* Re: [PATCH] ci: avoid building from the same commit in parallel
@ 2023-08-23 8:42 5% ` Johannes Schindelin
0 siblings, 0 replies; 24+ results
From: Johannes Schindelin @ 2023-08-23 8:42 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, rsbecker, 'Jeff King', 'Taylor Blau',
'Andy Koppe'
[-- Attachment #1: Type: text/plain, Size: 4624 bytes --]
Hi Junio,
On Tue, 22 Aug 2023, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Right, we'd need that `concurrency: ${{ github.sha }}` attribute on
> > the `config` job.
>
> That was my first thought, but I am not sure how it would work.
>
> Doesn't skip-if-redundant grab the workflow runs that have succeeded
> and then see if one for the same commit already exists? If you used
> concurrency on the 'config', what gets serialized between two jobs
> for the same commit is only the 'config' phase, so 'master' may wait
> starting (because 'config' is what everybody else 'needs' it) while
> 'config' phase of 'main' runs, and then when it gets to the turn of
> 'config' phase of 'master', it would not find the run for the same
> commit being done for 'main' completed yet, would it?
Yes, that's true.
But there is a silver lining: the `concurrency` can not only be specified
on the job level, but also on the workflow run level.
I tested this, and present the corresponding patch at the end of this
mail.
> > BTW there is another caveat. According to the documentation, if a job
> > is queued while another job is already queued, that other job is
> > canceled in favor of the latest one.
>
> Yes, that was the impression I got; your second one will wait (so
> you need a working skip-if-redundant to turn it into noop), but the
> third and subsequent ones are discarded without starting, which
> unfortunately is what we may want to see happen.
It's actually the last one that is still pending, while the intermediate
ones will be canceled before they are started (see also the attached
screenshot). The message that is shown in the web UI reads like this:
𐤈 CI: .github#L1
Canceling since a higher priority waiting request for '<SHA>' exists
See https://github.com/dscho/git/actions/runs/5948890677 for an example.
Here is the patch:
-- snipsnap --
From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 21 Aug 2023 17:31:26 -0700
Subject: [PATCH] ci: avoid building from the same commit in parallel
At times, we may need to push the same commit to multiple branches
in the same push. Rewinding 'next' to rebuild on top of 'master'
soon after a release is such an occasion. Making sure 'main' stays
in sync with 'master' to help those who expect that primary branch
of the project is named either of these is another.
We already use the branch name as a "concurrency group" key, but
that does not address the situation illustrated above.
Let's introduce another `concurrency` attribute, using the commit
hash as the concurrency group key, on the workflow run level, to
address this. This will hold any workflow run in the queued state
when there is already a workflow run targeting the same commit,
until that latter run completed. The `skip-if-redundant` check of
the second run will then have a chance to see whether the first
run succeeded.
The only caveat with this strategy is that only one workflow run
will be kept in the queued state by the `concurrency` feature: if
another run targeting the same commit is triggered, the
previously-queued run will be canceled. Considering the benefit,
this seems the smaller price to pay than to overload Git's build
agent pool with undesired workflow runs.
Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
.github/workflows/main.yml | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 30492eacddf..1739a0278dc 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -5,6 +5,19 @@ on: [push, pull_request]
env:
DEVELOPER: 1
+# If more than one workflow run is triggered for the very same commit hash
+# (which happens when multiple branches pointing to the same commit), only
+# the first one is allowed to run, the second will be kept in the "queued"
+# state. This allows a successful completion of the first run to be reused
+# in the second run via the `skip-if-redundant` logic in the `config` job.
+#
+# The only caveat is that if a workflow run is triggered for the same commit
+# hash that another run is already being held, that latter run will be
+# canceled. For more details about the `concurrency` attribute, see:
+# https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#concurrency
+concurrency:
+ group: ${{ github.sha }}
+
jobs:
ci-config:
name: config
--
2.42.0.rc2.windows.1
[-- Attachment #2: Type: image/png, Size: 43278 bytes --]
^ permalink raw reply related [relevance 5%]
* [PATCH] ci: avoid building from the same commit in parallel
@ 2023-08-22 0:31 6% ` Junio C Hamano
0 siblings, 1 reply; 24+ results
From: Junio C Hamano @ 2023-08-22 0:31 UTC (permalink / raw)
To: git
Cc: rsbecker, 'Jeff King', 'Taylor Blau',
'Andy Koppe', Johannes Schindelin
At times, we may need to push the same commit to multiple branches
in the same push. Rewinding 'next' to rebuild on top of 'master'
soon after a release is such an occasion. Making sure 'main' stays
in sync with 'master' to help those who expect that primary branch
of the project is named either of these is another.
We used to use the branch name as the "concurrency group" key, but
by switching to use the commit object name would make sure the
builds for the same commit would happen serially, and by the time
the second job becomes ready to run, the first job's outcome would
be available and mking it unnecessary to run the second job.
Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* There are tons of concurrency groups defined, but as a trial
change, here is to cover the "regular" matrix that consumes the
most resources (linux-asan-ubsan is the worst culprit, it seems).
.github/workflows/main.yml | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 30492eacdd..27b151aadf 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -240,8 +240,7 @@ jobs:
needs: ci-config
if: needs.ci-config.outputs.enabled == 'yes'
concurrency:
- group: ${{ matrix.vector.jobname }}-${{ matrix.vector.pool }}-${{ github.ref }}
- cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}
+ group: ${{ matrix.vector.jobname }}-${{ matrix.vector.pool }}-${{ github.sha }}
strategy:
fail-fast: false
matrix:
--
2.42.0
^ permalink raw reply related [relevance 6%]
* Re: [PATCH v9 4/6] notes.c: introduce '--separator=<paragraph-break>' option
@ 2023-05-10 19:19 6% ` Kristoffer Haugsbakk
0 siblings, 0 replies; 24+ results
From: Kristoffer Haugsbakk @ 2023-05-10 19:19 UTC (permalink / raw)
To: Teng Long; +Cc: avarab, git, Junio C Hamano, sunshine, tenglong.tl
I realize that this series is going to be merged to `master`.[1] I was
trying out this new change since I might have some use for it when the
next version is released.
On Fri, Apr 28, 2023, at 11:23, Teng Long wrote:
> From: Teng Long <dyroneteng@gmail.com>
>
> When adding new notes or appending to an existing notes, we will
> insert a blank line between the paragraphs, like:
The test case[2] `append: specify an empty separator` demonstrates that
`--separator=""` is the same as the default behavior, namely to add a
blank line. It has (according to the commit messages) been like this
since v5 of this patch.[3]
v4 of this patch special-cased `--separator=""` to mean “no
separator”. And this was the same behavior as the original
`--no-blankline`,[4] which eventually mutated into `--separator`.
Why was this changed to act the same as the default behavior (add a
blank line)? I can’t seem to find a note for it on the cover letter of
v5 or in the relevant replies.
It seemed that v4 of this patch (with special-cased empty argument) was
perhaps based on Eric Sunshine’s suggestion:[5]
> Taking a step back, perhaps think of this in terms of "separator". The
> default behavior is to insert "\n" as a separator between notes. If
> you add a --separator option, then users could supply their own
> separator, such as "----\n" or, in your case, "" to suppress the blank
> line.
(And then reiterated in a v4 email [6])
Was the idea perhaps to eventually (separately) add a separate option
which functions like `--each-message-is-line-not-paragraph`, like what
was mentioned in [7]?
Maybe I’ve missed something. (I probably have.)
[1] https://lore.kernel.org/git/xmqqpm785erg.fsf@gitster.g/T/#md9b20801457c3eb24dc0e793f5dfbeae2f2707fd
[2] On `next`, 74a8c73209 (Sync with 'master', 2023-05-09)
[3] https://lore.kernel.org/git/a74c96d6dd23f2f1df6d3492093f3fd27451e24c.1676551077.git.dyroneteng@gmail.com/
Commit message on v4:
> * --separator='': we specify an empty separator which means will
> append the message directly without inserting any separator at
> first.
Commit message on v5:
> * --separator='': we specify an empty separator which has the same
> behavour with --separator='\n' and or not specified the option.
[4] https://lore.kernel.org/git/20221013055654.39628-1-tenglong.tl@alibaba-inc.com/
[5] https://lore.kernel.org/git/CAPig+cRcezSp4Rqt1Y9bD-FT6+7b0g9qHfbGRx65AOnw2FQXKg@mail.gmail.com/
[6] https://lore.kernel.org/git/CAPig+cSF7Fp3oM4TRU1QbiSzTeKNd1qGtqU7goPc1r-p4g8mkg@mail.gmail.com/
[7] https://lore.kernel.org/git/xmqqh6yh3nk4.fsf@gitster.g/
--
Kristoffer Haugsbakk
^ permalink raw reply [relevance 6%]
* Re: [PATCH 0/2] Documentation/howto/maintain-git.txt: a pair of bugfixes
@ 2022-11-01 22:23 6% ` Junio C Hamano
0 siblings, 0 replies; 24+ results
From: Junio C Hamano @ 2022-11-01 22:23 UTC (permalink / raw)
To: Jeff King; +Cc: Taylor Blau, git
Jeff King <peff@peff.net> writes:
> Why not have a single file with all of the topics, with two "###"
> markers?
Mostly because jch and seen are built in two distinct steps, and I
need to be able to rearrange lines in the redo-jch.sh file either in
the editor or by running "Meta/redo-jch.sh -u", without touching
what is in redo-seen.sh file (meaning: what comes after your final
"###" cut lines).
After replacing the existing topic branches and creating new ones
for new topics, decide which topics to merge down to master and next
and edit redo-jch.sh (while looking at the what's cooking report
and/or output from "Meta/cook -w"). If we merge some to 'master',
then tentatively write '###' marker before these topics. Then
$ git checkout --detach master
$ Meta/redo-jch.sh -c1
$ edit RelNotes
$ git commit -a -s -m "${N}th batch"
$ Meta/round -coccinelle ;# and other tests as necessary
$ Meta/Reintegrate master.. >P
$ git checkout master
$ sh P && git commit --amend --no-edit --reset-author
will rebuild the 'master' (but this is needed only when 'master'
gets updated in the day's integration). After that
$ git checkout -B jch master
$ Meta/redo-jch.sh
and use
$ git branch --no-merged jch --no-merged seen --sort=-committerdate '??/*'
to see any topics that are not in last 'seen' and 'jch' we just
rebuilt. They are either replaced in 'seen' or new ones. Then
merge some of them that you are more confident than others to 'jch'
and test and update the redo-jch.sh script.
$ git merge xy/xxy
$ git merge fr/otz
...
$ Meta/round ;# or whatever tests that are appropriate
$ Meta/redo-jch.sh -u
This "-u"pdate step can be done without disturbing what should later
build on top for 'seen' by having the script separately.
When day's integration contains an update to 'next', there is
another step here:
$ git checkout --detach next
$ Meta/redo-jch.sh -c1
$ git merge -m "Sync with 'master'" --no-log master
$ Meta/round ;# or whatever tests that are appropriate
$ git diff 'jch^{/^### match next}' ;# must be empty
After that, queue new topics that are more questionable on 'seen',
and the rest:
$ git checkotu -B seen
$ git merge ni/tfol
$ git merge yo/min
...
$ Meta/round ;# or whatever tests that are appropriate
... here, new topics may be worse than what I initially thought
... that they need to be ejected from 'seen'
$ git reset --hard jch && git merge yo/min ... && Meta/round
And once satisfied with the new topics, queue the remainder.
$ Meta/redo-seen.sh
$ Meta/round ;# or whatever tests that are appropriate
$ Meta/redo-seen.sh -u ;# finally
Being able to rebuild the redo-* script for only the 'jch' part,
independent from the 'seen' part, is quite essential in the
workflow, because I may not yet know how day's 'seen' would look
like, when I am recording the topics and integration order for 'jch'.
^ permalink raw reply [relevance 6%]
* Re: [PATCH] merge-ort: make informational messages from recursive merges clearer
2022-03-02 2:32 0% ` Elijah Newren
2022-03-02 4:21 0% ` Elijah Newren
@ 2022-03-02 6:53 0% ` Junio C Hamano
1 sibling, 0 replies; 24+ results
From: Junio C Hamano @ 2022-03-02 6:53 UTC (permalink / raw)
To: Elijah Newren; +Cc: Elijah Newren via GitGitGadget, Git Mailing List
Elijah Newren <newren@gmail.com> writes:
>> > Yup, I found that the messages on inner conflicts, especially when
>> > they "cancel out" at the outer merge, are mostly noise that carries
>> > very little useful information (by being noisy, the user gets a sense
>> > of how complex the histories being merged are). Reducing the default
>> > messaging level would probably be a good idea.
>>
>> Here is what I just had to scroll through to update 'next' by
>> merging back 'master', only to grab the updates to the release
>> notes. Needless to say, this would have been somewhat baffling
>> if I didn't know to expect it.
>>
>> It would be good to squelch it before we hear another complaints
>> from old-timer power users ;-)
>
> I'll submit a patch soon.
>
>>
>> $ git merge -m 'Sync with master' --no-log master
>> From inner merge: Auto-merging blame.c
>> From inner merge: Auto-merging builtin/am.c
>> From inner merge: Auto-merging builtin/blame.c
>> From inner merge: Auto-merging builtin/clone.c
>> From inner merge: Auto-merging builtin/clone.c
>> From inner merge: Auto-merging builtin/commit.c
>> From inner merge: Auto-merging builtin/fetch.c
>> From inner merge: Auto-merging builtin/fetch.c
>> From inner merge: Auto-merging builtin/grep.c
>> From inner merge: Auto-merging builtin/hash-object.c
>> From inner merge: Auto-merging builtin/log.c
>> From inner merge: Auto-merging builtin/log.c
>> From inner merge: Auto-merging builtin/pack-objects.c
>> From inner merge: Auto-merging builtin/pull.c
>> From inner merge: Auto-merging builtin/pull.c
>> From inner merge: Auto-merging builtin/rebase.c
>> From inner merge: Auto-merging builtin/rebase.c
>> From inner merge: Auto-merging builtin/reflog.c
>> From inner merge: CONFLICT (content): Merge conflict in builtin/reflog.c
>> Auto-merging builtin/reflog.c
Thanks. I think "From inner merge: " can be removed even when we
emit these messages, as the indentation would make it clear that the
indented ones are different from the outermost ones.
^ permalink raw reply [relevance 0%]
* Re: [PATCH] merge-ort: make informational messages from recursive merges clearer
2022-03-02 2:32 0% ` Elijah Newren
@ 2022-03-02 4:21 0% ` Elijah Newren
2022-03-02 6:53 0% ` Junio C Hamano
1 sibling, 0 replies; 24+ results
From: Elijah Newren @ 2022-03-02 4:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Elijah Newren via GitGitGadget, Git Mailing List
On Tue, Mar 1, 2022 at 6:32 PM Elijah Newren <newren@gmail.com> wrote:
>
> On Mon, Feb 28, 2022 at 10:44 PM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Junio C Hamano <gitster@pobox.com> writes:
> >
> > >> One other thing to note here, that I didn't notice until typing up this
> > >> commit message, is that merge-recursive does not print any messages from
> > >> the inner merges by default; the extra verbosity has to be requested.
> > >> merge-ort currently has no verbosity controls and always prints these.
> > >> We may also want to change that, but for now, just make the output
> > >> clearer with these extra markings and indentation.
> > >
> > > Yup, I found that the messages on inner conflicts, especially when
> > > they "cancel out" at the outer merge, are mostly noise that carries
> > > very little useful information (by being noisy, the user gets a sense
> > > of how complex the histories being merged are). Reducing the default
> > > messaging level would probably be a good idea.
> >
> > Here is what I just had to scroll through to update 'next' by
> > merging back 'master', only to grab the updates to the release
> > notes. Needless to say, this would have been somewhat baffling
> > if I didn't know to expect it.
> >
> > It would be good to squelch it before we hear another complaints
> > from old-timer power users ;-)
>
> I'll submit a patch soon.
https://lore.kernel.org/git/pull.1167.git.1646194761463.gitgitgadget@gmail.com/,
which would drop all the "From inner merge:" lines below.
>
> >
> > $ git merge -m 'Sync with master' --no-log master
> > From inner merge: Auto-merging blame.c
> > From inner merge: Auto-merging builtin/am.c
> > From inner merge: Auto-merging builtin/blame.c
> > From inner merge: Auto-merging builtin/clone.c
> > From inner merge: Auto-merging builtin/clone.c
> > From inner merge: Auto-merging builtin/commit.c
> > From inner merge: Auto-merging builtin/fetch.c
> > From inner merge: Auto-merging builtin/fetch.c
> > From inner merge: Auto-merging builtin/grep.c
> > From inner merge: Auto-merging builtin/hash-object.c
> > From inner merge: Auto-merging builtin/log.c
> > From inner merge: Auto-merging builtin/log.c
> > From inner merge: Auto-merging builtin/pack-objects.c
> > From inner merge: Auto-merging builtin/pull.c
> > From inner merge: Auto-merging builtin/pull.c
> > From inner merge: Auto-merging builtin/rebase.c
> > From inner merge: Auto-merging builtin/rebase.c
> > From inner merge: Auto-merging builtin/reflog.c
> > From inner merge: CONFLICT (content): Merge conflict in builtin/reflog.c
> > Auto-merging builtin/reflog.c
> > From inner merge: Auto-merging builtin/reset.c
> > From inner merge: Auto-merging builtin/sparse-checkout.c
> > From inner merge: Auto-merging builtin/sparse-checkout.c
> > From inner merge: Auto-merging builtin/submodule--helper.c
> > From inner merge: Auto-merging builtin/submodule--helper.c
> > From inner merge: CONFLICT (content): Merge conflict in builtin/submodule--helper.c
> > Auto-merging builtin/submodule--helper.c
> > From inner merge: Auto-merging builtin/worktree.c
> > From inner merge: Auto-merging cache.h
> > From inner merge: Auto-merging config.c
> > From inner merge: Auto-merging config.h
> > From inner merge: Auto-merging diff-merges.c
> > From inner merge: Auto-merging diff.c
> > From inner merge: Auto-merging git.c
> > From inner merge: Auto-merging gpg-interface.c
> > From inner merge: Auto-merging grep.c
> > From inner merge: Auto-merging grep.c
> > From inner merge: Auto-merging notes-merge.c
> > From inner merge: Auto-merging object-name.c
> > From inner merge: Auto-merging pack-bitmap-write.c
> > From inner merge: Auto-merging parse-options.c
> > From inner merge: CONFLICT (content): Merge conflict in parse-options.c
> > From inner merge: Auto-merging parse-options.h
> > From inner merge: CONFLICT (content): Merge conflict in parse-options.h
> > From inner merge: Auto-merging refs.c
> > From inner merge: Auto-merging revision.c
> > From inner merge: Auto-merging sequencer.c
> > From inner merge: Auto-merging sequencer.c
> > From inner merge: Auto-merging sparse-index.c
> > From inner merge: Auto-merging submodule-config.c
> > From inner merge: Auto-merging t/t1091-sparse-checkout-builtin.sh
> > From inner merge: CONFLICT (content): Merge conflict in t/t1091-sparse-checkout-builtin.sh
> > Auto-merging t/t1091-sparse-checkout-builtin.sh
> > From inner merge: Auto-merging t/t1512-rev-parse-disambiguation.sh
> > From inner merge: Auto-merging t/t4202-log.sh
> > From inner merge: Auto-merging t/t4202-log.sh
> > From inner merge: Auto-merging t/t4202-log.sh
> > From inner merge: Auto-merging t/t4202-log.sh
> > From inner merge: Auto-merging t/t4202-log.sh
> > From inner merge: Auto-merging t/t5316-pack-delta-depth.sh
> > From inner merge: Auto-merging t/t6120-describe.sh
> > From inner merge: Auto-merging t/t6120-describe.sh
> > From inner merge: Auto-merging worktree.c
> > Merge made by the 'ort' strategy.
> > Documentation/RelNotes/2.36.0.txt | 29 +++++++++++++++++++++++++++++
> > 1 file changed, 29 insertions(+)
> >
> >
^ permalink raw reply [relevance 0%]
* Re: [PATCH] merge-ort: make informational messages from recursive merges clearer
2022-03-01 6:44 5% ` Junio C Hamano
@ 2022-03-02 2:32 0% ` Elijah Newren
2022-03-02 4:21 0% ` Elijah Newren
2022-03-02 6:53 0% ` Junio C Hamano
0 siblings, 2 replies; 24+ results
From: Elijah Newren @ 2022-03-02 2:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Elijah Newren via GitGitGadget, Git Mailing List
On Mon, Feb 28, 2022 at 10:44 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> >> One other thing to note here, that I didn't notice until typing up this
> >> commit message, is that merge-recursive does not print any messages from
> >> the inner merges by default; the extra verbosity has to be requested.
> >> merge-ort currently has no verbosity controls and always prints these.
> >> We may also want to change that, but for now, just make the output
> >> clearer with these extra markings and indentation.
> >
> > Yup, I found that the messages on inner conflicts, especially when
> > they "cancel out" at the outer merge, are mostly noise that carries
> > very little useful information (by being noisy, the user gets a sense
> > of how complex the histories being merged are). Reducing the default
> > messaging level would probably be a good idea.
>
> Here is what I just had to scroll through to update 'next' by
> merging back 'master', only to grab the updates to the release
> notes. Needless to say, this would have been somewhat baffling
> if I didn't know to expect it.
>
> It would be good to squelch it before we hear another complaints
> from old-timer power users ;-)
I'll submit a patch soon.
>
> $ git merge -m 'Sync with master' --no-log master
> From inner merge: Auto-merging blame.c
> From inner merge: Auto-merging builtin/am.c
> From inner merge: Auto-merging builtin/blame.c
> From inner merge: Auto-merging builtin/clone.c
> From inner merge: Auto-merging builtin/clone.c
> From inner merge: Auto-merging builtin/commit.c
> From inner merge: Auto-merging builtin/fetch.c
> From inner merge: Auto-merging builtin/fetch.c
> From inner merge: Auto-merging builtin/grep.c
> From inner merge: Auto-merging builtin/hash-object.c
> From inner merge: Auto-merging builtin/log.c
> From inner merge: Auto-merging builtin/log.c
> From inner merge: Auto-merging builtin/pack-objects.c
> From inner merge: Auto-merging builtin/pull.c
> From inner merge: Auto-merging builtin/pull.c
> From inner merge: Auto-merging builtin/rebase.c
> From inner merge: Auto-merging builtin/rebase.c
> From inner merge: Auto-merging builtin/reflog.c
> From inner merge: CONFLICT (content): Merge conflict in builtin/reflog.c
> Auto-merging builtin/reflog.c
> From inner merge: Auto-merging builtin/reset.c
> From inner merge: Auto-merging builtin/sparse-checkout.c
> From inner merge: Auto-merging builtin/sparse-checkout.c
> From inner merge: Auto-merging builtin/submodule--helper.c
> From inner merge: Auto-merging builtin/submodule--helper.c
> From inner merge: CONFLICT (content): Merge conflict in builtin/submodule--helper.c
> Auto-merging builtin/submodule--helper.c
> From inner merge: Auto-merging builtin/worktree.c
> From inner merge: Auto-merging cache.h
> From inner merge: Auto-merging config.c
> From inner merge: Auto-merging config.h
> From inner merge: Auto-merging diff-merges.c
> From inner merge: Auto-merging diff.c
> From inner merge: Auto-merging git.c
> From inner merge: Auto-merging gpg-interface.c
> From inner merge: Auto-merging grep.c
> From inner merge: Auto-merging grep.c
> From inner merge: Auto-merging notes-merge.c
> From inner merge: Auto-merging object-name.c
> From inner merge: Auto-merging pack-bitmap-write.c
> From inner merge: Auto-merging parse-options.c
> From inner merge: CONFLICT (content): Merge conflict in parse-options.c
> From inner merge: Auto-merging parse-options.h
> From inner merge: CONFLICT (content): Merge conflict in parse-options.h
> From inner merge: Auto-merging refs.c
> From inner merge: Auto-merging revision.c
> From inner merge: Auto-merging sequencer.c
> From inner merge: Auto-merging sequencer.c
> From inner merge: Auto-merging sparse-index.c
> From inner merge: Auto-merging submodule-config.c
> From inner merge: Auto-merging t/t1091-sparse-checkout-builtin.sh
> From inner merge: CONFLICT (content): Merge conflict in t/t1091-sparse-checkout-builtin.sh
> Auto-merging t/t1091-sparse-checkout-builtin.sh
> From inner merge: Auto-merging t/t1512-rev-parse-disambiguation.sh
> From inner merge: Auto-merging t/t4202-log.sh
> From inner merge: Auto-merging t/t4202-log.sh
> From inner merge: Auto-merging t/t4202-log.sh
> From inner merge: Auto-merging t/t4202-log.sh
> From inner merge: Auto-merging t/t4202-log.sh
> From inner merge: Auto-merging t/t5316-pack-delta-depth.sh
> From inner merge: Auto-merging t/t6120-describe.sh
> From inner merge: Auto-merging t/t6120-describe.sh
> From inner merge: Auto-merging worktree.c
> Merge made by the 'ort' strategy.
> Documentation/RelNotes/2.36.0.txt | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
>
^ permalink raw reply [relevance 0%]
* Re: [PATCH] merge-ort: make informational messages from recursive merges clearer
@ 2022-03-01 6:44 5% ` Junio C Hamano
2022-03-02 2:32 0% ` Elijah Newren
0 siblings, 1 reply; 24+ results
From: Junio C Hamano @ 2022-03-01 6:44 UTC (permalink / raw)
To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren
Junio C Hamano <gitster@pobox.com> writes:
>> One other thing to note here, that I didn't notice until typing up this
>> commit message, is that merge-recursive does not print any messages from
>> the inner merges by default; the extra verbosity has to be requested.
>> merge-ort currently has no verbosity controls and always prints these.
>> We may also want to change that, but for now, just make the output
>> clearer with these extra markings and indentation.
>
> Yup, I found that the messages on inner conflicts, especially when
> they "cancel out" at the outer merge, are mostly noise that carries
> very little useful information (by being noisy, the user gets a sense
> of how complex the histories being merged are). Reducing the default
> messaging level would probably be a good idea.
Here is what I just had to scroll through to update 'next' by
merging back 'master', only to grab the updates to the release
notes. Needless to say, this would have been somewhat baffling
if I didn't know to expect it.
It would be good to squelch it before we hear another complaints
from old-timer power users ;-)
$ git merge -m 'Sync with master' --no-log master
From inner merge: Auto-merging blame.c
From inner merge: Auto-merging builtin/am.c
From inner merge: Auto-merging builtin/blame.c
From inner merge: Auto-merging builtin/clone.c
From inner merge: Auto-merging builtin/clone.c
From inner merge: Auto-merging builtin/commit.c
From inner merge: Auto-merging builtin/fetch.c
From inner merge: Auto-merging builtin/fetch.c
From inner merge: Auto-merging builtin/grep.c
From inner merge: Auto-merging builtin/hash-object.c
From inner merge: Auto-merging builtin/log.c
From inner merge: Auto-merging builtin/log.c
From inner merge: Auto-merging builtin/pack-objects.c
From inner merge: Auto-merging builtin/pull.c
From inner merge: Auto-merging builtin/pull.c
From inner merge: Auto-merging builtin/rebase.c
From inner merge: Auto-merging builtin/rebase.c
From inner merge: Auto-merging builtin/reflog.c
From inner merge: CONFLICT (content): Merge conflict in builtin/reflog.c
Auto-merging builtin/reflog.c
From inner merge: Auto-merging builtin/reset.c
From inner merge: Auto-merging builtin/sparse-checkout.c
From inner merge: Auto-merging builtin/sparse-checkout.c
From inner merge: Auto-merging builtin/submodule--helper.c
From inner merge: Auto-merging builtin/submodule--helper.c
From inner merge: CONFLICT (content): Merge conflict in builtin/submodule--helper.c
Auto-merging builtin/submodule--helper.c
From inner merge: Auto-merging builtin/worktree.c
From inner merge: Auto-merging cache.h
From inner merge: Auto-merging config.c
From inner merge: Auto-merging config.h
From inner merge: Auto-merging diff-merges.c
From inner merge: Auto-merging diff.c
From inner merge: Auto-merging git.c
From inner merge: Auto-merging gpg-interface.c
From inner merge: Auto-merging grep.c
From inner merge: Auto-merging grep.c
From inner merge: Auto-merging notes-merge.c
From inner merge: Auto-merging object-name.c
From inner merge: Auto-merging pack-bitmap-write.c
From inner merge: Auto-merging parse-options.c
From inner merge: CONFLICT (content): Merge conflict in parse-options.c
From inner merge: Auto-merging parse-options.h
From inner merge: CONFLICT (content): Merge conflict in parse-options.h
From inner merge: Auto-merging refs.c
From inner merge: Auto-merging revision.c
From inner merge: Auto-merging sequencer.c
From inner merge: Auto-merging sequencer.c
From inner merge: Auto-merging sparse-index.c
From inner merge: Auto-merging submodule-config.c
From inner merge: Auto-merging t/t1091-sparse-checkout-builtin.sh
From inner merge: CONFLICT (content): Merge conflict in t/t1091-sparse-checkout-builtin.sh
Auto-merging t/t1091-sparse-checkout-builtin.sh
From inner merge: Auto-merging t/t1512-rev-parse-disambiguation.sh
From inner merge: Auto-merging t/t4202-log.sh
From inner merge: Auto-merging t/t4202-log.sh
From inner merge: Auto-merging t/t4202-log.sh
From inner merge: Auto-merging t/t4202-log.sh
From inner merge: Auto-merging t/t4202-log.sh
From inner merge: Auto-merging t/t5316-pack-delta-depth.sh
From inner merge: Auto-merging t/t6120-describe.sh
From inner merge: Auto-merging t/t6120-describe.sh
From inner merge: Auto-merging worktree.c
Merge made by the 'ort' strategy.
Documentation/RelNotes/2.36.0.txt | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
^ permalink raw reply [relevance 5%]
* Re: [PATCH v2 0/8] Add a new --remerge-diff capability to show & log
2021-12-27 21:11 5% ` Elijah Newren
@ 2022-01-10 15:48 0% ` Ævar Arnfjörð Bjarmason
0 siblings, 0 replies; 24+ results
From: Ævar Arnfjörð Bjarmason @ 2022-01-10 15:48 UTC (permalink / raw)
To: Elijah Newren
Cc: Elijah Newren via GitGitGadget, Git Mailing List, Jeff King,
Jonathan Nieder, Sergey Organov, Bagas Sanjaya, Neeraj Singh
On Mon, Dec 27 2021, Elijah Newren wrote:
> On Sun, Dec 26, 2021 at 2:28 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>>
>> On Sat, Dec 25 2021, Elijah Newren via GitGitGadget wrote:
>>
>> > === FURTHER BACKGROUND (original cover letter material) ==
>> >
>> > Here are some example commits you can try this out on (with git show
>> > --remerge-diff $COMMIT):
>> >
>> > * git.git conflicted merge: 07601b5b36
>> > * git.git non-conflicted change: bf04590ecd
>> > * linux.git conflicted merge: eab3540562fb
>> > * linux.git non-conflicted change: 223cea6a4f05
>> >
>> > Many more can be found by just running git log --merges --remerge-diff in
>> > your repository of choice and searching for diffs (most merges tend to be
>> > clean and unmodified and thus produce no diff but a search of '^diff' in the
>> > log output tends to find the examples nicely).
>> >
>> > Some basic high level details about this new option:
>> >
>> > * This option is most naturally compared to --cc, though the output seems
>> > to be much more understandable to most users than --cc output.
>> > * Since merges are often clean and unmodified, this new option results in
>> > an empty diff for most merges.
>> > * This new option shows things like the removal of conflict markers, which
>> > hunks users picked from the various conflicted sides to keep or remove,
>> > and shows changes made outside of conflict markers (which might reflect
>> > changes needed to resolve semantic conflicts or cleanups of e.g.
>> > compilation warnings or other additional changes an integrator felt
>> > belonged in the merged result).
>> > * This new option does not (currently) work for octopus merges, since
>> > merge-ort is specific to two-parent merges[1].
>> > * This option will not work on a read-only or full filesystem[2].
>> > * We discussed this capability at Git Merge 2020, and one of the
>> > suggestions was doing a periodic git gc --auto during the operation (due
>> > to potential new blobs and trees created during the operation). I found a
>> > way to avoid that; see [2].
>> > * This option is faster than you'd probably expect; it handles 33.5 merge
>> > commits per second in linux.git on my computer; see below.
>> >
>> > In regards to the performance point above, the timing for running the
>> > following command:
>> >
>> > time git log --min-parents=2 --max-parents=2 $DIFF_FLAG | wc -l
>>
>> I've been trying to come up with some other useful recipies for this new
>> option (which is already very useful, thanks!)
>
> I'm glad you like it. :-)
>
>> Some of these (if correct) are suggestions for incorporating into the
>> (now rather sparse) documentation. I.e. walking users through how to use
>> this, and how (if at all) it combines with other options.
>>
>> I wanted to find all merges between "master".."seen" for which Junio's
>> had to resolve a conflict, a naïve version is:
>>
>> $ git log --oneline --remerge-diff -p --min-parents=2 origin/master..origin/seen|grep ^diff -B1 | grep Merge
>> [...]
>
> I think the naive version is
> $ git log --remerge-diff --min-parents=2 origin/master..origin/seen
> <search for "^diff" using your pager's search functionality>
>
> Where the "--min-parents=2 origin/master..origin/seen" comes from your
> problem description ("find all merges between master..seen").
>
> You can add --oneline to format it, though it's an orthogonal concern.
> Also, adding -p is unnecessary: --remerge-diff, like --cc, implies -p.
>
>> But I found that this new option nicely integrates with --diff-filter,
>> i.e. we'll end up showing a diff, and the diff machinery allows you to
>> to filter on it.
>>
>> It seems to me like all the diffs you show fall under "M", so for
>
> Yes, the diffs I happened to pick all fell under "M", but by no means
> should you rely on that happening for all merges in history. For
> example, make a new merge commit, then add a completely new file (or
> delete a file, or rename a file, or copy a file, or change its
> mode/type), stage the new/deleted/renamed/copied/changed file, and run
> "git commit --amend".
>
> So, although --diff-filter=M can be interesting, I would not rely on it.
*Nod* I hadn't thought of those (in retrospect, rather obvious) cases.
>> master..seen (2ae0a9cb829..61055c2920d) this is equivalent (and the
>> output is the same as the above):
>>
>> $ git -P log --oneline --remerge-diff --no-patch --min-parents=2 --diff-filter=M origin/master..origin/seen
>> 95daa54b1c3 Merge branch 'hn/reftable-fixes' into seen
>> 26c4c09dd34 Merge branch 'gc/fetch-negotiate-only-early-return' into seen
>> e3dc8d073f6 Merge branch 'gc/branch-recurse-submodules' into seen
>> aeada898196 Merge branch 'js/branch-track-inherit' into seen
>> 4dd30e0da45 Merge branch 'jh/builtin-fsmonitor-part2' into seen
>> 337743b17d0 Merge branch 'ab/config-based-hooks-2' into seen
>> 261672178c0 Merge branch 'pw/fix-some-issues-in-reset-head' into seen
>> 1296d35b041 Merge branch 'ms/customizable-ident-expansion' into seen
>> 7a3d7d05126 Merge branch 'ja/i18n-similar-messages' into seen
>> eda714bb8bc Merge branch 'tb/midx-bitmap-corruption-fix' into seen
>> ba02295e3f8 Merge branch 'jh/p4-human-unit-numbers' into jch
>> 751773fc38b Merge branch 'es/test-chain-lint' into jch
>> ec17879f495 Merge branch 'tb/cruft-packs' into tb/midx-bitmap-corruption-fix
>>
>> However for "origin/master..origin/next" (next = 510f9eba9a2 currently)
>> we'll oddly show this with "-p":
>>
>> 9af51fd1d0d Sync with 'master'
>> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
>> CONFLICT (content): Merge conflict in t/lib-gpg.sh
>> d6f56f3248e Merge branch 'es/test-chain-lint' into next
>> diff --git a/t/t4126-apply-empty.sh b/t/t4126-apply-empty.sh
>> CONFLICT (content): Merge conflict in t/t4126-apply-empty.sh
>> index 996c93329c6..33860d38290 100755
>> --- a/t/t4126-apply-empty.sh
>> +++ b/t/t4126-apply-empty.sh
>> [...]
>>
>> The "oddly" applying only to that "9af51fd1d0d Sync with 'master'", not
>> the second d6f56f3248e, which shows the sort of conflict I'd expect. The
>> two-line "diff" of:
>>
>> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
>> CONFLICT (content): Merge conflict in t/lib-gpg.sh
>>
>> Shows up with -p --remerge-diff, not a mere -p. I also tried the other
>> --diff-merges=* options, that behavior is new in
>> --diff-merges=remerge. Is this a bug?
>
> Ugh, this is related to my comment elsewhere that conflicts from inner
> merges are not nicely differentiated. If I also apply my other series
> (which has not yet been submitted), this instead appears as follows:
>
> $ git show --oneline --remerge-diff 9af51fd1d0d
> 9af51fd1d0 Sync with 'master'
> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
> From inner merge: CONFLICT (content): Merge conflict in t/lib-gpg.sh
>
> and the addition of the "From inner merge: " text makes it clearer why
> that line appears. This is an interesting case where a conflict
> notice _only_ appears in the inner merge (i.e. the merge of merge
> bases), which means that both sides on the outer merge changed the
> relevant portion of the file in the same way, so the outer merge had
> no conflict.
>
> However, instead of trying to differentiate messages from inner
> merges, I think for --remerge-diff's purposes we should just drop all
> notices that come from the inner merges. Those conflict notices might
> be helpful when initially resolving a merge, but at the --remerge-diff
> level, they're more likely to be distracting than helpful.
Thanks. I see that v3 addresses this. Will try it out...
>> My local build also has a --pickaxe-patch option. It's something I
>> submitted on-list before[1] and have been meaning to re-roll.
>>
>> I'm discussing it here because it skips the stripping of the "+ " and "-
>> " prefixes under -G<regex> and allows you to search through the -U<n>
>> context. With that I'm able to do:
>>
>> git log --oneline --remerge-diff -p --min-parents=2 --pickaxe-patch -G'^\+' --diff-filter=M origin/master..origin/seen
>>
>> I.e. on top of the above filter only show those diffs that have
>> additions. FAICT the conflicting diffs where the committer of the merge
>> conflict picked one side or the other will only have "-" lines".
>>
>> So those diffs that have additions look to be those where the person
>> doing the merge needed to combine the two.
>>
>> Well, usually. E.g. 26c4c09dd34 (Merge branch
>> 'gc/fetch-negotiate-only-early-return' into seen, 2021-12-25) in that
>> range shows that isn't strictly true. Most such deletion-only diffs are
>> less interesting in picking one side or the other of the conflict, but
>> that one combines the two:
>>
>> -<<<<<<< d3419aac9f4 (Merge branch 'pw/add-p-hunk-split-fix' into seen)
>> warning(_("protocol does not support --negotiate-only, exiting"));
>> - return 1;
>> -=======
>> - warning(_("Protocol does not support --negotiate-only, exiting."));
>> result = 1;
>> goto cleanup;
>> ->>>>>>> 495e8601f28 (builtin/fetch: die on --negotiate-only and --recurse-submodules)
>>
>> Which I guess is partially commentary and partially a request (either
>> for this series, or some follow-up) for something like a
>> --remerge-diff-filter option. I.e. it would be very useful to be able to
>> filter on some combination of:
>>
>> * Which side(s) of the conflict(s) were picked, or a combination?
>> * Is there "new work" in the diff to resolve the conflict?
>> AFIACT this will always mean we'll have "+ " lines.
>
> Do any of the following count as "new work"? :
>
> * the deletion of a file (perhaps one that had no conflict but was
> deleted anyway)
> * mode changes (again, perhaps on files that had no conflict)
> * renames of files/directories?
>
> If so, searching for "^+" lines might be insufficient, but it depends
> on what you mean by new work.
Yes, I think at least for the use-cases I had in mind the useful thing
is knowing which state a merge commit is in:
A: "Vanilla" merge (i.e. no --remerge-diff output)
B: non-"vanilla" merge" (i.e. ew have --remerge-diff output)
What I'm requesting/musing about here (which is very much in the
category of stuff we can/should do later, and shouldn't prevent this
"good enough for now" series going in) is being able to disambiguate
those "B" cases.
I.e. being able to see if their/ours side "won", some mixture of the two
etc.
>> Or maybe that's not useful at all, and just -G<rx> (maybe combined with
>> my --pickaxe-patch) will cover it?
>
> I'd rather wait until we have a good idea of the potential range of
> usecases before adding a filter. (And I think for now, the -G and
> --pickaxe-patch are probably good enough for this usecase.) These
> particular usecases you point out are interesting; thanks for
> detailing them. Here's some others to consider:
>
> * Finding out when text was added or removed: `git log
> --remerge-diff -S<text>` (note that with only -p instead of
> --remerge-diff, that command will annoyingly misses cases where a
> merge introduced or removed the text)
> * Finding out how a merge differed from one run with some
> non-default options (e.g. `git show --remerge-diff -Xours` or `git
> show --remerge-diff -Xno-space-change`; although show doesn't take -X
> options so this is just an idea at this point)
> * Finding out how a merge would have differed had it been run with
> different options (so instead of comparing a remerge to the merge
> recorded in history, compare one remerge with default options with a
> different merge that uses e.g. -Xno-space-change)
>
> Also, I've got a follow-up series that also introduces a
> --remerge-diff-only flag which:
> * For single parent commits that cannot be identified as a revert or
> cherry-pick, do not show a diff.
> * For single parent commits that can be identified as a revert or
> cherry-pick, instead of showing a diff against the parent of the
> commit, redo the revert or cherry-pick in memory and show a diff
> against that.
> * For merge commits, act the same as --remerge-diff
*nod*
^ permalink raw reply [relevance 0%]
* Re: [PATCH v2 0/8] Add a new --remerge-diff capability to show & log
2021-12-26 21:52 7% ` Ævar Arnfjörð Bjarmason
@ 2021-12-27 21:11 5% ` Elijah Newren
2022-01-10 15:48 0% ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 24+ results
From: Elijah Newren @ 2021-12-27 21:11 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Elijah Newren via GitGitGadget, Git Mailing List, Jeff King,
Jonathan Nieder, Sergey Organov, Bagas Sanjaya, Neeraj Singh
On Sun, Dec 26, 2021 at 2:28 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> On Sat, Dec 25 2021, Elijah Newren via GitGitGadget wrote:
>
> > === FURTHER BACKGROUND (original cover letter material) ==
> >
> > Here are some example commits you can try this out on (with git show
> > --remerge-diff $COMMIT):
> >
> > * git.git conflicted merge: 07601b5b36
> > * git.git non-conflicted change: bf04590ecd
> > * linux.git conflicted merge: eab3540562fb
> > * linux.git non-conflicted change: 223cea6a4f05
> >
> > Many more can be found by just running git log --merges --remerge-diff in
> > your repository of choice and searching for diffs (most merges tend to be
> > clean and unmodified and thus produce no diff but a search of '^diff' in the
> > log output tends to find the examples nicely).
> >
> > Some basic high level details about this new option:
> >
> > * This option is most naturally compared to --cc, though the output seems
> > to be much more understandable to most users than --cc output.
> > * Since merges are often clean and unmodified, this new option results in
> > an empty diff for most merges.
> > * This new option shows things like the removal of conflict markers, which
> > hunks users picked from the various conflicted sides to keep or remove,
> > and shows changes made outside of conflict markers (which might reflect
> > changes needed to resolve semantic conflicts or cleanups of e.g.
> > compilation warnings or other additional changes an integrator felt
> > belonged in the merged result).
> > * This new option does not (currently) work for octopus merges, since
> > merge-ort is specific to two-parent merges[1].
> > * This option will not work on a read-only or full filesystem[2].
> > * We discussed this capability at Git Merge 2020, and one of the
> > suggestions was doing a periodic git gc --auto during the operation (due
> > to potential new blobs and trees created during the operation). I found a
> > way to avoid that; see [2].
> > * This option is faster than you'd probably expect; it handles 33.5 merge
> > commits per second in linux.git on my computer; see below.
> >
> > In regards to the performance point above, the timing for running the
> > following command:
> >
> > time git log --min-parents=2 --max-parents=2 $DIFF_FLAG | wc -l
>
> I've been trying to come up with some other useful recipies for this new
> option (which is already very useful, thanks!)
I'm glad you like it. :-)
> Some of these (if correct) are suggestions for incorporating into the
> (now rather sparse) documentation. I.e. walking users through how to use
> this, and how (if at all) it combines with other options.
>
> I wanted to find all merges between "master".."seen" for which Junio's
> had to resolve a conflict, a naïve version is:
>
> $ git log --oneline --remerge-diff -p --min-parents=2 origin/master..origin/seen|grep ^diff -B1 | grep Merge
> [...]
I think the naive version is
$ git log --remerge-diff --min-parents=2 origin/master..origin/seen
<search for "^diff" using your pager's search functionality>
Where the "--min-parents=2 origin/master..origin/seen" comes from your
problem description ("find all merges between master..seen").
You can add --oneline to format it, though it's an orthogonal concern.
Also, adding -p is unnecessary: --remerge-diff, like --cc, implies -p.
> But I found that this new option nicely integrates with --diff-filter,
> i.e. we'll end up showing a diff, and the diff machinery allows you to
> to filter on it.
>
> It seems to me like all the diffs you show fall under "M", so for
Yes, the diffs I happened to pick all fell under "M", but by no means
should you rely on that happening for all merges in history. For
example, make a new merge commit, then add a completely new file (or
delete a file, or rename a file, or copy a file, or change its
mode/type), stage the new/deleted/renamed/copied/changed file, and run
"git commit --amend".
So, although --diff-filter=M can be interesting, I would not rely on it.
> master..seen (2ae0a9cb829..61055c2920d) this is equivalent (and the
> output is the same as the above):
>
> $ git -P log --oneline --remerge-diff --no-patch --min-parents=2 --diff-filter=M origin/master..origin/seen
> 95daa54b1c3 Merge branch 'hn/reftable-fixes' into seen
> 26c4c09dd34 Merge branch 'gc/fetch-negotiate-only-early-return' into seen
> e3dc8d073f6 Merge branch 'gc/branch-recurse-submodules' into seen
> aeada898196 Merge branch 'js/branch-track-inherit' into seen
> 4dd30e0da45 Merge branch 'jh/builtin-fsmonitor-part2' into seen
> 337743b17d0 Merge branch 'ab/config-based-hooks-2' into seen
> 261672178c0 Merge branch 'pw/fix-some-issues-in-reset-head' into seen
> 1296d35b041 Merge branch 'ms/customizable-ident-expansion' into seen
> 7a3d7d05126 Merge branch 'ja/i18n-similar-messages' into seen
> eda714bb8bc Merge branch 'tb/midx-bitmap-corruption-fix' into seen
> ba02295e3f8 Merge branch 'jh/p4-human-unit-numbers' into jch
> 751773fc38b Merge branch 'es/test-chain-lint' into jch
> ec17879f495 Merge branch 'tb/cruft-packs' into tb/midx-bitmap-corruption-fix
>
> However for "origin/master..origin/next" (next = 510f9eba9a2 currently)
> we'll oddly show this with "-p":
>
> 9af51fd1d0d Sync with 'master'
> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
> CONFLICT (content): Merge conflict in t/lib-gpg.sh
> d6f56f3248e Merge branch 'es/test-chain-lint' into next
> diff --git a/t/t4126-apply-empty.sh b/t/t4126-apply-empty.sh
> CONFLICT (content): Merge conflict in t/t4126-apply-empty.sh
> index 996c93329c6..33860d38290 100755
> --- a/t/t4126-apply-empty.sh
> +++ b/t/t4126-apply-empty.sh
> [...]
>
> The "oddly" applying only to that "9af51fd1d0d Sync with 'master'", not
> the second d6f56f3248e, which shows the sort of conflict I'd expect. The
> two-line "diff" of:
>
> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
> CONFLICT (content): Merge conflict in t/lib-gpg.sh
>
> Shows up with -p --remerge-diff, not a mere -p. I also tried the other
> --diff-merges=* options, that behavior is new in
> --diff-merges=remerge. Is this a bug?
Ugh, this is related to my comment elsewhere that conflicts from inner
merges are not nicely differentiated. If I also apply my other series
(which has not yet been submitted), this instead appears as follows:
$ git show --oneline --remerge-diff 9af51fd1d0d
9af51fd1d0 Sync with 'master'
diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
From inner merge: CONFLICT (content): Merge conflict in t/lib-gpg.sh
and the addition of the "From inner merge: " text makes it clearer why
that line appears. This is an interesting case where a conflict
notice _only_ appears in the inner merge (i.e. the merge of merge
bases), which means that both sides on the outer merge changed the
relevant portion of the file in the same way, so the outer merge had
no conflict.
However, instead of trying to differentiate messages from inner
merges, I think for --remerge-diff's purposes we should just drop all
notices that come from the inner merges. Those conflict notices might
be helpful when initially resolving a merge, but at the --remerge-diff
level, they're more likely to be distracting than helpful.
> My local build also has a --pickaxe-patch option. It's something I
> submitted on-list before[1] and have been meaning to re-roll.
>
> I'm discussing it here because it skips the stripping of the "+ " and "-
> " prefixes under -G<regex> and allows you to search through the -U<n>
> context. With that I'm able to do:
>
> git log --oneline --remerge-diff -p --min-parents=2 --pickaxe-patch -G'^\+' --diff-filter=M origin/master..origin/seen
>
> I.e. on top of the above filter only show those diffs that have
> additions. FAICT the conflicting diffs where the committer of the merge
> conflict picked one side or the other will only have "-" lines".
>
> So those diffs that have additions look to be those where the person
> doing the merge needed to combine the two.
>
> Well, usually. E.g. 26c4c09dd34 (Merge branch
> 'gc/fetch-negotiate-only-early-return' into seen, 2021-12-25) in that
> range shows that isn't strictly true. Most such deletion-only diffs are
> less interesting in picking one side or the other of the conflict, but
> that one combines the two:
>
> -<<<<<<< d3419aac9f4 (Merge branch 'pw/add-p-hunk-split-fix' into seen)
> warning(_("protocol does not support --negotiate-only, exiting"));
> - return 1;
> -=======
> - warning(_("Protocol does not support --negotiate-only, exiting."));
> result = 1;
> goto cleanup;
> ->>>>>>> 495e8601f28 (builtin/fetch: die on --negotiate-only and --recurse-submodules)
>
> Which I guess is partially commentary and partially a request (either
> for this series, or some follow-up) for something like a
> --remerge-diff-filter option. I.e. it would be very useful to be able to
> filter on some combination of:
>
> * Which side(s) of the conflict(s) were picked, or a combination?
> * Is there "new work" in the diff to resolve the conflict?
> AFIACT this will always mean we'll have "+ " lines.
Do any of the following count as "new work"? :
* the deletion of a file (perhaps one that had no conflict but was
deleted anyway)
* mode changes (again, perhaps on files that had no conflict)
* renames of files/directories?
If so, searching for "^+" lines might be insufficient, but it depends
on what you mean by new work.
> Or maybe that's not useful at all, and just -G<rx> (maybe combined with
> my --pickaxe-patch) will cover it?
I'd rather wait until we have a good idea of the potential range of
usecases before adding a filter. (And I think for now, the -G and
--pickaxe-patch are probably good enough for this usecase.) These
particular usecases you point out are interesting; thanks for
detailing them. Here's some others to consider:
* Finding out when text was added or removed: `git log
--remerge-diff -S<text>` (note that with only -p instead of
--remerge-diff, that command will annoyingly misses cases where a
merge introduced or removed the text)
* Finding out how a merge differed from one run with some
non-default options (e.g. `git show --remerge-diff -Xours` or `git
show --remerge-diff -Xno-space-change`; although show doesn't take -X
options so this is just an idea at this point)
* Finding out how a merge would have differed had it been run with
different options (so instead of comparing a remerge to the merge
recorded in history, compare one remerge with default options with a
different merge that uses e.g. -Xno-space-change)
Also, I've got a follow-up series that also introduces a
--remerge-diff-only flag which:
* For single parent commits that cannot be identified as a revert or
cherry-pick, do not show a diff.
* For single parent commits that can be identified as a revert or
cherry-pick, instead of showing a diff against the parent of the
commit, redo the revert or cherry-pick in memory and show a diff
against that.
* For merge commits, act the same as --remerge-diff
^ permalink raw reply [relevance 5%]
* Re: [PATCH v2 0/8] Add a new --remerge-diff capability to show & log
@ 2021-12-26 21:52 7% ` Ævar Arnfjörð Bjarmason
2021-12-27 21:11 5% ` Elijah Newren
0 siblings, 1 reply; 24+ results
From: Ævar Arnfjörð Bjarmason @ 2021-12-26 21:52 UTC (permalink / raw)
To: Elijah Newren via GitGitGadget
Cc: git, Jeff King, Jonathan Nieder, Sergey Organov, Bagas Sanjaya,
Neeraj Singh, Elijah Newren
On Sat, Dec 25 2021, Elijah Newren via GitGitGadget wrote:
> === FURTHER BACKGROUND (original cover letter material) ==
>
> Here are some example commits you can try this out on (with git show
> --remerge-diff $COMMIT):
>
> * git.git conflicted merge: 07601b5b36
> * git.git non-conflicted change: bf04590ecd
> * linux.git conflicted merge: eab3540562fb
> * linux.git non-conflicted change: 223cea6a4f05
>
> Many more can be found by just running git log --merges --remerge-diff in
> your repository of choice and searching for diffs (most merges tend to be
> clean and unmodified and thus produce no diff but a search of '^diff' in the
> log output tends to find the examples nicely).
>
> Some basic high level details about this new option:
>
> * This option is most naturally compared to --cc, though the output seems
> to be much more understandable to most users than --cc output.
> * Since merges are often clean and unmodified, this new option results in
> an empty diff for most merges.
> * This new option shows things like the removal of conflict markers, which
> hunks users picked from the various conflicted sides to keep or remove,
> and shows changes made outside of conflict markers (which might reflect
> changes needed to resolve semantic conflicts or cleanups of e.g.
> compilation warnings or other additional changes an integrator felt
> belonged in the merged result).
> * This new option does not (currently) work for octopus merges, since
> merge-ort is specific to two-parent merges[1].
> * This option will not work on a read-only or full filesystem[2].
> * We discussed this capability at Git Merge 2020, and one of the
> suggestions was doing a periodic git gc --auto during the operation (due
> to potential new blobs and trees created during the operation). I found a
> way to avoid that; see [2].
> * This option is faster than you'd probably expect; it handles 33.5 merge
> commits per second in linux.git on my computer; see below.
>
> In regards to the performance point above, the timing for running the
> following command:
>
> time git log --min-parents=2 --max-parents=2 $DIFF_FLAG | wc -l
I've been trying to come up with some other useful recipies for this new
option (which is already very useful, thanks!)
Some of these (if correct) are suggestions for incorporating into the
(now rather sparse) documentation. I.e. walking users through how to use
this, and how (if at all) it combines with other options.
I wanted to find all merges between "master".."seen" for which Junio's
had to resolve a conflict, a naïve version is:
$ git log --oneline --remerge-diff -p --min-parents=2 origin/master..origin/seen|grep ^diff -B1 | grep Merge
[...]
But I found that this new option nicely integrates with --diff-filter,
i.e. we'll end up showing a diff, and the diff machinery allows you to
to filter on it.
It seems to me like all the diffs you show fall under "M", so for
master..seen (2ae0a9cb829..61055c2920d) this is equivalent (and the
output is the same as the above):
$ git -P log --oneline --remerge-diff --no-patch --min-parents=2 --diff-filter=M origin/master..origin/seen
95daa54b1c3 Merge branch 'hn/reftable-fixes' into seen
26c4c09dd34 Merge branch 'gc/fetch-negotiate-only-early-return' into seen
e3dc8d073f6 Merge branch 'gc/branch-recurse-submodules' into seen
aeada898196 Merge branch 'js/branch-track-inherit' into seen
4dd30e0da45 Merge branch 'jh/builtin-fsmonitor-part2' into seen
337743b17d0 Merge branch 'ab/config-based-hooks-2' into seen
261672178c0 Merge branch 'pw/fix-some-issues-in-reset-head' into seen
1296d35b041 Merge branch 'ms/customizable-ident-expansion' into seen
7a3d7d05126 Merge branch 'ja/i18n-similar-messages' into seen
eda714bb8bc Merge branch 'tb/midx-bitmap-corruption-fix' into seen
ba02295e3f8 Merge branch 'jh/p4-human-unit-numbers' into jch
751773fc38b Merge branch 'es/test-chain-lint' into jch
ec17879f495 Merge branch 'tb/cruft-packs' into tb/midx-bitmap-corruption-fix
However for "origin/master..origin/next" (next = 510f9eba9a2 currently)
we'll oddly show this with "-p":
9af51fd1d0d Sync with 'master'
diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
CONFLICT (content): Merge conflict in t/lib-gpg.sh
d6f56f3248e Merge branch 'es/test-chain-lint' into next
diff --git a/t/t4126-apply-empty.sh b/t/t4126-apply-empty.sh
CONFLICT (content): Merge conflict in t/t4126-apply-empty.sh
index 996c93329c6..33860d38290 100755
--- a/t/t4126-apply-empty.sh
+++ b/t/t4126-apply-empty.sh
[...]
The "oddly" applying only to that "9af51fd1d0d Sync with 'master'", not
the second d6f56f3248e, which shows the sort of conflict I'd expect. The
two-line "diff" of:
diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
CONFLICT (content): Merge conflict in t/lib-gpg.sh
Shows up with -p --remerge-diff, not a mere -p. I also tried the other
--diff-merges=* options, that behavior is new in
--diff-merges=remerge. Is this a bug?
My local build also has a --pickaxe-patch option. It's something I
submitted on-list before[1] and have been meaning to re-roll.
I'm discussing it here because it skips the stripping of the "+ " and "-
" prefixes under -G<regex> and allows you to search through the -U<n>
context. With that I'm able to do:
git log --oneline --remerge-diff -p --min-parents=2 --pickaxe-patch -G'^\+' --diff-filter=M origin/master..origin/seen
I.e. on top of the above filter only show those diffs that have
additions. FAICT the conflicting diffs where the committer of the merge
conflict picked one side or the other will only have "-" lines".
So those diffs that have additions look to be those where the person
doing the merge needed to combine the two.
Well, usually. E.g. 26c4c09dd34 (Merge branch
'gc/fetch-negotiate-only-early-return' into seen, 2021-12-25) in that
range shows that isn't strictly true. Most such deletion-only diffs are
less interesting in picking one side or the other of the conflict, but
that one combines the two:
-<<<<<<< d3419aac9f4 (Merge branch 'pw/add-p-hunk-split-fix' into seen)
warning(_("protocol does not support --negotiate-only, exiting"));
- return 1;
-=======
- warning(_("Protocol does not support --negotiate-only, exiting."));
result = 1;
goto cleanup;
->>>>>>> 495e8601f28 (builtin/fetch: die on --negotiate-only and --recurse-submodules)
Which I guess is partially commentary and partially a request (either
for this series, or some follow-up) for something like a
--remerge-diff-filter option. I.e. it would be very useful to be able to
filter on some combination of:
* Which side(s) of the conflict(s) were picked, or a combination?
* Is there "new work" in the diff to resolve the conflict?
AFIACT this will always mean we'll have "+ " lines.
Or maybe that's not useful at all, and just -G<rx> (maybe combined with
my --pickaxe-patch) will cover it?
1. https://lore.kernel.org/git/20190424152215.16251-3-avarab@gmail.com/
^ permalink raw reply [relevance 7%]
* [PATCH] t1300: fix unset of GIT_CONFIG_NOSYSTEM leaking into subsequent tests
@ 2021-04-23 5:47 5% ` Patrick Steinhardt
0 siblings, 0 replies; 24+ results
From: Patrick Steinhardt @ 2021-04-23 5:47 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Eric Sunshine
[-- Attachment #1: Type: text/plain, Size: 1921 bytes --]
In order to test whether the new GIT_CONFIG_SYSTEM environment variable
behaves as expected, we unset GIT_CONFIG_NOSYSTEM in one of our tests in
t1300. But because tests are not executed in a subshell, this unset
leaks into all subsequent tests and may thus cause them to fail in some
environments. These failures are easily reproducable with `make
prefix=/root test`.
Fix the issue by not using `sane_unset GIT_CONFIG_NOSYSTEM`, but instead
just manually add it to the environment of the two command invocations
which need it.
Reported-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
This patch applies on top of 47e6f16901 (Sync with master, 2021-04-20),
which is the tip of next at the time of writing.
t/t1300-config.sh | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 0f92dfe6fb..ec599baeba 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -2092,21 +2092,20 @@ test_expect_success 'override global and system config' '
git config --show-scope --list >output &&
test_cmp expect output &&
- sane_unset GIT_CONFIG_NOSYSTEM &&
-
cat >expect <<-EOF &&
system system.config=true
global global.config=true
local local.config=true
EOF
- GIT_CONFIG_SYSTEM=custom-system-config GIT_CONFIG_GLOBAL=custom-global-config \
+ GIT_CONFIG_NOSYSTEM=false GIT_CONFIG_SYSTEM=custom-system-config GIT_CONFIG_GLOBAL=custom-global-config \
git config --show-scope --list >output &&
test_cmp expect output &&
cat >expect <<-EOF &&
local local.config=true
EOF
- GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_GLOBAL=/dev/null git config --show-scope --list >output &&
+ GIT_CONFIG_NOSYSTEM=false GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_GLOBAL=/dev/null \
+ git config --show-scope --list >output &&
test_cmp expect output
'
--
2.31.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [relevance 5%]
* [Outreachy][PATCH 00/10] Finish converting git bisect to C part 2
@ 2020-02-26 10:14 6% Miriam Rubio
0 siblings, 0 replies; 24+ results
From: Miriam Rubio @ 2020-02-26 10:14 UTC (permalink / raw)
To: git; +Cc: Miriam Rubio
These patches correspond to a second part of patch series
of Outreachy project "Finish converting `git bisect` from shell to C"
started by Pranit Bauva and Tanushree Tumane
(https://public-inbox.org/git/pull.117.git.gitgitgadget@gmail.com) and
continued by me.
This second part depends on mr/bisect-in-c-1 and it is formed by
reimplementations of some `git bisect` subcommands and removal of some
temporary subcommands.
These patch series emails were generated from:
https://gitlab.com/mirucam/git/commits/git-bisect-work-part2.
General changes
---------------
* Rebase on next branch: 5900a2a8f9 (Sync with master, 2020-02-25).
* Improve commit messages.
* Amend patch series titles.
* Reorder commits.
Specific changes
----------------
[1/10] bisect--helper: introduce new `write_in_file()` function
* New patch to refactor code in `write_in_file()` function and use it in
`write_terms()`.
--
[2/10] bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell functions in C
* Fix `repo_init_revisions()` and bisect_next_all` calls after
rebase on master.
* Add blank line.
* Remove goto statements in `bisect_skipped_commits()`.
* Add `clear_commit_marks()` in `process_skipped_commits()`.
* Use `write_in_file()` in `bisect_successful()`.
* Change header return type and internal returns to enum in
`bisect_next()` and `bisect_auto_next()`.
* Improve code comments.
--
[3/10] bisect--helper: finish porting `bisect_start()` to C
* Change header return type and internal returns to enum in `bisect_start()`.
* Improve code comments.
--
[6/10] bisect--helper: reimplement `bisect_autostart` shell function in C
* Change an internal return to enum in `bisect_autostart()`.
--
[7/10] bisect--helper: reimplement `bisect_state` & `bisect_head` shell functions in C
* Change header return type and internal returns to enum in `bisect_state()`.
--
Miriam Rubio (1):
bisect--helper: introduce new `write_in_file()` function
Pranit Bauva (9):
bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell
functions in C
bisect--helper: finish porting `bisect_start()` to C
bisect--helper: retire `--bisect-clean-state` subcommand
bisect--helper: retire `--next-all` subcommand
bisect--helper: reimplement `bisect_autostart` shell function in C
bisect--helper: reimplement `bisect_state` & `bisect_head` shell
functions in C
bisect--helper: retire `--check-expected-revs` subcommand
bisect--helper: retire `--write-terms` subcommand
bisect--helper: retire `--bisect-autostart` subcommand
bisect.c | 11 ++
builtin/bisect--helper.c | 388 ++++++++++++++++++++++++++++++++++-----
git-bisect.sh | 145 +--------------
3 files changed, 358 insertions(+), 186 deletions(-)
--
2.25.0
^ permalink raw reply [relevance 6%]
* Re: Error in `git': free(): invalid pointer (was Re: [PATCH] sequencer: improve config handling)
[not found] ` <18737953.1042351513802399608.JavaMail.defaultUser@defaultHost>
@ 2017-12-21 15:02 6% ` Kaartic Sivaraam
0 siblings, 0 replies; 24+ results
From: Kaartic Sivaraam @ 2017-12-21 15:02 UTC (permalink / raw)
To: phillip.wood@talktalk.net
Cc: Git Mailing List, Johannes Schindelin, Junio C Hamano
On Thursday 21 December 2017 02:09 AM, phillip.wood@talktalk.net wrote:
>
>
> Hi Kaartic
>
> I'm replying off list as I've only got access to webmail at the
> moment.
No issues brought the CCs including the list to ensure we don't miss things.
> Are you able to do a backtrace with debugging symbols
> please, I'm finding it hard to glean anything from the backtrace
> here.
That's confusing, I already had the following in my `config.mak` while
compiling,
DEVELOPER=1
DEVELOPERS=1
CFLAGS += -g -O2
CFLAGS += -DFLEX_ARRAY=2048
NO_GETTEXT=1
prefix=/home/$(USER)/.local
#CFLAGS += -Wno-unused-value
As you can see the `-g` flag is there in CFLAGS.
If this output doesn't give much sense could you be specific about what
output you want i.e., by mentioning the command, showing a sample etc.,
I'm not sure how to go about debugging with "git rebase" as it's a script.
> Also does it definitely bisect to this patch or the merge of
> this branch?
>
"git bisect" speaking,
28d6daed4f119940ace31e523b3b272d3d153d04 is the first bad commit
commit 28d6daed4f119940ace31e523b3b272d3d153d04
Author: Phillip Wood <phillip.wood@dunelm.org.uk>
Date: Wed Dec 13 11:46:21 2017 +0000
sequencer: improve config handling
The previous config handling relied on global variables, called
git_default_config() even when the key had already been handled by
git_sequencer_config() and did not initialize the diff configuration
variables. Improve this by: i) loading the default values for message
cleanup and gpg signing of commits into struct replay_opts;
ii) restructuring the code to return immediately once a key is
handled; and iii) calling git_diff_basic_config(). Note that
unfortunately it is not possible to return early if the key is handled
by git_gpg_config() as it does not indicate to the caller if the key
has been handled or not.
The sequencer should probably have been calling
git_diff_basic_config() before as it creates a patch when there are
conflicts. The shell version uses 'diff-tree' to create the patch so
calling git_diff_basic_config() should match that. Although 'git
commit' calls git_diff_ui_config() I don't think the output of
print_commit_summary() is affected by anything that is loaded by that
as print_commit_summary() always turns on rename detection so would
ignore the value in the user's configuration anyway. The other values
loaded by git_diff_ui_config() are about the formatting of patches so
are not relevant to print_commit_summary().
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
:040000 040000 bbd487cc285ae12cefb97a9661de4ff27d182727 9460ac528bf68f331088412f62e5d6a39305854b M builtin
:100644 100644 99452a0e89777999418c116386f924be396ed04a 7051b20b76292e3b539875f0dc38108a734d2164 M sequencer.c
:100644 100644 77cb174b2aaf3972ebb9e6ec379252be96dedd3d 3a5072c2ab9088c237b83d92deae3c801289e543 M sequencer.h
FWIW, I started the bisection with `v2.11.0` as good (because "git
rebase -i HEAD~10 worked there) and the tip of `next` (cfbfd45ee (Sync
with master, 2017-12-19)) as bad.
Here's the bisect log for a shorter range of bisection (which was
possible after I discovered the bad commit in the previous bisect)
git bisect start
# bad: [cfbfd45ee6e49007fdeb00008904064ba98f65e0] Sync with master
git bisect bad cfbfd45ee6e49007fdeb00008904064ba98f65e0
# good: [a4212f7ebd7158a1e237d776fb4bbd7a295d0bda] Merge branch 'pw/sequencer-in-process-commit' into next
git bisect good a4212f7ebd7158a1e237d776fb4bbd7a295d0bda
# good: [bdae4af87053490adad2dc9fb184d6d050d46a4c] Merge branch 'sg/setup-doc-update'
git bisect good bdae4af87053490adad2dc9fb184d6d050d46a4c
# bad: [6d12e08217360cfc042ec484129771f73ad7b97f] Merge branch 'rs/strbuf-read-once-reset-length' into next
git bisect bad 6d12e08217360cfc042ec484129771f73ad7b97f
# bad: [1b791d2503742b060f91b8159b85c06335634bd8] Merge branch 'ab/simplify-perl-makefile' into next
git bisect bad 1b791d2503742b060f91b8159b85c06335634bd8
# bad: [abfed699db9f0f0e6f03f5ee3e9d137944ba4216] Merge branch 'sb/clone-recursive-submodule-doc' into next
git bisect bad abfed699db9f0f0e6f03f5ee3e9d137944ba4216
# bad: [ec4d2b9c843b8b338ffa8906eea70095b3098c1e] Merge branch 'pw/sequencer-in-process-commit' into next
git bisect bad ec4d2b9c843b8b338ffa8906eea70095b3098c1e
# good: [5279b80103bde3ec5d6108fa8f43de2017668039] Merge branch 'tb/check-crlf-for-safe-crlf' into next
git bisect good 5279b80103bde3ec5d6108fa8f43de2017668039
# bad: [28d6daed4f119940ace31e523b3b272d3d153d04] sequencer: improve config handling
git bisect bad 28d6daed4f119940ace31e523b3b272d3d153d04
# first bad commit: [28d6daed4f119940ace31e523b3b272d3d153d04] sequencer: improve config handling
--
Kaartic
^ permalink raw reply [relevance 6%]
* [RFC PATCH v3 0/4] give more useful error messages while renaming branch
@ 2017-11-02 6:54 5% ` Kaartic Sivaraam
0 siblings, 0 replies; 24+ results
From: Kaartic Sivaraam @ 2017-11-02 6:54 UTC (permalink / raw)
To: git
In builtin/branch, the error messages weren't handled directly by the branch
renaming function and was left to the other function. Though this avoids
redundancy this gave unclear error messages in some cases. So, make builtin/branch
give more useful error messages.
Changes in v3:
Incorporated suggestions from v2 to improve code and commit message. To
be more precise about the code part,
In 2/4 slightly re-ordered the parameters to move the flag parameters to
the end.
In 3/4, changed the return type of the branchname validation functions to
be the enum (whose values they return) as suggested by Stefan.
Dropped the PATCH 3/5 of v2 as there was another series[1] that did the
refactor and got merged to 'next'. I have now re-rolled the series over
'next' [pointing at 273055501 (Sync with master, 2017-10-24)].
This has made the code in 3/4 a little clumsy (at least to me) as I
tried to achieve to achieve what the previous patches did with the new
validate*_branchname functionS. Let me know, if it looks too bad.
So this could go on top of 'next' without any conflicts but in case I
missed something, let me know. The series could be found in my fork[2].
Any feedback welcome.
Thanks,
Kaartic
[1] : https://public-inbox.org/git/20171013051132.3973-1-gitster@pobox.com
[2] : https://github.com/sivaraam/git/tree/work/branch-revamp
Kaartic Sivaraam (4):
branch: improve documentation and naming of 'create_branch()'
branch: re-order function arguments to group related arguments
branch: introduce dont_fail parameter for branchname validation
builtin/branch: give more useful error messages when renaming
branch.c | 63 ++++++++++++++++++++++++++++++------------------------
branch.h | 57 ++++++++++++++++++++++++++++++++++++++----------
builtin/branch.c | 49 ++++++++++++++++++++++++++++++++++--------
builtin/checkout.c | 11 +++++-----
4 files changed, 127 insertions(+), 53 deletions(-)
--
2.15.0.rc2.401.g3db9995f9
^ permalink raw reply [relevance 5%]
* [RFC PATCH v3 0/4] give more useful error messages while renaming branch
@ 2017-11-02 6:52 5% Kaartic Sivaraam
0 siblings, 0 replies; 24+ results
From: Kaartic Sivaraam @ 2017-11-02 6:52 UTC (permalink / raw)
To: git
In builtin/branch, the error messages weren't handled directly by the branch
renaming function and was left to the other function. Though this avoids
redundancy this gave unclear error messages in some cases. So, make builtin/branch
give more useful error messages.
Changes in v3:
Incorporated suggestions from v2 to improve code and commit message. To
be more precise about the code part,
In 2/4 slightly re-ordered the parameters to move the flag parameters to
the end.
In 3/4, changed the return type of the branchname validation functions to
be the enum (whose values they return) as suggested by Stefan.
Dropped the PATCH 3/5 of v2 as there was another series[1] that did the
refactor and got merged to 'next'. I have now re-rolled the series over
'next' [pointing at 273055501 (Sync with master, 2017-10-24)].
This has made the code in 3/4 a little clumsy (at least to me) as I
tried to achieve to achieve what the previous patches did with the new
validate*_branchname functionS. Let me know, if it looks too bad.
So this could go on top of 'next' without any conflicts but in case I
missed something, let me know. The series could be found in my fork[2].
Any feedback welcome.
Thanks,
Kaartic
[1] : https://public-inbox.org/git/20171013051132.3973-1-gitster@pobox.com
[2] : https://github.com/sivaraam/git/tree/work/branch-revamp
Kaartic Sivaraam (4):
branch: improve documentation and naming of 'create_branch()'
branch: re-order function arguments to group related arguments
branch: introduce dont_fail parameter for branchname validation
builtin/branch: give more useful error messages when renaming
branch.c | 63 ++++++++++++++++++++++++++++++------------------------
branch.h | 57 ++++++++++++++++++++++++++++++++++++++----------
builtin/branch.c | 49 ++++++++++++++++++++++++++++++++++--------
builtin/checkout.c | 11 +++++-----
4 files changed, 127 insertions(+), 53 deletions(-)
--
2.15.0.rc2.401.g3db9995f9
^ permalink raw reply [relevance 5%]
* [RFD] should all merge bases be equal?
@ 2016-10-17 22:28 6% Junio C Hamano
0 siblings, 0 replies; 24+ results
From: Junio C Hamano @ 2016-10-17 22:28 UTC (permalink / raw)
To: git
People can see how fast the usual merges I see everyday are by
looking at output from
$ git log --first-parent --format='%ct %s' master..pu
and noticing the seconds since epoch. Most of the days, these are
recreated directly on top of 'master' from scratch, and they get
timestamps that are very close to each other (or the same), meaning
that I am getting multiple merges per second.
Being accustomed how fast my merges go, there is one merge that
hiccups for me every once in a few days: merging back from 'master'
to 'next'. This happens after having multiple topics (that by
definition have to be the ones that were already merged to 'next'
some time ago) to 'master', and 'master' may also have its own
commit (e.g. update to "RelNotes") and merges of side branches that
were not in 'next' (e.g. merge from submaintainers like i18n, etc.)
The reason why this merge is slow is because it typically have many
merge bases. For example, today's tip of 'next' is:
commit 6021889cc14df07d4366829367d2c4a11d1eaa56
Merge: 4868def05e 659889482a
Author: Junio C Hamano <gitster@pobox.com>
Date: Mon Oct 17 14:02:05 2016 -0700
Sync with master
* master:
Tenth batch for 2.11
l10n: de.po: translate 260 new messages
l10n: de.po: fix translation of autostash
l10n: ru.po: update Russian translation
which is a merge that has 12 merge bases:
$ git merge-base --all 4868def05e 659889482a | git name-rev --stdin
3cdd5d19178a54d2e51b5098d43b57571241d0ab (ko/master)
641c900b2c3a8c3d385eb353b3801a5f49ddbb47 (js/reset-usage)
30cfe72d37ed8c174cae43923769730a94549dae (rs/pretty-format-color-doc-fix)
654311bf6ee0fbf530c088271c2c76d46f31f82d (da/mergetool-diff-order)
72710165c932edb2b8552aef7aef2f357dde4746 (sb/submodule-config-doc-drop-path)
842a516cb02a53cf0291ff67ed6f8517966345c0 (js/regexec-buf)
62fe0eb4804c297486a1d421a4f893865fcbc911 (jk/quarantine-received-objects)
a94bb683970a111b467a36590ca36e52754ad504 (rs/cocci)
e8c42cb9ce6a566aad797cc6c5bc1279d608d819 (jk/ref-symlink-loop)
22d3b8de1b625813faec6f3d6ffe66124837b78b (jk/clone-copy-alternates-fix)
7431596ab1f05a13adb93b44108f27cfd6578a31 (nd/commit-p-doc)
5275c3081c2b2c6166a2fc6b253a3acb20f8ae89 (dt/http-empty-auth)
The tip of each topic that was merged recently to 'master' since
'master' was last merged to 'next' becomes a valid merge-base by
design of the workflow. We merge a topic to 'master' whose tip
has been already in 'next' for a while, so the tip of 'next' before
merging 'master' back is a descendant of the tips of these topics,
and the tip of 'master' before I make this merge has just become a
descendant of the tips of these topics. That makes them common
ancestors between 'master' and 'next'.
But for the purpose of figuring out the 3-way merge result, I
suspect that they are pretty much useless common ancestor to use as
the merge base. The first one in the list, the old 'master' that
was merged the last time to 'next', would be a lot more useful one.
And of course, if I do this:
$ git checkout next
$ git merge master ;# this is slow due to the 12-base above
$ git checkout HEAD^ ;# detach at the previous state
$ git merge-recursive ko/master -- HEAD master
the merge is instantaneous. I'd get only what truly happened
uniquely on 'master', e.g. RelNotes update and i18n merge.
I am wondering if it is worth trying to optimize it by taking
advantage of the specific workflow (i.e. give me an option to use
when merging 'master' back to 'next') and allows me to exclude the
tips of these topic branches. Would I be making the result open to
the criss-cross merge gotchas the "recursive merge" machinery was
designed to prevent in the first place by doing so? Offhand, I do
not think that would be the case.
Assuming that it is a good idea, there is another question of how to
tell the more meaningful merge bases (ko/master in this case) out of
the less useful ones (all the others). I think it would be
sufficient to reject commits that are not on the first-parent chain
of neither branch being merged. Among the 12 merge bases, ko/master
is the only one that appears on the first-parent chain from 'master'
being merged (it does not appear on the first-parent chain from
'next'). All others were topic tips that by definition were merged
as second parent to integration branches ('next' and later 'master').
The place to do this would be a new option to 'merge-base'; instead
of "--all", perhaps "--major" option gives only the major merge bases
(with the definition of 'major' being the above heuristics), and then
"git merge-recursive" would learn "-Xmajor-base-only" strategy option,
or something along that line.
Thoughts?
^ permalink raw reply [relevance 6%]
* Re: [PATCH v6 00/17] Port branch.c to use ref-filter's printing options
@ 2016-05-17 19:02 6% ` Karthik Nayak
0 siblings, 0 replies; 24+ results
From: Karthik Nayak @ 2016-05-17 19:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git
On Tue, May 17, 2016 at 11:22 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Hello, sorry for the confusion, it's built on top of 'next' which contains
>> f307218 (t6302: simplify non-gpg cases). The merge conflict is due to the
>> commit made by you 1cca17df (Documentation: fix linkgit references).
>
> That is not "confusion", but an "incorrect piece of information".
>
> The series does not seem to apply on 'next', either.
>
> Where did you exactly rebase on top of? It is not on f307218, it is
> not on 'next', 'next@{1}',... 'next@{8}'.
>
> f3072180 (t6302: simplify non-gpg cases, 2016-05-09) was merged to
> 'next' at 9fcb98b2 (Merge branch 'es/test-gpg-tags' into next,
> 2016-05-10), but the series does not seem to apply there, either.
>
> $ git co 9fcb98b2
> Applying: ref-filter: implement %(if), %(then), and %(else) atoms
> error: patch failed: Documentation/git-for-each-ref.txt:181
> error: Documentation/git-for-each-ref.txt: patch does not apply
> Patch failed at 0001 ref-filter: implement %(if), %(then), and %(else) atoms
> The copy of the patch that failed is found in: .git/rebase-apply/patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
>
> Not that a series built on top of any 'next' is directly usable.
> You are forcing me to identify which topics in 'next' you depend on,
> and build a topic that does not contain anything unrelated that is
> in 'next' before starting to apply these patches. Can you pick a
> more appropriate place to base these patches on, please? Why isn't
> this based on 'master', for example?
Hello,
Sorry for that.
The only reason I haven't based it on 'master' is because it doesn't contain
'f307218'.
➔ git branch --contains=f307218
next
ref-filter
Now speaking of which, this is based on next.
➔ git branch -v
* next 78b384c Sync with master
And Idk what the problem is but it seems to apply perfectly on top of it [1]
➔ git am v6-00*
Applying: ref-filter: implement %(if), %(then), and %(else) atoms
Applying: ref-filter: include reference to 'used_atom' within 'atom_value'
Applying: ref-filter: implement %(if:equals=<string>) and
%(if:notequals=<string>)
Applying: ref-filter: modify "%(objectname:short)" to take length
Applying: ref-filter: move get_head_description() from branch.c
Applying: ref-filter: introduce format_ref_array_item()
Applying: ref-filter: make %(upstream:track) prints "[gone]" for
invalid upstreams
Applying: ref-filter: add support for %(upstream:track,nobracket)
Applying: ref-filter: make "%(symref)" atom work with the ':short' modifier
Applying: ref-filter: introduce refname_atom_parser_internal()
Applying: ref-filter: introduce symref_atom_parser() and refname_atom_parser()
Applying: ref-filter: make remote_ref_atom_parser() use
refname_atom_parser_internal()
Applying: ref-filter: add `:dir` and `:base` options for ref printing atoms
Applying: ref-filter: allow porcelain to translate messages in the output
Applying: branch, tag: use porcelain output
Applying: branch: use ref-filter printing APIs
Applying: branch: implement '--format' option
[1] : https://github.com/KarthikNayak/git/commits/ref-filter
--
Regards,
Karthik Nayak
^ permalink raw reply [relevance 6%]
* Re: t7063-status-untracked-cache.sh test failure on next
@ 2015-10-21 16:22 6% ` Ramsay Jones
0 siblings, 0 replies; 24+ results
From: Ramsay Jones @ 2015-10-21 16:22 UTC (permalink / raw)
To: Torsten Bögershausen, Junio C Hamano; +Cc: GIT Mailing-list, David Turner
On 21/10/15 17:05, Torsten Bögershausen wrote:
> On 21.10.15 16:37, Ramsay Jones wrote:
>> Hi Junio,
>>
>> While testing the next branch today, I had a test failure, viz:
>>
>> $ tail ntest-out-fail
>> Test Summary Report
>> -------------------
>> t7063-status-untracked-cache.sh (Wstat: 256 Tests: 32 Failed: 22)
>
> Does this patch help ?
> (Recently send & tested by David. I just copy & paste the diff)
> []
>
No, that patch is already in next and was part of the build
that failed:
$ git log -1 --oneline 9b680fbd3
9b680fb t7063: fix flaky untracked-cache test
$ git branch --contains 9b680fbd3
* next
pu
$
Again, I haven't been able to reproduce the failure ...
[I should have said the this is today's next branch
@ 3b31934 Sync with master. This is on Linux Mint 17.2]
Thanks.
ATB,
Ramsay Jones
> Signed-off-by: David Turner <dturner@twopensource.com>
> ---
> t/t7063-status-untracked-cache.sh | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh
> index 37a24c1..0e8d0d4 100755
> --- a/t/t7063-status-untracked-cache.sh
> +++ b/t/t7063-status-untracked-cache.sh
> @@ -412,7 +412,9 @@ test_expect_success 'create/modify files, some of which are gitignored' '
> echo two bis >done/two &&
> echo three >done/three && # three is gitignored
> echo four >done/four && # four is gitignored at a higher level
> - echo five >done/five # five is not gitignored
> + echo five >done/five && # five is not gitignored
> + echo test >base && #we need to ensure that the root dir is touched
> + rm base
> '
>
>
^ permalink raw reply [relevance 6%]
* BUG: check-ref-format and rev-parse can not handle branches with an @ in their name combined with @{u}
@ 2014-01-14 23:04 0% Keith Derrick
0 siblings, 0 replies; 24+ results
From: Keith Derrick @ 2014-01-14 23:04 UTC (permalink / raw)
To: git@vger.kernel.org
I couldn't find a duplicate in the JIRA instance.
According to the documentation of check-ref-format, a branch name such
as @mybranch is valid. Yet 'git check-ref-format --branch @mybranch@{u}'
claims @mybranch is an invalid branch name. yet I am able to create the
branch (which would seem the obvious place to check for an invalid
branch name)
Pick any repository with an upstream remote (I cloned git itself from
github). The following will reproduce the bug
> $cd ~/play/gitsrc/
> $
> $git status
> On branch master
> Your branch is up-to-date with 'origin/master'.
>
> nothing to commit, working directory clean
> $git checkout -t -b @mybranch origin/master
> Branch @mybranch set up to track remote branch master from origin.
> Switched to a new branch '@mybranch'
> $git branch -av
> * @mybranch 14598b9 Sync with 1.8.5.3
> master 14598b9 Sync with 1.8.5.3
> remotes/origin/HEAD -> origin/master
> remotes/origin/maint 4224916 Git 1.8.5.3
> remotes/origin/master 14598b9 Sync with 1.8.5.3
> remotes/origin/next b139ac2 Sync with master
> remotes/origin/pu 3d58c42 Merge branch 'ab/subtree-doc' into pu
> remotes/origin/todo 1066e10 What's cooking (2014/01 #02)
> $git check-ref-format --branch @mybranch
> @mybranch
> $git check-ref-format --branch @mybranch@{u}
> fatal: '@mybranch@{u}' is not a valid branch name
> $git rev-parse --abbrev-ref @mybranch
> @mybranch
> $git rev-parse --abbrev-ref @mybranch@{u}
> @mybranch@{u}
> fatal: ambiguous argument '@mybranch@{u}': unknown revision or path
> not in the working tree.
> Use '--' to separate paths from revisions, like this:
> 'git <command> [<revision>...] -- [<file>...]'
> $
The same problem appears if the single '@' is anywhere in the branch name.
I *think* the problem lies in 'interpret_empty_at' in sha1_name.c with
these statements
> if (next != name + 1)
> return -1;
Which is trying to allow the sequence '@@' but I'm not certain enough of
unintended consequences to suggest a patch.
In our real-world example, we wanted to use a naming convention where a
branch name beginning with @ was intended to be a long-lived branch (for
example, a support branch for an official release). Thus, our sequence
above actually begins with something like 'git checkout -t
origin/@release1'.
We hit this problem some considerable time after initiating the naming
convention, so it's too late to turn back for us.
As a work around, if you have the branch checked out, then using HEAD
instead of the branch name works
> $git checkout @mybranch
> Switched to branch '@mybranch'
> Your branch is up-to-date with 'origin/master'.
> $git check-ref-format --branch HEAD@{u}
> origin/master
> $git rev-parse --abbrev-ref HEAD@{u}
> origin/master
> $
Envronment:
> $git version
> git version 1.8.5.2
> $lsb_release -a
> No LSB modules are available.
> Distributor ID: Ubuntu
> Description: Ubuntu 12.04.3 LTS
> Release: 12.04
> Codename: precise
Keith Derrick
^ permalink raw reply [relevance 0%]
* [PATCH 0/6 (v2)] Detecting HEAD more reliably while cloning
@ 2008-12-01 14:12 6% Junio C Hamano
0 siblings, 0 replies; 24+ results
From: Junio C Hamano @ 2008-12-01 14:12 UTC (permalink / raw)
To: git
This is another approach to the same problem that a repository cloned from
another repository whose default branch is not 'master' can use 'master'
as the default.
The current code has to guess where the HEAD in the original repository
points at, because the original protocol tells what object each ref points
at but does not talk about which other ref a symbolic ref points at. The
implication of this is that if you prepare another branch that points at
your master, like this:
$ git checkout -b another master
and keep that other branch checked out (and in sync with 'master'), a
clone made from such a repository may incorrectly have its HEAD pointing
at 'master', not 'another'.
Instead of introducing a full-fledged protocol extension, this round hides
the new information in the same place as the server capabilities list that
is used to implement protocol extension is hidden from older clients.
This way, it does not have to work around the code structure imposed by
the transport API, does not have to introduce an extra round trip, and
does not have to trigger an annoying (but harmless) error message when an
older client contacts a new uploader.
[1/6] get_remote_heads(): refactor code to read "server capabilities"
[2/6] connect.c::read_extra_info(): prepare to receive more than server
capabilities
[3/6] connect.c::read_extra_info(): find where HEAD points at
[4/6] clone: find the current branch more explicitly
[5/6] upload-pack: send the HEAD information
[6/6] clone: test the new HEAD detection logic
The first four are the client side, the fifth one is the uploader side,
and the last one is the test. After storing these patches in separate
files, you would build this history (on top of 'master'):
git am 1 2 3 4
git reset --hard HEAD~4 5---------------M---6
git am 5 / /
git merge HEAD@{2} ---1---2---3---4
git am 6
builtin-clone.c | 24 +++++++++++++++++++-----
connect.c | 47 +++++++++++++++++++++++++++++++++++++++++++----
t/t5601-clone.sh | 11 +++++++++++
upload-pack.c | 14 +++++++++++---
4 files changed, 84 insertions(+), 12 deletions(-)
^ permalink raw reply [relevance 6%]
* [PATCH 0/5] Detecting HEAD more reliably while cloning
@ 2008-11-30 9:57 6% ` Junio C Hamano
0 siblings, 0 replies; 24+ results
From: Junio C Hamano @ 2008-11-30 9:57 UTC (permalink / raw)
To: git
This is a "works but is unsatisfactory from my acceptance standard" WIP
for review and improvements. It tries to introduce a protocol extension
that lets "git clone" discover where the HEAD points at more reliably.
The current code has to guess, because the original protocol tells what
object each ref points at but does not talk about which other ref a
symbolic ref points at. The implication of this is that if you prepare
another branch that points at your master, like this:
$ git checkout -b another master
and keep that other branch checked out (and in sync with 'master'), a
clone made from such a repository may incorrectly have its HEAD pointing
at 'master', not 'another'.
Here are the five patches to remedy the problem.
[PATCH 1/5] upload-pack.c: refactor receive_needs()
[PATCH 2/5] get_remote_heads(): do not assume that the conversation is one-way
[PATCH 3/5] clone: find the current branch more explicitly
[PATCH 4/5] upload-pack: implement protocol extension "symbolic-ref"
[PATCH 5/5] clone: test the new HEAD detection logic
The first one is a mere clean-up and is not absolutely necessary. The
second one is a preparatory step and it is needed for later steps (it by
itself does not change any behaviour).
The third one and the fourth one implement the receiver and and the sender
end respectively. It is better to test these applying each of them
independently on top of the second one and then merge the result, so that
what happens during the transition period during which old client talks to
new server (and vice versa) can be tested. The new feature is activated
only when the updated client talks to the new server, so the test appears
at the end, as a separate patch.
In other words, after storing these five patches in five separate files,
you would build this history (on top of 'master'):
git am 1 2 3
git reset --hard HEAD^ 4---M---5
git am 4 / /
git merge HEAD@{2} ---1---2---3
git am 5
The reason I say it is unsatisfactory is mostly because the protocol
extension for this is very hard in the face of ls-remote which receives
what the upload-pack says but disconnects without saying anything after
that. The upload-pack side needs to check if the receiver wants to
trigger protocol extension, but reading from the socket when talking to an
old client will trigger an error message from it, although it is actually
harmless. When git-clone runs locally, you can actually observe the error
message arising from this issue, by running t5601 after applying 1 2 and 4
but not 3 (i.e. the state after "git am 4" in the above sequence) under
"sh -x". We could trivially fix this by giving an extra parameter to
packet_read_line() and safe_read() to tell them that it is Ok if the other
end gives them an EOF if we wanted to, but I left the visible-but-harmless
breakage as is to illustrate the issue for this round.
builtin-clone.c | 24 +++++++--
builtin-fetch-pack.c | 2 +-
builtin-send-pack.c | 7 ++-
cache.h | 2 +-
connect.c | 40 ++++++++++++++-
t/t5601-clone.sh | 11 ++++
transport.c | 4 +-
upload-pack.c | 140 ++++++++++++++++++++++++++++++++------------------
8 files changed, 166 insertions(+), 64 deletions(-)
^ permalink raw reply [relevance 6%]
* Re: [PATCH 6/6] Teach core object handling functions about gitlinks
@ 2007-04-11 23:54 5% ` Martin Waitz
0 siblings, 0 replies; 24+ results
From: Martin Waitz @ 2007-04-11 23:54 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List, Junio C Hamano
[-- Attachment #1: Type: text/plain, Size: 9635 bytes --]
hoi :)
On Wed, Apr 11, 2007 at 08:16:10AM -0700, Linus Torvalds wrote:
> Branches in submodules actually in many ways are *more* important than
> branches in supermodules - it's just that with the CVS mentality, you
> would never actually see that, because CVS obviously doesn't really
> support such a notion.
I fully agree with you about the importance of submodule branches.
In fact, I want to make them even more important and useable!
And by the way, I long forgot about CVS ;-)
> So I'd argue that branches in submodules give you:
>
> - you can develop the submodule *independently* of the supermodule, but
> still be able to easily merge back and forth.
>
> Quite often, the submodule would be developed entirely _outside_ of the
> supermodule, and the "branch" that gets the most development would thus
> actually be the "vendor branch", entirely outside the supermodule. Call
> that the "main" branch or whatever, inside the supermodule it would
> often be something like the remote "remotes/origin/master" branch.
>
> So inside the supermodule, the HEAD would generally point to something
> that is *not* necessarily the "main development" branch, because the
> supermodule maintainer would quite logically and often have his own
> modifications to the original project on that branch. It migth be a
> detached branch, or just a local branch inside the submodule.
I fully agree.
> - branches inside submodules are *also* very useful even inside the
> supermodule, ie they again allow topic work to be fetched into the
> submodule *without* having to actually be part of the supermodule,
> or as a way to track a certain experimental branch of the supermodule.
>
> I suspect that most supermodule usage is as an "integrator" branch,
> which means that the supermodule tends to follow the "main
> development", and the whole point of the supermodule is largely to have
> a collection of "stable things that work together".
>
> In contrast, branches within submodules are useful for doing all the
> development that is *not* yet ready to be committed to the supermodule,
> exactly because it's not yet been tested in the full "make World" kind
> of situation.
I fully agree.
You are just so much better in describing things than I am...
> > Whenever you do a checkout in the supermodule you also have to update
> > the submodule and this update has to change the same thing which is read
> > above.
>
> I suspect (but will not guarantee) that the right approach is that a
> supermodule checkout usually just uses a "detached HEAD" setup. Within the
> context of the supermodule, only the actual commit SHA1 matters, not what
> branch it was developed on (side note: I haven't decided if we should
> allow the SHA1 to be a signed tag object too - the current patches
> obviously don't care since they never follow the SHA1 anyway, and it might
> be a good idea).
If you use a detached HEAD then you can no longer switch back to it
once you used some other (independent) branch (for testing or whatever).
This is my main argument: If you just update some 'special'
refs/heads/from-supermodule (or whatever, maybe get it from
.gitmodules/config) you can still switch between branches, making them
more useful IMHO.
If we create some other way to easily get to the commit referenced by
the index of the supermodule then a detached HEAD is ok for me, too.
But why create two things (this not-yet-existing way to get the
supermodule index entry, plus submodules HEAD) for the same thing?
Why not simply create a new refs/heads/whatever?
This is easy and everybody knows how to work with it.
> So I strongly suspect (and that is what the patch series embodies) that as
> far as the supermodule is concerned, it should *not* matter at all what
> branch the subproject was on. The subproject can use branches for
> development, and the supermodule really doesn't care what the local
> branchname was when a commit was made - because branch-names are *local*
> things, and a branch that is called "experimental" in one environment
> might be called "master" in another.
Fully agree.
Please don't confuse my "I always want to use one dedicated branch" with
"I always want to use one special branch from the submodule project".
This refs/heads/whatever I am talking about is _purely_ for ease of
use of the submodule inside the supermodule. It is in no way linked
to the branchnames that are used by the submodule project.
Well, besides that you can merge back and forth between them, of course.
> So once the commit hits the superproject, the branch identities just go
> away (only as far as the superproject is concerned, of course - the
> subproject still stays with whatever branches it has), and the only thing
> that matters is the commit SHA1.
Fully agree.
> > Updating the branch which HEAD points to is dangerous.
>
> I would strongly suggest that the *superproject* never really change the
> status of the subproject HEAD, except it updates it for "pull/reset", and
> then it just would use whatever the subproject decided to use.
>
> The subproject HEAD policy would be entirely under the control of the
> subproject. If the subproject wants to use a branch to track the
> superproject, go wild: have a real branch that is called "my-integration"
> and make HEAD a symref to that (and thus any work in the superproject will
> update that branch - something that is visible when you pull directly from
> that subproject!)
So you now have this nice "my-integration" branch lying next to other
independent (not-supermodule-related) branches.
If you want to _switch_ to one of these unrelated branches you obviously
have to change HEAD, and suddenly your unrelated branches are
considered to be part of the supermodule (ok, not yet part of its
index of course, but now all supermodule operations would work on
this unrelated branch).
I want to preserve these unrelated branches and see them as a strong
feature. Branches in submodules should be independent from the
supermodule _because_ the supermodule has no notion of which branch
is used.
> But quite often, I suspect that a subproject would just use a detached
> HEAD. The subproject may have branches of its own, of course, but you can
> think of HEAD as not being connected to any of it's "own" branches, but
> simply being the "superproject branch". That's a fairly accurate picture
> of reality, and using "detached HEAD" sounds like a very natural thing to
> do in that situation.
Only that you loose your nice detached HEAD view once you start using
those nice branches inside your submodule.
> So I really think you can do both, and I think using HEAD inside the
> superproject gives you exactly that flexibility - you can decide on a
> per-subproject basis whether HEAD should track a real local branch in a
> subproject, or whether it should be detached.
>
> (Side note: if you do *not* use detatched HEAD, I suspect the .gitmodules
> file could also contain the branchname to be used for the subproject
> tracking, but I think that's a detail, and quite debatable)
>
> > So my advice is:
> > Always read and write one dedicated branch (hardcoded "master" or
> > configurable) when the supermodule wants to access a submodule.
>
> So the main reasons I don't think that is a good idea are:
>
> - it's less flexible: see above on why you might want to use a dedicated
> branch *or* just detached HEAD, and why you might want to choose your
> own name for the dedicated branch.
In terms of flexibility it is important what you can do with the
submodule. Being able to use branches just like in a normal
repository ("switch the branch to go to an other, unrelated branch")
is a plus for me.
A detached HEAD does not give the same level of flexibility as a real
head.
> - it's also going to be quite confusing when the superproject sees
> something *else* than what is actually checked out.
Well, the user explicitly expressed his intent to switch to another
branch! In a normal repository you are not confused about the working
directory not being in sync with "master", and we always prominently state
which branch you are on. Of course this has to be clear for submodules,
too. So if you do git-status in the supermodule it should print some
"submodule is on different branch"-dirty marker.
At least I had some situations where I wanted to use something like
this: use some experimental brach which should not be directly touched
by the supermodule. Instead provide a method ("git merge
from-supermodule") to sync your working branch with new stuff from
the supermodule.
> This is an equally strong argument for just using HEAD - when we
> actually implement a
>
> git diff --subproject
>
> flag that recurses into the subproject, if you don't use HEAD inside
> the subproject, that suddenly becomes a *very* confusing thing.
This is right. Suddenly we have one more player in the field which
you can diff against.
Before submodules:
tree <-> index <-> working file
submodules always using HEAD:
tree <-> index <-> submodule HEAD <-> submodule working dir
submodules using some dedicated branch:
tree <-> index <-> subm. "from-supermodule" <-> subm. HEAD <-> subm. wd
I haven't thought about which diff really makes sense in which
situation.
--
Martin Waitz
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [relevance 5%]
* git merge (resolve) _is_ stupid
@ 2006-07-31 8:44 7% Junio C Hamano
0 siblings, 0 replies; 24+ results
From: Junio C Hamano @ 2006-07-31 8:44 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin
While dealing with Jakub's gitweb changes, I found that merge we
use _is_ very stupid. Perhaps it is not a surprise...
My "next" was at 688a750, and "master" was at b63fafd. I had
already merged gitweb changes from Luben to "master" at this
point, and "next" was in sync with "master" with respect to
gitweb/, i.e. this gave empty:
$ git diff master next gitweb/
Jakub's gitweb changes were stored in jn/web branch which
contained 16 patches on top of "master".
However, pulling "jn/web" (594e212) into "next" using resolve
strategy produced interesting conflicts. Essentially, it said
that some changes done by Luben (which has already merged to
"master" and precedes what Jakub did in "jn/web") conflict with
what "jn/web" does, which is quite bogus.
This merge has three merge-bases, but
$ git log master..next -- gitweb/
shows only two merges, and that is not so surprising given that
at this point gitweb/ files are identical between master and
next.
More interestingly, if I merge "master" to "next" first and then
merge "jn/web" on top of the result, the resolve strategy does
the right thing. This is understandable -- by merging "master"
into "next", merge base between "next" and "jn/web" becomes
"master" and nothing else.
Using recursive strategy resolved this merge correctly, taking
gitweb/ files from "jn/web" branch.
By the way, the "recur" strategy in "next" produces the correct
result, but it produces a funny error in the middle (that is why
Johannes is CC'ed).
error: Could not read 0100000000000000000000000000000000000000
: gitster; git merge -s recur 'test merge' next jn/web
Merging next with 594e212bc849039a204deef1d16c2eddcc451532
Merging:
688a75071490101dbc660e3304aafb7a13e28807 Merge branch '__/setup-n-mv' into next
594e212bc849039a204deef1d16c2eddcc451532 gitweb: Ref refactoring - use git_get_referencing for marking tagged/head commits
found 3 common ancestor(s):
7061cf0f205e86613c3a3306fdfedf2a5dcc8a65 Merge branch 'lt/setup' into __/setup-n-mv
acb0f6f33760b43c1fc9617a45346ab3738f021a gitweb.cgi: git_blame2: slight optimization reading the blame lines
2d023581c9a0ae5efdebfd0084d54d09669a25d5 Set datarootdir in config.mak.in
Merging:
7061cf0f205e86613c3a3306fdfedf2a5dcc8a65 Merge branch 'lt/setup' into __/setup-n-mv
acb0f6f33760b43c1fc9617a45346ab3738f021a gitweb.cgi: git_blame2: slight optimization reading the blame lines
found 1 common ancestor(s):
634331061599a82968daddae2d2c0896b6137d4c gitweb.cgi: Centralize printing of the page path
Auto-merging gitweb/gitweb.cgi
Merging:
virtual merged tree
2d023581c9a0ae5efdebfd0084d54d09669a25d5 Set datarootdir in config.mak.in
error: Could not read 0100000000000000000000000000000000000000
found 1 common ancestor(s):
7b8cf0cf2973cc8df3bdd36b9b36542b1f04d70a Rename man1 and man7 variables to man1dir and man7dir
Auto-merging .gitignore
Auto-merging INSTALL
Auto-merging Makefile
Auto-merging Makefile
Merge made by recur.
Documentation/git-tar-tree.txt | 5
Makefile | 3
git.c | 76 ++---
gitweb/gitweb.cgi | 609 +++++++++++++++++-----------------------
setup.c | 4
5 files changed, 300 insertions(+), 397 deletions(-)
^ permalink raw reply [relevance 7%]
Results 1-24 of 24 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2006-07-31 8:44 7% git merge (resolve) _is_ stupid Junio C Hamano
2007-04-10 4:12 [PATCH 0/6] Initial subproject support (RFC?) Linus Torvalds
2007-04-10 4:20 ` [PATCH 6/6] Teach core object handling functions about gitlinks Linus Torvalds
2007-04-11 8:06 ` Martin Waitz
2007-04-11 15:16 ` Linus Torvalds
2007-04-11 23:54 5% ` Martin Waitz
2008-11-30 9:57 [PATCH 5/5] clone: test the new HEAD detection logic Junio C Hamano
2008-11-30 9:57 ` [PATCH 4/5] upload-pack: implement protocol extension "symbolic-ref" Junio C Hamano
2008-11-30 9:57 ` [PATCH 3/5] clone: find the current branch more explicitly Junio C Hamano
2008-11-30 9:57 ` [PATCH 2/5] get_remote_heads(): do not assume that the conversation is one-way Junio C Hamano
2008-11-30 9:57 ` [PATCH 1/5] upload-pack.c: refactor receive_needs() Junio C Hamano
2008-11-30 9:57 6% ` [PATCH 0/5] Detecting HEAD more reliably while cloning Junio C Hamano
2008-12-01 14:12 6% [PATCH 0/6 (v2)] " Junio C Hamano
2014-01-14 23:04 0% BUG: check-ref-format and rev-parse can not handle branches with an @ in their name combined with @{u} Keith Derrick
2015-10-21 14:37 t7063-status-untracked-cache.sh test failure on next Ramsay Jones
2015-10-21 16:05 ` Torsten Bögershausen
2015-10-21 16:22 6% ` Ramsay Jones
2016-05-15 10:45 [PATCH v6 00/17] Port branch.c to use ref-filter's printing options Karthik Nayak
2016-05-16 22:12 ` Junio C Hamano
2016-05-17 8:04 ` Karthik Nayak
2016-05-17 17:52 ` Junio C Hamano
2016-05-17 19:02 6% ` Karthik Nayak
2016-10-17 22:28 6% [RFD] should all merge bases be equal? Junio C Hamano
2017-09-25 8:20 [RFC PATCH v2 0/5] Give more useful error messages when renaming a branch Kaartic Sivaraam
2017-11-02 6:54 5% ` [RFC PATCH v3 0/4] give more useful error messages while renaming branch Kaartic Sivaraam
2017-11-02 6:52 5% Kaartic Sivaraam
2017-12-11 23:44 [PATCH v5 0/9] sequencer: don't fork git commit Junio C Hamano
2017-12-13 11:46 ` [PATCH] sequencer: improve config handling Phillip Wood
2017-12-20 18:33 ` Error in `git': free(): invalid pointer (was Re: [PATCH] sequencer: improve config handling) Kaartic Sivaraam
[not found] ` <18737953.1042351513802399608.JavaMail.defaultUser@defaultHost>
2017-12-21 15:02 6% ` Kaartic Sivaraam
2020-02-26 10:14 6% [Outreachy][PATCH 00/10] Finish converting git bisect to C part 2 Miriam Rubio
2021-04-21 20:46 [PATCH v5 3/3] config: allow overriding of global and system configuration SZEDER Gábor
2021-04-23 5:47 5% ` [PATCH] t1300: fix unset of GIT_CONFIG_NOSYSTEM leaking into subsequent tests Patrick Steinhardt
2021-12-21 18:05 [PATCH 0/9] Add a new --remerge-diff capability to show & log Elijah Newren via GitGitGadget
2021-12-25 7:59 ` [PATCH v2 0/8] " Elijah Newren via GitGitGadget
2021-12-26 21:52 7% ` Ævar Arnfjörð Bjarmason
2021-12-27 21:11 5% ` Elijah Newren
2022-01-10 15:48 0% ` Ævar Arnfjörð Bjarmason
2022-02-17 6:38 [PATCH] merge-ort: make informational messages from recursive merges clearer Elijah Newren via GitGitGadget
2022-02-17 23:10 ` Junio C Hamano
2022-03-01 6:44 5% ` Junio C Hamano
2022-03-02 2:32 0% ` Elijah Newren
2022-03-02 4:21 0% ` Elijah Newren
2022-03-02 6:53 0% ` Junio C Hamano
2022-10-31 23:47 [PATCH 0/2] Documentation/howto/maintain-git.txt: a pair of bugfixes Taylor Blau
2022-11-01 8:01 ` Jeff King
2022-11-01 22:23 6% ` Junio C Hamano
2023-04-28 9:23 [PATCH v9 0/6] notes.c: introduce "--separator" option Teng Long
2023-04-28 9:23 ` [PATCH v9 4/6] notes.c: introduce '--separator=<paragraph-break>' option Teng Long
2023-05-10 19:19 6% ` Kristoffer Haugsbakk
2023-07-15 10:37 [PATCH] pretty: add %(decorate[:<options>]) format Andy Koppe
2023-07-15 16:07 ` [PATCH v2] " Andy Koppe
2023-07-19 18:16 ` Glen Choo
2023-08-11 18:59 ` Andy Koppe
2023-08-15 18:13 ` Junio C Hamano
2023-08-15 18:28 ` Andy Koppe
2023-08-15 19:01 ` Junio C Hamano
2023-08-15 19:29 ` main != master at github.com/git/git Andy Koppe
2023-08-15 22:16 ` Taylor Blau
2023-08-16 2:24 ` Jeff King
2023-08-16 13:30 ` rsbecker
2023-08-18 0:35 ` Junio C Hamano
2023-08-21 14:56 ` Johannes Schindelin
2023-08-21 16:17 ` Junio C Hamano
2023-08-22 0:31 6% ` [PATCH] ci: avoid building from the same commit in parallel Junio C Hamano
2023-08-22 4:36 ` Junio C Hamano
2023-08-22 4:48 ` Johannes Schindelin
2023-08-22 15:31 ` Junio C Hamano
2023-08-23 8:42 5% ` Johannes Schindelin
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).