worktree add: shouldn't dwim if -b or -B is given
PATCH 1/1 worktree add: shouldn't dwim if -b or -B is given
From: Yoichi NAKAYAMA <[email protected]>
'git worktree add <path> <branch>' DWIMs <branch> to a remote-tracking branch when neither -b, -B, nor --detach is given.
However, 'git worktree add -b <new-branch> <path> <branch>' can still DWIM <branch>, causing <new-branch> to be ignored.
This is a regression introduced in v2.42.0
(128e5496b325640f0a09cc1d5b1e346c069b410f).
---
worktree add: shouldn't dwim if -b or -B is given
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2192%2Fyoichi%2Fworktree-add-should-not-dwim-with-b-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2192/yoichi/worktree-add-should-not-dwim-with-b-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2192
builtin/worktree.c | 2 +- t/t2400-worktree-add.sh | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-)
builtin/worktree.c +1 -1
@@ -897,7 +897,7 @@ static int add(int ac, const char **av, const char *prefix,/* DWIM: Infer --orphan when repo has no refs. */ opts.orphan = (!s) &&dwim_orphan (&opts, !!opt_track,1 ); - }else if (ac ==2 ) { + }else if (ac ==2 && !new_branch) {struct object_id oid;struct commit *commit;char *remote;
t/t2400-worktree-add.sh +10 -0
@@ -621,6 +621,16 @@ test_expect_success '"add" <path> <branch> dwims' ' ) ' +test_expect_success '"add" <path> <branch> does not dwim with -b' ' + test_when_finished rm -rf repo_upstream repo_dwim foo && + setup_remote_repo repo_upstream repo_dwim && + git init repo_dwim && + ( + cd repo_dwim && + test_must_fail git worktree add -b branch ../foo foo + ) +' + test_expect_success '"add" <path> <branch> dwims with checkout.defaultRemote' ' test_when_finished rm -rf repo_upstream repo_dwim foo && setup_remote_repo repo_upstream repo_dwim &&
base-commit: 5b2471720c93ee30e5764a19f3d3b3ae9ec9712a
Signature · 1 line
gitgitgadget
Quoted 29 lines · Yoichi NAKAYAMA via GitGitGadget
From: Yoichi NAKAYAMA <[email protected]>
'git worktree add <path> <branch>' DWIMs <branch> to a remote-tracking branch when neither -b, -B, nor --detach is given.
However, 'git worktree add -b <new-branch> <path> <branch>' can still DWIM <branch>, causing <new-branch> to be ignored.
This is a regression introduced in v2.42.0
(128e5496b325640f0a09cc1d5b1e346c069b410f).Signed-off-by Yoichi NAKAYAMA <[email protected]>---
worktree add: shouldn't dwim if -b or -B is givenPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2192%2Fyoichi%2Fworktree-add-should-not-dwim-with-b-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2192/yoichi/worktree-add-should-not-dwim-with-b-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2192builtin/worktree.c | 2 +- t/t2400-worktree-add.sh | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-)builtin/worktree.c +0
-0
builtin/worktree.c static int add(int ac, const char **av, const char *prefix, · line 903
Quoted 8 lines
builtin/worktree.c +1
-1@@ -897,7 +897,7 @@ static int add(int ac, const char **av, const char *prefix,/* DWIM: Infer --orphan when repo has no refs. */ opts.orphan = (!s) &&dwim_orphan (&opts, !!opt_track,1 ); - }else if (ac ==2 ) { + }else if (ac ==2 && !new_branch) {struct object_id oid;struct commit *commit;char *remote;
This part checks 'branch' (assigned from av[1] earlier) to see if it names a commit. When it does not, the code checks if it is the name of a unique remote-tracking branch; if it is, the code uses that as 'branch', which is the origin to be used to fork 'new_branch' (av[1] in this case) from.
Your observation is correct that this would overwrite 'new_branch' if it were supplied.
Stepping back a bit, though, does this change the behavior when 'branch' *does* resolve to a commit (hence, the DWIM is already bypassed and 'new_branch' or 'branch' are not nuked)? When 'ac' is equal to 2 and 'new_branch' is supplied, we used to call:
if (!strcmp(branch, "HEAD"))
can_use_local_refs(&opts);inside the block you are now skipping. It looks to me that this patch also changes behavior when the user says:
$ git worktree add -b <new-branch> <path> HEAD
by not calling can_use_local_refs(), whose only effect in this context is that it may issue a warning() to the user. I do not know offhand what the ramifications of this difference are.
I wonder if we want to skip only the dwim part inside of this "else if" arm, e.g.
builtin/worktree.c +2 -0
@@ -898,6 +898,7 @@ static int add(int ac, const char **av, const char *prefix,/* DWIM: Infer --orphan when repo has no refs. */ opts.orphan = (!s) &&dwim_orphan (&opts, !!opt_track,1 ); }else if (ac ==2 ) { +if (!newbranch) {struct object_id oid;struct commit *commit;char *remote;
@@ -910,6 +911,7 @@ static int add(int ac, const char **av, const char *prefix, branch = new_branch_to_free = remote; } } + }if (!strcmp (branch,"HEAD" ))can_use_local_refs (&opts);
Note that above diff is with broken indentation to help reduce the patch noise to illustrate where the new block boundary would be.
Quoted 7 lines
t/t2400-worktree-add.sh +0
-0@@ -621,6 +621,16 @@ test_expect_success '"add" <path> <branch> dwims' ' ) '
t/t2400-worktree-add.sh test_expect_success '"add" <path> <branch> dwims' ' · line 632
Quoted 10 lines
t/t2400-worktree-add.sh +9
-0+test_expect_success '"add" <path> <branch> does not dwim with -b' ' + test_when_finished rm -rf repo_upstream repo_dwim foo && + setup_remote_repo repo_upstream repo_dwim && + git init repo_dwim && + ( + cd repo_dwim && + test_must_fail git worktree add -b branch ../foo foo + ) +'
Two comments.
* You call setup_remote_repo(), which takes two names, runs 'git init' on each of them, and prepares those repositories. Do you need to run 'git init repo_dwim' to initialize it again? What for?
* Expecting a failure to create a worktree may be fine, but can we say how we expect the command to fail as well? Perhaps you can capture the error message while ensuring that the command fails, and then check the error message to make sure it failed in the expected way, or something like that.
Thanks.
Quoted 8 lines · Junio C Hamano
Stepping back a bit, though, does this change the behavior when 'branch' *does* resolve to a commit (hence, the DWIM is already bypassed and '
new_branch' or 'branch' are not nuked)? When 'ac' is equal to 2 and 'new_branch' is supplied, we used to call:if (!strcmp(branch, "HEAD")) can_use_local_refs(&opts);
Quoted 8 lines
inside the block you are now skipping. It looks to me that this patch also changes behavior when the user says:
$ git worktree add -b <new-branch> <path> HEADby not calling
can_use_local_refs(), whose only effect in this context is that it may issue awarning()to the user. I do not know offhand what the ramifications of this difference are.
It is better to emit warnings for the given arguments whenever possible, so I think we should not skip the step mentioned above.
I'll modify the condition.
Quoted 20 lines
Quoted 7 lines
t/t2400-worktree-add.sh +0
-0@@ -621,6 +621,16 @@ test_expect_success '"add" <path> <branch> dwims' ' ) 't/t2400-worktree-add.sh test_expect_success '"add" <path> <branch> dwims' ' · line 632
Quoted 10 lines
t/t2400-worktree-add.sh +9
-0+test_expect_success '"add" <path> <branch> does not dwim with -b' ' + test_when_finished rm -rf repo_upstream repo_dwim foo && + setup_remote_repo repo_upstream repo_dwim && + git init repo_dwim && + ( + cd repo_dwim && + test_must_fail git worktree add -b branch ../foo foo + ) +'Two comments.
Quoted 4 lines
* You call
setup_remote_repo(), which takes two names, runs 'git init' on each of them, and prepares those repositories. Do you need to run 'git initrepo_dwim' to initialize it again? What for?
'git init' was unnecessary. It was included because I copied an existing test; I'll remove it.
Quoted 5 lines
* Expecting a failure to create a worktree may be fine, but can we say how we expect the command to fail as well? Perhaps you can capture the error message while ensuring that the command fails, and then check the error message to make sure it failed in the expected way, or something like that.
Expected error is 'fatal: invalid reference: foo'. I'll add test_grep.
Thanks,
Signature · 1 line
Yoichi NAKAYAMA
PATCH v2 1/1 worktree add: shouldn't dwim if -b or -B is given
In reply to Yoichi NAKAYAMA via GitGitGadgetFrom: Yoichi NAKAYAMA <[email protected]>
git worktree add <path> <branch> DWIMs <branch> to a remote-tracking branch when neither -b, -B, nor --detach is given.
However, git worktree add -b <new-branch> <path> <branch> can still DWIM <branch>, causing <new-branch> to be ignored.
This is a regression introduced in v2.42.0
(128e5496b325640f0a09cc1d5b1e346c069b410f).
---
worktree add: shouldn't dwim if -b or -B is given
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2192%2Fyoichi%2Fworktree-add-should-not-dwim-with-b-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2192/yoichi/worktree-add-should-not-dwim-with-b-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2192
Range-diff vs v1:
1: 908a32fb56 ! 1: b00f6c2fa1 worktree add: shouldn't dwim if -b or -B is given
@@ Metadata
## Commit message ##
worktree add: shouldn't dwim if -b or -B is given
- 'git worktree add <path> <branch>' DWIMs <branch> to a
+ git worktree add <path> <branch> DWIMs <branch> to a
remote-tracking branch when neither -b, -B, nor --detach
is given.
- However, 'git worktree add -b <new-branch> <path> <branch>' can
+ However, git worktree add -b <new-branch> <path> <branch> can
still DWIM <branch>, causing <new-branch> to be ignored.
This is a regression introduced in v2.42.0
@@ Commit message
## builtin/worktree.c ##
@@ builtin/worktree.c: static int add(int ac, const char **av, const char *prefix,
-
/* DWIM: Infer --orphan when repo has no refs. */
opts.orphan = (!s) && dwim_orphan(&opts, !!opt_track, 1);
-- } else if (ac == 2) {
-+ } else if (ac == 2 && !new_branch) {
- struct object_id oid;
- struct commit *commit;
- char *remote;
+ } else if (ac == 2) {
+- struct object_id oid;
+- struct commit *commit;
+- char *remote;
+-
+- commit = lookup_commit_reference_by_name(branch);
+- if (!commit) {
+- remote = unique_tracking_name(branch, &oid, NULL);
+- if (remote) {
+- new_branch = branch;
+- branch = new_branch_to_free = remote;
++ if (!new_branch) {
++ struct object_id oid;
++ struct commit *commit;
++ char *remote;
++
++ commit = lookup_commit_reference_by_name(branch);
++ if (!commit) {
++ remote = unique_tracking_name(branch, &oid, NULL);
++ if (remote) {
++ new_branch = branch;
++ branch = new_branch_to_free = remote;
++ }
+ }
+ }
+
## t/t2400-worktree-add.sh ##
@@ t/t2400-worktree-add.sh: test_expect_success '"add" <path> <branch> dwims' '
@@ t/t2400-worktree-add.sh: test_expect_success '"add" <path> <branch> dwims' '
'
+test_expect_success '"add" <path> <branch> does not dwim with -b' '
-+ test_when_finished rm -rf repo_upstream repo_dwim foo &&
++ test_when_finished rm -rf repo_upstream repo_dwim wt &&
+ setup_remote_repo repo_upstream repo_dwim &&
-+ git init repo_dwim &&
+ (
+ cd repo_dwim &&
-+ test_must_fail git worktree add -b branch ../foo foo
++ test_must_fail git worktree add -b branch ../wt foo 2>actual &&
++ test_grep "^fatal: invalid reference: foo" actual
+ )
+'
+builtin/worktree.c | 22 ++++++++++++---------- t/t2400-worktree-add.sh | 10 ++++++++++ 2 files changed, 22 insertions(+), 10 deletions(-)
builtin/worktree.c +12 -10
@@ -898,16 +898,18 @@ static int add(int ac, const char **av, const char *prefix,/* DWIM: Infer --orphan when repo has no refs. */ opts.orphan = (!s) &&dwim_orphan (&opts, !!opt_track,1 ); }else if (ac ==2 ) { -struct object_id oid; -struct commit *commit; -char *remote; - - commit =lookup_commit_reference_by_name (branch); -if (!commit) { - remote =unique_tracking_name (branch, &oid,NULL ); -if (remote) { - new_branch = branch; - branch = new_branch_to_free = remote; +if (!new_branch) { +struct object_id oid; +struct commit *commit; +char *remote; + + commit =lookup_commit_reference_by_name (branch); +if (!commit) { + remote =unique_tracking_name (branch, &oid,NULL ); +if (remote) { + new_branch = branch; + branch = new_branch_to_free = remote; + } } }
t/t2400-worktree-add.sh +10 -0
@@ -621,6 +621,16 @@ test_expect_success '"add" <path> <branch> dwims' ' ) ' +test_expect_success '"add" <path> <branch> does not dwim with -b' ' + test_when_finished rm -rf repo_upstream repo_dwim wt && + setup_remote_repo repo_upstream repo_dwim && + ( + cd repo_dwim && + test_must_fail git worktree add -b branch ../wt foo 2>actual && + test_grep "^fatal: invalid reference: foo" actual + ) +' + test_expect_success '"add" <path> <branch> dwims with checkout.defaultRemote' ' test_when_finished rm -rf repo_upstream repo_dwim foo && setup_remote_repo repo_upstream repo_dwim &&
base-commit: 5b2471720c93ee30e5764a19f3d3b3ae9ec9712a
Signature · 1 line
gitgitgadget
Quoted 12 lines · Yoichi NAKAYAMA via GitGitGadget
Range-diff vs v1:
1:
908a32fb56! 1:b00f6c2fa1worktree add: shouldn't dwim if -b or -B is given
@@ Metadata
## Commit message ##
worktree add: shouldn't dwim if -b or -B is given- 'git worktree add <path> <branch>' DWIMs <branch> to a
+ git worktree add <path> <branch> DWIMs <branch> to a
remote-tracking branch when neither -b, -B, nor --detach is given.
Quoted 5 lines
- However, 'git worktree add -b <new-branch> <path> <branch>' can
+ However, git worktree add -b <new-branch> <path> <branch> can
still DWIM <branch>, causing <new-branch> to be ignored.This is a regression introduced in v2.42.0
I accidentally removed the quotes in the commit message. I'll fix it.
Signature · 1 line
Yoichi NAKAYAMA
PATCH v3 1/1 worktree add: shouldn't dwim if -b or -B is given
In reply to Yoichi NAKAYAMA via GitGitGadgetFrom: Yoichi NAKAYAMA <[email protected]>
'git worktree add <path> <branch>' DWIMs <branch> to a remote-tracking branch when neither -b, -B, nor --detach is given.
However, 'git worktree add -b <new-branch> <path> <branch>' can still DWIM <branch>, causing <new-branch> to be ignored.
This is a regression introduced in v2.42.0
(128e5496b325640f0a09cc1d5b1e346c069b410f).
---
worktree add: shouldn't dwim if -b or -B is given
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2192%2Fyoichi%2Fworktree-add-should-not-dwim-with-b-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2192/yoichi/worktree-add-should-not-dwim-with-b-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2192
Range-diff vs v2:
1: b00f6c2fa1 ! 1: 477ea27c8b worktree add: shouldn't dwim if -b or -B is given
@@ Metadata
## Commit message ##
worktree add: shouldn't dwim if -b or -B is given
- git worktree add <path> <branch> DWIMs <branch> to a
+ 'git worktree add <path> <branch>' DWIMs <branch> to a
remote-tracking branch when neither -b, -B, nor --detach is given.
- However, git worktree add -b <new-branch> <path> <branch> can
+ However, 'git worktree add -b <new-branch> <path> <branch>' can
still DWIM <branch>, causing <new-branch> to be ignored.
This is a regression introduced in v2.42.0
builtin/worktree.c | 22 ++++++++++++---------- t/t2400-worktree-add.sh | 10 ++++++++++ 2 files changed, 22 insertions(+), 10 deletions(-)
builtin/worktree.c +12 -10
@@ -898,16 +898,18 @@ static int add(int ac, const char **av, const char *prefix,/* DWIM: Infer --orphan when repo has no refs. */ opts.orphan = (!s) &&dwim_orphan (&opts, !!opt_track,1 ); }else if (ac ==2 ) { -struct object_id oid; -struct commit *commit; -char *remote; - - commit =lookup_commit_reference_by_name (branch); -if (!commit) { - remote =unique_tracking_name (branch, &oid,NULL ); -if (remote) { - new_branch = branch; - branch = new_branch_to_free = remote; +if (!new_branch) { +struct object_id oid; +struct commit *commit; +char *remote; + + commit =lookup_commit_reference_by_name (branch); +if (!commit) { + remote =unique_tracking_name (branch, &oid,NULL ); +if (remote) { + new_branch = branch; + branch = new_branch_to_free = remote; + } } }
t/t2400-worktree-add.sh +10 -0
@@ -621,6 +621,16 @@ test_expect_success '"add" <path> <branch> dwims' ' ) ' +test_expect_success '"add" <path> <branch> does not dwim with -b' ' + test_when_finished rm -rf repo_upstream repo_dwim wt && + setup_remote_repo repo_upstream repo_dwim && + ( + cd repo_dwim && + test_must_fail git worktree add -b branch ../wt foo 2>actual && + test_grep "^fatal: invalid reference: foo" actual + ) +' + test_expect_success '"add" <path> <branch> dwims with checkout.defaultRemote' ' test_when_finished rm -rf repo_upstream repo_dwim foo && setup_remote_repo repo_upstream repo_dwim &&
base-commit: 5b2471720c93ee30e5764a19f3d3b3ae9ec9712a
Signature · 1 line
gitgitgadget
Quoted 11 lines · Yoichi NAKAYAMA via GitGitGadget
From: Yoichi NAKAYAMA <[email protected]>
'git worktree add <path> <branch>' DWIMs <branch> to a remote-tracking branch when neither -b, -B, nor --detach is given.
However, 'git worktree add -b <new-branch> <path> <branch>' can still DWIM <branch>, causing <new-branch> to be ignored.
This is a regression introduced in v2.42.0
(128e5496b325640f0a09cc1d5b1e346c069b410f).
I'll commit with the latter half of the above message rewritten like this:
However, 'git worktree add -b <new-branch> <path> <branch>' can still DWIM <branch>, causing <new-branch> to be ignored. This is a regression introduced by 128e5496b3 (worktree add: extend DWIM to infer --orphan, 2023-05-17), which appeared in Git 2.42.
As we refer to an individual commit using "log -1
--pretty=reference" format.
Quoted 3 lines
Signed-off-by Yoichi NAKAYAMA <[email protected]>---
worktree add: shouldn't dwim if -b or -B is given
The change looks good to me.
Thanks.
Quoted 84 lines
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2192%2Fyoichi%2Fworktree-add-should-not-dwim-with-b-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2192/yoichi/worktree-add-should-not-dwim-with-b-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2192Range-diff vs v2:
1:
b00f6c2fa1! 1:477ea27c8bworktree add: shouldn't dwim if -b or -B is given
@@ Metadata
## Commit message ##
worktree add: shouldn't dwim if -b or -B is given- git worktree add <path> <branch> DWIMs <branch> to a
+ 'git worktree add <path> <branch>' DWIMs <branch> to a
remote-tracking branch when neither -b, -B, nor --detach is given.- However, git worktree add -b <new-branch> <path> <branch> can
+ However, 'git worktree add -b <new-branch> <path> <branch>' can
still DWIM <branch>, causing <new-branch> to be ignored.This is a regression introduced in v2.42.0
builtin/worktree.c | 22 ++++++++++++---------- t/t2400-worktree-add.sh | 10 ++++++++++ 2 files changed, 22 insertions(+), 10 deletions(-)builtin/worktree.c +12
-10@@ -898,16 +898,18 @@ static int add(int ac, const char **av, const char *prefix,/* DWIM: Infer --orphan when repo has no refs. */ opts.orphan = (!s) &&dwim_orphan (&opts, !!opt_track,1 ); }else if (ac ==2 ) { -struct object_id oid; -struct commit *commit; -char *remote; - - commit =lookup_commit_reference_by_name (branch); -if (!commit) { - remote =unique_tracking_name (branch, &oid,NULL ); -if (remote) { - new_branch = branch; - branch = new_branch_to_free = remote; +if (!new_branch) { +struct object_id oid; +struct commit *commit; +char *remote; + + commit =lookup_commit_reference_by_name (branch); +if (!commit) { + remote =unique_tracking_name (branch, &oid,NULL ); +if (remote) { + new_branch = branch; + branch = new_branch_to_free = remote; + } } }t/t2400-worktree-add.sh +10
-0@@ -621,6 +621,16 @@ test_expect_success '"add" <path> <branch> dwims' ' ) ' +test_expect_success '"add" <path> <branch> does not dwim with -b' ' + test_when_finished rm -rf repo_upstream repo_dwim wt && + setup_remote_repo repo_upstream repo_dwim && + ( + cd repo_dwim && + test_must_fail git worktree add -b branch ../wt foo 2>actual && + test_grep "^fatal: invalid reference: foo" actual + ) +' + test_expect_success '"add" <path> <branch> dwims with checkout.defaultRemote' ' test_when_finished rm -rf repo_upstream repo_dwim foo && setup_remote_repo repo_upstream repo_dwim &&base-commit:
5b2471720c93ee30e5764a19f3d3b3ae9ec9712a
Quoted 22 lines · Yoichi NAKAYAMA via GitGitGadget
builtin/worktree.c +4
-10@@ -898,16 +898,18 @@ static int add(int ac, const char **av, const char *prefix,/* DWIM: Infer --orphan when repo has no refs. */ opts.orphan = (!s) &&dwim_orphan (&opts, !!opt_track,1 ); }else if (ac ==2 ) { -struct object_id oid; -struct commit *commit; -char *remote; - - commit =lookup_commit_reference_by_name (branch); -if (!commit) { - remote =unique_tracking_name (branch, &oid,NULL ); -if (remote) { - new_branch = branch; - branch = new_branch_to_free = remote; +if (!new_branch) { +struct object_id oid; +struct commit *commit; +char *remote;
builtin/worktree.c static int add(int ac, const char **av, const char *prefix, · line 914
Quoted 10 lines
builtin/worktree.c +8
-0+ + commit =lookup_commit_reference_by_name (branch); +if (!commit) { + remote =unique_tracking_name (branch, &oid,NULL ); +if (remote) { + new_branch = branch; + branch = new_branch_to_free = remote; + } } }
To reduce nesting and match the style of the preceding conditions, I'd like to revise this part as follows:
(I'll use the commit message modified by Junio with 'log --pretty=reference')
Quoted 9 lines
builtin/worktree.c +0
-0}else if (ac <2 && new_branch) {} else if (ac < 2) { ... } else if (ac == 2 && new_branch) { if (!strcmp(branch, "HEAD")) can_use_local_refs(&opts); } else if (ac = 2) { ...
Thanks,
Signature · 1 line
Yoichi NAKAYAMA
PATCH v4 1/1 worktree add: shouldn't dwim if -b or -B is given
In reply to Yoichi NAKAYAMA via GitGitGadgetFrom: Yoichi NAKAYAMA <[email protected]>
'git worktree add <path> <branch>' DWIMs <branch> to a remote-tracking branch when neither -b, -B, nor --detach is given.
However, 'git worktree add -b <new-branch> <path> <branch>' can still DWIM <branch>, causing <new-branch> to be ignored. This is a regression introduced by 128e5496b3 (worktree add: extend DWIM to infer --orphan, 2023-05-17), which appeared in Git 2.42.
---
worktree add: shouldn't dwim if -b or -B is given
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2192%2Fyoichi%2Fworktree-add-should-not-dwim-with-b-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2192/yoichi/worktree-add-should-not-dwim-with-b-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/2192
Range-diff vs v3:
1: 477ea27c8b ! 1: 512b037bb7 worktree add: shouldn't dwim if -b or -B is given
@@ Commit message
is given.
However, 'git worktree add -b <new-branch> <path> <branch>' can
- still DWIM <branch>, causing <new-branch> to be ignored.
-
- This is a regression introduced in v2.42.0
- (128e5496b325640f0a09cc1d5b1e346c069b410f).
+ still DWIM <branch>, causing <new-branch> to be ignored. This is a
+ regression introduced by 128e5496b3 (worktree add: extend DWIM to
+ infer --orphan, 2023-05-17), which appeared in Git 2.42.
Signed-off-by: Yoichi NAKAYAMA <[email protected]>
## builtin/worktree.c ##
@@ builtin/worktree.c: static int add(int ac, const char **av, const char *prefix,
+
/* DWIM: Infer --orphan when repo has no refs. */
opts.orphan = (!s) && dwim_orphan(&opts, !!opt_track, 1);
++ } else if (ac == 2 && new_branch) {
++ if (!strcmp(branch, "HEAD"))
++ can_use_local_refs(&opts);
} else if (ac == 2) {
-- struct object_id oid;
-- struct commit *commit;
-- char *remote;
--
-- commit = lookup_commit_reference_by_name(branch);
-- if (!commit) {
-- remote = unique_tracking_name(branch, &oid, NULL);
-- if (remote) {
-- new_branch = branch;
-- branch = new_branch_to_free = remote;
-+ if (!new_branch) {
-+ struct object_id oid;
-+ struct commit *commit;
-+ char *remote;
-+
-+ commit = lookup_commit_reference_by_name(branch);
-+ if (!commit) {
-+ remote = unique_tracking_name(branch, &oid, NULL);
-+ if (remote) {
-+ new_branch = branch;
-+ branch = new_branch_to_free = remote;
-+ }
- }
- }
-
+ struct object_id oid;
+ struct commit *commit;
## t/t2400-worktree-add.sh ##
@@ t/t2400-worktree-add.sh: test_expect_success '"add" <path> <branch> dwims' 'builtin/worktree.c | 3 +++ t/t2400-worktree-add.sh | 10 ++++++++++ 2 files changed, 13 insertions(+)
builtin/worktree.c +3 -0
@@ -897,6 +897,9 @@ static int add(int ac, const char **av, const char *prefix,/* DWIM: Infer --orphan when repo has no refs. */ opts.orphan = (!s) &&dwim_orphan (&opts, !!opt_track,1 ); + }else if (ac ==2 && new_branch) { +if (!strcmp (branch,"HEAD" )) +can_use_local_refs (&opts); }else if (ac ==2 ) {struct object_id oid;struct commit *commit;
t/t2400-worktree-add.sh +10 -0
@@ -621,6 +621,16 @@ test_expect_success '"add" <path> <branch> dwims' ' ) ' +test_expect_success '"add" <path> <branch> does not dwim with -b' ' + test_when_finished rm -rf repo_upstream repo_dwim wt && + setup_remote_repo repo_upstream repo_dwim && + ( + cd repo_dwim && + test_must_fail git worktree add -b branch ../wt foo 2>actual && + test_grep "^fatal: invalid reference: foo" actual + ) +' + test_expect_success '"add" <path> <branch> dwims with checkout.defaultRemote' ' test_when_finished rm -rf repo_upstream repo_dwim foo && setup_remote_repo repo_upstream repo_dwim &&
base-commit: dea0ea3582e6980ddbc1173cc8e3e9f9db91cde0
Signature · 1 line
gitgitgadget
Quoted 49 lines · Yoichi NAKAYAMA via GitGitGadget
@@ builtin/worktree.c: static int add(int ac, const char **av, const char *prefix, + /* DWIM: Infer --orphan when repo has no refs. */ opts.orphan = (!s) && dwim_orphan(&opts, !!opt_track, 1); ++ } else if (ac == 2 && new_branch) { ++ if (!strcmp(branch, "HEAD")) ++ can_use_local_refs(&opts); } else if (ac == 2) { -- struct object_id oid; -- struct commit *commit; -- char *remote; -- -- commit = lookup_commit_reference_by_name(branch); -- if (!commit) { -- remote = unique_tracking_name(branch, &oid, NULL); -- if (remote) { -- new_branch = branch; -- branch = new_branch_to_free = remote; -+ if (!new_branch) { -+ struct object_id oid; -+ struct commit *commit; -+ char *remote; -+ -+ commit = lookup_commit_reference_by_name(branch); -+ if (!commit) { -+ remote = unique_tracking_name(branch, &oid, NULL); -+ if (remote) { -+ new_branch = branch; -+ branch = new_branch_to_free = remote; -+ } - } - } - + struct object_id oid; + struct commit *commit; ## t/t2400-worktree-add.sh ## @@ t/t2400-worktree-add.sh: test_expect_success '"add" <path> <branch> dwims' 'builtin/worktree.c | 3 +++ t/t2400-worktree-add.sh | 10 ++++++++++ 2 files changed, 13 insertions(+)builtin/worktree.c +0
-0
builtin/worktree.c static int add(int ac, const char **av, const char *prefix, · line 905
Quoted 9 lines
builtin/worktree.c +3
-0@@ -897,6 +897,9 @@ static int add(int ac, const char **av, const char *prefix,/* DWIM: Infer --orphan when repo has no refs. */ opts.orphan = (!s) &&dwim_orphan (&opts, !!opt_track,1 ); + }else if (ac ==2 && new_branch) { +if (!strcmp (branch,"HEAD" )) +can_use_local_refs (&opts); }else if (ac ==2 ) {struct object_id oid;struct commit *commit;
Ah, that is a much smaller patch, and does not share the same issue as the very initial iteration.
Will queue. Thanks.
Quoted 23 lines
t/t2400-worktree-add.sh +10
-0@@ -621,6 +621,16 @@ test_expect_success '"add" <path> <branch> dwims' ' ) ' +test_expect_success '"add" <path> <branch> does not dwim with -b' ' + test_when_finished rm -rf repo_upstream repo_dwim wt && + setup_remote_repo repo_upstream repo_dwim && + ( + cd repo_dwim && + test_must_fail git worktree add -b branch ../wt foo 2>actual && + test_grep "^fatal: invalid reference: foo" actual + ) +' + test_expect_success '"add" <path> <branch> dwims with checkout.defaultRemote' ' test_when_finished rm -rf repo_upstream repo_dwim foo && setup_remote_repo repo_upstream repo_dwim &&base-commit:
dea0ea3582e6980ddbc1173cc8e3e9f9db91cde0