{"thread":{"id":"58168","subject":"[PATCH v2] Add note that conflict resolution is still performed","startedAt":"2022-07-15T09:26:09Z","lastAt":"2022-08-03T21:01:09Z","messageCount":4,"participants":["Matthias Beyer","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"459130","messageId":"20220715092527.1567837-1-mail@beyermatthias.de","threadId":"58168","inReplyTo":"xmqq35f6fe0j.fsf@gitster.g","subject":"[PATCH v2] Add note that conflict resolution is still performed","fromName":"Matthias Beyer","fromEmail":"mail@beyermatthias.de","sentAt":"2022-07-15T09:25:27Z","receivedAt":"2022-07-15T09:26:09Z","isPatch":true,"sender":{"key":"mail@beyermatthias.de","avatar":null},"body":"We should note that conflict resolution is still performed, even if\n`--no-rerere-autoupdate` is specified, to make sure users do not get\nconfused by the setting and assume this disables rerere conflict\nresultion altogether.\n\nCC: Phillip Wood \nCC: Junio C Hamano \nSigned-off-by: Matthias Beyer \n---\n Documentation/git-cherry-pick.txt | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\nindex 78dcc9171f..b92aa1f9da 100644\n--- a/Documentation/git-cherry-pick.txt\n+++ b/Documentation/git-cherry-pick.txt\n@@ -160,6 +160,10 @@ effect to your index in a row.\n --no-rerere-autoupdate::\n \tAllow the rerere mechanism to update the index with the\n \tresult of auto-conflict resolution if possible.\n+\tThe `--no-rerere-autoupdate` option does not prevent the conflict\n+\tresolution, but prevents the index from being updated. This gives the\n+\tuser a chance for a final sanity check before using linkgit:git-add[1]\n+\tto add the result.\n \n SEQUENCER SUBCOMMANDS\n ---------------------\n-- \n2.36.0\n\n"},{"id":"459171","messageId":"xmqq35f2ysd9.fsf@gitster.g","threadId":"58168","inReplyTo":"20220715092527.1567837-1-mail@beyermatthias.de","subject":"Re: [PATCH v2] Add note that conflict resolution is still performed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-15T21:32:18Z","receivedAt":"2022-07-15T21:32:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthias Beyer writes:\n\n> We should note that conflict resolution is still performed, even if\n> `--no-rerere-autoupdate` is specified, to make sure users do not get\n> confused by the setting and assume this disables rerere conflict\n> resultion altogether.\n>\n> CC: Phillip Wood \n> CC: Junio C Hamano \n> Signed-off-by: Matthias Beyer \n> ---\n> Documentation/git-cherry-pick.txt | 4 ++++\n> 1 file changed, 4 insertions(+)\n>\n> diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\n> index 78dcc9171f..b92aa1f9da 100644\n> --- a/Documentation/git-cherry-pick.txt\n> +++ b/Documentation/git-cherry-pick.txt\n> @@ -160,6 +160,10 @@ effect to your index in a row.\n> --no-rerere-autoupdate::\n> \tAllow the rerere mechanism to update the index with the\n> \tresult of auto-conflict resolution if possible.\n> +\tThe `--no-rerere-autoupdate` option does not prevent the conflict\n> +\tresolution, but prevents the index from being updated. This gives the\n> +\tuser a chance for a final sanity check before using linkgit:git-add[1]\n> +\tto add the result.\n\n$ git grep -l \"^--rerere-autoupdate::\" Documentation\nDocumentation/git-am.txt\nDocumentation/git-cherry-pick.txt\nDocumentation/git-merge.txt\nDocumentation/git-rebase.txt\nDocumentation/git-revert.txt\n\nI made a cursory scan of these and I suspect the existing two-line\ndescription are shread among all of them. At this point it may make\nsense to split the description to a separate file to be included by\nthese places (the attached patch may be a starting point) in a\npatch, and then follow up with the text change in a follow-up patch.\n\nA tangent that may be worth thinking about, that does not have to be\npart of this topic (as it probably will involve code change).\n\nIt makes sense that \"--no-rerere-autoupdate\" does not disable the\n\"rerere\" mechanism (when it is enabled, of course), because it makes\nsense to reuse recorded resolution without updating the index with\nthe result.\n\nHowever, it may make sense to have \"--rerere-autoupdate\" option to\nenable the \"rerere\" mechanism when it is disabled, because with\n\"rerere\" disabled, there is nothing to auto-update.\n\nAnyway, here is the preliminary restructuring patch.\n\n----- >8 --------- >8 --------- >8 --------- >8 --------- >8 -----\nSubject: [PATCH] doc: consolidate --rerere-autoupdate description\n\nThe `--rerere-autoupdate` option is shared across 5 commands, and\nare described the same way because it works exactly the same way in\nthese commands.\n\nCreate a separate file and include it from the help pages for these\ncommands, so that we can improve the description at one place to\nimprove all of them at once, and keep them in sync.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/git-am.txt | 5 +----\n Documentation/git-cherry-pick.txt | 5 +----\n Documentation/git-merge.txt | 5 +----\n Documentation/git-rebase.txt | 5 +----\n Documentation/git-revert.txt | 5 +----\n Documentation/rerere-options.txt | 4 ++++\n 6 files changed, 9 insertions(+), 20 deletions(-)\n\ndiff --git c/Documentation/git-am.txt w/Documentation/git-am.txt\nindex 09107fb106..320da6c4f7 100644\n--- c/Documentation/git-am.txt\n+++ w/Documentation/git-am.txt\n@@ -112,10 +112,7 @@ default. You can use `--no-utf8` to override this.\n \tam.threeWay configuration variable. For more information,\n \tsee am.threeWay in linkgit:git-config[1].\n \n---rerere-autoupdate::\n---no-rerere-autoupdate::\n-\tAllow the rerere mechanism to update the index with the\n-\tresult of auto-conflict resolution if possible.\n+include::rerere-options.txt[]\n \n --ignore-space-change::\n --ignore-whitespace::\ndiff --git c/Documentation/git-cherry-pick.txt w/Documentation/git-cherry-pick.txt\nindex 78dcc9171f..1e8ac9df60 100644\n--- c/Documentation/git-cherry-pick.txt\n+++ w/Documentation/git-cherry-pick.txt\n@@ -156,10 +156,7 @@ effect to your index in a row.\n \tPass the merge strategy-specific option through to the\n \tmerge strategy. See linkgit:git-merge[1] for details.\n \n---rerere-autoupdate::\n---no-rerere-autoupdate::\n-\tAllow the rerere mechanism to update the index with the\n-\tresult of auto-conflict resolution if possible.\n+include::rerere-options.txt[]\n \n SEQUENCER SUBCOMMANDS\n ---------------------\ndiff --git c/Documentation/git-merge.txt w/Documentation/git-merge.txt\nindex 3125473cc1..fee1dc2df2 100644\n--- c/Documentation/git-merge.txt\n+++ w/Documentation/git-merge.txt\n@@ -90,10 +90,7 @@ invocations. The automated message can include the branch description.\n If `--log` is specified, a shortlog of the commits being merged\n will be appended to the specified message.\n \n---rerere-autoupdate::\n---no-rerere-autoupdate::\n-\tAllow the rerere mechanism to update the index with the\n-\tresult of auto-conflict resolution if possible.\n+include::rerere-options.txt[]\n \n --overwrite-ignore::\n --no-overwrite-ignore::\ndiff --git c/Documentation/git-rebase.txt w/Documentation/git-rebase.txt\nindex a872ab0fbd..ff0b643ec0 100644\n--- c/Documentation/git-rebase.txt\n+++ w/Documentation/git-rebase.txt\n@@ -376,10 +376,7 @@ See also INCOMPATIBLE OPTIONS below.\n +\n See also INCOMPATIBLE OPTIONS below.\n \n---rerere-autoupdate::\n---no-rerere-autoupdate::\n-\tAllow the rerere mechanism to update the index with the\n-\tresult of auto-conflict resolution if possible.\n+include::rerere-options.txt[]\n \n -S[]::\n --gpg-sign[=]::\ndiff --git c/Documentation/git-revert.txt w/Documentation/git-revert.txt\nindex 8463fe9cf7..0105a54c1a 100644\n--- c/Documentation/git-revert.txt\n+++ w/Documentation/git-revert.txt\n@@ -112,10 +112,7 @@ effect to your index in a row.\n \tPass the merge strategy-specific option through to the\n \tmerge strategy. See linkgit:git-merge[1] for details.\n \n---rerere-autoupdate::\n---no-rerere-autoupdate::\n-\tAllow the rerere mechanism to update the index with the\n-\tresult of auto-conflict resolution if possible.\n+include::rerere-options.txt[]\n \n --reference::\n \tInstead of starting the body of the log message with \"This\ndiff --git c/Documentation/rerere-options.txt w/Documentation/rerere-options.txt\nnew file mode 100644\nindex 0000000000..8f4849e272\n--- /dev/null\n+++ w/Documentation/rerere-options.txt\n@@ -0,0 +1,4 @@\n+--rerere-autoupdate::\n+--no-rerere-autoupdate::\n+\tAllow the rerere mechanism to update the index with the\n+\tresult of auto-conflict resolution if possible.\n"},{"id":"459172","messageId":"xmqqv8ryxcyz.fsf@gitster.g","threadId":"58168","inReplyTo":"xmqq35f2ysd9.fsf@gitster.g","subject":"Re: [PATCH v2] Add note that conflict resolution is still performed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-15T21:50:12Z","receivedAt":"2022-07-15T21:50:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> A tangent that may be worth thinking about, that does not have to be\n> part of this topic (as it probably will involve code change).\n>\n> It makes sense that \"--no-rerere-autoupdate\" does not disable the\n> \"rerere\" mechanism (when it is enabled, of course), because it makes\n> sense to reuse recorded resolution without updating the index with\n> the result.\n>\n> However, it may make sense to have \"--rerere-autoupdate\" option to\n> enable the \"rerere\" mechanism when it is disabled, because with\n> \"rerere\" disabled, there is nothing to auto-update.\n\nI think the damage to the code to implement the above may not be too\nbad. Here is an illustration (not even compile tested) of the idea:\n\n - Earlier, after reading the config, we asked is_rerere_enabled()\n and let its logic decide if rerere is to be used, giving no\n influence to the incoming \"flags\" parameter. Then the value of\n rerere_autoupdate read from the config is further adjusted by the\n value in \"flags\" parameter that relays --[no-]rerere-autoupdate\n from the command line.\n\n - Instead, after reading the config, we can tweak it with the\n \"flags\", and flip rerere_enabled to true if autoupdate is\n enabled, before making a call to is_rerere_enabled().\n\nI do not think we have a test that insists that rerere does not kick\nin in a repository where rerere is disabled and the command line\nasks for \"--rerere-autoupdate\", so I suspect that with this patch, I\nwould not be surprised if no test breaks. If we think this takes us\nin a good direction, we should add a few tests to make sure at least\nthe following two:\n\n - In a repository with rerere disabled, running a command with the\n \"--rerere-autoupdate\" option will enable rerere for that single\n invocation of the command.\n\n - In a repository with rerere enabled, running a command with the\n \"--no-rerere-autoupdate\" option does not disable rerere for that\n single invocation of the command.\n\n---\n\n rerere.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git c/rerere.c w/rerere.c\nindex 876ab435da..16d3e865e6 100644\n--- c/rerere.c\n+++ w/rerere.c\n@@ -872,11 +872,13 @@ int setup_rerere(struct repository *r, struct string_list *merge_rr, int flags)\n \tint fd;\n \n \tgit_rerere_config();\n+\tif (flags & (RERERE_AUTOUPDATE|RERERE_NOAUTOUPDATE))\n+\t\trerere_autoupdate = !!(flags & RERERE_AUTOUPDATE);\n+\tif (rerere_autoupdate)\n+\t\trerere_enabled = 1;\n \tif (!is_rerere_enabled())\n \t\treturn -1;\n \n-\tif (flags & (RERERE_AUTOUPDATE|RERERE_NOAUTOUPDATE))\n-\t\trerere_autoupdate = !!(flags & RERERE_AUTOUPDATE);\n \tif (flags & RERERE_READONLY)\n \t\tfd = 0;\n \telse\n"},{"id":"460574","messageId":"20220803205915.1550797-1-gitster@pobox.com","threadId":"58168","inReplyTo":"xmqq35f2ysd9.fsf@gitster.g","subject":"[PATCH] doc: clarify rerere-autoupdate","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-03T20:59:15Z","receivedAt":"2022-08-03T21:01:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The \"--[no-]rerere-autoupdate\" option controls what happens _after_\nthe rerere mechanism kicks in to reuse recorded resolutions and does\nnot prevent from the rerere mechanism to trigger in the first place.\n\nIt is unclear in the current text if \"--no-rerere-autoupdate\" stops\nthe auto-resolution. Rewrite the sentence to clarify.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/rerere-options.txt | 9 +++++++--\n 1 file changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/rerere-options.txt b/Documentation/rerere-options.txt\nindex 8f4849e272..c3321ddea2 100644\n--- a/Documentation/rerere-options.txt\n+++ b/Documentation/rerere-options.txt\n@@ -1,4 +1,9 @@\n --rerere-autoupdate::\n --no-rerere-autoupdate::\n-\tAllow the rerere mechanism to update the index with the\n-\tresult of auto-conflict resolution if possible.\n+\tAfter the rerere mechanism reuses a recorded resolution on\n+\tthe current conflict to update the files in the working\n+\ttree, allow it to also update the index with the result of\n+\tresolution. `--no-rerere-autoupdate` is a good way to\n+\tdouble-check what `rerere` did and catch potential\n+\tmismerges, before committing the result to the index with a\n+\tseparate `git add`.\n-- \n2.37.1-482-g94c4bae67b\n\n"}]}