Repository navigation
Conversation
Welcome to GitGitGadgetHi @tsukasaI, and welcome to GitGitGadget, the GitHub App to send patch series to the Git mailing list from GitHub Pull Requests. Please make sure that either:
You can CC potential reviewers by adding a footer to the PR description with the following syntax: NOTE: DO NOT copy/paste your CC list from a previous GGG PR's description, Also, it is a good idea to review the commit messages one last time, as the Git project expects them in a quite specific form:
It is in general a good idea to await the automated test ("Checks") in this Pull Request before contributing the patches, e.g. to avoid trivial issues such as unportable code. Contributing the patchesBefore you can contribute the patches, your GitHub username needs to be added to the list of permitted users. Any already-permitted user can do that, by adding a comment to your PR of the form Both the person who commented An alternative is the channel Once on the list of permitted usernames, you can contribute the patches to the Git mailing list by adding a PR comment If you want to see what email(s) would be sent for a After you submit, GitGitGadget will respond with another comment that contains the link to the cover letter mail in the Git mailing list archive. Please make sure to monitor the discussion in that thread and to address comments and suggestions (while the comments and suggestions will be mirrored into the PR by GitGitGadget, you will still want to reply via mail). If you do not want to subscribe to the Git mailing list just to be able to respond to a mail, you can download the mbox from the Git mailing list archive (click the curl -g --user "<EMailAddress>:<Password>" \
--url "imaps://imap.gmail.com/INBOX" -T /path/to/raw.txtTo iterate on your change, i.e. send a revised patch or patch series, you will first want to (force-)push to the same branch. You probably also want to modify your Pull Request description (or title). It is a good idea to summarize the revision by adding something like this to the cover letter (read: by editing the first comment on the PR, i.e. the PR description): To send a new iteration, just add another PR comment with the contents: Need help?New contributors who want advice are encouraged to join git-mentoring@googlegroups.com, where volunteers who regularly contribute to Git are willing to answer newbie questions, give advice, or otherwise provide mentoring to interested contributors. You must join in order to post or view messages, but anyone can join. You may also be able to find help in real time in the developer IRC channel, |
|
Hi, this is my first contribution to Git. This PR fixes a wrong hint shown by switch/checkout when writing the |
|
/allow |
|
User tsukasaI is now allowed to use GitGitGadget. WARNING: tsukasaI has no public email address set on GitHub; GitGitGadget needs an email address to Cc: you on your contribution, so that you receive any feedback on the Git mailing list. Go to https://github.com/settings/profile to make your preferred email public to let GitGitGadget know which email address to use. |
|
/submit |
|
Submitted as pull.2268.git.1792458904027.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
Junio C Hamano wrote on the Git mailing list (how to reply to this email): "tsukasa-ino via GitGitGadget" <gitgitgadget@gmail.com> writes:
> hint: git branch --set-upstream-to=origin/refs/heads/feat
>
> The hint says origin/refs/heads/feat, but the correct name is
> origin/feat. Running the command as printed fails, because the full
> ref name is not a valid remote-tracking branch name here.
Good observation. Thanks for noticing.
> +test_expect_success '--set-upstream-to hint uses short remote-tracking branch name on locked config' '
> + test_when_finished "git branch -D hint-local" &&
> + test_when_finished "git update-ref -d refs/remotes/hintremote/hintbranch" &&
> + test_config remote.hintremote.url . &&
> + test_config remote.hintremote.fetch "+refs/heads/*:refs/remotes/hintremote/*" &&
> + git update-ref refs/remotes/hintremote/hintbranch HEAD &&
> + git branch hint-local &&
> + test_when_finished "rm -f .git/config.lock" &&
> + >.git/config.lock &&
> + test_must_fail git -c core.configLockTimeout=0 \
> + branch --set-upstream-to=hintremote/hintbranch hint-local 2>err &&
> + test_grep "git branch --set-upstream-to=hintremote/hintbranch\$" err &&
> + test_grep ! "refs/heads/" err
Perhaps it makes sense to also test that the suggested command line
is accepted after all of the above?
> +'
Thanks. |
14eac91 to
f5524d1
Compare
|
Harald Nordgren wrote on the Git mailing list (how to reply to this email): > When git switch fails to write the tracking config, it prints a hint
> with the command to set the upstream by hand. For example, if
> .git/config.lock exists:
>
> error: could not lock config file .git/config: File exists
> error: unable to write upstream branch configuration
> hint:
> hint: After fixing the error cause you may try to fix up
> hint: the remote tracking information by invoking:
> hint: git branch --set-upstream-to=origin/refs/heads/feat
>
> The hint says origin/refs/heads/feat, but the correct name is
> origin/feat. Running the command as printed fails, because the full
> ref name is not a valid remote-tracking branch name here.
Very nice find!
> +test_expect_success '--set-upstream-to hint uses short remote-tracking branch name on locked config' '
> + test_when_finished "git branch -D hint-local" &&
> + test_when_finished "git update-ref -d refs/remotes/hintremote/hintbranch" &&
> + test_config remote.hintremote.url . &&
> + test_config remote.hintremote.fetch "+refs/heads/*:refs/remotes/hintremote/*" &&
> + git update-ref refs/remotes/hintremote/hintbranch HEAD &&
> + git branch hint-local &&
> + test_when_finished "rm -f .git/config.lock" &&
> + >.git/config.lock &&
> + test_must_fail git -c core.configLockTimeout=0 \
> + branch --set-upstream-to=hintremote/hintbranch hint-local 2>err &&
> + test_grep "git branch --set-upstream-to=hintremote/hintbranch\$" err &&
> + test_grep ! "refs/heads/" err
> +'
Would be good to test the full multiline hint message. After all, it's
the point of the topic.
Right now, you are asserting that the correct form
'hintremote/hintbranch' works, but we want to make sure that the hint
actually gives that.
Harald |
|
User |
When git switch fails to write the tracking config, it prints a hint
with the command to set the upstream by hand. For example, if
.git/config.lock exists:
error: could not lock config file .git/config: File exists
error: unable to write upstream branch configuration
hint:
hint: After fixing the error cause you may try to fix up
hint: the remote tracking information by invoking:
hint: git branch --set-upstream-to=origin/refs/heads/feat
The hint says origin/refs/heads/feat, but the correct name is
origin/feat. Running the command as printed fails, because the full
ref name is not a valid remote-tracking branch name here.
This has been wrong since a3f40ec (branch: accept multiple upstream
branches for tracking, 2021-12-20).
Strip refs/heads/ from the hint, as the success message in the same
function already does. The git config --add line for multiple upstreams
still needs the full ref name, so it is not changed.
Signed-off-by: tsukasa inoue <inoue.tsukasa.4@gmail.com>
setup_tracking() calls exit(1) when it cannot write the tracking configuration. This exit was introduced in 27852b2 (branch: report errors in tracking branch setup, 2016-02-22). Because of it, callers cannot continue after a tracking failure, even though the ref has already been created. Change setup_tracking() to return an error instead of exiting, and make create_branch() and dwim_and_setup_tracking() return that value too. This does not change behavior. Callers still exit with status 1 on failure, as before. Changing checkout Signed-off-by: tsukasa inoue <inoue.tsukasa.4@gmail.com>
When git switch -c <new> <remote>/<branch> (or checkout -b) cannot write the tracking config, for example because .git/config.lock exists, the worktree, the index and the new branch ref have already been updated. Git then exits with status 1 while HEAD still points to the original branch. The changes of the switch remain staged, and the post-checkout hook does not run. create_branch() now returns an error instead of exiting. Make checkout ignore that error so that the switch completes and the command exits with status 0. The error and the hint are still printed. Match push -u, clone and fetch, which also print the error and continue. The upstream can be set later with the command shown in the hint. Keep git branch <new> <start> failing when the tracking config cannot be written, so its behavior does not change; t3200 covers this. Making the switch fully atomic, undoing the worktree, the index, the ref and the config when the tracking config fails, is a separate topic. Signed-off-by: tsukasa inoue <inoue.tsukasa.4@gmail.com>
f5524d1 to
be94ac7
Compare
tsukasaI
changed the title
|
This branch is now known as |
|
This patch series was integrated into seen via git@6e1e80b. |
|
/submit |
|
Submitted as pull.2268.v2.git.1792618518.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
Junio C Hamano wrote on the Git mailing list (how to reply to this email): "tsukasa-ino via GitGitGadget" <gitgitgadget@gmail.com> writes:
> When the tracking config cannot be written (for example, because
> .git/config.lock exists), git switch -c and checkout -b stop halfway. The
> worktree, the index and the new branch ref are already updated, but HEAD
> stays on the old branch, and the changes are left staged. The hint printed
> at that point was also wrong: it suggested origin/refs/heads/.
>
> 1/3 strips refs/heads/ from the hint. 2/3 makes setup_tracking() return an
> error instead of calling exit(). No behavior change yet. 3/3 makes switch
> and checkout finish the switch and exit with status 0, as push -u, clone and
> fetch already do. git branch still fails.
>
> Changes since v1:
>
> * 1/3: the test now compares the whole hint message, as suggested by Harald
> Nordgren. It also checks that the command in the hint works once the lock
> is removed, as suggested by Junio
> (https://lore.kernel.org/git/xmqq7bjcup46.fsf@gitster.g/).
OK.
> * 2/3 and 3/3 are new. They implement the direction agreed on the bug
> report thread (https://lore.kernel.org/git/xmqqecdjsryk.fsf@gitster.g/).
I am not sure if these two should be part of the same topic as the
first one, or they should be treated separately. IOW, if the way
these two patches implement the fix proves to be contentious, does
it make sense to hold the change in 1/3 until 2&3/3 are resolved
(and vice versa), or can they be treated as two separate issues?
> Making the switch fully atomic, so that a failure undoes the worktree, the
> index, the ref and the config, is left for a separate topic.
Sensible. |
|
井上司 wrote on the Git mailing list (how to reply to this email): > I am not sure if these two should be part of the same topic as the
> first one, or they should be treated separately. IOW, if the way
> these two patches implement the fix proves to be contentious, does
> it make sense to hold the change in 1/3 until 2&3/3 are resolved
> (and vice versa), or can they be treated as two separate issues?
Thanks for the question. I checked whether the patches can be split.
- 2/3 applies without 1/3.
- 3/3 conflicts with 1/3 only in t3200, because its new test is placed
right after the one added by 1/3. The code does not depend on 1/3. The
hint sentence in the 3/3 commit message assumes the corrected hint, but
that is only a wording dependency.
So 1/3 can be taken on its own. 2/3 and 3/3 can be discussed separately,
and changes to them do not need to hold up 1/3. I will resend 2/3 and 3/3
as a separate series on top of the current master, so that 1/3 can be
applied first.
2026年10月22日(木) 7:20 Junio C Hamano <gitster@pobox.com>:
>
> "tsukasa-ino via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > When the tracking config cannot be written (for example, because
> > .git/config.lock exists), git switch -c and checkout -b stop halfway. The
> > worktree, the index and the new branch ref are already updated, but HEAD
> > stays on the old branch, and the changes are left staged. The hint printed
> > at that point was also wrong: it suggested origin/refs/heads/.
> >
> > 1/3 strips refs/heads/ from the hint. 2/3 makes setup_tracking() return an
> > error instead of calling exit(). No behavior change yet. 3/3 makes switch
> > and checkout finish the switch and exit with status 0, as push -u, clone and
> > fetch already do. git branch still fails.
> >
> > Changes since v1:
> >
> > * 1/3: the test now compares the whole hint message, as suggested by Harald
> > Nordgren. It also checks that the command in the hint works once the lock
> > is removed, as suggested by Junio
> > (https://lore.kernel.org/git/xmqq7bjcup46.fsf@gitster.g/).
>
> OK.
>
> > * 2/3 and 3/3 are new. They implement the direction agreed on the bug
> > report thread (https://lore.kernel.org/git/xmqqecdjsryk.fsf@gitster.g/).
>
> I am not sure if these two should be part of the same topic as the
> first one, or they should be treated separately. IOW, if the way
> these two patches implement the fix proves to be contentious, does
> it make sense to hold the change in 1/3 until 2&3/3 are resolved
> (and vice versa), or can they be treated as two separate issues?
>
> > Making the switch fully atomic, so that a failure undoes the worktree, the
> > index, the ref and the config, is left for a separate topic.
>
> Sensible. |
2 participants
When the tracking config cannot be written (for example, because
.git/config.lock exists), git switch -c and checkout -b stop halfway.
The worktree, the index and the new branch ref are already updated, but
HEAD stays on the old branch, and the changes are left staged. The hint
printed at that point was also wrong: it suggested
origin/refs/heads/.
1/3 strips refs/heads/ from the hint.
2/3 makes setup_tracking() return an error instead of calling exit().
No behavior change yet.
3/3 makes switch and checkout finish the switch and exit with status 0,
as push -u, clone and fetch already do. git branch still fails.
Changes since v1:
Harald Nordgren. It also checks that the command in the hint works
once the lock is removed, as suggested by Junio
(https://lore.kernel.org/git/xmqq7bjcup46.fsf@gitster.g/).
report thread (https://lore.kernel.org/git/xmqqecdjsryk.fsf@gitster.g/).
Making the switch fully atomic, so that a failure undoes the worktree,
the index, the ref and the config, is left for a separate topic.
cc: Harald Nordgren haraldnordgren@gmail.com