* [PATCH] checkout: don't check worktrees when not necessary @ 2015-05-31 23:16 Spencer Baugh 2015-06-01 10:23 ` Duy Nguyen 0 siblings, 1 reply; 5+ messages in thread From: Spencer Baugh @ 2015-05-31 23:16 UTC (permalink / raw) To: git; +Cc: Nguyễn Thái Ngọc Duy, Spencer Baugh When --patch or pathspecs are passed to git checkout, the working tree will not be switching branch, so there's no need to check if the branch that we are running checkout on is already checked out. Signed-off-by: Spencer Baugh <sbaugh@catern.com> --- builtin/checkout.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/checkout.c b/builtin/checkout.c index 2f92328..7039c5c 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1237,6 +1237,7 @@ static int parse_branchname_arg(int argc, const char **argv, char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag); if (head_ref && (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)) && + !(opts->patch_mode || opts->pathspec.nr) && !opts->ignore_other_worktrees) check_linked_checkouts(new); free(head_ref); -- 2.4.2.339.g77bd3ea ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] checkout: don't check worktrees when not necessary 2015-05-31 23:16 [PATCH] checkout: don't check worktrees when not necessary Spencer Baugh @ 2015-06-01 10:23 ` Duy Nguyen 2015-06-10 19:21 ` Spencer Baugh 0 siblings, 1 reply; 5+ messages in thread From: Duy Nguyen @ 2015-06-01 10:23 UTC (permalink / raw) To: Spencer Baugh; +Cc: git On Sun, May 31, 2015 at 07:16:29PM -0400, Spencer Baugh wrote: > --- a/builtin/checkout.c > +++ b/builtin/checkout.c > @@ -1237,6 +1237,7 @@ static int parse_branchname_arg(int argc, const char **argv, > char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag); > if (head_ref && > (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)) && > + !(opts->patch_mode || opts->pathspec.nr) && > !opts->ignore_other_worktrees) > check_linked_checkouts(new); > free(head_ref); Simple and effective. But if in future we add more options for non-switching-branch checkout, we need to update both places, here and near the end of cmd_checkout(). Perhaps we can move all this block inside checkout_branch() so we only need to test "opts->patch_mode || opts->pathspec.nr" once, at the end of cmd_checkout(). Something like below? I'm not opposed to your change, but if you go with it, you should cherry pick my test in the below patch. Or create a similar test. -- 8< -- diff --git a/builtin/checkout.c b/builtin/checkout.c index 2f92328..e9aee58 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1110,7 +1110,6 @@ static int parse_branchname_arg(int argc, const char **argv, { struct tree **source_tree = &opts->source_tree; const char **new_branch = &opts->new_branch; - int force_detach = opts->force_detach; int argcount = 0; unsigned char branch_rev[20]; const char *arg; @@ -1231,17 +1230,6 @@ static int parse_branchname_arg(int argc, const char **argv, else new->path = NULL; /* not an existing branch */ - if (new->path && !force_detach && !*new_branch) { - unsigned char sha1[20]; - int flag; - char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag); - if (head_ref && - (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)) && - !opts->ignore_other_worktrees) - check_linked_checkouts(new); - free(head_ref); - } - new->commit = lookup_commit_reference_gently(rev, 1); if (!new->commit) { /* not a commit */ @@ -1321,6 +1309,17 @@ static int checkout_branch(struct checkout_opts *opts, die(_("Cannot switch branch to a non-commit '%s'"), new->name); + if (new->path && !opts->force_detach && !opts->new_branch) { + unsigned char sha1[20]; + int flag; + char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag); + if (head_ref && + (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)) && + !opts->ignore_other_worktrees) + check_linked_checkouts(new); + free(head_ref); + } + if (opts->new_worktree) return prepare_linked_checkout(opts, new); diff --git a/t/t2025-checkout-to.sh b/t/t2025-checkout-to.sh index f8e4df4..a8d9336 100755 --- a/t/t2025-checkout-to.sh +++ b/t/t2025-checkout-to.sh @@ -28,6 +28,14 @@ test_expect_success 'checkout --to refuses to checkout locked branch' ' ! test -d .git/worktrees/zere ' +test_expect_success 'checking out paths not complaining about linked checkouts' ' + ( + cd existing_empty && + echo dirty >>init.t && + git checkout master -- init.t + ) +' + test_expect_success 'checkout --to a new worktree' ' git rev-parse HEAD >expect && git checkout --detach --to here master && -- 8< -- -- Duy ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] checkout: don't check worktrees when not necessary 2015-06-01 10:23 ` Duy Nguyen @ 2015-06-10 19:21 ` Spencer Baugh 2015-06-12 10:49 ` Nguyễn Thái Ngọc Duy 0 siblings, 1 reply; 5+ messages in thread From: Spencer Baugh @ 2015-06-10 19:21 UTC (permalink / raw) To: Duy Nguyen, Spencer Baugh; +Cc: git Duy Nguyen <pclouds@gmail.com> writes: > On Sun, May 31, 2015 at 07:16:29PM -0400, Spencer Baugh wrote: >> --- a/builtin/checkout.c >> +++ b/builtin/checkout.c >> @@ -1237,6 +1237,7 @@ static int parse_branchname_arg(int argc, const char **argv, >> char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag); >> if (head_ref && >> (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)) && >> + !(opts->patch_mode || opts->pathspec.nr) && >> !opts->ignore_other_worktrees) >> check_linked_checkouts(new); >> free(head_ref); > > Simple and effective. But if in future we add more options for > non-switching-branch checkout, we need to update both places, here and > near the end of cmd_checkout(). > > Perhaps we can move all this block inside checkout_branch() so we only > need to test "opts->patch_mode || opts->pathspec.nr" once, at the end > of cmd_checkout(). Something like below? > > I'm not opposed to your change, but if you go with it, you should > cherry pick my test in the below patch. Or create a similar test. Sorry for late reply, but I think your change is much better than mine so I'd suggest just using that instead. ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] checkout: don't check worktrees when not necessary 2015-06-10 19:21 ` Spencer Baugh @ 2015-06-12 10:49 ` Nguyễn Thái Ngọc Duy 2015-06-12 22:24 ` Junio C Hamano 0 siblings, 1 reply; 5+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2015-06-12 10:49 UTC (permalink / raw) To: git; +Cc: catern, Junio C Hamano, Nguyễn Thái Ngọc Duy When --patch or pathspecs are passed to git checkout, the working tree will not be switching branch, so there's no need to check if the branch that we are running checkout on is already checked out. Original-patch-by: Spencer Baugh <sbaugh@catern.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- builtin/checkout.c | 23 +++++++++++------------ t/t2025-checkout-to.sh | 8 ++++++++ 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 9b49f0e..e227f64 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1110,7 +1110,6 @@ static int parse_branchname_arg(int argc, const char **argv, { struct tree **source_tree = &opts->source_tree; const char **new_branch = &opts->new_branch; - int force_detach = opts->force_detach; int argcount = 0; unsigned char branch_rev[20]; const char *arg; @@ -1231,17 +1230,6 @@ static int parse_branchname_arg(int argc, const char **argv, else new->path = NULL; /* not an existing branch */ - if (new->path && !force_detach && !*new_branch) { - unsigned char sha1[20]; - int flag; - char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag); - if (head_ref && - (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)) && - !opts->ignore_other_worktrees) - check_linked_checkouts(new); - free(head_ref); - } - new->commit = lookup_commit_reference_gently(rev, 1); if (!new->commit) { /* not a commit */ @@ -1321,6 +1309,17 @@ static int checkout_branch(struct checkout_opts *opts, die(_("Cannot switch branch to a non-commit '%s'"), new->name); + if (new->path && !opts->force_detach && !opts->new_branch) { + unsigned char sha1[20]; + int flag; + char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag); + if (head_ref && + (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)) && + !opts->ignore_other_worktrees) + check_linked_checkouts(new); + free(head_ref); + } + if (opts->new_worktree) return prepare_linked_checkout(opts, new); diff --git a/t/t2025-checkout-to.sh b/t/t2025-checkout-to.sh index f8e4df4..a8d9336 100755 --- a/t/t2025-checkout-to.sh +++ b/t/t2025-checkout-to.sh @@ -28,6 +28,14 @@ test_expect_success 'checkout --to refuses to checkout locked branch' ' ! test -d .git/worktrees/zere ' +test_expect_success 'checking out paths not complaining about linked checkouts' ' + ( + cd existing_empty && + echo dirty >>init.t && + git checkout master -- init.t + ) +' + test_expect_success 'checkout --to a new worktree' ' git rev-parse HEAD >expect && git checkout --detach --to here master && -- 2.3.0.rc1.137.g477eb31 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] checkout: don't check worktrees when not necessary 2015-06-12 10:49 ` Nguyễn Thái Ngọc Duy @ 2015-06-12 22:24 ` Junio C Hamano 0 siblings, 0 replies; 5+ messages in thread From: Junio C Hamano @ 2015-06-12 22:24 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy; +Cc: git, catern Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes: > When --patch or pathspecs are passed to git checkout, the working tree > will not be switching branch, so there's no need to check if the branch > that we are running checkout on is already checked out. Yeah, I agree that having this check in parse_branchname_arg() does not make any sense, as that function is still trying to decide if we are switching to a different branch or not. The new location looks much more sensible. Will queue this on nd/multiple-work-trees (which has been in 'master' for about a month). Good thing that we caught it before it got in any tagged release. Thanks. > > Original-patch-by: Spencer Baugh <sbaugh@catern.com> > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> > --- > builtin/checkout.c | 23 +++++++++++------------ > t/t2025-checkout-to.sh | 8 ++++++++ > 2 files changed, 19 insertions(+), 12 deletions(-) > > diff --git a/builtin/checkout.c b/builtin/checkout.c > index 9b49f0e..e227f64 100644 > --- a/builtin/checkout.c > +++ b/builtin/checkout.c > @@ -1110,7 +1110,6 @@ static int parse_branchname_arg(int argc, const char **argv, > { > struct tree **source_tree = &opts->source_tree; > const char **new_branch = &opts->new_branch; > - int force_detach = opts->force_detach; > int argcount = 0; > unsigned char branch_rev[20]; > const char *arg; > @@ -1231,17 +1230,6 @@ static int parse_branchname_arg(int argc, const char **argv, > else > new->path = NULL; /* not an existing branch */ > > - if (new->path && !force_detach && !*new_branch) { > - unsigned char sha1[20]; > - int flag; > - char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag); > - if (head_ref && > - (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)) && > - !opts->ignore_other_worktrees) > - check_linked_checkouts(new); > - free(head_ref); > - } > - > new->commit = lookup_commit_reference_gently(rev, 1); > if (!new->commit) { > /* not a commit */ > @@ -1321,6 +1309,17 @@ static int checkout_branch(struct checkout_opts *opts, > die(_("Cannot switch branch to a non-commit '%s'"), > new->name); > > + if (new->path && !opts->force_detach && !opts->new_branch) { > + unsigned char sha1[20]; > + int flag; > + char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag); > + if (head_ref && > + (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)) && > + !opts->ignore_other_worktrees) > + check_linked_checkouts(new); > + free(head_ref); > + } > + > if (opts->new_worktree) > return prepare_linked_checkout(opts, new); > > diff --git a/t/t2025-checkout-to.sh b/t/t2025-checkout-to.sh > index f8e4df4..a8d9336 100755 > --- a/t/t2025-checkout-to.sh > +++ b/t/t2025-checkout-to.sh > @@ -28,6 +28,14 @@ test_expect_success 'checkout --to refuses to checkout locked branch' ' > ! test -d .git/worktrees/zere > ' > > +test_expect_success 'checking out paths not complaining about linked checkouts' ' > + ( > + cd existing_empty && > + echo dirty >>init.t && > + git checkout master -- init.t > + ) > +' > + > test_expect_success 'checkout --to a new worktree' ' > git rev-parse HEAD >expect && > git checkout --detach --to here master && ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-06-12 22:24 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-05-31 23:16 [PATCH] checkout: don't check worktrees when not necessary Spencer Baugh 2015-06-01 10:23 ` Duy Nguyen 2015-06-10 19:21 ` Spencer Baugh 2015-06-12 10:49 ` Nguyễn Thái Ngọc Duy 2015-06-12 22:24 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).