| Message ID | 20230107045757.30037-1-jacobabel@nullpo.dev (mailing list archive) |
|---|---|
| Headers | show |
| Series | worktree: Support `--orphan` when creating new worktrees | expand |
On Sat, Jan 07 2023, Jacob Abel wrote: > While working with the worktree based git workflow, I realised that setting > up a new git repository required switching between the traditional and > worktree based workflows. Searching online I found a SO answer [1] which > seemed to support this and which indicated that adding support for this should > not be technically difficult. > > This patchset has four parts: > * adding `-B` to the usage docs (noticed during dev and it seemed too small > to justify a separate submission) > * adding a helper fn to simplify testing for mutual exclusion of options > in `t/t2400-worktree-add.sh` > * adding orphan branch functionality (as is present in `git-switch`) > to `git-worktree-add` > * adding an advise for using --orphan when `git worktree add` fails due to > a bad ref. > > Changes from v6: > * Removed helper save_param_arr() introduced in v6 from t2400 (2/4) [2]. > * Reverted changes introduced in v6 to test_wt_add_excl() from t2400 (2/4) [3]. > * Changed test_wt_add_excl() to use `local opts="$*"` (2/4) [3]. > * Added check to test_wt_add_excl() to better validate test results (2/4). > * Re-add &&-chains to test_wt_add_excl() (2/4) [4]. > * Reverted changes introduced in v6 to test_wt_add_empty_repo_orphan_hint() > from t2400 (4/4) [3]. > * Changed test_wt_add_empty_repo_orphan_hint() to use `local opts="$*"` (4/4) [3]. > * Added check to test_wt_add_empty_repo_orphan_hint() to better validate test > results (4/4). > * Re-add &&-chains to test_wt_add_empty_repo_orphan_hint() (2/4) [4]. This round looks good to me, except for a small tiny (and new) portability issue that needs fixing: > Range-diff against v6: > 1: a9ef3eca7b = 1: a9ef3eca7b worktree add: include -B in usage docs > 2: c03c112f79 ! 2: d124cc481c worktree add: refactor opt exclusion tests > @@ t/t2400-worktree-add.sh: test_expect_success '"add" no auto-vivify with --detach > -test_expect_success '"add" -b/-B mutually exclusive' ' > - test_must_fail git worktree add -b poodle -B poodle bamboo main > -' > -+# Saves parameter sequence/array as a string so they can be safely stored in a > -+# variable and restored with `eval "set -- $arr"`. Sourced from > -+# https://stackoverflow.com/a/27503158/15064705 > -+save_param_arr () { > -+ local i > -+ for i; > -+ do > -+ # For each argument: > -+ # 1. Append "\n" after each entry > -+ # 2. Convert "'" into "'\''" > -+ # 3. Prepend "'" before each entry > -+ # 4. Append " \" after each entry > -+ printf "%s\\n" "$i" | sed " > -+ s/'/'\\\\''/g > -+ 1s/^/'/ > -+ \$s/\$/' \\\\/ > -+ " > -+ done > -+ echo " " > -+} > - > +- > -test_expect_success '"add" -b/--detach mutually exclusive' ' > - test_must_fail git worktree add -b poodle --detach bamboo main > -' Good to get rid of this. > +# Helper function to test mutually exclusive options. > ++# > ++# Note: Quoted arguments containing spaces are not supported. > +test_wt_add_excl () { > -+ local arr=$(save_param_arr "$@") > -+ test_expect_success "'worktree add' with $* has mutually exclusive options" ' > -+ eval "set -- $arr" && > -+ test_must_fail git worktree add "$@" > ++ local opts="$*" && > ++ test_expect_success "'worktree add' with '$opts' has mutually exclusive options" ' > ++ test_must_fail git worktree add $opts 2>actual && > ++ grep -P "fatal:( options)? .* cannot be used together" actual This is the new unportable code, the "-P" option is specific to GNU grep, and here you're relying on the "?" syntax along with other ERE-like syntax. It looks like the minimal fix here is to just change -P to -E, which we can use, you're not using any PCRE-syntax, but just syntax that's part of ERE.
On 23/01/09 01:26PM, Ævar Arnfjörð Bjarmason wrote: > > On Sat, Jan 07 2023, Jacob Abel wrote: > > > [...] > > This round looks good to me, except for a small tiny (and new) > portability issue that needs fixing: > > > [...] > > Good to get rid of this. > > > [...] > > ++ grep -P "fatal:( options)? .* cannot be used together" actual > > This is the new unportable code, the "-P" option is specific to GNU > grep, and here you're relying on the "?" syntax along with other > ERE-like syntax. > > It looks like the minimal fix here is to just change -P to -E, which we > can use, you're not using any PCRE-syntax, but just syntax that's part > of ERE. Understood. I have made the change and prepared a new revision already if this is all that needs to be changed. I can submit it at any time however I was unsure of whether it was considered bad etiquette to submit new revisions so close to each other.
On Mon, Jan 09 2023, Jacob Abel wrote: > On 23/01/09 01:26PM, Ævar Arnfjörð Bjarmason wrote: >> >> On Sat, Jan 07 2023, Jacob Abel wrote: >> >> > [...] >> >> This round looks good to me, except for a small tiny (and new) >> portability issue that needs fixing: >> >> > [...] >> >> Good to get rid of this. >> >> > [...] >> > ++ grep -P "fatal:( options)? .* cannot be used together" actual >> >> This is the new unportable code, the "-P" option is specific to GNU >> grep, and here you're relying on the "?" syntax along with other >> ERE-like syntax. >> >> It looks like the minimal fix here is to just change -P to -E, which we >> can use, you're not using any PCRE-syntax, but just syntax that's part >> of ERE. > > Understood. I have made the change and prepared a new revision already if this > is all that needs to be changed. I can submit it at any time however I was > unsure of whether it was considered bad etiquette to submit new revisions so > close to each other. It would be good to have that new revision soon in this case. Generally, if you submit a series you're expected to wait to give more comments time to trickle in. But as a series gets more "ready" a re-roll can come sooner, e.g. in this case where at least I'm fairly confident that this is (knock on wood) "the last bug". So once that version's in Junio can hopefully queue it for graduation to next soon-ish.
On 23/01/09 06:21PM, Ævar Arnfjörð Bjarmason wrote: > > On Mon, Jan 09 2023, Jacob Abel wrote: > > > On 23/01/09 01:26PM, Ævar Arnfjörð Bjarmason wrote: > >> > >> On Sat, Jan 07 2023, Jacob Abel wrote: > >> > >> > [...] > >> > >> This round looks good to me, except for a small tiny (and new) > >> portability issue that needs fixing: > >> > >> > [...] > >> > >> Good to get rid of this. > >> > >> > [...] > >> > ++ grep -P "fatal:( options)? .* cannot be used together" actual > >> > >> This is the new unportable code, the "-P" option is specific to GNU > >> grep, and here you're relying on the "?" syntax along with other > >> ERE-like syntax. > >> > >> It looks like the minimal fix here is to just change -P to -E, which we > >> can use, you're not using any PCRE-syntax, but just syntax that's part > >> of ERE. > > > > Understood. I have made the change and prepared a new revision already if this > > is all that needs to be changed. I can submit it at any time however I was > > unsure of whether it was considered bad etiquette to submit new revisions so > > close to each other. > > It would be good to have that new revision soon in this case. > > Generally, if you submit a series you're expected to wait to give more > comments time to trickle in. > > But as a series gets more "ready" a re-roll can come sooner, e.g. in > this case where at least I'm fairly confident that this is (knock on > wood) "the last bug". > > So once that version's in Junio can hopefully queue it for graduation to > next soon-ish. OK. That makes sense. In that case I'll get that sent out right now.
While working with the worktree based git workflow, I realised that setting up a new git repository required switching between the traditional and worktree based workflows. Searching online I found a SO answer [1] which seemed to support this and which indicated that adding support for this should not be technically difficult. This patchset has four parts: * adding `-B` to the usage docs (noticed during dev and it seemed too small to justify a separate submission) * adding a helper fn to simplify testing for mutual exclusion of options in `t/t2400-worktree-add.sh` * adding orphan branch functionality (as is present in `git-switch`) to `git-worktree-add` * adding an advise for using --orphan when `git worktree add` fails due to a bad ref. Changes from v6: * Removed helper save_param_arr() introduced in v6 from t2400 (2/4) [2]. * Reverted changes introduced in v6 to test_wt_add_excl() from t2400 (2/4) [3]. * Changed test_wt_add_excl() to use `local opts="$*"` (2/4) [3]. * Added check to test_wt_add_excl() to better validate test results (2/4). * Re-add &&-chains to test_wt_add_excl() (2/4) [4]. * Reverted changes introduced in v6 to test_wt_add_empty_repo_orphan_hint() from t2400 (4/4) [3]. * Changed test_wt_add_empty_repo_orphan_hint() to use `local opts="$*"` (4/4) [3]. * Added check to test_wt_add_empty_repo_orphan_hint() to better validate test results (4/4). * Re-add &&-chains to test_wt_add_empty_repo_orphan_hint() (2/4) [4]. 1. https://stackoverflow.com/a/68717229/15064705/ 2. https://lore.kernel.org/git/xmqq5ydvpu1o.fsf@gitster.g/ 3. https://lore.kernel.org/git/xmqqo7rbsuyh.fsf@gitster.g/ 4. https://lore.kernel.org/git/221228.868risuf5x.gmgdl@evledraar.gmail.com/ Jacob Abel (4): worktree add: include -B in usage docs worktree add: refactor opt exclusion tests worktree add: add --orphan flag worktree add: add hint to direct users towards --orphan Documentation/config/advice.txt | 4 ++ Documentation/git-worktree.txt | 17 +++++- advice.c | 1 + advice.h | 1 + builtin/worktree.c | 65 ++++++++++++++++++++--- t/t2400-worktree-add.sh | 94 +++++++++++++++++++++++++++++---- 6 files changed, 164 insertions(+), 18 deletions(-) Range-diff against v6: 1: a9ef3eca7b = 1: a9ef3eca7b worktree add: include -B in usage docs 2: c03c112f79 ! 2: d124cc481c worktree add: refactor opt exclusion tests @@ t/t2400-worktree-add.sh: test_expect_success '"add" no auto-vivify with --detach -test_expect_success '"add" -b/-B mutually exclusive' ' - test_must_fail git worktree add -b poodle -B poodle bamboo main -' -+# Saves parameter sequence/array as a string so they can be safely stored in a -+# variable and restored with `eval "set -- $arr"`. Sourced from -+# https://stackoverflow.com/a/27503158/15064705 -+save_param_arr () { -+ local i -+ for i; -+ do -+ # For each argument: -+ # 1. Append "\n" after each entry -+ # 2. Convert "'" into "'\''" -+ # 3. Prepend "'" before each entry -+ # 4. Append " \" after each entry -+ printf "%s\\n" "$i" | sed " -+ s/'/'\\\\''/g -+ 1s/^/'/ -+ \$s/\$/' \\\\/ -+ " -+ done -+ echo " " -+} - +- -test_expect_success '"add" -b/--detach mutually exclusive' ' - test_must_fail git worktree add -b poodle --detach bamboo main -' +# Helper function to test mutually exclusive options. ++# ++# Note: Quoted arguments containing spaces are not supported. +test_wt_add_excl () { -+ local arr=$(save_param_arr "$@") -+ test_expect_success "'worktree add' with $* has mutually exclusive options" ' -+ eval "set -- $arr" && -+ test_must_fail git worktree add "$@" ++ local opts="$*" && ++ test_expect_success "'worktree add' with '$opts' has mutually exclusive options" ' ++ test_must_fail git worktree add $opts 2>actual && ++ grep -P "fatal:( options)? .* cannot be used together" actual + ' +} 3: 9b93e2493a = 3: b66ea4d309 worktree add: add --orphan flag 4: 737fca6986 ! 4: b779606121 worktree add: add hint to direct users towards --orphan @@ t/t2400-worktree-add.sh: test_expect_success '"add" worktree with orphan branch, test_cmp expect .git/worktrees/orphan-with-lock-reason/locked ' ++# Note: Quoted arguments containing spaces are not supported. +test_wt_add_empty_repo_orphan_hint () { -+ local context="$1" -+ shift -+ local arr=$(save_param_arr "$@") ++ local context="$1" && ++ shift && ++ local opts="$*" && + test_expect_success "'worktree add' show orphan hint in empty repo w/ $context" ' -+ eval "set -- $arr" && + test_when_finished "rm -rf empty_repo" && + GIT_DIR="empty_repo" git init --bare && -+ test_must_fail git -C empty_repo worktree add "$@" foobar/ 2> actual && ++ test_must_fail git -C empty_repo worktree add $opts foobar/ 2>actual && ++ ! grep "error: unknown switch" actual && + grep "hint: If you meant to create a worktree containing a new orphan branch" actual + ' +} -- 2.38.2