{"thread":{"id":"55853","subject":"[PATCH] rerere: enable by default","startedAt":"2021-06-06T12:31:01Z","lastAt":"2021-06-07T16:42:44Z","messageCount":8,"participants":["Junio C Hamano","brian m. carlson","Andrei Rybak","Ævar Arnfjörð Bjarmason","Felipe Contreras"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"426531","messageId":"xmqqfsxvxjj2.fsf@gitster.g","threadId":"55853","inReplyTo":null,"subject":"[PATCH] rerere: enable by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-06T12:30:25Z","receivedAt":"2021-06-06T12:31:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"By default, the rerere machinery has been disabled since a bug in\nthe machinery could screw up the end user's data at the most\nstressful time during the end user's workday (i.e. during conflict\nresolution).\n\nIt however has been in wide use without causing much trouble (other\nthan, obviously, replaying a broken conflict resolution that was\nrecorded earlier when the user made a mismerge), and it is about\ntime to enable it by default.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/config/rerere.txt |  5 ++---\n rerere.c                        | 12 +++---------\n t/t2030-unresolve-info.sh       |  3 +++\n 3 files changed, 8 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/config/rerere.txt b/Documentation/config/rerere.txt\nindex 40abdf6a6b..45e97fc0fa 100644\n--- a/Documentation/config/rerere.txt\n+++ b/Documentation/config/rerere.txt\n@@ -7,6 +7,5 @@ rerere.enabled::\n \tActivate recording of resolved conflicts, so that identical\n \tconflict hunks can be resolved automatically, should they be\n \tencountered again.  By default, linkgit:git-rerere[1] is\n-\tenabled if there is an `rr-cache` directory under the\n-\t`$GIT_DIR`, e.g. if \"rerere\" was previously used in the\n-\trepository.\n+\tenabled, but this configuration can be set to 'false' to\n+\tturn it off.\ndiff --git a/rerere.c b/rerere.c\nindex d83d58df4f..83e740d730 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -18,8 +18,7 @@\n #define THREE_STAGED 2\n void *RERERE_RESOLVED = &RERERE_RESOLVED;\n \n-/* if rerere_enabled == -1, fall back to detection of .git/rr-cache */\n-static int rerere_enabled = -1;\n+static int rerere_enabled = 1; /* default to true */\n \n /* automatically update cleanly resolved paths to the index */\n static int rerere_autoupdate;\n@@ -852,16 +851,11 @@ static GIT_PATH_FUNC(git_path_rr_cache, \"rr-cache\")\n \n static int is_rerere_enabled(void)\n {\n-\tint rr_cache_exists;\n-\n \tif (!rerere_enabled)\n \t\treturn 0;\n \n-\trr_cache_exists = is_directory(git_path_rr_cache());\n-\tif (rerere_enabled < 0)\n-\t\treturn rr_cache_exists;\n-\n-\tif (!rr_cache_exists && mkdir_in_gitdir(git_path_rr_cache()))\n+\tif (!is_directory(git_path_rr_cache()) &&\n+\t    mkdir_in_gitdir(git_path_rr_cache()))\n \t\tdie(_(\"could not create directory '%s'\"), git_path_rr_cache());\n \treturn 1;\n }\ndiff --git a/t/t2030-unresolve-info.sh b/t/t2030-unresolve-info.sh\nindex be6c84c52a..8c571ddf92 100755\n--- a/t/t2030-unresolve-info.sh\n+++ b/t/t2030-unresolve-info.sh\n@@ -46,6 +46,7 @@ prime_resolve_undo () {\n }\n \n test_expect_success setup '\n+\tgit config rerere.enabled false &&\n \tmkdir fi &&\n \tprintf \"a\\0a\" >binary &&\n \tgit add binary &&\n@@ -127,6 +128,8 @@ test_expect_success 'unmerge with plumbing' '\n '\n \n test_expect_success 'rerere and rerere forget' '\n+\t# from here on, use rerere.\n+\tgit config rerere.enabled true &&\n \tmkdir .git/rr-cache &&\n \tprime_resolve_undo &&\n \techo record the resolution &&\n-- \n2.32.0-189-ge1556ffcec\n\n"},{"id":"426552","messageId":"YL1uudVxwcDUg2+T@camp.crustytoothpaste.net","threadId":"55853","inReplyTo":"xmqqfsxvxjj2.fsf@gitster.g","subject":"Re: [PATCH] rerere: enable by default","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-06-07T00:56:25Z","receivedAt":"2021-06-07T00:57:00Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-06-06 at 12:30:25, Junio C Hamano wrote:\n> By default, the rerere machinery has been disabled since a bug in\n> the machinery could screw up the end user's data at the most\n> stressful time during the end user's workday (i.e. during conflict\n> resolution).\n> \n> It however has been in wide use without causing much trouble (other\n> than, obviously, replaying a broken conflict resolution that was\n> recorded earlier when the user made a mismerge), and it is about\n> time to enable it by default.\n\nYes, I think this makes sense.  I've been using it for quite a while\nwithout problems and I'm usually quite good at breaking things, and if\nwe can make the user experience better, which I think rerere does,\nthat's definitely a good thing.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"426563","messageId":"xmqqtumauhnc.fsf@gitster.g","threadId":"55853","inReplyTo":"YL1uudVxwcDUg2+T@camp.crustytoothpaste.net","subject":"Re: [PATCH] rerere: enable by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-07T09:51:35Z","receivedAt":"2021-06-07T09:51:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> On 2021-06-06 at 12:30:25, Junio C Hamano wrote:\n>> By default, the rerere machinery has been disabled since a bug in\n>> the machinery could screw up the end user's data at the most\n>> stressful time during the end user's workday (i.e. during conflict\n>> resolution).\n>> \n>> It however has been in wide use without causing much trouble (other\n>> than, obviously, replaying a broken conflict resolution that was\n>> recorded earlier when the user made a mismerge), and it is about\n>> time to enable it by default.\n>\n> Yes, I think this makes sense.  I've been using it for quite a while\n> without problems and I'm usually quite good at breaking things, and if\n> we can make the user experience better, which I think rerere does,\n> that's definitely a good thing.\n\nI agree in principle, but new tests added by topics in flight need\nto get prepared for the change.  There are, for example, misguided\nchecks that insist that the standard error stream out of \"git merge\"\nmust be absolutely empty, which will break when rerere machinery\nstarts reporting that it prepared the preimage so that it can record\nthe resolution.  Not just rerere, but such a test is brittle and\nwill break when other inner parts of the system starts reporting\nmore (e.g. progress) to the standard error stream.\n\nI'd prefer to see these tests cleaned up in the right way (as\nopposed to setting rerere.enabled to false upfront, which hides\nthese suboptimal tests) before this change to enable rerere by\ndefault lands, so it may need a sizeable preparatory work.\n\nThanks.\n\n\n\n\n"},{"id":"426564","messageId":"720154c0-eedb-db92-1b61-1df2e22fb128@gmail.com","threadId":"55853","inReplyTo":"xmqqfsxvxjj2.fsf@gitster.g","subject":"Re: [PATCH] rerere: enable by default","fromName":"Andrei Rybak","fromEmail":"rybak.a.v@gmail.com","sentAt":"2021-06-07T10:34:07Z","receivedAt":"2021-06-07T10:35:19Z","isPatch":true,"sender":{"key":"rybak.a.v@gmail.com","avatar":"https://avatars.githubusercontent.com/u/624072?v=4"},"body":"On 06/06/2021 14:30, Junio C Hamano wrote:\n> By default, the rerere machinery has been disabled since a bug in\n> the machinery could screw up the end user's data at the most\n> stressful time during the end user's workday (i.e. during conflict\n> resolution).\n> \n> It however has been in wide use without causing much trouble (other\n> than, obviously, replaying a broken conflict resolution that was\n> recorded earlier when the user made a mismerge), and it is about\n> time to enable it by default.\n> \n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>   Documentation/config/rerere.txt |  5 ++---\n>   rerere.c                        | 12 +++---------\n>   t/t2030-unresolve-info.sh       |  3 +++\n>   3 files changed, 8 insertions(+), 12 deletions(-)\n> \n> diff --git a/Documentation/config/rerere.txt b/Documentation/config/rerere.txt\n> index 40abdf6a6b..45e97fc0fa 100644\n> --- a/Documentation/config/rerere.txt\n> +++ b/Documentation/config/rerere.txt\n> @@ -7,6 +7,5 @@ rerere.enabled::\n>   \tActivate recording of resolved conflicts, so that identical\n>   \tconflict hunks can be resolved automatically, should they be\n>   \tencountered again.  By default, linkgit:git-rerere[1] is\n> -\tenabled if there is an `rr-cache` directory under the\n> -\t`$GIT_DIR`, e.g. if \"rerere\" was previously used in the\n> -\trepository.\n> +\tenabled, but this configuration can be set to 'false' to\n\nAt the moment, backticks are used more often than single quotes\nfor formatting of boolean values in documentation:\n\n     $ git grep -oE \"\\`(false|true)\\`\" -- Documentation/*.txt Documentation/config/*.txt | wc -l\n     121\n     $ git grep -oE \"'(false|true)'\" -- Documentation/*.txt Documentation/config/*.txt | wc -l\n     53\n\n> +\tturn it off.\n"},{"id":"426565","messageId":"877dj654or.fsf@evledraar.gmail.com","threadId":"55853","inReplyTo":"xmqqfsxvxjj2.fsf@gitster.g","subject":"Re: [PATCH] rerere: enable by default","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-07T09:50:35Z","receivedAt":"2021-06-07T10:50:58Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Jun 06 2021, Junio C Hamano wrote:\n\n> By default, the rerere machinery has been disabled since a bug in\n> the machinery could screw up the end user's data at the most\n> stressful time during the end user's workday (i.e. during conflict\n> resolution).\n\nWhat was that bug & in what commit was it fixed? Makes sense to note\nthat here.\n\n> It however has been in wide use without causing much trouble (other\n> than, obviously, replaying a broken conflict resolution that was\n> recorded earlier when the user made a mismerge), and it is about\n> time to enable it by default.\n>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>  Documentation/config/rerere.txt |  5 ++---\n>  rerere.c                        | 12 +++---------\n>  t/t2030-unresolve-info.sh       |  3 +++\n>  3 files changed, 8 insertions(+), 12 deletions(-)\n>\n> diff --git a/Documentation/config/rerere.txt b/Documentation/config/rerere.txt\n> index 40abdf6a6b..45e97fc0fa 100644\n> --- a/Documentation/config/rerere.txt\n> +++ b/Documentation/config/rerere.txt\n> @@ -7,6 +7,5 @@ rerere.enabled::\n>  \tActivate recording of resolved conflicts, so that identical\n>  \tconflict hunks can be resolved automatically, should they be\n>  \tencountered again.  By default, linkgit:git-rerere[1] is\n> -\tenabled if there is an `rr-cache` directory under the\n> -\t`$GIT_DIR`, e.g. if \"rerere\" was previously used in the\n> -\trepository.\n> +\tenabled, but this configuration can be set to 'false' to\n> +\tturn it off.\n> diff --git a/rerere.c b/rerere.c\n> index d83d58df4f..83e740d730 100644\n> --- a/rerere.c\n> +++ b/rerere.c\n> @@ -18,8 +18,7 @@\n>  #define THREE_STAGED 2\n>  void *RERERE_RESOLVED = &RERERE_RESOLVED;\n>  \n> -/* if rerere_enabled == -1, fall back to detection of .git/rr-cache */\n> -static int rerere_enabled = -1;\n> +static int rerere_enabled = 1; /* default to true */\n>  \n>  /* automatically update cleanly resolved paths to the index */\n>  static int rerere_autoupdate;\n> @@ -852,16 +851,11 @@ static GIT_PATH_FUNC(git_path_rr_cache, \"rr-cache\")\n>  \n>  static int is_rerere_enabled(void)\n>  {\n> -\tint rr_cache_exists;\n> -\n>  \tif (!rerere_enabled)\n>  \t\treturn 0;\n>  \n> -\trr_cache_exists = is_directory(git_path_rr_cache());\n> -\tif (rerere_enabled < 0)\n> -\t\treturn rr_cache_exists;\n> -\n> -\tif (!rr_cache_exists && mkdir_in_gitdir(git_path_rr_cache()))\n> +\tif (!is_directory(git_path_rr_cache()) &&\n> +\t    mkdir_in_gitdir(git_path_rr_cache()))\n>  \t\tdie(_(\"could not create directory '%s'\"), git_path_rr_cache());\n>  \treturn 1;\n>  }\n> diff --git a/t/t2030-unresolve-info.sh b/t/t2030-unresolve-info.sh\n> index be6c84c52a..8c571ddf92 100755\n> --- a/t/t2030-unresolve-info.sh\n> +++ b/t/t2030-unresolve-info.sh\n> @@ -46,6 +46,7 @@ prime_resolve_undo () {\n>  }\n>  \n>  test_expect_success setup '\n> +\tgit config rerere.enabled false &&\n>  \tmkdir fi &&\n>  \tprintf \"a\\0a\" >binary &&\n>  \tgit add binary &&\n> @@ -127,6 +128,8 @@ test_expect_success 'unmerge with plumbing' '\n>  '\n>  \n>  test_expect_success 'rerere and rerere forget' '\n> +\t# from here on, use rerere.\n> +\tgit config rerere.enabled true &&\n>  \tmkdir .git/rr-cache &&\n>  \tprime_resolve_undo &&\n>  \techo record the resolution &&\n\nIt seems strange to disable it at the very beginning, but then not have\nany test until you turn it on actually rely on that except that \"mkdir\"\nin the second hunk, i.e. this on top works:\n\t\n\tdiff --git a/t/t2030-unresolve-info.sh b/t/t2030-unresolve-info.sh\n\tindex 8c571ddf92..7195ded9f9 100755\n\t--- a/t/t2030-unresolve-info.sh\n\t+++ b/t/t2030-unresolve-info.sh\n\t@@ -46,7 +46,6 @@ prime_resolve_undo () {\n\t }\n\t \n\t test_expect_success setup '\n\t-\tgit config rerere.enabled false &&\n\t \tmkdir fi &&\n\t \tprintf \"a\\0a\" >binary &&\n\t \tgit add binary &&\n\t@@ -130,7 +129,6 @@ test_expect_success 'unmerge with plumbing' '\n\t test_expect_success 'rerere and rerere forget' '\n\t \t# from here on, use rerere.\n\t \tgit config rerere.enabled true &&\n\t-\tmkdir .git/rr-cache &&\n\t \tprime_resolve_undo &&\n\t \techo record the resolution &&\n\t \tgit rerere &&\n\nSo the only impact of that rerere.enabled=false early is to make sure\nwe're not creating the .git/rr-cache.\n\nThose tests will fail if we turned on rerere.autoUpdate=true there, so\nisn't it a better test coverage to assert that even with\nrerere.enabled=true we don't impact these merge resolutions in the\nearlier tests?\n\nIt also seems to make sense to change the main rerere tests to assert\nthat we have the default set to this new value, i.e.:\n\t\n\tdiff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh\n\tindex 9f8c76dffb..f319f08852 100755\n\t--- a/t/t4200-rerere.sh\n\t+++ b/t/t4200-rerere.sh\n\t@@ -89,8 +89,8 @@ test_expect_success 'activate rerere, old style (conflicting merge)' '\n\t '\n\t \n\t test_expect_success 'rerere.enabled works, too' '\n\t+\ttest_path_is_dir .git/rr-cache &&\n\t \trm -rf .git/rr-cache &&\n\t-\tgit config rerere.enabled true &&\n\t \tgit reset --hard &&\n\t \ttest_must_fail git merge first &&\n\t \n\t@@ -100,8 +100,8 @@ test_expect_success 'rerere.enabled works, too' '\n\t '\n\t \n\t test_expect_success 'set up rr-cache' '\n\t+\ttest_path_is_dir .git/rr-cache &&\n\t \trm -rf .git/rr-cache &&\n\t-\tgit config rerere.enabled true &&\n\t \tgit reset --hard &&\n\t \ttest_must_fail git merge first &&\n\t \tsha1=$(perl -pe \"s/\t.*//\" .git/MERGE_RR) &&\n\t@@ -668,7 +668,6 @@ test_expect_success 'test simple stage 1 handling' '\n\t \t(\n\t \t\tcd stage_1_handling &&\n\t \n\t-\t\tgit config rerere.enabled true &&\n\t \t\tgit checkout A^0 &&\n\t \t\ttest_must_fail git merge B^0\n\t \t)\n\nAs an aside: This pre-dates your change here, but I thought the config\nreading in rerere.c was a bit odd, why do we call the one-off config\nreading functions when we're about to iterate over the whole set anyway?\nTurns out we did it the way I'd expect before 633e5ad326 (rerere.c:\nreplace `git_config()` with `git_config_get_*()` family, 2014-08-07).\n"},{"id":"426571","messageId":"xmqqpmwyue8q.fsf@gitster.g","threadId":"55853","inReplyTo":"877dj654or.fsf@evledraar.gmail.com","subject":"Re: [PATCH] rerere: enable by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-07T11:05:09Z","receivedAt":"2021-06-07T11:05:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> On Sun, Jun 06 2021, Junio C Hamano wrote:\n>\n>> By default, the rerere machinery has been disabled since a bug in\n>> the machinery could screw up the end user's data at the most\n>> stressful time during the end user's workday (i.e. during conflict\n>> resolution).\n>\n> What was that bug & in what commit was it fixed? Makes sense to note\n> that here.\n\nThere is no such bug ;-).\n\nWriting buggy code, not thinking about it carefully enough and\njumping up and down yelling this shiny new toy must be merged down\nimmediately is something we see on this list from others, but it is\nthe total opposite of how I operate.  I just am extra cautious and\neven after I am reasonably sure the code would not break, I prefer\nto have volunteers to opt into testing.\n\n> \t@@ -130,7 +129,6 @@ test_expect_success 'unmerge with plumbing' '\n> \t test_expect_success 'rerere and rerere forget' '\n> \t \t# from here on, use rerere.\n> \t \tgit config rerere.enabled true &&\n> \t-\tmkdir .git/rr-cache &&\n> \t \tprime_resolve_undo &&\n> \t \techo record the resolution &&\n> \t \tgit rerere &&\n>\n> So the only impact of that rerere.enabled=false early is to make sure\n> we're not creating the .git/rr-cache.\n\nNot really.  Unresolve is about recording the initial conflict in\nthe index, so it is far easier to see its effect if you do not\nenable rerere, when you are manually debugging these earlier tests.\n\nAnd later test do check how it works with rerere enabled, but the\nway the original sequence of tests enable it is with the \"mkdir\".\nI.e. \"if rerere.enabled is not set either way, presence of the\ndirectory means it is already enabled\".  The new test sequence\nuses the configuration variable explicitly, because in the new world\norder, the presence of the directory does not mean a thing.\n"},{"id":"426604","messageId":"87v96p4w3f.fsf@evledraar.gmail.com","threadId":"55853","inReplyTo":"xmqqpmwyue8q.fsf@gitster.g","subject":"Re: [PATCH] rerere: enable by default","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-07T12:56:08Z","receivedAt":"2021-06-07T13:56:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Jun 07 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n>\n>> On Sun, Jun 06 2021, Junio C Hamano wrote:\n>>\n>>> By default, the rerere machinery has been disabled since a bug in\n>>> the machinery could screw up the end user's data at the most\n>>> stressful time during the end user's workday (i.e. during conflict\n>>> resolution).\n>>\n>> What was that bug & in what commit was it fixed? Makes sense to note\n>> that here.\n>\n> There is no such bug ;-).\n>\n> Writing buggy code, not thinking about it carefully enough and\n> jumping up and down yelling this shiny new toy must be merged down\n> immediately is something we see on this list from others, but it is\n> the total opposite of how I operate.  I just am extra cautious and\n> even after I am reasonably sure the code would not break, I prefer\n> to have volunteers to opt into testing.\n\nThanks. I misread that, and was (perhaps mis-)remembering the previous\nthread about this discussing some past bugs...\n\n>> \t@@ -130,7 +129,6 @@ test_expect_success 'unmerge with plumbing' '\n>> \t test_expect_success 'rerere and rerere forget' '\n>> \t \t# from here on, use rerere.\n>> \t \tgit config rerere.enabled true &&\n>> \t-\tmkdir .git/rr-cache &&\n>> \t \tprime_resolve_undo &&\n>> \t \techo record the resolution &&\n>> \t \tgit rerere &&\n>>\n>> So the only impact of that rerere.enabled=false early is to make sure\n>> we're not creating the .git/rr-cache.\n>\n> Not really.  Unresolve is about recording the initial conflict in\n> the index, so it is far easier to see its effect if you do not\n> enable rerere, when you are manually debugging these earlier tests.\n>\n> And later test do check how it works with rerere enabled, but the\n> way the original sequence of tests enable it is with the \"mkdir\".\n> I.e. \"if rerere.enabled is not set either way, presence of the\n> directory means it is already enabled\".  The new test sequence\n> uses the configuration variable explicitly, because in the new world\n> order, the presence of the directory does not mean a thing.\n\nI mean you added \"from here on, use rerere\", but if I instrument the\ntests to set rerere.enabled=false they also pass, sans the .git/rr-cache\ndirectory being created.\n\nSo we weren't \"using rerere\", we were just writing the data on the side.\n\nWe *do* get a failure in test #3, \"rm records reset clears\", if we set\nrerere.autoUpdate=true. So it seems to me that what those first tests\nwould benefit more from not having the addition of your\nrerere.enabled=false line early on.\n\nAfter all it's more interesting to test that the merge resolution is not\ndifferent in any way under rerere.enabled=true & rerere.autoUpdate=false\nthan to not write the rr-cache data at all. It seems like having just\none test with rerere.enabled=false & \"I did not write .git/rr-cache\"\nwould make for better coverage.\n\nI.e. this on top works:\n\t\n\tdiff --git a/t/t2030-unresolve-info.sh b/t/t2030-unresolve-info.sh\n\tindex 8c571ddf92..b010f504b0 100755\n\t--- a/t/t2030-unresolve-info.sh\n\t+++ b/t/t2030-unresolve-info.sh\n\t@@ -46,11 +46,12 @@ prime_resolve_undo () {\n\t }\n\t \n\t test_expect_success setup '\n\t-\tgit config rerere.enabled false &&\n\t \tmkdir fi &&\n\t \tprintf \"a\\0a\" >binary &&\n\t \tgit add binary &&\n\t+\t! test_path_is_dir .git/rr-cache &&\n\t \ttest_commit initial fi/le first &&\n\t+\ttest_path_is_dir .git/rr-cache &&\n\t \tgit branch side &&\n\t \tgit branch another &&\n\t \tprintf \"a\\0b\" >binary &&\n\t@@ -128,9 +129,6 @@ test_expect_success 'unmerge with plumbing' '\n\t '\n\t \n\t test_expect_success 'rerere and rerere forget' '\n\t-\t# from here on, use rerere.\n\t-\tgit config rerere.enabled true &&\n\t-\tmkdir .git/rr-cache &&\n\t \tprime_resolve_undo &&\n\t \techo record the resolution &&\n\t \tgit rerere &&\n\t@@ -156,7 +154,6 @@ test_expect_success 'rerere and rerere forget' '\n\t \n\t test_expect_success 'rerere and rerere forget (subdirectory)' '\n\t \trm -fr .git/rr-cache &&\n\t-\tmkdir .git/rr-cache &&\n\t \tprime_resolve_undo &&\n\t \techo record the resolution &&\n\t \t(cd fi && git rerere) &&\n\nAs an aside when testing this I found that I could make it segfault by\nnot doing the mkdir() in setup_rerere() and without:\n\n\tdiff --git a/rerere.c b/rerere.c\n\tindex 83e740d730..06fb86d5b7 100644\n\t--- a/rerere.c\n\t+++ b/rerere.c\n\t@@ -731,7 +731,10 @@ static void do_rerere_one_path(struct index_state *istate,\n\t \t/* Has the user resolved it already? */\n\t \tif (variant >= 0) {\n\t \t\tif (!handle_file(istate, path, NULL, NULL)) {\n\t-\t\t\tcopy_file(rerere_path(id, \"postimage\"), path, 0666);\n\t+\t\t\tconst char *dst = rerere_path(id, \"postimage\");\n\t+\t\t\tif (copy_file(dst, path, 0666))\n\t+\t\t\t\tdie_errno(_(\"could not copy '%s' to '%s'\"),\n\t+\t\t\t\t\t  path, dst);\n\t \t\t\tid->collection->status[variant] |= RR_HAS_POSTIMAGE;\n\t \t\t\tfprintf_ln(stderr, _(\"Recorded resolution for '%s'.\"), path);\n\t \t\t\tfree_rerere_id(rr_item);\n\t\n\nI.e. we make the hard assumption that the directory has been created,\nand don't check the return value(s) of various subsequent file copying\netc. So there's a rare segfault in the wild if the \"setup_rerere()\"\nraces with something that removes the .git/rr-cache (perhaps git-gc will\nremove it entirely?).\n"},{"id":"426627","messageId":"60be4c4071b80_39c0a208ac@natae.notmuch","threadId":"55853","inReplyTo":"xmqqfsxvxjj2.fsf@gitster.g","subject":"RE: [PATCH] rerere: enable by default","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-07T16:41:36Z","receivedAt":"2021-06-07T16:42:44Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> By default, the rerere machinery has been disabled since a bug in\n> the machinery could screw up the end user's data at the most\n> stressful time during the end user's workday (i.e. during conflict\n> resolution).\n> \n> It however has been in wide use without causing much trouble (other\n> than, obviously, replaying a broken conflict resolution that was\n> recorded earlier when the user made a mismerge), and it is about\n> time to enable it by default.\n\nThat is a good diea.\n\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\nBut did you really come up with it all by yourself?\n\nCould perhaps this mail [1] sent 5 days prior have anything to do with it?\n\n> If the defaults make your life hard, then shouldn't we change the\n> defaults?\n\n> But, if we are already on this topic... who wants rerere disabled by\n> default?\n\nCheers.\n\n[1] https://lore.kernel.org/git/60b61089ba63d_e40ca20894@natae.notmuch/\n\n-- \nFelipe Contreras\n"}]}