* Re: [PATCH v2 2/3] builtin/rebase.c: Emit warning when rebasing without a forkpoint
From: Phillip Wood @ 2023-09-04 10:16 UTC (permalink / raw)
To: Junio C Hamano, Wesley Schwengle; +Cc: git
In-Reply-To: <xmqqlednuagl.fsf@gitster.g>
On 03/09/2023 05:50, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> If you rewind to lose commits from the branch you are (re)building
>> against, and what was rewound and discarded was part of the work you
>> are building, whether it is on a local branch or on a remote branch
>> that contains what you have already pushed, they will be discarded,
>> it is by design, and it is a known deficiency with the fork-point
>> heuristics. How the fork-point heuristics breaks down is rather
>> well known ...
>
> Another tangent, this time very closely related to this topic, is
> that it may be worth warning when the fork-point heuristics chooses
> the base commit that is different from the original upstream,
> regardless of how we ended up using fork-point heuristics.
I think that is a good idea and would help to mitigate the surprise that
some users have expressed when --fork-point kicks and they didn't know
about it. I think we may want to compare "branch_base" which holds the
merge-base of HEAD and upstream with "restrict_revision" to decide when
to warn.
Best Wishes
Phillip
> Experienced users may not be confused when the heuristics kicks in
> and when it does not (e.g. because they configured, because they
> used the "lazy" form, or because they gave "--fork-point" from the
> command line explicitly), but they still may get surprising results
> if a reflog entry chosen to be used as the base by the heuristics is
> not what they expected to be used, and can lose their work that way.
> Imagine that you pushed your work to the remote that is a shared
> repository, and then continued building on top of it, while others
> rewound the remote branch to eject your work, and your "git fetch"
> updated the remote-tracking branch. You'll be pretty much in the
> same situation you had in your reproduction recipe that rewound your
> own local branch that you used to build your derived work on and
> would lose your work the same way, if you do not notice that the
> remote branch has been rewound (and the fork-point heuristics chose
> a "wrong" commit from the reflog of your remote-tracking branch.
>
> Perhaps something along the lines of this (not even compile tested,
> though)... It might even be useful to show a shortlog between the
> .restrict_revision and .upstream, which is the list of commits that
> is potentially lost, but that might turn out to be excessively loud
> and noisy in the workflow of those who do benefit from the
> fork-point heuristics because their project rewinds branches too
> often and too wildly for them to manually keep track of. I dunno.
>
>
> builtin/rebase.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git c/builtin/rebase.c w/builtin/rebase.c
> index 50cb85751f..432a97e205 100644
> --- c/builtin/rebase.c
> +++ w/builtin/rebase.c
> @@ -1721,9 +1721,15 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
> if (keep_base && options.reapply_cherry_picks)
> options.upstream = options.onto;
>
> - if (options.fork_point > 0)
> + if (options.fork_point > 0) {
> options.restrict_revision =
> get_fork_point(options.upstream_name, options.orig_head);
> + if (options.restrict_revision &&
> + options.restrict_revision != options.upstream)
> + warning(_("fork-point heuristics using %s from the reflog of %s"),
> + oid_to_hex(&options.restrict_revision->object.oid),
> + options.upstream_name);
> + }
>
> if (repo_read_index(the_repository) < 0)
> die(_("could not read index"));
>
^ permalink raw reply
* Re: [PATCH] sequencer: update abort safety file more sparingly
From: Oswald Buddenhagen @ 2023-09-04 12:48 UTC (permalink / raw)
To: phillip.wood
Cc: git, Stephan Beyer, Johannes Schindelin, Rohit Ashiwal,
Junio C Hamano
In-Reply-To: <4e0628ab-c39c-410d-864b-b7c74f9e04b1@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3052 bytes --]
On Mon, Sep 04, 2023 at 11:05:22AM +0100, Phillip Wood wrote:
>On 03/09/2023 21:18, Oswald Buddenhagen wrote:
>> On Sun, Sep 03, 2023 at 08:48:14PM +0100, Phillip Wood wrote:
>>> On 03/09/2023 20:25, Oswald Buddenhagen wrote:
>>>> On Sun, Sep 03, 2023 at 07:40:00PM +0100, Phillip Wood wrote:
>>>>> it only matters for "cherry-pick --skip"
>>>>>
>>>> that doesn't seem right. a --skip is just a --continue with a prior
>>>> reset, more or less.
>>>
>>> sequencer_skip() calls rollback_is_safe() which checks the abort
>>> safety file.
>>>
>> that's weird. can you think of a good reason for doing that?
>
>I think it is clear from the code - so it does not reset changes after
>the user has committed the conflict resolution.
>
yeah, i've researched that meanwhile.
the background is that the machinery that was originally introduced for
abort safety was later (ab-)used for checking whether a --skip makes
sense (de81ca3f36, "cherry-pick/revert: add --skip option", 2019-07-02).
this made the state file a misnomer (not that "abort-safety" was a
particularly good name to start with - i'd have used "latest-head" or
some such).
but the implementation made no sense to me, so i read the mailing list
archive. the result is the attached patch.
however, even so, it seems kinda wrong to me: going by HEAD means that
dropping commits would also trigger it, which would make the given
advice misleading.
in fact, the situation this code path is covering is fundamentally
different from the normal merge conflict: rather than letting the user
resolve it and us finishing the commit, we are rescheduling the pick.
but that means that --skip needs to skip whatever the next command is?
that doesn't sound right.
also, i just tried --continue after a path conflict, and it apparently
did the same as --skip, so something is really wrong.
also, when we have no _HEAD, actually attempting to `reset --merge` is
pointless, no?
oh, and i just noticed that the git-prompt is buggy: it doesn't tell me
about the interrupted multi-pick nested into an interrupted rebase.
ugh, and rebase lets me continue despite still being in the multi-pick.
and the path conflict check is made ineffective by the file in question
being in .gitignore?! (i force-added config.mak.autogen for testing, and
cherry-picking over it goes through just fine.)
>> i think i'd aim for an object-oriented-ish design with an
>> encapsulated state, lazy loading getters, lazy setters, and a commit
>> entry point (or maybe several partial ones). no idea how that would
>> play out.
>
>I've been working on something similar
>
awesome!
(well, except for the rebase nightmare in my own series i expect because
of this.)
>to only write the state to disc when the sequencer stops for user
>interaction.
>
note that this must cover ctrl-c as well, because the sequencer state
must be consistent with HEAD. of course one could also delay updating
HEAD, but that hinges on no relevant hooks being present, i think?
git-replay has a huge advantage here ...
regards
[-- Attachment #2: 0001-sequencer-improve-comment-in-sequencer_skip.patch --]
[-- Type: text/x-diff, Size: 1823 bytes --]
From eb81dc1d5ecb7d9d3cf4608b93c30250392f6fc7 Mon Sep 17 00:00:00 2001
From: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Date: Mon, 4 Sep 2023 13:07:04 +0200
Subject: [PATCH] sequencer: improve comment in sequencer_skip()
It wasn't clear under which circumstances the described path would be
relevant.
Change-Id: Ie9fd8a619dad4daf163c5efdb2f9a9eccf17307d
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
sequencer.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index c0ff165b83..11d2368ab1 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3359,12 +3359,13 @@ int sequencer_skip(struct repository *r, struct replay_opts *opts)
* If the corresponding .git/<ACTION>_HEAD exists, we know that the
* action is in progress and we can skip the commit.
*
- * Otherwise we check that the last instruction was related to the
- * particular subcommand we're trying to execute and barf if that's not
- * the case.
- *
- * Finally we check that the rollback is "safe", i.e., has the HEAD
- * moved? In this case, it doesn't make sense to "reset the merge" and
+ * But if the action would have overwritten an untracked file, no
+ * corresponding _HEAD file exists.
+ * In this case we fall back to checking that the last instruction was
+ * related to the particular subcommand we're trying to execute and barf
+ * if that's not the case.
+ * We also check that the rollback is "safe", i.e., whether the HEAD
+ * moved. If it did, it doesn't make sense to "reset the merge" and
* "skip the commit" as the user already handled this by committing. But
* we'd not want to barf here, instead give advice on how to proceed. We
* only need to check that when .git/<ACTION>_HEAD doesn't exist because
--
2.42.0.324.gb1ea313d68
^ permalink raw reply related
* Potential bug in `git checkout --quiet`
From: Radovan Haluška @ 2023-09-04 12:56 UTC (permalink / raw)
To: git
Hello,
I believe I have found a bug in the `git checkout —quiet` command. Below, you can find the output of `git bugreport`. Thank you
Best,
Radovan Haluska
——
Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.
What did you do before the bug happened? (Steps to reproduce your issue)
```
git clone --quiet --branch master --depth 1 --no-checkout --filter blob:none \
git@github.com:acatai/Strategy-Card-Game-AI-Competition.git locm-agents
cd locm-agents
git sparse-checkout set --no-cone
git sparse-checkout add 'contest-2022-08-COG/ByteRL'
git checkout --quiet
```
What did you expect to happen? (Expected behavior)
I expected to receive no output from any of the commands above.
What happened instead? (Actual behavior)
I received an output from the last command even though the `--quiet` switch was specified
What's different between what you expected and what actually happened?
This shouldn't have been printed on the screen:
'''
remote: Enumerating objects: 28, done.
remote: Counting objects: 100% (28/28), done.
remote: Compressing objects: 100% (27/27), done.
remote: Total 28 (delta 0), reused 25 (delta 0), pack-reused 0
Receiving objects: 100% (28/28), 31.40 MiB | 4.94 MiB/s, done.
'''
Anything else you want to add:
Please review the rest of the bug report below.
You can delete any lines you don't wish to share.
[System Info]
git version:
git version 2.42.0
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon
uname: Darwin 22.5.0 Darwin Kernel Version 22.5.0: Mon Apr 24 20:51:50 PDT 2023; root:xnu-8796.121.2~5/RELEASE_X86_64 x86_64
compiler info: clang: 14.0.3 (clang-1403.0.22.14.1)
libc info: no libc information available
$SHELL (typically, interactive shell): /bin/zsh
[Enabled Hooks]
not run from a git repository - no hooks to show
^ permalink raw reply
* Re: Potential bug in `git checkout --quiet`
From: Bagas Sanjaya @ 2023-09-04 13:36 UTC (permalink / raw)
To: Radovan Haluška, git
Cc: Elijah Newren, William Sprent,
Ævar Arnfjörð Bjarmason, Junio C Hamano
In-Reply-To: <9419E14B-5933-4773-B1BA-899A7DA75D96@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1437 bytes --]
On Mon, Sep 04, 2023 at 02:56:37PM +0200, Radovan Haluška wrote:
> What did you do before the bug happened? (Steps to reproduce your issue)
>
> ```
> git clone --quiet --branch master --depth 1 --no-checkout --filter blob:none \
> git@github.com:acatai/Strategy-Card-Game-AI-Competition.git locm-agents
> cd locm-agents
> git sparse-checkout set --no-cone
> git sparse-checkout add 'contest-2022-08-COG/ByteRL'
> git checkout --quiet
> ```
>
> What did you expect to happen? (Expected behavior)
>
> I expected to receive no output from any of the commands above.
>
> What happened instead? (Actual behavior)
>
> I received an output from the last command even though the `--quiet` switch was specified
>
> What's different between what you expected and what actually happened?
>
> This shouldn't have been printed on the screen:
>
> '''
> remote: Enumerating objects: 28, done.
> remote: Counting objects: 100% (28/28), done.
> remote: Compressing objects: 100% (27/27), done.
> remote: Total 28 (delta 0), reused 25 (delta 0), pack-reused 0
> Receiving objects: 100% (28/28), 31.40 MiB | 4.94 MiB/s, done.
> '''
>
I can reproduce this bug on v2.40.0 using your reproducer above. Yet,
`git checkout --quiet` on normal repos (not partial ones) works as
expected. Cc'ing people working on sparse-checkout recently.
Thanks.
--
An old man doll... just what I always wanted! - Clara
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3 5/7] rebase: fix rewritten list for failed pick
From: Phillip Wood @ 2023-09-04 14:31 UTC (permalink / raw)
To: Johannes Schindelin, Phillip Wood via GitGitGadget
Cc: git, Junio C Hamano, Stefan Haller, Eric Sunshine, Glen Choo,
Phillip Wood
In-Reply-To: <304553b3-3668-0b98-c6dc-3c047fd9e34c@gmx.de>
Hi Dscho
On 23/08/2023 09:55, Johannes Schindelin wrote:
> Hi Phillip,
>
> On Tue, 1 Aug 2023, Phillip Wood via GitGitGadget wrote:
>
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> git rebase keeps a list that maps the OID of each commit before it was
>> rebased to the OID of the equivalent commit after the rebase. This list
>> is used to drive the "post-rewrite" hook that is called at the end of a
>> successful rebase. When a rebase stops for the user to resolve merge
>> conflicts the OID of the commit being picked is written to
>> ".git/rebase-merge/stopped-sha". Then when the rebase is continued that
>> OID is added to the list of rewritten commits. Unfortunately if a commit
>> cannot be picked because it would overwrite an untracked file we still
>> write the "stopped-sha1" file. This means that when the rebase is
>> continued the commit is added into the list of rewritten commits even
>> though it has not been picked yet.
>>
>> Fix this by not calling error_with_patch() for failed commands. The pick
>> has failed so there is nothing to commit and therefore we do not want to
>> set up the state files for committing staged changes when the rebase
>> continues. This change means we no-longer write a patch for the failed
>> command or display the error message printed by error_with_patch(). As
>> the command has failed the patch isn't really useful and in any case the
>> user can inspect the commit associated with the failed command by
>> inspecting REBASE_HEAD. Unless the user has disabled it we already print
>> an advice message that is more helpful than the message from
>> error_with_patch() which the user will still see. Even if the advice is
>> disabled the user will see the messages from the merge machinery
>> detailing the problem.
>>
>> To simplify writing REBASE_HEAD in this case pick_one_commit() is
>> modified to avoid duplicating the code that adds the failed command
>> back into the todo list.
>
> This motivates the change well, and answered all but one of the questions
> I had about it, being:
>
>> diff --git a/sequencer.c b/sequencer.c
>> index 62277e7bcc1..e25abfd2fb4 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> [...]
>> @@ -4658,12 +4659,8 @@ static int pick_one_commit(struct repository *r,
>> check_todo);
>> if (is_rebase_i(opts) && res < 0) {
>> /* Reschedule */
>> - advise(_(rescheduled_advice),
>> - get_item_line_length(todo_list, todo_list->current),
>> - get_item_line(todo_list, todo_list->current));
>> - todo_list->current--;
>
> Why is it okay to remove this decrement?
>
> Here is why: The code that calls `save_todo()` in the `if (reschedule)`
> block of the loop of `pick_commits()` _duplicates_ the logic that is
> removed here, including the advice and the decrementing of `current`.
>
> I had to instrument the code before and after this patch to figure this
> out, as I had missed the fact that the now-remaining code also decremented
> the `current` attribute.
>
> So: All is good with this patch. If you'd like to amend the commit message
> accordingly, I would not be opposed, but I could now live equally as
> easily without it.
I'll try and add something to the commit message when I re-roll
Thanks
Phillip
>> - if (save_todo(todo_list, opts))
>> - return -1;
>> + *reschedule = 1;
>> + return -1;
>> }
>> if (item->command == TODO_EDIT) {
>> struct commit *commit = item->commit;
>
> I'd like to point out how delighted I am about this careful test case:
>
>> diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
>> index 96ae0edf1e1..4938ebb1c17 100755
>> --- a/t/t3430-rebase-merges.sh
>> +++ b/t/t3430-rebase-merges.sh
>> @@ -165,12 +165,12 @@ test_expect_success 'failed `merge -C` writes patch (may be rescheduled, too)' '
>> test_config sequence.editor \""$PWD"/replace-editor.sh\" &&
>> test_tick &&
>> test_must_fail git rebase -ir HEAD &&
>> + test_cmp_rev REBASE_HEAD H^0 &&
>> grep "^merge -C .* G$" .git/rebase-merge/done &&
>> grep "^merge -C .* G$" .git/rebase-merge/git-rebase-todo &&
>> - test_path_is_file .git/rebase-merge/patch &&
>> + test_path_is_missing .git/rebase-merge/patch &&
>>
>> : fail because of merge conflict &&
>> - rm G.t .git/rebase-merge/patch &&
>> git reset --hard conflicting-G &&
>> test_must_fail git rebase --continue &&
>> ! grep "^merge -C .* G$" .git/rebase-merge/git-rebase-todo &&
>> diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh
>> index 5f3ff051ca2..ad7f8c6f002 100755
>> --- a/t/t5407-post-rewrite-hook.sh
>> +++ b/t/t5407-post-rewrite-hook.sh
>> @@ -17,6 +17,12 @@ test_expect_success 'setup' '
>> git checkout A^0 &&
>> test_commit E bar E &&
>> test_commit F foo F &&
>> + git checkout B &&
>> + git merge E &&
>> + git tag merge-E &&
>> + test_commit G G &&
>> + test_commit H H &&
>> + test_commit I I &&
>> git checkout main &&
>>
>> test_hook --setup post-rewrite <<-EOF
>> @@ -173,6 +179,48 @@ test_fail_interactive_rebase () {
>> )
>> }
>>
>> +test_expect_success 'git rebase with failed pick' '
>> + clear_hook_input &&
>> + cat >todo <<-\EOF &&
>> + exec >bar
>> + merge -C merge-E E
>> + exec >G
>> + pick G
>> + exec >H 2>I
>> + pick H
>> + fixup I
>> + EOF
>> +
>> + (
>> + set_replace_editor todo &&
>> + test_must_fail git rebase -i D D 2>err
>> + ) &&
>> + grep "would be overwritten" err &&
>> + rm bar &&
>> +
>> + test_must_fail git rebase --continue 2>err &&
>> + grep "would be overwritten" err &&
>> + rm G &&
>> +
>> + test_must_fail git rebase --continue 2>err &&
>> + grep "would be overwritten" err &&
>> + rm H &&
>> +
>> + test_must_fail git rebase --continue 2>err &&
>> + grep "would be overwritten" err &&
>> + rm I &&
>> +
>> + git rebase --continue &&
>> + echo rebase >expected.args &&
>> + cat >expected.data <<-EOF &&
>> + $(git rev-parse merge-E) $(git rev-parse HEAD~2)
>> + $(git rev-parse G) $(git rev-parse HEAD~1)
>> + $(git rev-parse H) $(git rev-parse HEAD)
>> + $(git rev-parse I) $(git rev-parse HEAD)
>> + EOF
>> + verify_hook_input
>> +'
>> +
>> test_expect_success 'git rebase -i (unchanged)' '
>> git reset --hard D &&
>> clear_hook_input &&
>
> Here is my ACK.
>
> Thank you,
> Johannes
^ permalink raw reply
* Re: [PATCH v3 6/7] rebase --continue: refuse to commit after failed command
From: Phillip Wood @ 2023-09-04 14:37 UTC (permalink / raw)
To: Johannes Schindelin, Phillip Wood via GitGitGadget
Cc: git, Junio C Hamano, Stefan Haller, Eric Sunshine, Glen Choo,
Phillip Wood
In-Reply-To: <a5bfea5f-0d0d-f7ed-3f72-37e3db6f5b2c@gmx.de>
Hi Dscho
On 23/08/2023 10:01, Johannes Schindelin wrote:
> Hi Phillip,
>
> On Tue, 1 Aug 2023, Phillip Wood via GitGitGadget wrote:
>
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> If a commit cannot be picked because it would overwrite an untracked
>> file then "git rebase --continue" should refuse to commit any staged
>> changes as the commit was not picked. This is implemented by refusing to
>> commit if the message file is missing. The message file is chosen for
>> this check because it is only written when "git rebase" stops for the
>> user to resolve merge conflicts.
>>
>> Existing commands that refuse to commit staged changes when continuing
>> such as a failed "exec" rely on checking for the absence of the author
>> script in run_git_commit(). This prevents the staged changes from being
>> committed but prints
>>
>> error: could not open '.git/rebase-merge/author-script' for
>> reading
>>
>> before the message about not being able to commit. This is confusing to
>> users and so checking for the message file instead improves the user
>> experience. The existing test for refusing to commit after a failed exec
>> is updated to check that we do not print the error message about a
>> missing author script anymore.
>
> I am delighted to see an improvement of the user experience!
>
> However, I could imagine that users would still be confused when seeing
> the advice about staged changes, even if nothing was staged at all.
If nothing is staged then this message wont trigger because is_clean
will be false.
> Could you introduce a new advice message specifically for the case where
> untracked files are in the way and prevent changes from being applied?
We have an advice message now that is printed when the rebase stops in
that case. The message here is printed when the user runs "rebase
--continue" with staged changes and we're not expecting to commit
anything because the commit couldn't be picked or we're containing from
a break command or bad exec/label/reset etc.
> P.S.: To save both you and me time, here is my ACK for patch 7/7
> (actually, the entire patch series, but _maybe_ you want to change
> "impove" -> "improve" in the cover letter's subject) ;-)
Thanks for taking the time to read through the patches and for you
comments. I'll fix the typo when I re-roll
Best Wishes
Phillip
>>
>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>> ---
>> sequencer.c | 5 +++++
>> t/t3404-rebase-interactive.sh | 18 +++++++++++++++++-
>> t/t3430-rebase-merges.sh | 4 ++++
>> 3 files changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/sequencer.c b/sequencer.c
>> index e25abfd2fb4..a90b015e79c 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -4977,6 +4977,11 @@ static int commit_staged_changes(struct repository *r,
>>
>> is_clean = !has_uncommitted_changes(r, 0);
>>
>> + if (!is_clean && !file_exists(rebase_path_message())) {
>> + const char *gpg_opt = gpg_sign_opt_quoted(opts);
>> +
>> + return error(_(staged_changes_advice), gpg_opt, gpg_opt);
>> + }
>> if (file_exists(rebase_path_amend())) {
>> struct strbuf rev = STRBUF_INIT;
>> struct object_id head, to_amend;
>> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
>> index 6d3788c588b..a8ad398956a 100755
>> --- a/t/t3404-rebase-interactive.sh
>> +++ b/t/t3404-rebase-interactive.sh
>> @@ -604,7 +604,8 @@ test_expect_success 'clean error after failed "exec"' '
>> echo "edited again" > file7 &&
>> git add file7 &&
>> test_must_fail git rebase --continue 2>error &&
>> - test_i18ngrep "you have staged changes in your working tree" error
>> + test_i18ngrep "you have staged changes in your working tree" error &&
>> + test_i18ngrep ! "could not open.*for reading" error
>> '
>>
>> test_expect_success 'rebase a detached HEAD' '
>> @@ -1290,6 +1291,11 @@ test_expect_success 'rebase -i commits that overwrite untracked files (pick)' '
>> test_cmp_rev REBASE_HEAD I &&
>> rm file6 &&
>> test_path_is_missing .git/rebase-merge/patch &&
>> + echo changed >file1 &&
>> + git add file1 &&
>> + test_must_fail git rebase --continue 2>err &&
>> + grep "error: you have staged changes in your working tree" err &&
>> + git reset --hard HEAD &&
>> git rebase --continue &&
>> test_cmp_rev HEAD I
>> '
>> @@ -1310,6 +1316,11 @@ test_expect_success 'rebase -i commits that overwrite untracked files (squash)'
>> test_cmp_rev REBASE_HEAD I &&
>> rm file6 &&
>> test_path_is_missing .git/rebase-merge/patch &&
>> + echo changed >file1 &&
>> + git add file1 &&
>> + test_must_fail git rebase --continue 2>err &&
>> + grep "error: you have staged changes in your working tree" err &&
>> + git reset --hard HEAD &&
>> git rebase --continue &&
>> test $(git cat-file commit HEAD | sed -ne \$p) = I &&
>> git reset --hard original-branch2
>> @@ -1330,6 +1341,11 @@ test_expect_success 'rebase -i commits that overwrite untracked files (no ff)' '
>> test_cmp_rev REBASE_HEAD I &&
>> rm file6 &&
>> test_path_is_missing .git/rebase-merge/patch &&
>> + echo changed >file1 &&
>> + git add file1 &&
>> + test_must_fail git rebase --continue 2>err &&
>> + grep "error: you have staged changes in your working tree" err &&
>> + git reset --hard HEAD &&
>> git rebase --continue &&
>> test $(git cat-file commit HEAD | sed -ne \$p) = I
>> '
>> diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
>> index 4938ebb1c17..804ff819782 100755
>> --- a/t/t3430-rebase-merges.sh
>> +++ b/t/t3430-rebase-merges.sh
>> @@ -169,6 +169,10 @@ test_expect_success 'failed `merge -C` writes patch (may be rescheduled, too)' '
>> grep "^merge -C .* G$" .git/rebase-merge/done &&
>> grep "^merge -C .* G$" .git/rebase-merge/git-rebase-todo &&
>> test_path_is_missing .git/rebase-merge/patch &&
>> + echo changed >file1 &&
>> + git add file1 &&
>> + test_must_fail git rebase --continue 2>err &&
>> + grep "error: you have staged changes in your working tree" err &&
>>
>> : fail because of merge conflict &&
>> git reset --hard conflicting-G &&
>> --
>> gitgitgadget
>>
>>
^ permalink raw reply
* Is "bare"ness in the context of multiple worktrees weird? Bitmap error in git gc.
From: Tao Klerks @ 2023-09-04 14:41 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Taylor Blau, Patrick Steinhardt
Hi folks,
I work with a project that often, or typically, benefits from having
multiple worktrees where users are working with different versions of
the code.
Worktrees make particular sense in this project for all the usual
reasons, plus the fact that it is a very *large* repo - multiple gigs
of repo, and very fast-moving; cloning and fetching less often is
nice.
One problem that we found users to have, early on when we introduced
worktrees, was that it was not obvious to users that there was (or why
there was) a "main" worktree, containing the actual ".git" repo, and
"subsidiary" worktrees, in the same directory location. Git by default
makes worktrees *subfolders* of the initial/main worktree/folder, but
we put them alongside each other to make other project tooling be able
to treat all worktrees consistently; in daily use there is absolutely
no difference between them, in all the "project tooling" there cannot
be a practical difference... but if you choose to delete a worktree,
and it happens to be the "special" one that contains the repo...
you've just lost stuff that you didn't expect to lose (any local
branches and stashes, basically).
Because worktree use was so useful/widespread/critical on this
project, and we already had a custom cloning process that introduced
selective refspecs etc, we introduced a special clone topology: the
initial clone is a bare repo, and that folder gets a specific clear
name (ending in .git). Then we create worktrees attached to that bare
repo.
Generally speaking, this has worked *very* well: I would recommend it
generally as a recognized/supported local-repo-setup. The most
important thing that makes this *possible* is the fact that "git
rev-parse --is-bare-repository" returns True in the bare repo folder,
where the lack of index and HEAD shouldn't bother git, and it returns
False in any one of the worktrees. It feels like things were designed
to work this way, even though I can find no explicit mention of this
topology in the docs.
However, from time to time something weird happens: Today I finally
started to understand why I was seeing a crazy GC error about bitmaps,
intermittently: It seems to be because "git gc" wants to create
bitmaps in bare repos, but can't do so properly when we're in a
partial clone... or something like that?
EG repro:
```
git clone --bare https://github.com/ksylor/ohshitgit dangit_shared.git
--filter=blob:none
git -C dangit_shared.git worktree add $PWD/dangit_wt3
cd dangit_wt3/
echo "this is some new unique blob in the repo" > new_blob.txt
git add new_blob.txt && git commit -m "new blob"
cd ../dangit_shared.git/
git gc
```
This yields, at the end of the GC run:
```
warning: Failed to write bitmap index. Packfile doesn't have full
closure (object bf86ed1b2602ac3a8d4724bcdf6707b156673aac is missing)
fatal: failed to write bitmap index
fatal: failed to run repack
```
On the other hand, running "git gc" in one of the worktrees works fine
(except you first need to delete a couple of ".tmp-*" files from the
"objects/pack" folder, if you already got the error above).
I at first thought this was a bug - but as I realized the problematic
behavior was tied to the "core.bare" setting (and its expression at
runtime through "git rev-parse --is-bare-repository"), it became more
obvious that maybe the system could/should be able to make assumptions
about the kind of repo that has this "true", and assume there are no
local-object-derived non-promisor packfiles (or whatever it is about
this example that makes things unhappy).
So, I guess I'm confused as to what "core.bare" is supposed to mean:
Is it intended to mean "there is no index nor HEAD here, and that's
good, don't worry" (in which case my setup is presumably "supported",
and the gc behavior is buggy?), or is it intended to mean "this is the
kind of repository in which there are no worktrees" (in which case I
am abusing the system and get the errors I deserve)?
CC Dscho, who made the worktree support that I rely on work about 16
years ago it seems, and Taylor Blau and Patrick Steinhardt, who I
think have made changes in the area of the code concerned with whether
or not we try to create bitmaps, and might have an opinion as to
whether the assumptions made by "gc" at the moment are unsafe, or my
use or "core.bare" in this context is wrong.
Thanks for any feedback!
Tao
^ permalink raw reply
* Re: Is "bare"ness in the context of multiple worktrees weird? Bitmap error in git gc.
From: Tao Klerks @ 2023-09-04 14:59 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Taylor Blau, Patrick Steinhardt
In-Reply-To: <CAPMMpoixKnr4BkKd8jeU+79Edhqtu4R7m8=BX4ZSYKdBHDzK=w@mail.gmail.com>
On Mon, Sep 4, 2023 at 4:41 PM Tao Klerks <tao@klerks.biz> wrote:
>
> we introduced a special clone topology: the
> initial clone is a bare repo, and that folder gets a specific clear
> name (ending in .git). Then we create worktrees attached to that bare
> repo.
>
> Generally speaking, this has worked *very* well: I would recommend it
> generally as a recognized/supported local-repo-setup. The most
> important thing that makes this *possible* is the fact that "git
> rev-parse --is-bare-repository" returns True in the bare repo folder,
> where the lack of index and HEAD shouldn't bother git, and it returns
> False in any one of the worktrees. It feels like things were designed
> to work this way, even though I can find no explicit mention of this
> topology in the docs.
I should add that I only recently discovered "git clone
--separate-git-dir", which I at first though was a formal expression
of this setup... until I understood that the relationship between the
"GITDIR" and the worktree that you end up with is not "Bare repo vs
worktree", but rather... "orphaned repo / repo that doesn't know about
its worktree, vs worktree".
It seems, to me, that "my setup" makes a lot more sense than what you
end up with when you use "--separate-git-dir", and that the behavior
there predates the current "mutual reference" model of
worktrees-to-their-repo. If "my" use of "core.bare" in the example
above is sound - then should the implementation of
"--separate-git-dir" be changed to produce a bare repo with a
"worktrees" folder, like you get if you clone bare and add a worktree
in two separate steps?
(I say "change the implementation", but I guess I really mean
introducing a new option for the new behavior, and deprecate the old
option)
Dscho, I assume you would have the strongest opinion about this?
^ permalink raw reply
* Re: Is "bare"ness in the context of multiple worktrees weird? Bitmap error in git gc.
From: Tao Klerks @ 2023-09-04 15:29 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Taylor Blau, Patrick Steinhardt
In-Reply-To: <CAPMMpoj7s=ewXJfJyxvrcHjpmOOWEWBvZ94OOuVmYs2UQ482HA@mail.gmail.com>
On Mon, Sep 4, 2023 at 4:59 PM Tao Klerks <tao@klerks.biz> wrote:
>
>
> It seems, to me, that "my setup" makes a lot more sense than what you
> end up with when you use "--separate-git-dir", and that the behavior
> there predates the current "mutual reference" model of
> worktrees-to-their-repo. If "my" use of "core.bare" in the example
> above is sound - then should the implementation of
> "--separate-git-dir" be changed to produce a bare repo with a
> "worktrees" folder, like you get if you clone bare and add a worktree
> in two separate steps?
>
And to confuse matters further, I just stumbled across
https://github.com/git/git/blob/master/contrib/workdir/git-new-workdir
- I don't understand when you would want to use that vs, again, a bare
repo with one or more worktrees properly attached via two-way
references, their own indexes, their own reflogs, etc.
Is it the case that this contrib script predates the current "git
worktree" support?
^ permalink raw reply
* Re: [PATCH v2 1/3] range-diff: treat notes like `log`
From: Kristoffer Haugsbakk @ 2023-09-04 17:10 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Denton Liu, Jeff King
In-Reply-To: <94b9535b-8c2a-eb8f-90fb-cd0f998ec57e@gmx.de>
Hi Dscho
On Sun, Sep 3, 2023, at 14:17, Johannes Schindelin wrote:
> Hi Kristoffer,
>
> On Fri, 1 Sep 2023, Kristoffer Haugsbakk wrote:
>
> > [snip]
>
> Very well explained.
>
> The root cause for this is 8cf51561d1e (range-diff: fix a crash in parsing
> git-log output, 2020-04-15) which added the `--notes` argument in
> `read_patches()`' call. The commit message explains why this is needed:
> the (necessary) `--pretty=medium` would turn off the notes, therefore
> `--notes` had to be added to reinstate the original behavior (except, as
> you pointed out, in the case `--notes=<ref>` was specified).
That's interesting. I didn't find that commit in my history spelunking.
>> Remedy this by co-opting the `--standard-notes` option which has been
>> deprecated since ab18b2c0df[2] and which is currently only documented in
>> `pretty-options`.
>
> This sounds a bit less desirable, though, than passing the `--notes`
> argument only as needed. This patch (on top of `notes-range-diff` in your
> fork) lets the new test still pass while leaving `--standard-notes`
> deprecated:
>
> [snip]
I like it. I didn't like my solution but it was the only thing that I
could get to work. I would like to use your solution instead. Thank you.
Maybe you could supply a commit message for v3? v3 would then consist of
two commits:
1. Your fix
2. Those two tests
Or should it be handled in some other way?
Cheers
--
Kristoffer Haugsbakk
^ permalink raw reply
* Re: Is "bare"ness in the context of multiple worktrees weird? Bitmap error in git gc.
From: Kristoffer Haugsbakk @ 2023-09-04 17:42 UTC (permalink / raw)
To: Tao Klerks; +Cc: Johannes Schindelin, Taylor Blau, Patrick Steinhardt, git
In-Reply-To: <CAPMMpohpKJdopSpZu+ehE0MZrH8cksgtY1NEHFyZz2jj+LOKhA@mail.gmail.com>
On Mon, Sep 4, 2023, at 17:29, Tao Klerks wrote:
> On Mon, Sep 4, 2023 at 4:59 PM Tao Klerks <tao@klerks.biz> wrote:
>>
>>
>> [snip]
>>
>
> And to confuse matters further, I just stumbled across
> https://github.com/git/git/blob/master/contrib/workdir/git-new-workdir
> - I don't understand when you would want to use that vs, again, a bare
> repo with one or more worktrees properly attached via two-way
> references, their own indexes, their own reflogs, etc.
>
> Is it the case that this contrib script predates the current "git
> worktree" support?
Yes, according to VonC[1] and the 2.05 release notes.[2]
🔗 1: https://stackoverflow.com/a/30185564/1725151
🔗 2: https://github.com/git/git/blob/22aca1b3ac10af7188dccf033b44a36926f04d4b/Documentation/RelNotes/2.5.0.txt#L25-L27
--
Kristoffer Haugsbakk
^ permalink raw reply
* Re: Is "bare"ness in the context of multiple worktrees weird? Bitmap error in git gc.
From: Kristoffer Haugsbakk @ 2023-09-04 17:56 UTC (permalink / raw)
To: Tao Klerks; +Cc: Johannes Schindelin, Taylor Blau, Patrick Steinhardt, git
In-Reply-To: <CAPMMpoixKnr4BkKd8jeU+79Edhqtu4R7m8=BX4ZSYKdBHDzK=w@mail.gmail.com>
Hi Tao
Context for my own use: I use the default clone (named the same as the
upstream repository) as the main worktree and name worktrees according to
some topic. So if the repository is named `work-application` then I might
have worktrees named things like `deployment-work`, `next-version-work`,
and things like that. All of them sibling directories since they are all
Intellij projects (to your point about making tooling treat them the same
way). I usually use the main worktree so I am fine with one worktree being
*special* (that it contains the `.git` directory).
I can understand that the main worktree/linked worktree dichotomy might
feel artificial if you use, say, ten different wotrkees equally often. Or
maybe one worktree per branch.
And then from that vantage point it might feel wasteful to dedicate an
unused main worktree—with its own working tree—to just sit somewhere for
its `.git` directory, essentially.
On Mon, Sep 4, 2023, at 16:41, Tao Klerks wrote:
> Because worktree use was so useful/widespread/critical on this project,
> and we already had a custom cloning process that introduced selective
> refspecs etc, we introduced a special clone topology: the initial clone
> is a bare repo, and that folder gets a specific clear name (ending in
> .git). Then we create worktrees attached to that bare repo.
This is interesting as a Git user. I've been encountering questions on
StackOverflow where the questioner is using a bare repository which they
make (or try to make) worktrees from. I've been telling them that making
worktrees from a bare repository is a contradiction:[1]
> Bare repositories don’t have worktrees per definition. Or at least
> that’s what `man gitglossary says`. Of course what `git worktree` allows
> you to do trumps that. But it might be ill-defined.
The glossary says under “worktree” (on Git 2.42):
> A repository can have zero (i.e. bare repository) or one or more
> worktrees attached to it.
And as someone who never has needed to use a bare repository + worktrees
I've just left it at that.
🔗 1: https://stackoverflow.com/a/76273222/1725151
Cheers
^ permalink raw reply
* Re: [PATCH] var: avoid a segmentation fault when `HOME` is unset
From: brian m. carlson @ 2023-09-04 20:26 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
In-Reply-To: <pull.1580.git.1693808487058.gitgitgadget@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 770 bytes --]
On 2023-09-04 at 06:21:26, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The code introduced in 576a37fccbf (var: add attributes files locations,
> 2023-06-27) paid careful attention to use `xstrdup()` for pointers known
> never to be `NULL`, and `xstrdup_or_null()` otherwise.
>
> One spot was missed, though: `git_attr_global_file()` can return `NULL`,
> when the `HOME` variable is not set (and neither `XDG_CONFIG_HOME`), a
> scenario not too uncommon in certain server scenarios.
>
> Fix this, and add a test case to avoid future regressions.
Looks good to me. Thanks for the patch, and my apologies for the
oversight.
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply
* [PATCH] advice: improve hint for diverging branches.
From: Konstantin Pereiaslov via GitGitGadget @ 2023-09-04 20:59 UTC (permalink / raw)
To: git; +Cc: Konstantin Pereiaslov, Konstantin Pereiaslov
From: Konstantin Pereiaslov <perk11@perk11.info>
Added a description of what the offered options will do and for pull
also offered a 3rd option during a pull - a hard reset.
This option should be helpful for the new users that accidentally
committed into the wrong branch which is a scenario I saw very
often.
The resulting tooltip looks like this for pull:
hint: Diverging branches can't be fast-forwarded.
Consider the following options:
hint:
hint: To merge remote changes into your branch:
hint: git merge --no-ff
hint:
hint: To apply your changes on top of remote changes:
hint: git rebase
hint:
hint: To discard your local changes and apply the remote changes:
hint: git reset --hard refs/remotes/upstream/branch-name
hint:
hint: Disable this message with "git config advice.diverging false"
There is some danger because it's semi-destructive, but so are
other options offered if user doesn't know the commands to
revert back. Additionally, I think "To discard your local changes"
wording describes the danger clearly enough.
And for merge I improved the wording and added a description of what
the commands do:
hint: Diverging branches can't be fast-forwarded.
hint: Consider the following options:
hint:
hint: To merge changes into your branch:
hint: git merge --no-ff
hint:
hint: To apply your changes on top:
hint: git rebase
hint:
hint: Disable this message with "git config advice.diverging false"
Signed-off-by: Konstantin Pereiaslov <perk11@perk11.info>
---
Improve hint for diverging branches.
I have seen a lot of developers not know what to do when they try to do
a git pull on a master branch with the intention of updating that branch
to the latest version, but see an error about branches diverging because
they accidentally committed their changes to that branch. They then
spend their time resolving conflicts and still not getting the intended
result. The suggestion to do a hard reset should be something that helps
in this situation.
I'm not sure if a new config option needs to be created as technically
these are two different advice now. I'm also not sure if "refs/remotes"
part of the refspec is necessary, that is what I found the functions in
pull.c are returning. I think "upstream/branch-name" should be the same
thing, but kept it as is ( git reset --hard
refs/remotes/upstream/branch-name) for now. Please feel free to chime
in.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1570%2Fperk11%2Fdiverging-advice-improvements-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1570/perk11/diverging-advice-improvements-v1
Pull-Request: https://github.com/git/git/pull/1570
Documentation/config/advice.txt | 3 ++-
advice.c | 27 +++++++++++++++++++++++----
advice.h | 3 ++-
builtin/merge.c | 2 +-
builtin/pull.c | 20 ++++++++++++++++++--
5 files changed, 46 insertions(+), 9 deletions(-)
diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
index c548a91e676..f3daa232ace 100644
--- a/Documentation/config/advice.txt
+++ b/Documentation/config/advice.txt
@@ -137,7 +137,8 @@ advice.*::
is asked to update index entries outside the current sparse
checkout.
diverging::
- Advice shown when a fast-forward is not possible.
+ Advice shown when a fast-forward is not possible during merge
+ or pull operation.
worktreeAddOrphan::
Advice shown when a user tries to create a worktree from an
invalid reference, to instruct how to create a new orphan
diff --git a/advice.c b/advice.c
index 50c79443ba7..8fc4fb19932 100644
--- a/advice.c
+++ b/advice.c
@@ -220,19 +220,38 @@ void NORETURN die_conclude_merge(void)
die(_("Exiting because of unfinished merge."));
}
-void NORETURN die_ff_impossible(void)
+void NORETURN die_ff_impossible_during_merge(void)
{
advise_if_enabled(ADVICE_DIVERGING,
- _("Diverging branches can't be fast-forwarded, you need to either:\n"
+ _("Diverging branches can't be fast-forwarded.\n"
+ "Consider the following options:\n"
"\n"
+ "To merge changes into your branch:\n"
"\tgit merge --no-ff\n"
"\n"
- "or:\n"
- "\n"
+ "To apply your changes on top:\n"
"\tgit rebase\n"));
die(_("Not possible to fast-forward, aborting."));
}
+void NORETURN die_ff_impossible_during_pull(const char *upstream_branch_spec)
+{
+ advise_if_enabled(ADVICE_DIVERGING,
+ _("Diverging branches can't be fast-forwarded. "
+ "Consider the following options:\n"
+ "\n"
+ "To merge remote changes into your branch:\n"
+ "\tgit merge --no-ff\n"
+ "\n"
+ "To apply your changes on top of remote changes:\n"
+ "\tgit rebase\n"
+ "\n"
+ "To discard your local changes and apply the remote changes:\n"
+ "\tgit reset --hard %s\n"), upstream_branch_spec);
+ die(_("Not possible to fast-forward, aborting."));
+}
+
+
void advise_on_updating_sparse_paths(struct string_list *pathspec_list)
{
struct string_list_item *item;
diff --git a/advice.h b/advice.h
index 2affbe14261..f87369a5471 100644
--- a/advice.h
+++ b/advice.h
@@ -71,7 +71,8 @@ void advise_if_enabled(enum advice_type type, const char *advice, ...);
int error_resolve_conflict(const char *me);
void NORETURN die_resolve_conflict(const char *me);
void NORETURN die_conclude_merge(void);
-void NORETURN die_ff_impossible(void);
+void NORETURN die_ff_impossible_during_merge(void);
+void NORETURN die_ff_impossible_during_pull(const char *upstream_branch_spec);
void advise_on_updating_sparse_paths(struct string_list *pathspec_list);
void detach_advice(const char *new_name);
void advise_on_moving_dirty_path(struct string_list *pathspec_list);
diff --git a/builtin/merge.c b/builtin/merge.c
index de68910177f..8358f137f1d 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1675,7 +1675,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
}
if (fast_forward == FF_ONLY)
- die_ff_impossible();
+ die_ff_impossible_during_merge();
if (autostash)
create_autostash(the_repository,
diff --git a/builtin/pull.c b/builtin/pull.c
index be2b2c9ebc9..51d30e6f918 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -765,6 +765,20 @@ static const char *get_tracking_branch(const char *remote, const char *refspec)
refspec_item_clear(&spec);
return merge_branch;
}
+/**
+ * Returns the branch the pull is performed from.
+ * If remote is NULL or refspec is NULL, configured upstream remote of the
+ * current branch is used.
+ * If refspec is NULL, the current upstream branch is used.
+ */
+static const char *get_pull_branch(const char *remote, const char *refspec)
+{
+ if (refspec == NULL || remote == NULL) {
+ return get_upstream_branch(remote);
+ }
+
+ return get_tracking_branch(remote, refspec);
+}
/**
* Given the repo and refspecs, sets fork_point to the point at which the
@@ -1112,8 +1126,10 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
/* ff-only takes precedence over rebase */
if (opt_ff && !strcmp(opt_ff, "--ff-only")) {
- if (divergent)
- die_ff_impossible();
+ if (divergent) {
+ const char* pull_branch_spec = get_pull_branch(repo, *refspecs);
+ die_ff_impossible_during_pull(pull_branch_spec);
+ }
opt_rebase = REBASE_FALSE;
}
/* If no action specified and we can't fast forward, then warn. */
base-commit: d814540bb75bbd2257f9a6bf59661a84fe8cf3cf
--
gitgitgadget
^ permalink raw reply related
* Re: Is "bare"ness in the context of multiple worktrees weird? Bitmap error in git gc.
From: Eric Sunshine @ 2023-09-05 0:26 UTC (permalink / raw)
To: Tao Klerks; +Cc: git, Johannes Schindelin, Taylor Blau, Patrick Steinhardt
In-Reply-To: <CAPMMpoixKnr4BkKd8jeU+79Edhqtu4R7m8=BX4ZSYKdBHDzK=w@mail.gmail.com>
On Mon, Sep 4, 2023 at 10:41 AM Tao Klerks <tao@klerks.biz> wrote:
> I work with a project that often, or typically, benefits from having
> multiple worktrees where users are working with different versions of
> the code.
>
> Worktrees make particular sense in this project for all the usual
> reasons, plus the fact that it is a very *large* repo - multiple gigs
> of repo, and very fast-moving; cloning and fetching less often is
> nice.
>
> One problem that we found users to have, early on when we introduced
> worktrees, was that it was not obvious to users that there was (or why
> there was) a "main" worktree, containing the actual ".git" repo, and
> "subsidiary" worktrees, in the same directory location. Git by default
> makes worktrees *subfolders* of the initial/main worktree/folder, but
This is not accurate. There is no default location for new worktrees;
git-worktree creates the new worktree at the location specified by the
user:
git worktree add [<options>] <path> [<commit>]
where <path> -- the only mandatory argument -- specifies the location.
> we put them alongside each other to make other project tooling be able
> to treat all worktrees consistently; in daily use there is absolutely
> no difference between them, in all the "project tooling" there cannot
> be a practical difference... but if you choose to delete a worktree,
> and it happens to be the "special" one that contains the repo...
> you've just lost stuff that you didn't expect to lose (any local
> branches and stashes, basically).
>
> Because worktree use was so useful/widespread/critical on this
> project, and we already had a custom cloning process that introduced
> selective refspecs etc, we introduced a special clone topology: the
> initial clone is a bare repo, and that folder gets a specific clear
> name (ending in .git). Then we create worktrees attached to that bare
> repo.
>
> Generally speaking, this has worked *very* well: I would recommend it
> generally as a recognized/supported local-repo-setup. The most
> important thing that makes this *possible* is the fact that "git
> rev-parse --is-bare-repository" returns True in the bare repo folder,
> where the lack of index and HEAD shouldn't bother git, and it returns
> False in any one of the worktrees. It feels like things were designed
> to work this way, even though I can find no explicit mention of this
> topology in the docs.
It indeed was designed to work this way. It is perfectly legitimate to
create worktrees attached to a bare repository[1].
[1]: Support for bare repositories in conjunction with multiple-
worktrees, however, came after the initial implementation of multiple-
worktrees. An unfortunate side-effect is that established terminology
became somewhat confusing. In particular, in a bare repository
scenario, the term "main worktree" refers to the bare repository, not
to the "blessed" worktree containing the ".git/" directory (since
there is no such worktree in this case).
> However, from time to time something weird happens: Today I finally
> started to understand why I was seeing a crazy GC error about bitmaps,
> intermittently: It seems to be because "git gc" wants to create
> bitmaps in bare repos, but can't do so properly when we're in a
> partial clone... or something like that?
>
> EG repro:
> ```
> git clone --bare https://github.com/ksylor/ohshitgit dangit_shared.git
> --filter=blob:none
> git -C dangit_shared.git worktree add $PWD/dangit_wt3
> cd dangit_wt3/
> echo "this is some new unique blob in the repo" > new_blob.txt
> git add new_blob.txt && git commit -m "new blob"
> cd ../dangit_shared.git/
> git gc
> ```
>
> This yields, at the end of the GC run:
> ```
> warning: Failed to write bitmap index. Packfile doesn't have full
> closure (object bf86ed1b2602ac3a8d4724bcdf6707b156673aac is missing)
> fatal: failed to write bitmap index
> fatal: failed to run repack
> ```
>
> On the other hand, running "git gc" in one of the worktrees works fine
> (except you first need to delete a couple of ".tmp-*" files from the
> "objects/pack" folder, if you already got the error above).
Worktrees appear to be a red-herring. It's possible to reproduce this
error without them. For instance:
% git clone --bare --filter=blob:none
https://github.com/ksylor/ohshitgit dangit_shared.git
% git clone dangit_shared.git foop
% cd foop
% echo nothing >nothing
% git add nothing
% git commit -m nothing
fatal: unable to read dbbb0682a7690b62ccf51b2a8648fa71ac671348
% git push origin master
% cd ../dangit_shared.git
% git gc
...
warning: Failed to write bitmap index. Packfile doesn't have full
closure (object bf86ed1b2602ac3a8d4724bcdf6707b156673aac is missing)
fatal: failed to write bitmap index
fatal: failed to run repack
> I at first thought this was a bug - but as I realized the problematic
> behavior was tied to the "core.bare" setting (and its expression at
> runtime through "git rev-parse --is-bare-repository"), it became more
> obvious that maybe the system could/should be able to make assumptions
> about the kind of repo that has this "true", and assume there are no
> local-object-derived non-promisor packfiles (or whatever it is about
> this example that makes things unhappy).
>
> So, I guess I'm confused as to what "core.bare" is supposed to mean:
> Is it intended to mean "there is no index nor HEAD here, and that's
> good, don't worry" (in which case my setup is presumably "supported",
> and the gc behavior is buggy?), or is it intended to mean "this is the
> kind of repository in which there are no worktrees" (in which case I
> am abusing the system and get the errors I deserve)?
The former, meaning that your setup should be supported. Citing
documentation for `core.bare`:
If true this repository is assumed to be bare and has no working
directory associated with it. If this is the case a number of
commands that require a working directory will be disabled, such
as git-add(1) or git-merge(1).
On Mon, Sep 4, 2023 at 10:59 AM Tao Klerks <tao@klerks.biz> wrote:
> I should add that I only recently discovered "git clone
> --separate-git-dir", which I at first though was a formal expression
> of this setup... until I understood that the relationship between the
> "GITDIR" and the worktree that you end up with is not "Bare repo vs
> worktree", but rather... "orphaned repo / repo that doesn't know about
> its worktree, vs worktree".
>
> It seems, to me, that "my setup" makes a lot more sense than what you
> end up with when you use "--separate-git-dir", and that the behavior
> there predates the current "mutual reference" model of
> worktrees-to-their-repo. If "my" use of "core.bare" in the example
> above is sound - then should the implementation of
> "--separate-git-dir" be changed to produce a bare repo with a
> "worktrees" folder, like you get if you clone bare and add a worktree
> in two separate steps?
`--separate-git-dir` predates multiple-worktree support by several
years and is distinct in purpose from --bare and multiple-worktrees
(in fact, a couple somewhat recent fixes [2,3] were needed to prevent
--separate-git-dir from breaking worktree administrative data). My
understand from scanning history is that --separate-git-dir was
introduced in aid of submodule support and perhaps other use-cases.
[2]: 42264bc841 (init: teach --separate-git-dir to repair linked
worktrees, 2020-08-31)
[3]: 59d876ccd6 (init: make --separate-git-dir work from within linked
worktree, 2020-08-31)
On Mon, Sep 4, 2023 at 11:29 AM Tao Klerks <tao@klerks.biz> wrote:
> And to confuse matters further, I just stumbled across
> https://github.com/git/git/blob/master/contrib/workdir/git-new-workdir
> - I don't understand when you would want to use that vs, again, a bare
> repo with one or more worktrees properly attached via two-way
> references, their own indexes, their own reflogs, etc.
>
> Is it the case that this contrib script predates the current "git
> worktree" support?
git-new-workdir predates git-worktree by quite a few years and, as I
understand it, remains in-tree because it fills a niche not entirely
filled by git-worktree.
^ permalink raw reply
* git gui - add new tool command not working
From: Riza Dindir @ 2023-09-05 14:03 UTC (permalink / raw)
To: git
Hello,
I am on slackware 15.0.
I have a problem with git gui. Yesterday I was able to add tool
commands. But today I am only able to open the "Add New Tool Command"
dialog using Tools | Add. I can not close the opened dialog with the
esc key (the escape key can close the other dialogs that are opened).
But I can close the dialog for the add new tool command using the
cancel button on the dialog. Also I can not click on the Name and
Command textbox to enter anything.
Is there a log file that the git gui tool is writing? What might there be wrong?
Regards
^ permalink raw reply
* Re: [PATCH 1/2] ci: allow branch selection through "vars"
From: Johannes Schindelin @ 2023-09-05 10:51 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20230905073013.GI199565@coredump.intra.peff.net>
Hi Jeff,
On Tue, 5 Sep 2023, Jeff King wrote:
> [...] coupled with Phillip's use cases in the other part of the thread,
> maybe we should have a JSON-formatted CI_CONFIG variable instead.
>
> That requires the developer to hand-write a bit of JSON, but it's not
> too bad (and again, I really think it's only a couple folks using this).
>
> What do you think?
Thank you for asking my opinion. The `[no ci]` support described in
https://github.blog/changelog/2021-02-08-github-actions-skip-pull-request-and-push-workflows-with-skip-ci/
solves the problem adequately and with a lot less complexity than the
current or the `vars.`-based solution. In my opinion.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH 1/2] ci: allow branch selection through "vars"
From: Jeff King @ 2023-09-05 7:30 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <d000f469-2b7d-e037-c92e-013034490461@gmx.de>
On Sun, Sep 03, 2023 at 10:59:37AM +0200, Johannes Schindelin wrote:
> > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> > index 1b41278a7f..c364abb8f8 100644
> > --- a/.github/workflows/main.yml
> > +++ b/.github/workflows/main.yml
> > @@ -21,6 +21,7 @@ concurrency:
> > jobs:
> > ci-config:
> > name: config
> > + if: vars.CI_BRANCHES == '' || contains(vars.CI_BRANCHES, github.ref_name)
>
> This might be too loose a check, as branch names that are a substring of
> any name listed in `CI_BRANCHES` would be false positive match. For
> example, if `CI_BRANCHES` was set to `maint next seen`, a branch called
> `see` would be a false match.
Yes, I wrote it about it in the commit message. :)
My assumption is that this may be good enough, just because we are
realistically talking about the needs of a handful of Git developers.
Folks doing one-off patches would just push to their forks and get CI
(which they'd want in order to use GGG anyway). This is for people with
more exotic workflows, and my guess is that half a dozen people would
use this at all.
But we can make it more robust if we think somebody will actually run
into it in practice.
> Due to the absence of a `concat()` function (for more details, see
> https://docs.github.com/en/actions/learn-github-actions/expressions#functions),
> I fear that we'll have to resort to something like `contains(format(' {0} ',
> vars.CI_BRANCHES), format(' {0} ', github.ref_name))`.
Yeah, I had imagined checking startsWith() and endsWith(), but
auto-inserting the leading/trailing space as you suggest is even
shorter.
I think that contains() is more robust if used against an actual list
data structure. But there doesn't seem to be any split()-type function.
So I don't see how to get one short of using fromJSON(). But coupled
with Phillip's use cases in the other part of the thread, maybe we
should have a JSON-formatted CI_CONFIG variable instead.
That requires the developer to hand-write a bit of JSON, but it's not
too bad (and again, I really think it's only a couple folks using this).
What do you think?
-Peff
^ permalink raw reply
* Re: [PATCH v2 1/3] range-diff: treat notes like `log`
From: Johannes Schindelin @ 2023-09-05 10:56 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: git, Denton Liu, Jeff King
In-Reply-To: <8642763e-d3e7-49c4-b2ea-d5e4bebfbca5@app.fastmail.com>
Hi Kristoffer,
On Mon, 4 Sep 2023, Kristoffer Haugsbakk wrote:
> I would like to use your solution instead. Thank you.
Please do! You're welcome.
> Maybe you could supply a commit message for v3? v3 would then consist of
> two commits:
>
> 1. Your fix
> 2. Those two tests
>
> Or should it be handled in some other way?
Personally, I'd prefer it if you just squashed the changes into your
patch. If you want, I'd be delighted about a Helped-by (or even
Co-authored-by) footer, but taking ownership would be fine with me, too.
Ciao,
Johannes
^ permalink raw reply
* [PATCH] show doc: redirect user to git log manual instead of git diff-tree
From: Han Young @ 2023-09-05 12:12 UTC (permalink / raw)
To: git; +Cc: Han Young
While git show accepts options that apply to the git diff-tree command,
some options do not make sense in the context of git show.
The options of git show are handled using the machinery of git log.
The git log manual page is a better place to look into than git diff-tree
for options that are not in the git show manual page.
Signed-off-by: Han Young <hanyang.tony@bytedance.com>
---
Documentation/git-show.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-show.txt b/Documentation/git-show.txt
index 2b1bc7288d..13f63f5210 100644
--- a/Documentation/git-show.txt
+++ b/Documentation/git-show.txt
@@ -26,7 +26,7 @@ with --name-only).
For plain blobs, it shows the plain contents.
-The command takes options applicable to the 'git diff-tree' command to
+The command takes options applicable to the 'git log' command to
control how the changes the commit introduces are shown.
This manual page describes only the most frequently used options.
--
2.40.0
^ permalink raw reply related
* Re: Is "bare"ness in the context of multiple worktrees weird? Bitmap error in git gc.
From: Eric Sunshine @ 2023-09-05 5:43 UTC (permalink / raw)
To: Junio C Hamano
Cc: Tao Klerks, git, Johannes Schindelin, Taylor Blau,
Patrick Steinhardt
In-Reply-To: <xmqqedjdtoh5.fsf@gitster.g>
On Mon, Sep 4, 2023 at 9:09 PM Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> > This is not accurate. There is no default location for new worktrees;
> > git-worktree creates the new worktree at the location specified by the
> > user:
> >
> > git worktree add [<options>] <path> [<commit>]
> >
> > where <path> -- the only mandatory argument -- specifies the location.
>
> All correct. The per-worktree part of the repository data does live
> in a subdirectory of the ".git" directory and that was probably what
> Tao had in mind, though.
That could be. I read Tao's explanation as meaning that people do this:
git clone foo.git foo
cd foo
git worktree add bar
git worktree add baz
rather than (perhaps) this:
git clone foo.git foo
cd foo
git worktree add ../bar
git worktree add ../baz
But it's possible I misunderstood.
> >> Is it the case that this contrib script predates the current "git
> >> worktree" support?
> >
> > git-new-workdir predates git-worktree by quite a few years and, as I
> > understand it, remains in-tree because it fills a niche not entirely
> > filled by git-worktree.
>
> I actually think there is no longer a valid workflow whose support
> by "worktree" is still insufficient and the script has outlived its
> usefulness. I have been a heavy user of the new-workdir script to
> maintain my build environments, but I always have the HEAD of these
> workdir's detached, so I can easily switch my arrangement to use the
> "git worktree" without losing any flexibility.
My response was based upon my recollection of the periodic message
which shows up on the mailing list reporting a bug or submitting an
improvement for git-new-workdir, accompanied by a statement that
git-new-workdir is still a better fit for the user's particular
use-case. But I've never used it myself, so it's good to hear from
someone (you) who does use it.
> Perhaps we should remove it, possibly leaving a tombstone file like
> how we removed stuff from the contrib/examples directory.
Perhaps.
^ permalink raw reply
* Re: [PATCH] name-rev: use OPT_HIDDEN_BOOL for --peel-tag
From: Jeff King @ 2023-09-05 7:17 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List
In-Reply-To: <5a86c8f8-fcdc-fee9-8af5-aa5ecb036d2e@web.de>
On Sat, Sep 02, 2023 at 08:38:34PM +0200, René Scharfe wrote:
> adfc1857bd (describe: fix --contains when a tag is given as input,
> 2013-07-18) added the option --peel-tag, defining it using a positional
> struct option initializer and a comment indicating that it's intended to
> be a hidden OPT_BOOL. 4741edd549 (Remove deprecated OPTION_BOOLEAN for
> parsing arguments, 2013-08-03) added the macro OPT_HIDDEN_BOOL, which
> allows to express this more succinctly. Use it.
Yeah, this is a definite improvement.
OPT_HIDDEN_BOOL() itself is a little funny to me. I guess back then we
did not have the "_F" variants, but really it is just:
OPT_BOOL_F(0, "peel-tag", &peel_tag,
N_("dereference tags in the input (internal use)")),
PARSE_OPT_HIDDEN);
which would remove one more special case (after all, being hidden is
orthogonal to the type). But there are enough of them that maybe having
a special name for this is worth it. I dunno. But we could probably
simplify the definition, at least. :)
-Peff
^ permalink raw reply
* Re: [PATCH 0/2] replacing ci/config/allow-ref with a repo variable
From: Jeff King @ 2023-09-05 7:24 UTC (permalink / raw)
To: phillip.wood; +Cc: git, Johannes Schindelin
In-Reply-To: <d3d1109b-3a1f-4e8b-be8d-6581d45f1b81@gmail.com>
On Mon, Sep 04, 2023 at 10:56:15AM +0100, Phillip Wood wrote:
> > Yes, I think it would be possible to do something like:
> >
> > if: |
> > (vars.CI_BRANCHES == '' || contains(vars.CI_BRANCHES, github.ref_name)) &&
> > !contains(vars.CI_BRANCHES_REJECT, github.ref_name)
> >
> > It doesn't allow globbing, though. Do you need that?
>
> Oh I'd missed that, yes I do. All the globs are prefix matches but I'm not
> sure that helps.
It does make it easier. There's no globbing function available to us,
but if we know something is a prefix, there's a startsWith() we can use.
It does seem we're getting a combinatorial expansion of things to check,
though:
- full names to accept
- full names to reject
- prefixes to accept
- prefixes to reject
I wrote "prefixes" but I'm actually not sure how feasible that is. That
implies iterating over the list of prefixes, which I'm not sure we can
do.
-Peff
^ permalink raw reply
* Re: [PATCH v2 10/10] parse-options: mark unused parameters in noop callback
From: Jeff King @ 2023-09-05 7:09 UTC (permalink / raw)
To: René Scharfe; +Cc: git, Junio C Hamano
In-Reply-To: <3bf1bd4a-9d28-32c1-7838-09acd2c85d03@web.de>
On Sat, Sep 02, 2023 at 01:37:08PM +0200, René Scharfe wrote:
> --- >8 ---
> Subject: [PATCH] parse-options: let NULL callback be a noop
>
> Allow OPTION_CALLBACK options to have a NULL callback function pointer
> and do nothing in that case, yet still handle arguments as usual. Use
> this to replace parse_opt_noop_cb().
>
> We lose the ability to distinguish between a forgotten function pointer
> and intentional noop, but that will be caught by a compiler warning
> about an unused function in many cases and having an option do nothing
> is a benign failure mode.
Yeah, my concern would be missing an accidental NULL here. I guess that
is pretty unlikely in practice, though. I'd guess the worst case would
be somebody accidentally putting the function into the "opt->value" slot
where the compiler might then think it is used (though I don't know
off-hand if it would complain about an implicit conversion of a function
pointer into a "void *").
> We also lose the ability to add a warning to the callback, but we
> haven't done that since it was added by 6acec0380b (parseopt: add
> OPT_NOOP_NOARG, 2011-09-28), so we probably won't do it soon. We can
> add a callback back when we want to show a warning.
>
> By letting go of features we don't need this patch simplifies the
> parse-options API and gets rid of an exported empty function.
Your patch looks correct to me. I probably wouldn't have bothered to
write it, but now you did. :) My inclination would be to go with my
dumb-simple one for this series, and it looks like you may have
collected a few slight-more-risky-but-maybe-worthwhile parseopt cleanups
on top that could make their own series.
-Peff
^ permalink raw reply
* [PATCH v3 04/10] checkout-index: delay automatic setting of to_tempfile
From: Jeff King @ 2023-09-05 7:12 UTC (permalink / raw)
To: René Scharfe; +Cc: git, Junio C Hamano
In-Reply-To: <c7855b08-46ee-5df0-4b0f-67ea57d84b18@web.de>
On Sat, Sep 02, 2023 at 08:20:43AM +0200, René Scharfe wrote:
> Am 31.08.23 um 23:20 schrieb Jeff King:
> > @@ -269,6 +268,11 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
> > state.base_dir = "";
> > state.base_dir_len = strlen(state.base_dir);
> >
> > + if (to_tempfile < 0)
> > + to_tempfile = (checkout_stage == CHECKOUT_ALL);
> > + if (!to_tempfile && checkout_stage == CHECKOUT_ALL)
> > + die("--stage=all and --no-temp are incompatible");
>
> How about making this message translatable from the start and following
> the convention from 12909b6b8a (i18n: turn "options are incompatible"
> into "cannot be used together", 2022-01-05) to reuse the existing
> translations?
Good catch. I forgot that we had standardized some of these. The other
error messages in the file aren't translated, but I don't think that's
an intentional choice (even though it is plumbing). Some of them are
obviously quite old and don't match our usual style (like starting with
"checkout-index: ").
Rather than re-send the whole series, I _think_ this is the only patch I
would change in a re-roll (if you buy me "sure, go ahead and send it on
top" evasions in my other responses).
So here's a replacement patch 4 that fixes up the message.
-- >8 --
Subject: checkout-index: delay automatic setting of to_tempfile
Using --stage=all requires writing to tempfiles, since we cannot put
multiple stages into a single file. So --stage=all implies --temp.
But we do so by setting to_tempfile in the options callback for --stage,
rather than after all options have been parsed. This leads to two bugs:
1. If you run "checkout-index --stage=all --stage=2", this should not
imply --temp, but it currently does. The callback cannot just unset
to_tempfile when it sees the "2" value, because it no longer knows
if its value was from the earlier --stage call, or if the user
specified --temp explicitly.
2. If you run "checkout-index --stage=all --no-temp", the --no-temp
will overwrite the earlier implied --temp. But this mode of
operation cannot work, and the command will fail with "<path>
already exists" when trying to write the higher stages.
We can fix both by lazily setting to_tempfile. We'll make it a tristate,
with -1 as "not yet given", and have --stage=all enable it only after
all options are parsed. Likewise, after all options are parsed we can
detect and reject the bogus "--no-temp" case.
Note that this does technically change the behavior for "--stage=all
--no-temp" for paths which have only one stage present (which
accidentally worked before, but is now forbidden). But this behavior was
never intended, and you'd have to go out of your way to try to trigger
it.
The new tests cover both cases, as well the general "--stage=all implies
--temp", as most of the other tests explicitly say "--temp". Ironically,
the test "checkout --temp within subdir" is the only one that _doesn't_
use "--temp", and so was implicitly covering this case. But it seems
reasonable to have a more explicit test alongside the other related
ones.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/checkout-index.c | 9 +++++++--
t/t2004-checkout-cache-temp.sh | 20 ++++++++++++++++++++
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index f62f13f2b5..526c210bcb 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -24,7 +24,7 @@
static int nul_term_line;
static int checkout_stage; /* default to checkout stage0 */
static int ignore_skip_worktree; /* default to 0 */
-static int to_tempfile;
+static int to_tempfile = -1;
static char topath[4][TEMPORARY_FILENAME_LENGTH + 1];
static struct checkout state = CHECKOUT_INIT;
@@ -196,7 +196,6 @@ static int option_parse_stage(const struct option *opt,
BUG_ON_OPT_NEG(unset);
if (!strcmp(arg, "all")) {
- to_tempfile = 1;
checkout_stage = CHECKOUT_ALL;
} else {
int ch = arg[0];
@@ -269,6 +268,12 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
state.base_dir = "";
state.base_dir_len = strlen(state.base_dir);
+ if (to_tempfile < 0)
+ to_tempfile = (checkout_stage == CHECKOUT_ALL);
+ if (!to_tempfile && checkout_stage == CHECKOUT_ALL)
+ die(_("options '%s' and '%s' cannot be used together"),
+ "--stage=all", "--no-temp");
+
/*
* when --prefix is specified we do not want to update cache.
*/
diff --git a/t/t2004-checkout-cache-temp.sh b/t/t2004-checkout-cache-temp.sh
index b16d69ca4a..45dd1bc858 100755
--- a/t/t2004-checkout-cache-temp.sh
+++ b/t/t2004-checkout-cache-temp.sh
@@ -117,6 +117,26 @@ test_expect_success 'checkout all stages/one file to temporary files' '
test $(cat $s3) = tree3path1)
'
+test_expect_success '--stage=all implies --temp' '
+ rm -f path* .merge_* actual &&
+ git checkout-index --stage=all -- path1 &&
+ test_path_is_missing path1
+'
+
+test_expect_success 'overriding --stage=all resets implied --temp' '
+ rm -f path* .merge_* actual &&
+ git checkout-index --stage=all --stage=2 -- path1 &&
+ echo tree2path1 >expect &&
+ test_cmp expect path1
+'
+
+test_expect_success '--stage=all --no-temp is rejected' '
+ rm -f path* .merge_* actual &&
+ test_must_fail git checkout-index --stage=all --no-temp -- path1 2>err &&
+ grep -v "already exists" err &&
+ grep "options .--stage=all. and .--no-temp. cannot be used together" err
+'
+
test_expect_success 'checkout some stages/one file to temporary files' '
rm -f path* .merge_* actual &&
git checkout-index --stage=all --temp -- path2 >actual &&
--
2.42.0.567.gc396a4a104
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox