git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v7 28/31] merge-recursive: fix remaining directory rename + dirty overwrite cases

From
Elijah Newren <newren@gmail.com>
Date
Feb 5, 2018, 22:18 UTC
Message-ID
<CABPp-BHpC9EqSM_qmBxo3J3jbSrHNR-UkDpC0amztwfCYg8Low@mail.gmail.com>
In-Reply-To
<CAGZ79kYwDnL1AJ79Z02qxse1ZjKd1zqORqKTxpZGHmCqPhTHJw@mail.gmail.com>
On Mon, Feb 5, 2018 at 1:52 PM, Stefan Beller <sbeller@google.com> wrote:
> On Tue, Jan 30, 2018 at 3:25 PM, Elijah Newren <newren@gmail.com> wrote:
Show 10 quoted lines
>> +               /*
>> +                * Stupid double negatives in remove_file; it somehow manages
>> +                * to repeatedly mess me up.  So, just for myself:
>> +                *    1) update_wd iff !ren_src_was_dirty.
>> +                *    2) no_wd iff !update_wd
>> +                *    3) so, no_wd == !!ren_src_was_dirty == ren_src_was_dirty
>> +                */
>
> Not sure iff this comment is at the right place and is a good addition to
> the code base.

Fair enough, and I should apologize for letting my frustration come through there. However, what if I replaced the first two lines of the comment with:

"Because the double negatives somehow keep confusing me..."
so that it reads:
        /*
         * Because the double negatives somehow keep confusing me...
         *    1) update_wd iff !ren_src_was_dirty.
         *    2) no_wd iff !update_wd
         *    3) so, no_wd == !!ren_src_was_dirty == ren_src_was_dirty
         */

Even if my wording was suboptimal, the rest of the comment did seem pretty important because I messed up the line after the comment multiple times. (You'd think that the odds of getting it right should be 50/50 and that a simple inversion would fix it, so one could only mess the line up once, but I'm apparently special). And then I came back to look at it later and was still confused. For some reason, I seem to need the longer explanation.

> However it hints at the underlying issue of a bad API that is provided
> by remove_file ?
I'd definitely agree with that.
Previous: Stefan BellerNext: Elijah Newren
Message 6 of 63 in “Add directory rename detection to git”
  1. 00/31 Add directory rename detection to gitElijah Newren, Jan 30, 2018
  2. Elijah NewrenJan 30, 2018
  3. 24/31 merge-recursive: when comparing files, don't include treesElijah Newren, Jan 30, 2018
  4. 28/31 merge-recursive: fix remaining directory rename + dirty overwrite casesElijah Newren, Jan 30, 2018
  5. Stefan BellerFeb 5, 2018
  6. Elijah NewrenFeb 5, 2018
  7. 05/31 directory rename detection: files/directories in the way of some renamesElijah Newren, Jan 30, 2018
  8. 06/31 directory rename detection: testcases checking which side did the renameElijah Newren, Jan 30, 2018
  9. 10/31 directory rename detection: tests for handling overwriting untracked filesElijah Newren, Jan 30, 2018
  10. 15/31 merge-recursive: make !o->detect_rename codepath more obviousElijah Newren, Jan 30, 2018
  11. Stefan BellerFeb 2, 2018
  12. 14/31 merge-recursive: fix leaks of allocated renames and diff_filepairsElijah Newren, Jan 30, 2018
  13. Stefan BellerFeb 2, 2018
  14. 23/31 merge-recursive: check for file level conflicts then get new nameElijah Newren, Jan 30, 2018
  15. 30/31 merge-recursive: avoid spurious rename/rename conflict from dir renamesElijah Newren, Jan 30, 2018
  16. 16/31 merge-recursive: split out code for determining diff_filepairsElijah Newren, Jan 30, 2018
  17. Stefan BellerFeb 3, 2018
  18. Elijah NewrenFeb 3, 2018
  19. 11/31 directory rename detection: tests for handling overwriting dirty filesElijah Newren, Jan 30, 2018
  20. 22/31 merge-recursive: add computation of collisions due to dir rename & mergingElijah Newren, Jan 30, 2018
  21. 31/31 merge-recursive: ensure we write updates for directory-renamed fileElijah Newren, Jan 30, 2018
  22. Stefan BellerFeb 5, 2018
  23. 20/31 merge-recursive: check for directory level conflictsElijah Newren, Jan 30, 2018
  24. Stefan BellerFeb 5, 2018
  25. Elijah NewrenFeb 5, 2018
  26. 25/31 merge-recursive: apply necessary modifications for directory renamesElijah Newren, Jan 30, 2018
  27. SZEDER GáborFeb 16, 2018
  28. 17/31 merge-recursive: add a new hashmap for storing directory renamesElijah Newren, Jan 30, 2018
  29. Stefan BellerFeb 3, 2018
  30. Elijah NewrenFeb 3, 2018
  31. Johannes SixtFeb 4, 2018
  32. Elijah NewrenFeb 5, 2018
  33. Junio C HamanoFeb 5, 2018
  34. Stefan BellerFeb 5, 2018
  35. Elijah NewrenFeb 5, 2018
  36. 21/31 merge-recursive: add a new hashmap for storing file collisionsElijah Newren, Jan 30, 2018
  37. Stefan BellerFeb 5, 2018
  38. 29/31 directory rename detection: new testcases showcasing a pair of bugsElijah Newren, Jan 30, 2018
  39. 27/31 merge-recursive: fix overwriting dirty files involved in renamesElijah Newren, Jan 30, 2018
  40. Stefan BellerFeb 5, 2018
  41. Elijah NewrenFeb 5, 2018
  42. 03/31 directory rename detection: testcases to avoid taking detection too farElijah Newren, Jan 30, 2018
  43. 01/31 directory rename detection: basic testcasesElijah Newren, Jan 30, 2018
  44. 04/31 directory rename detection: partially renamed directory testcase/discussionElijah Newren, Jan 30, 2018
  45. 19/31 merge-recursive: add get_directory_renames()Elijah Newren, Jan 30, 2018
  46. Stefan BellerFeb 3, 2018
  47. Elijah NewrenFeb 3, 2018
  48. Elijah NewrenFeb 4, 2018
  49. Eric SunshineFeb 4, 2018
  50. Eric SunshineFeb 4, 2018
  51. Stefan BellerFeb 5, 2018
  52. 18/31 merge-recursive: make a helper function for cleanup for handle_renamesElijah Newren, Jan 30, 2018
  53. Stefan BellerFeb 3, 2018
  54. 02/31 directory rename detection: directory splitting testcasesElijah Newren, Jan 30, 2018
  55. 13/31 merge-recursive: introduce new functions to handle rename logicElijah Newren, Jan 30, 2018
  56. Stefan BellerFeb 2, 2018
  57. 12/31 merge-recursive: move the get_renames() functionElijah Newren, Jan 30, 2018
  58. Stefan BellerFeb 2, 2018
  59. 09/31 directory rename detection: miscellaneous testcases to complete coverageElijah Newren, Jan 30, 2018
  60. 08/31 directory rename detection: testcases exploring possibly suboptimal mergesElijah Newren, Jan 30, 2018
  61. 26/31 merge-recursive: avoid clobbering untracked files with directory renamesElijah Newren, Jan 30, 2018
  62. 07/31 directory rename detection: more involved edge/corner testcasesElijah Newren, Jan 30, 2018
  63. Stefan BellerFeb 5, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.