[PATCH v2] Add note that conflict resolution is still performed

[PATCH v2] Add note that conflict resolution is still performed

[Matthias Beyer]

We should note that conflict resolution is still performed, even if
`--no-rerere-autoupdate` is specified, to make sure users do not get
confused by the setting and assume this disables rerere conflict
resultion altogether.

CC: Phillip Wood <phillip.wood@dunelm.org.uk>
CC: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
---
 Documentation/git-cherry-pick.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index 78dcc9171f..b92aa1f9da 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -160,6 +160,10 @@ effect to your index in a row.
 --no-rerere-autoupdate::
 	Allow the rerere mechanism to update the index with the
 	result of auto-conflict resolution if possible.
+	The `--no-rerere-autoupdate` option does not prevent the conflict
+	resolution, but prevents the index from being updated. This gives the
+	user a chance for a final sanity check before using linkgit:git-add[1]
+	to add the result.
 
 SEQUENCER SUBCOMMANDS
 ---------------------
-- 
2.36.0

Re: [PATCH v2] Add note that conflict resolution is still performed

[Junio C Hamano]

Matthias Beyer <mail@beyermatthias.de> writes:

> We should note that conflict resolution is still performed, even if
> `--no-rerere-autoupdate` is specified, to make sure users do not get
> confused by the setting and assume this disables rerere conflict
> resultion altogether.
>
> CC: Phillip Wood <phillip.wood@dunelm.org.uk>
> CC: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
> ---
>  Documentation/git-cherry-pick.txt | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
> index 78dcc9171f..b92aa1f9da 100644
> --- a/Documentation/git-cherry-pick.txt
> +++ b/Documentation/git-cherry-pick.txt
> @@ -160,6 +160,10 @@ effect to your index in a row.
>  --no-rerere-autoupdate::
>  	Allow the rerere mechanism to update the index with the
>  	result of auto-conflict resolution if possible.
> +	The `--no-rerere-autoupdate` option does not prevent the conflict
> +	resolution, but prevents the index from being updated. This gives the
> +	user a chance for a final sanity check before using linkgit:git-add[1]
> +	to add the result.

$ git grep -l "^--rerere-autoupdate::" Documentation
Documentation/git-am.txt
Documentation/git-cherry-pick.txt
Documentation/git-merge.txt
Documentation/git-rebase.txt
Documentation/git-revert.txt

I made a cursory scan of these and I suspect the existing two-line
description are shread among all of them.  At this point it may make
sense to split the description to a separate file to be included by
these places (the attached patch may be a starting point) in a
patch, and then follow up with the text change in a follow-up patch.

A tangent that may be worth thinking about, that does not have to be
part of this topic (as it probably will involve code change).

It makes sense that "--no-rerere-autoupdate" does not disable the
"rerere" mechanism (when it is enabled, of course), because it makes
sense to reuse recorded resolution without updating the index with
the result.

However, it may make sense to have "--rerere-autoupdate" option to
enable the "rerere" mechanism when it is disabled, because with
"rerere" disabled, there is nothing to auto-update.

Anyway, here is the preliminary restructuring patch.

----- >8 --------- >8 --------- >8 --------- >8 --------- >8 -----
Subject: [PATCH] doc: consolidate --rerere-autoupdate description

The `--rerere-autoupdate` option is shared across 5 commands, and
are described the same way because it works exactly the same way in
these commands.

Create a separate file and include it from the help pages for these
commands, so that we can improve the description at one place to
improve all of them at once, and keep them in sync.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-am.txt          | 5 +----
 Documentation/git-cherry-pick.txt | 5 +----
 Documentation/git-merge.txt       | 5 +----
 Documentation/git-rebase.txt      | 5 +----
 Documentation/git-revert.txt      | 5 +----
 Documentation/rerere-options.txt  | 4 ++++
 6 files changed, 9 insertions(+), 20 deletions(-)

diff --git c/Documentation/git-am.txt w/Documentation/git-am.txt
index 09107fb106..320da6c4f7 100644
--- c/Documentation/git-am.txt
+++ w/Documentation/git-am.txt
@@ -112,10 +112,7 @@ default.   You can use `--no-utf8` to override this.
 	am.threeWay configuration variable. For more information,
 	see am.threeWay in linkgit:git-config[1].
 
---rerere-autoupdate::
---no-rerere-autoupdate::
-	Allow the rerere mechanism to update the index with the
-	result of auto-conflict resolution if possible.
+include::rerere-options.txt[]
 
 --ignore-space-change::
 --ignore-whitespace::
diff --git c/Documentation/git-cherry-pick.txt w/Documentation/git-cherry-pick.txt
index 78dcc9171f..1e8ac9df60 100644
--- c/Documentation/git-cherry-pick.txt
+++ w/Documentation/git-cherry-pick.txt
@@ -156,10 +156,7 @@ effect to your index in a row.
 	Pass the merge strategy-specific option through to the
 	merge strategy.  See linkgit:git-merge[1] for details.
 
---rerere-autoupdate::
---no-rerere-autoupdate::
-	Allow the rerere mechanism to update the index with the
-	result of auto-conflict resolution if possible.
+include::rerere-options.txt[]
 
 SEQUENCER SUBCOMMANDS
 ---------------------
diff --git c/Documentation/git-merge.txt w/Documentation/git-merge.txt
index 3125473cc1..fee1dc2df2 100644
--- c/Documentation/git-merge.txt
+++ w/Documentation/git-merge.txt
@@ -90,10 +90,7 @@ invocations. The automated message can include the branch description.
 If `--log` is specified, a shortlog of the commits being merged
 will be appended to the specified message.
 
---rerere-autoupdate::
---no-rerere-autoupdate::
-	Allow the rerere mechanism to update the index with the
-	result of auto-conflict resolution if possible.
+include::rerere-options.txt[]
 
 --overwrite-ignore::
 --no-overwrite-ignore::
diff --git c/Documentation/git-rebase.txt w/Documentation/git-rebase.txt
index a872ab0fbd..ff0b643ec0 100644
--- c/Documentation/git-rebase.txt
+++ w/Documentation/git-rebase.txt
@@ -376,10 +376,7 @@ See also INCOMPATIBLE OPTIONS below.
 +
 See also INCOMPATIBLE OPTIONS below.
 
---rerere-autoupdate::
---no-rerere-autoupdate::
-	Allow the rerere mechanism to update the index with the
-	result of auto-conflict resolution if possible.
+include::rerere-options.txt[]
 
 -S[<keyid>]::
 --gpg-sign[=<keyid>]::
diff --git c/Documentation/git-revert.txt w/Documentation/git-revert.txt
index 8463fe9cf7..0105a54c1a 100644
--- c/Documentation/git-revert.txt
+++ w/Documentation/git-revert.txt
@@ -112,10 +112,7 @@ effect to your index in a row.
 	Pass the merge strategy-specific option through to the
 	merge strategy.  See linkgit:git-merge[1] for details.
 
---rerere-autoupdate::
---no-rerere-autoupdate::
-	Allow the rerere mechanism to update the index with the
-	result of auto-conflict resolution if possible.
+include::rerere-options.txt[]
 
 --reference::
 	Instead of starting the body of the log message with "This
diff --git c/Documentation/rerere-options.txt w/Documentation/rerere-options.txt
new file mode 100644
index 0000000000..8f4849e272
--- /dev/null
+++ w/Documentation/rerere-options.txt
@@ -0,0 +1,4 @@
+--rerere-autoupdate::
+--no-rerere-autoupdate::
+	Allow the rerere mechanism to update the index with the
+	result of auto-conflict resolution if possible.

Re: [PATCH v2] Add note that conflict resolution is still performed

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> A tangent that may be worth thinking about, that does not have to be
> part of this topic (as it probably will involve code change).
>
> It makes sense that "--no-rerere-autoupdate" does not disable the
> "rerere" mechanism (when it is enabled, of course), because it makes
> sense to reuse recorded resolution without updating the index with
> the result.
>
> However, it may make sense to have "--rerere-autoupdate" option to
> enable the "rerere" mechanism when it is disabled, because with
> "rerere" disabled, there is nothing to auto-update.

I think the damage to the code to implement the above may not be too
bad.  Here is an illustration (not even compile tested) of the idea:

 - Earlier, after reading the config, we asked is_rerere_enabled()
   and let its logic decide if rerere is to be used, giving no
   influence to the incoming "flags" parameter.  Then the value of
   rerere_autoupdate read from the config is further adjusted by the
   value in "flags" parameter that relays --[no-]rerere-autoupdate
   from the command line.

 - Instead, after reading the config, we can tweak it with the
   "flags", and flip rerere_enabled to true if autoupdate is
   enabled, before making a call to is_rerere_enabled().

I do not think we have a test that insists that rerere does not kick
in in a repository where rerere is disabled and the command line
asks for "--rerere-autoupdate", so I suspect that with this patch, I
would not be surprised if no test breaks.  If we think this takes us
in a good direction, we should add a few tests to make sure at least
the following two:

 - In a repository with rerere disabled, running a command with the
   "--rerere-autoupdate" option will enable rerere for that single
   invocation of the command.

 - In a repository with rerere enabled, running a command with the
   "--no-rerere-autoupdate" option does not disable rerere for that
   single invocation of the command.

---

 rerere.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git c/rerere.c w/rerere.c
index 876ab435da..16d3e865e6 100644
--- c/rerere.c
+++ w/rerere.c
@@ -872,11 +872,13 @@ int setup_rerere(struct repository *r, struct string_list *merge_rr, int flags)
 	int fd;
 
 	git_rerere_config();
+	if (flags & (RERERE_AUTOUPDATE|RERERE_NOAUTOUPDATE))
+		rerere_autoupdate = !!(flags & RERERE_AUTOUPDATE);
+	if (rerere_autoupdate)
+		rerere_enabled = 1;
 	if (!is_rerere_enabled())
 		return -1;
 
-	if (flags & (RERERE_AUTOUPDATE|RERERE_NOAUTOUPDATE))
-		rerere_autoupdate = !!(flags & RERERE_AUTOUPDATE);
 	if (flags & RERERE_READONLY)
 		fd = 0;
 	else

[PATCH] doc: clarify rerere-autoupdate

[Junio C Hamano]

The "--[no-]rerere-autoupdate" option controls what happens _after_
the rerere mechanism kicks in to reuse recorded resolutions and does
not prevent from the rerere mechanism to trigger in the first place.

It is unclear in the current text if "--no-rerere-autoupdate" stops
the auto-resolution.  Rewrite the sentence to clarify.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/rerere-options.txt | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/rerere-options.txt b/Documentation/rerere-options.txt
index 8f4849e272..c3321ddea2 100644
--- a/Documentation/rerere-options.txt
+++ b/Documentation/rerere-options.txt
@@ -1,4 +1,9 @@
 --rerere-autoupdate::
 --no-rerere-autoupdate::
-	Allow the rerere mechanism to update the index with the
-	result of auto-conflict resolution if possible.
+	After the rerere mechanism reuses a recorded resolution on
+	the current conflict to update the files in the working
+	tree, allow it to also update the index with the result of
+	resolution.  `--no-rerere-autoupdate` is a good way to
+	double-check what `rerere` did and catch potential
+	mismerges, before committing the result to the index with a
+	separate `git add`.
-- 
2.37.1-482-g94c4bae67b