Re: What's cooking in git.git (Jan 2021, #02; Fri, 8)

Junio C Hamano [off-list ref] writes:

Here is how eventual merge of the topic into 'master' would resolve
conflicts.  The straight merge chain from master..seen would have
such a merge with the resolution shown below, but 'next' already has
a mismerge, so I'll be applying a single-parent commit to fix the
mismerge directly on 'next' (which I'll send in a separate message).

And this is the 'next' mismerge fix.

----- >8 ----- >8 ----- >8 ----- >8 ----- >8 -----
Subject: [PATCH] Fix mismerge of ds/maintenance-part-4 topic

At 1f98c859 (Merge branch 'ds/maintenance-part-4' into next, 2021-01-08)
the ds/maintenance-part-4 topic, which had an incorrect invocation
of internal "git config" when $(pwd) had ERE special characters, was
merged into the mainline that added "--fixed-value" feature to the
"git config" command to address this exact issue, and also changed
how test-tool stubbed out crontab calls.

The semantic conflicts for the latter have been fully resolved at
that commit, but four places that "git config" was invoked to look
for entries with the same value as "$(pwd)" were not adjusted to use
the "--fixed-value".  They should have been.

Helped-by: Emily Shaffer [off-list ref]
Helped-by: Taylor Blau [off-list ref]
Helped-by: Derrick Stolee [off-list ref]
Signed-off-by: Junio C Hamano <redacted>
---
 t/t7900-maintenance.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 2e0c8a4c31..1074009cc0 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh

@@ -487,7 +487,7 @@ test_expect_success 'start and stop macOS maintenance' '
 	GIT_TEST_MAINT_SCHEDULER=launchctl:./print-args git maintenance start &&
 
 	# start registers the repo
-	git config --get --global maintenance.repo "$(pwd)" &&
+	git config --get --global --fixed-value maintenance.repo "$(pwd)" &&
 
 	ls "$HOME/Library/LaunchAgents" >actual &&
 	cat >expect <<-\EOF &&

@@ -512,7 +512,7 @@ test_expect_success 'start and stop macOS maintenance' '
 	GIT_TEST_MAINT_SCHEDULER=launchctl:./print-args git maintenance stop &&
 
 	# stop does not unregister the repo
-	git config --get --global maintenance.repo "$(pwd)" &&
+	git config --get --global --fixed-value maintenance.repo "$(pwd)" &&
 
 	printf "bootout gui/[UID] $pfx/Library/LaunchAgents/org.git-scm.git.%s.plist\n" \
 		hourly daily weekly >expect &&

@@ -538,7 +538,7 @@ test_expect_success 'start and stop Windows maintenance' '
 	GIT_TEST_MAINT_SCHEDULER="schtasks:./print-args" git maintenance start &&
 
 	# start registers the repo
-	git config --get --global maintenance.repo "$(pwd)" &&
+	git config --get --global --fixed-value maintenance.repo "$(pwd)" &&
 
 	for frequency in hourly daily weekly
 	do

@@ -551,7 +551,7 @@ test_expect_success 'start and stop Windows maintenance' '
 	GIT_TEST_MAINT_SCHEDULER="schtasks:./print-args" git maintenance stop &&
 
 	# stop does not unregister the repo
-	git config --get --global maintenance.repo "$(pwd)" &&
+	git config --get --global --fixed-value maintenance.repo "$(pwd)" &&
 
 	printf "/delete /tn Git Maintenance (%s) /f\n" \
 		hourly daily weekly >expect &&

-- 
2.30.0-386-gfb533afdb4

On 1/15/2021 1:38 AM, Junio C Hamano wrote:

Junio C Hamano [off-list ref] writes:

quoted

Here is how eventual merge of the topic into 'master' would resolve
conflicts.  The straight merge chain from master..seen would have
such a merge with the resolution shown below, but 'next' already has
a mismerge, so I'll be applying a single-parent commit to fix the
mismerge directly on 'next' (which I'll send in a separate message).

And this is the 'next' mismerge fix.

----- >8 ----- >8 ----- >8 ----- >8 ----- >8 -----
Subject: [PATCH] Fix mismerge of ds/maintenance-part-4 topic

At 1f98c859 (Merge branch 'ds/maintenance-part-4' into next, 2021-01-08)
the ds/maintenance-part-4 topic, which had an incorrect invocation
of internal "git config" when $(pwd) had ERE special characters, was
merged into the mainline that added "--fixed-value" feature to the
"git config" command to address this exact issue, and also changed
how test-tool stubbed out crontab calls.

The semantic conflicts for the latter have been fully resolved at
that commit, but four places that "git config" was invoked to look
for entries with the same value as "$(pwd)" were not adjusted to use
the "--fixed-value".  They should have been.

Thanks for dealing with this fallout. Sorry for the mixup.

Helped-by: Emily Shaffer [off-list ref]

Would it be appropriate to convert this to a Co-authored-by?

Thanks,
-Stolee

Derrick Stolee [off-list ref] writes:

Thanks for dealing with this fallout. Sorry for the mixup.

quoted

Helped-by: Emily Shaffer [off-list ref]

Would it be appropriate to convert this to a Co-authored-by?

Or more like "Inspired-by-a-patch-by".  Also you three have about
the same amount of input and deserve credit.

Sorry, but it already is on 'next', which will be discarded in a few
months when the new cycle begins, so I am not sure if it is worth
reverting and reapplying with updated credit trailers.

FWIW, I ended up redoing the merge and did "checkout -m" to recreate
conflicts, and then looked for "config.*pwd" in the block of text
that came from the "part-4" topic and fixed them up manually.  I
then compared the result of this new merge with the earlier merge
that had the issue.  The resulting diff is what went into the patch
you are responding to.  It is good that we had Emily's input as a
comparison material to see that both of us independently touched the
same places to fix.

By the way, the merge into 'master' will directly reuse the conflict
resolution of e47c3632 (Merge branch 'ds/maintenance-part-4' into
jch, 2021-01-14) from the rerere database, without any separate
"merge fix" commit.

Thanks.

On Fri, Jan 15, 2021 at 11:44:50AM -0800, Junio C Hamano wrote:

Derrick Stolee [off-list ref] writes:

quoted

Thanks for dealing with this fallout. Sorry for the mixup.

quoted

Helped-by: Emily Shaffer [off-list ref]

Would it be appropriate to convert this to a Co-authored-by?

Or more like "Inspired-by-a-patch-by".  Also you three have about
the same amount of input and deserve credit.

Sorry, but it already is on 'next', which will be discarded in a few
months when the new cycle begins, so I am not sure if it is worth
reverting and reapplying with updated credit trailers.

FWIW, I ended up redoing the merge and did "checkout -m" to recreate
conflicts, and then looked for "config.*pwd" in the block of text
that came from the "part-4" topic and fixed them up manually.  I
then compared the result of this new merge with the earlier merge
that had the issue.  The resulting diff is what went into the patch
you are responding to.  It is good that we had Emily's input as a
comparison material to see that both of us independently touched the
same places to fix.

Thanks for the action on this, glad to see it resolved in 'next' when I
pull today.

 - Emily