[PATCH 0/2] checkout: introduce "--to-branch" option

[PATCH 0/2] checkout: introduce "--to-branch" option

[ZheNing Hu via GitGitGadget]

Sometimes people expect to checkout to a commit (or a commit which tag
reference to) and develop on its corresponding branch. This may take some
steps:

$ git checkout v1.1
$ git log  # search for the branch dev1
$ git checkout dev1


collapse the above steps into one step:

$ git checkout --to-branch v1.1
Switched to branch 'refs/heads/dev1'


This will help users switch to the unqiue branch quickly.

Thanks.

ZheNing Hu (2):
  checkout: handling branch_info memory leak
  checkout: introduce "--to-branch" option

 Documentation/git-checkout.txt |  8 +++-
 builtin/checkout.c             | 49 +++++++++++++++++++-
 t/t2018-checkout-branch.sh     | 85 ++++++++++++++++++++++++++++++++++
 t/t9902-completion.sh          |  1 +
 4 files changed, 140 insertions(+), 3 deletions(-)


base-commit: abe6bb3905392d5eb6b01fa6e54d7e784e0522aa
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1095%2Fadlternative%2Fcheckout-w-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1095/adlternative/checkout-w-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1095
-- 
gitgitgadget

[PATCH 1/2] checkout: handling branch_info memory leak

[ZheNing Hu via GitGitGadget]

From: ZheNing Hu <adlternative@gmail.com>

On the checkout code path, we dynamically allocate
memory for `refname` and `path` in `struct branch_info`,
but did not free their memory at the end. So introduce
branch_info_clear() which can free branch_info memory.

Signed-off-by: ZheNing Hu <adlternative@gmail.com>
---
 builtin/checkout.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index cbf73b8c9f6..3eeac3147f2 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -103,6 +103,15 @@ struct branch_info {
 	char *checkout;
 };
 
+static void branch_info_clear(struct branch_info *branch_info) {
+	if (branch_info->path) {
+		free((char *)branch_info->path);
+		branch_info->path = NULL;
+	}
+	if (branch_info->refname)
+		FREE_AND_NULL(branch_info->refname);
+}
+
 static int post_checkout_hook(struct commit *old_commit, struct commit *new_commit,
 			      int changed)
 {
@@ -1578,6 +1587,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
 {
 	struct branch_info new_branch_info;
 	int parseopt_flags = 0;
+	int ret = 0;
 
 	memset(&new_branch_info, 0, sizeof(new_branch_info));
 	opts->overwrite_ignore = 1;
@@ -1768,9 +1778,11 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
 
 	UNLEAK(opts);
 	if (opts->patch_mode || opts->pathspec.nr)
-		return checkout_paths(opts, &new_branch_info);
+		ret = checkout_paths(opts, &new_branch_info);
 	else
-		return checkout_branch(opts, &new_branch_info);
+		ret = checkout_branch(opts, &new_branch_info);
+	branch_info_clear(&new_branch_info);
+	return ret;
 }
 
 int cmd_checkout(int argc, const char **argv, const char *prefix)
-- 
gitgitgadget

[PATCH 2/2] checkout: introduce "--to-branch" option

[ZheNing Hu via GitGitGadget]

From: ZheNing Hu <adlternative@gmail.com>

When we want checkout to a branch (e.g. dev1) which reference
to a commit, but sometimes we only remember the tag (e.g. v1.1)
on it, we will use `git checkout v1.1` to find the commit first,
git will be in the state of deatching HEAD, so we have to search the
branches on the commit and checkout the branch we perfer. This will
be a bit cumbersome.

Introduce "--to-branch" option, `git checkout --to-branch <tag>`
and `git checkout --to-branch <commit>` will search all branches
and find a unique branch reference to the commit (or the commit which
the tag reference to) and checkout to it. If the commit have more
than one branches, it will report error "here are more than one
branch on commit".

Signed-off-by: ZheNing Hu <adlternative@gmail.com>
---
 Documentation/git-checkout.txt |  8 +++-
 builtin/checkout.c             | 33 +++++++++++++
 t/t2018-checkout-branch.sh     | 85 ++++++++++++++++++++++++++++++++++
 t/t9902-completion.sh          |  1 +
 4 files changed, 126 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index d473c9bf387..2a240699fd9 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 [verse]
 'git checkout' [-q] [-f] [-m] [<branch>]
 'git checkout' [-q] [-f] [-m] --detach [<branch>]
-'git checkout' [-q] [-f] [-m] [--detach] <commit>
+'git checkout' [-q] [-f] [-m] [--detach] [-w|--to-branch] <commit>
 'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] <new_branch>] [<start_point>]
 'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <pathspec>...
 'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] --pathspec-from-file=<file> [--pathspec-file-nul]
@@ -210,6 +210,12 @@ variable.
 	`<commit>` is not a branch name.  See the "DETACHED HEAD" section
 	below for details.
 
+-w::
+--to-branch::
+	Rather than checking out a commit to work on it, checkout out
+	to the unique branch on it. If there are multiple branches on
+	the commit, the checkout will fail.
+
 --orphan <new_branch>::
 	Create a new 'orphan' branch, named `<new_branch>`, started from
 	`<start_point>` and switch to it.  The first commit made on this
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 3eeac3147f2..696248b05c0 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -28,6 +28,7 @@
 #include "xdiff-interface.h"
 #include "entry.h"
 #include "parallel-checkout.h"
+#include "../ref-filter.h"
 
 static const char * const checkout_usage[] = {
 	N_("git checkout [<options>] <branch>"),
@@ -70,6 +71,7 @@ struct checkout_opts {
 	int empty_pathspec_ok;
 	int checkout_index;
 	int checkout_worktree;
+	int to_branch;
 	const char *ignore_unmerged_opt;
 	int ignore_unmerged;
 	int pathspec_file_nul;
@@ -1512,6 +1514,35 @@ static int checkout_branch(struct checkout_opts *opts,
 		    (flag & REF_ISSYMREF) && is_null_oid(&rev))
 			return switch_unborn_to_new_branch(opts);
 	}
+	if (opts->to_branch) {
+		struct ref_filter filter;
+		struct ref_array array;
+		int i;
+		int count = 0;
+		const char *unused_pattern = NULL;
+
+		memset(&array, 0, sizeof(array));
+		memset(&filter, 0, sizeof(filter));
+		filter.kind = FILTER_REFS_BRANCHES;
+		filter.name_patterns = &unused_pattern;
+		filter_refs(&array, &filter, filter.kind);
+		for (i = 0; i < array.nr; i++) {
+			if (oideq(&array.items[i]->objectname, &new_branch_info->oid)) {
+				if (count)
+					die(_("here are more than one branch on commit %s"), oid_to_hex(&new_branch_info->oid));
+				count++;
+				if (new_branch_info->refname)
+					free((char *)new_branch_info->refname);
+				new_branch_info->refname = xstrdup(array.items[i]->refname);
+				if (new_branch_info->path)
+					free((char *)new_branch_info->path);
+				new_branch_info->path = xstrdup(array.items[i]->refname);
+				new_branch_info->name = new_branch_info->path;
+			}
+		}
+		ref_array_clear(&array);
+	}
+
 	return switch_branches(opts, new_branch_info);
 }
 
@@ -1797,6 +1828,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT_BOOL('l', NULL, &opts.new_branch_log, N_("create reflog for new branch")),
 		OPT_BOOL(0, "guess", &opts.dwim_new_local_branch,
 			 N_("second guess 'git checkout <no-such-branch>' (default)")),
+		OPT_BOOL('w', "to-branch", &opts.to_branch,
+			 N_("checkout to a branch from a commit or a tag")),
 		OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode (default)")),
 		OPT_END()
 	};
diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index 93be1c0eae5..53e45cfe7fd 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -270,4 +270,89 @@ test_expect_success 'checkout -b rejects an extra path argument' '
 	test_i18ngrep "Cannot update paths and switch to branch" err
 '
 
+test_expect_success 'checkout -w with oid' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		git branch -M main &&
+		test_commit initial file1 &&
+		HEAD1=$(git rev-parse --verify HEAD) &&
+		git switch -c dev &&
+		test_commit step2 file2 &&
+		HEAD2=$(git rev-parse --verify HEAD) &&
+
+		git checkout -w $HEAD1 &&
+
+		echo "refs/heads/main" >ref.expect &&
+		git rev-parse --symbolic-full-name HEAD >ref.actual &&
+		test_cmp ref.expect ref.actual &&
+
+		echo "$HEAD1" >oid.expect &&
+		git rev-parse --verify HEAD >oid.actual &&
+		test_cmp oid.expect oid.actual &&
+
+		git checkout -w $HEAD2 &&
+
+		echo "refs/heads/dev" >ref.expect &&
+		git rev-parse --symbolic-full-name HEAD >ref.actual &&
+		test_cmp ref.expect ref.actual &&
+
+		echo "$HEAD2" >oid.expect &&
+		git rev-parse --verify HEAD >oid.actual &&
+		test_cmp oid.expect oid.actual
+	)
+'
+
+test_expect_success 'checkout -w with tag' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		git branch -M main &&
+		test_commit initial file1 &&
+		git tag v1 &&
+		HEAD1=$(git rev-parse --verify HEAD) &&
+		git switch -c dev &&
+		test_commit step2 file2 &&
+		git tag v2 &&
+		HEAD2=$(git rev-parse --verify HEAD) &&
+
+		git checkout -w v1 &&
+
+		echo "refs/heads/main" >ref.expect &&
+		git rev-parse --symbolic-full-name HEAD >ref.actual &&
+		test_cmp ref.expect ref.actual &&
+
+		echo "$HEAD1" >oid.expect &&
+		git rev-parse --verify HEAD >oid.actual &&
+		test_cmp oid.expect oid.actual &&
+
+		git checkout -w v2 &&
+
+		echo "refs/heads/dev" >ref.expect &&
+		git rev-parse --symbolic-full-name HEAD >ref.actual &&
+		test_cmp ref.expect ref.actual &&
+
+		echo "$HEAD2" >oid.expect &&
+		git rev-parse --verify HEAD >oid.actual &&
+		test_cmp oid.expect oid.actual
+	)
+'
+
+test_expect_success 'checkout -w with branches' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		git branch -M main &&
+		test_commit initial file1 &&
+		HEAD1=$(git rev-parse --verify HEAD) &&
+		git tag v1 &&
+		git branch dev &&
+		test_must_fail git checkout -w $HEAD1 &&
+		test_must_fail git checkout -w dev
+	)
+'
+
 test_done
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 518203fbe07..4ec134338c2 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -2021,6 +2021,7 @@ test_expect_success 'double dash "git checkout"' '
 	--orphan=Z
 	--ours Z
 	--theirs Z
+	--to-branch Z
 	--merge Z
 	--conflict=Z
 	--patch Z
-- 
gitgitgadget

Re: [PATCH 1/2] checkout: handling branch_info memory leak

[Ævar Arnfjörð Bjarmason]

On Fri, Dec 10 2021, ZheNing Hu via GitGitGadget wrote:

> From: ZheNing Hu <adlternative@gmail.com>
>
> On the checkout code path, we dynamically allocate
> memory for `refname` and `path` in `struct branch_info`,
> but did not free their memory at the end. So introduce
> branch_info_clear() which can free branch_info memory.

This (partially) duplicates my 9081a421a6d (checkout: fix "branch info"
memory leaks, 2021-11-16) which is in "next" already.

Re: [PATCH 2/2] checkout: introduce "--to-branch" option

[Christian Couder]

On Fri, Dec 10, 2021 at 7:22 AM ZheNing Hu via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: ZheNing Hu <adlternative@gmail.com>
>
> When we want checkout to a branch (e.g. dev1) which reference
> to a commit, but sometimes we only remember the tag (e.g. v1.1)
> on it, we will use `git checkout v1.1` to find the commit first,
> git will be in the state of deatching HEAD, so we have to search the

s/deatching/detached/

It's possible, and likely faster, to use something like `git log -1
v1.1` or `git show <sha>` to see which branches point to the tag or
commit, as the branches are added as decoration to the output.

> branches on the commit and checkout the branch we perfer. This will

s/perfer/prefer/

> be a bit cumbersome.
>
> Introduce "--to-branch" option, `git checkout --to-branch <tag>`
> and `git checkout --to-branch <commit>` will search all branches
> and find a unique branch reference to the commit (or the commit which
> the tag reference to) and checkout to it. If the commit have more

s/have/has/

> than one branches, it will report error "here are more than one

s/branches/branch/

> branch on commit".

Maybe reporting something like "more than one branch point to commit
XXX" would be a bit better.

> Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> ---
>  Documentation/git-checkout.txt |  8 +++-
>  builtin/checkout.c             | 33 +++++++++++++
>  t/t2018-checkout-branch.sh     | 85 ++++++++++++++++++++++++++++++++++
>  t/t9902-completion.sh          |  1 +
>  4 files changed, 126 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
> index d473c9bf387..2a240699fd9 100644
> --- a/Documentation/git-checkout.txt
> +++ b/Documentation/git-checkout.txt
> @@ -10,7 +10,7 @@ SYNOPSIS
>  [verse]
>  'git checkout' [-q] [-f] [-m] [<branch>]
>  'git checkout' [-q] [-f] [-m] --detach [<branch>]
> -'git checkout' [-q] [-f] [-m] [--detach] <commit>
> +'git checkout' [-q] [-f] [-m] [--detach] [-w|--to-branch] <commit>

It's a bit strange that --detach can be used along with the new
option, as the purpose of the new option is to not detach. It makes
one wonder what happens when both --detach and --to-branch are used.

I wonder if all the following lines:

      git checkout [-q] [-f] [-m] [<branch>]
      git checkout [-q] [-f] [-m] --detach [<branch>]
      git checkout [-q] [-f] [-m] [--detach] <commit>

could be replaced with just:

      git checkout [-q] [-f] [-m] [--detach|--to-branch] <commitish>

>  'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] <new_branch>] [<start_point>]
>  'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <pathspec>...
>  'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] --pathspec-from-file=<file> [--pathspec-file-nul]
> @@ -210,6 +210,12 @@ variable.
>         `<commit>` is not a branch name.  See the "DETACHED HEAD" section
>         below for details.
>
> +-w::
> +--to-branch::

Using a short option name like "-w" might not be a good idea at this
point. Maybe if many people use the long option a lot, they will want
a short option name, but we can add it then instead of using one of
the few left right now.

> +       Rather than checking out a commit to work on it, checkout out

"checking out a commit to work on it" might not describe well that it
works when we pass a tag too and that we checkout the underlying
commit in the detached HEAD mode by default.

> +       to the unique branch on it. If there are multiple branches on
> +       the commit, the checkout will fail.

It might be a bit better to say that a branch "points to" a commit,
rather than that it is "on" a commit.

>  --orphan <new_branch>::
>         Create a new 'orphan' branch, named `<new_branch>`, started from
>         `<start_point>` and switch to it.  The first commit made on this
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 3eeac3147f2..696248b05c0 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -28,6 +28,7 @@
>  #include "xdiff-interface.h"
>  #include "entry.h"
>  #include "parallel-checkout.h"
> +#include "../ref-filter.h"

Are you sure that "../" is needed above?

>  static const char * const checkout_usage[] = {
>         N_("git checkout [<options>] <branch>"),
> @@ -70,6 +71,7 @@ struct checkout_opts {
>         int empty_pathspec_ok;
>         int checkout_index;
>         int checkout_worktree;
> +       int to_branch;
>         const char *ignore_unmerged_opt;
>         int ignore_unmerged;
>         int pathspec_file_nul;
> @@ -1512,6 +1514,35 @@ static int checkout_branch(struct checkout_opts *opts,
>                     (flag & REF_ISSYMREF) && is_null_oid(&rev))
>                         return switch_unborn_to_new_branch(opts);
>         }
> +       if (opts->to_branch) {
> +               struct ref_filter filter;
> +               struct ref_array array;
> +               int i;
> +               int count = 0;
> +               const char *unused_pattern = NULL;
> +
> +               memset(&array, 0, sizeof(array));
> +               memset(&filter, 0, sizeof(filter));
> +               filter.kind = FILTER_REFS_BRANCHES;
> +               filter.name_patterns = &unused_pattern;
> +               filter_refs(&array, &filter, filter.kind);
> +               for (i = 0; i < array.nr; i++) {
> +                       if (oideq(&array.items[i]->objectname, &new_branch_info->oid)) {
> +                               if (count)
> +                                       die(_("here are more than one branch on commit %s"), oid_to_hex(&new_branch_info->oid));
> +                               count++;
> +                               if (new_branch_info->refname)
> +                                       free((char *)new_branch_info->refname);
> +                               new_branch_info->refname = xstrdup(array.items[i]->refname);
> +                               if (new_branch_info->path)
> +                                       free((char *)new_branch_info->path);
> +                               new_branch_info->path = xstrdup(array.items[i]->refname);
> +                               new_branch_info->name = new_branch_info->path;
> +                       }
> +               }
> +               ref_array_clear(&array);

It might be my personal taste, but I would find it cleaner and easier
to understand if a separate function to find the branch we are looking
for was called, instead of adding all the code here.

> +       }
> +
>         return switch_branches(opts, new_branch_info);
>  }
>
> @@ -1797,6 +1828,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>                 OPT_BOOL('l', NULL, &opts.new_branch_log, N_("create reflog for new branch")),
>                 OPT_BOOL(0, "guess", &opts.dwim_new_local_branch,
>                          N_("second guess 'git checkout <no-such-branch>' (default)")),
> +               OPT_BOOL('w', "to-branch", &opts.to_branch,
> +                        N_("checkout to a branch from a commit or a tag")),
>                 OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode (default)")),
>                 OPT_END()
>         };
> diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
> index 93be1c0eae5..53e45cfe7fd 100755
> --- a/t/t2018-checkout-branch.sh
> +++ b/t/t2018-checkout-branch.sh

I plan to look at the tests after we decide how the new option relates
to --detach.

Thanks!

Re: [PATCH 2/2] checkout: introduce "--to-branch" option

[Ævar Arnfjörð Bjarmason]

On Fri, Dec 10 2021, ZheNing Hu via GitGitGadget wrote:

> From: ZheNing Hu <adlternative@gmail.com>
>
> When we want checkout to a branch (e.g. dev1) which reference
> to a commit, but sometimes we only remember the tag (e.g. v1.1)
> on it, we will use `git checkout v1.1` to find the commit first,
> git will be in the state of deatching HEAD, so we have to search the
> branches on the commit and checkout the branch we perfer. This will
> be a bit cumbersome.
>
> Introduce "--to-branch" option, `git checkout --to-branch <tag>`
> and `git checkout --to-branch <commit>` will search all branches
> and find a unique branch reference to the commit (or the commit which
> the tag reference to) and checkout to it. If the commit have more
> than one branches, it will report error "here are more than one
> branch on commit".
>
> Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> ---
>  Documentation/git-checkout.txt |  8 +++-
>  builtin/checkout.c             | 33 +++++++++++++
>  t/t2018-checkout-branch.sh     | 85 ++++++++++++++++++++++++++++++++++
>  t/t9902-completion.sh          |  1 +
>  4 files changed, 126 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
> index d473c9bf387..2a240699fd9 100644
> --- a/Documentation/git-checkout.txt
> +++ b/Documentation/git-checkout.txt
> @@ -10,7 +10,7 @@ SYNOPSIS
>  [verse]
>  'git checkout' [-q] [-f] [-m] [<branch>]
>  'git checkout' [-q] [-f] [-m] --detach [<branch>]
> -'git checkout' [-q] [-f] [-m] [--detach] <commit>
> +'git checkout' [-q] [-f] [-m] [--detach] [-w|--to-branch] <commit>
>  'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] <new_branch>] [<start_point>]
>  'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <pathspec>...
>  'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] --pathspec-from-file=<file> [--pathspec-file-nul]
> @@ -210,6 +210,12 @@ variable.
>  	`<commit>` is not a branch name.  See the "DETACHED HEAD" section
>  	below for details.
>  
> +-w::
> +--to-branch::
> +	Rather than checking out a commit to work on it, checkout out
> +	to the unique branch on it. If there are multiple branches on
> +	the commit, the checkout will fail.
> +

So basically what this option implements is something that could be done
as a shellscript of:
	
	git_checkout_branch_from_oid () {
		rev=$1
		git for-each-ref --format='%(refname:strip=2)' --points-at $rev >/tmp/found
		if test $(wc -l </tmp/found) -ne 1
	        then
			echo "Goldilocks error in finding $rev: $(cat /tmp/found)"
	                return 1
		fi
	        git checkout $found
	}

Which is not to say that it isn't useful, but that I think adding this
to "git checkout" specifically is adding this to the wrong level. Isn't
this useful to most things that parse revisions? I.e. wouldn't a better
interface be via the peel syntax?

    oid=$(git rev-parse HEAD)
    git checkout $oid^{tobranch}

Doing it that way would allow any arbitrary command that takes revisions
now access to that, and we could have e.g. "^{tobranches}" too, so you
could do:

    git for-each-ref --format='%(refname:strip=2)' $oid^{tobranches}

Or:

    git log $oid^{tobranches}

I think implementing that is a bit harder. It's peel_onion() in
object-name.c. I think parse_branchname_arg() via get_oid_mb() is now
only capable of filling in an OID for a given name, and then checking
out that name comes as a separate step, and you can't just return
e.g. "master".

But I don't think anything stops us from adjusting those functions a bit
so that get_oid_with_context(() and friends could pass down say an
optional "struct string_list *", and the "peel" could then be expanded
to that.

Similar to how we have "git chekout -", and the "-" is understood by
some commands, but not all (via some opt-in whose location I forget...).

Re: [PATCH 2/2] checkout: introduce "--to-branch" option

[Bagas Sanjaya]

On 10/12/21 13.22, ZheNing Hu via GitGitGadget wrote:
> +-w::
> +--to-branch::
> +	Rather than checking out a commit to work on it, checkout out
> +	to the unique branch on it. If there are multiple branches on
> +	the commit, the checkout will fail.
> +

Did you mean unique branch that contains the commit?

-- 
An old man doll... just what I always wanted! - Clara

Re: [PATCH 2/2] checkout: introduce "--to-branch" option

[Junio C Hamano]

"ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: ZheNing Hu <adlternative@gmail.com>
>
> When we want checkout to a branch (e.g. dev1) which reference
> to a commit, but sometimes we only remember the tag (e.g. v1.1)
> on it, we will use `git checkout v1.1` to find the commit first,
> git will be in the state of deatching HEAD, so we have to search the
> branches on the commit and checkout the branch we perfer. This will
> be a bit cumbersome.
>
> Introduce "--to-branch" option, `git checkout --to-branch <tag>`
> and `git checkout --to-branch <commit>` will search all branches
> and find a unique branch reference to the commit (or the commit which
> the tag reference to) and checkout to it. If the commit have more
> than one branches, it will report error "here are more than one
> branch on commit".

Sorry, but the above explanation does not make any sense to me.  

It is unclear if you mean "dev1" exactly point at the commit tagged
as v1.1, or you want the branch "dev1" that is a descedanant of
v1.1.  Without telling that to the reader, the above explanation is
useless.

And whether you meant the former or the latter, neither use case does
not make much sense.

First, suppose you meant "checkout --to-branch v1.1" to find a
branch whose tip exactly points at v1.1.  You instead check out
"dev1" branch, and work on it to advance its history.  When you are
done, you may go to another branch and work on something else.

But then what?  When you need another topic that also needs to be
later merge-able to v1.1, "checkout --to-branch v1.1" no longer will
be able to find "dev1", because, well, you have already used it to
build something else.

So, "--to-branch v1.1" that finds and checks out a branch whose tip
exactly points at v1.1 would be pretty useless.

So let's correct the unwritten assumption and say "--to-branch v1.1"
finds a branch that is descendant of the tag.  It is like I have
maint-2.33 branch to prepare for v2.33.1, v2.33.2,... maintenance
releases and being able to find maint-2.33 by saying v2.33.2 (or
v2.33.1) _might_ be convenient.

But that would only be true if there is only one single branch per
family of tags (in the above example, v2.33.* tags).  You cannot use
the workflow where many topic branches run in parallel, and get
merged to the integration branch(es) only after they are ready,
because you need bugfix-1-for-v2.33, bugfix-2-for-v2.33,... branches
all forked from v2.33.0 (or a commit with a later tag in the v2.33.*
family) to cook these independent fixes that are destined for the
maint-2.33 integration branch, so you cannot uniquely find maint-2.33
by saying v2.33.0 or v2.33.1 or whatever.

I also sense that the first paragraph of the proposed log message
for this commit hints that the user needs a bit more studying of
existing tools.  When we know v1.1 but do not know if we already
have branches that are based on it, we DO NOT do "git checkout v1.1".
Instead the first thing we would do is "git branch --contains v1.1"
(add "--no-merged main" to exclude the branches that have already
graduated to 'main').

So, for this partcular topic, what I would recommend is *not* jump
in and add a new feature, but to study what's available and build a
workflow around the existing features.

Re: [PATCH 2/2] checkout: introduce "--to-branch" option

[ZheNing Hu]

Christian Couder <christian.couder@gmail.com> 于2021年12月10日周五 16:34写道：

> >  [verse]
> >  'git checkout' [-q] [-f] [-m] [<branch>]
> >  'git checkout' [-q] [-f] [-m] --detach [<branch>]
> > -'git checkout' [-q] [-f] [-m] [--detach] <commit>
> > +'git checkout' [-q] [-f] [-m] [--detach] [-w|--to-branch] <commit>
>
> It's a bit strange that --detach can be used along with the new
> option, as the purpose of the new option is to not detach. It makes
> one wonder what happens when both --detach and --to-branch are used.
>

When both --detach and --to-branch are used, --detach will work...
Of course, it should be reasonable to prevent them from being used at the
same time.

> I wonder if all the following lines:
>
>       git checkout [-q] [-f] [-m] [<branch>]
>       git checkout [-q] [-f] [-m] --detach [<branch>]
>       git checkout [-q] [-f] [-m] [--detach] <commit>
>
> could be replaced with just:
>
>       git checkout [-q] [-f] [-m] [--detach|--to-branch] <commitish>
>

Well, it will be much more concise.

> >  'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] <new_branch>] [<start_point>]
> >  'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <pathspec>...
> >  'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] --pathspec-from-file=<file> [--pathspec-file-nul]
> > @@ -210,6 +210,12 @@ variable.
> >         `<commit>` is not a branch name.  See the "DETACHED HEAD" section
> >         below for details.
> >
> > +-w::
> > +--to-branch::
>
> Using a short option name like "-w" might not be a good idea at this
> point. Maybe if many people use the long option a lot, they will want
> a short option name, but we can add it then instead of using one of
> the few left right now.
>

Makes sense.

> > +       Rather than checking out a commit to work on it, checkout out
>
> "checking out a commit to work on it" might not describe well that it
> works when we pass a tag too and that we checkout the underlying
> commit in the detached HEAD mode by default.
>
> > +       to the unique branch on it. If there are multiple branches on
> > +       the commit, the checkout will fail.
>
> It might be a bit better to say that a branch "points to" a commit,
> rather than that it is "on" a commit.
>

OK.

> >  static const char * const checkout_usage[] = {
> >         N_("git checkout [<options>] <branch>"),
> > @@ -70,6 +71,7 @@ struct checkout_opts {
> >         int empty_pathspec_ok;
> >         int checkout_index;
> >         int checkout_worktree;
> > +       int to_branch;
> >         const char *ignore_unmerged_opt;
> >         int ignore_unmerged;
> >         int pathspec_file_nul;
> > @@ -1512,6 +1514,35 @@ static int checkout_branch(struct checkout_opts *opts,
> >                     (flag & REF_ISSYMREF) && is_null_oid(&rev))
> >                         return switch_unborn_to_new_branch(opts);
> >         }
> > +       if (opts->to_branch) {
> > +               struct ref_filter filter;
> > +               struct ref_array array;
> > +               int i;
> > +               int count = 0;
> > +               const char *unused_pattern = NULL;
> > +
> > +               memset(&array, 0, sizeof(array));
> > +               memset(&filter, 0, sizeof(filter));
> > +               filter.kind = FILTER_REFS_BRANCHES;
> > +               filter.name_patterns = &unused_pattern;
> > +               filter_refs(&array, &filter, filter.kind);
> > +               for (i = 0; i < array.nr; i++) {
> > +                       if (oideq(&array.items[i]->objectname, &new_branch_info->oid)) {
> > +                               if (count)
> > +                                       die(_("here are more than one branch on commit %s"), oid_to_hex(&new_branch_info->oid));
> > +                               count++;
> > +                               if (new_branch_info->refname)
> > +                                       free((char *)new_branch_info->refname);
> > +                               new_branch_info->refname = xstrdup(array.items[i]->refname);
> > +                               if (new_branch_info->path)
> > +                                       free((char *)new_branch_info->path);
> > +                               new_branch_info->path = xstrdup(array.items[i]->refname);
> > +                               new_branch_info->name = new_branch_info->path;
> > +                       }
> > +               }
> > +               ref_array_clear(&array);
>
> It might be my personal taste, but I would find it cleaner and easier
> to understand if a separate function to find the branch we are looking
> for was called, instead of adding all the code here.
>

You are right.

> > +       }
> > +
> >         return switch_branches(opts, new_branch_info);
> >  }
> >
> > @@ -1797,6 +1828,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
> >                 OPT_BOOL('l', NULL, &opts.new_branch_log, N_("create reflog for new branch")),
> >                 OPT_BOOL(0, "guess", &opts.dwim_new_local_branch,
> >                          N_("second guess 'git checkout <no-such-branch>' (default)")),
> > +               OPT_BOOL('w', "to-branch", &opts.to_branch,
> > +                        N_("checkout to a branch from a commit or a tag")),
> >                 OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode (default)")),
> >                 OPT_END()
> >         };
> > diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
> > index 93be1c0eae5..53e45cfe7fd 100755
> > --- a/t/t2018-checkout-branch.sh
> > +++ b/t/t2018-checkout-branch.sh
>
> I plan to look at the tests after we decide how the new option relates
> to --detach.
>
> Thanks!

Thanks.
--
ZheNing Hu

Re: [PATCH 2/2] checkout: introduce "--to-branch" option

[ZheNing Hu]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> 于2021年12月10日周五 17:24写道：
> > +-w::
> > +--to-branch::
> > +     Rather than checking out a commit to work on it, checkout out
> > +     to the unique branch on it. If there are multiple branches on
> > +     the commit, the checkout will fail.
> > +
>
> So basically what this option implements is something that could be done
> as a shellscript of:
>
>         git_checkout_branch_from_oid () {
>                 rev=$1
>                 git for-each-ref --format='%(refname:strip=2)' --points-at $rev >/tmp/found
>                 if test $(wc -l </tmp/found) -ne 1
>                 then
>                         echo "Goldilocks error in finding $rev: $(cat /tmp/found)"
>                         return 1
>                 fi
>                 git checkout $found
>         }
>

Yes, this is the effect I expect, and it can indeed be done through
the shellscript.

> Which is not to say that it isn't useful, but that I think adding this
> to "git checkout" specifically is adding this to the wrong level. Isn't
> this useful to most things that parse revisions? I.e. wouldn't a better
> interface be via the peel syntax?
>
>     oid=$(git rev-parse HEAD)
>     git checkout $oid^{tobranch}
>
> Doing it that way would allow any arbitrary command that takes revisions
> now access to that, and we could have e.g. "^{tobranches}" too, so you
> could do:
>
>     git for-each-ref --format='%(refname:strip=2)' $oid^{tobranches}
>
> Or:
>
>     git log $oid^{tobranches}
>

Very good inspiration, putting "oid -> branches" in peel will be more elegant
and useful.

> I think implementing that is a bit harder. It's peel_onion() in
> object-name.c. I think parse_branchname_arg() via get_oid_mb() is now
> only capable of filling in an OID for a given name, and then checking
> out that name comes as a separate step, and you can't just return
> e.g. "master".
>
> But I don't think anything stops us from adjusting those functions a bit
> so that get_oid_with_context(() and friends could pass down say an
> optional "struct string_list *", and the "peel" could then be expanded
> to that.
>

I agree.

> Similar to how we have "git chekout -", and the "-" is understood by
> some commands, but not all (via some opt-in whose location I forget...).

"-" is parsed as "@{-1}".

Thanks.
--
ZheNing Hu

Re: [PATCH 2/2] checkout: introduce "--to-branch" option

[ZheNing Hu]

Bagas Sanjaya <bagasdotme@gmail.com> 于2021年12月10日周五 19:51写道：
>
> On 10/12/21 13.22, ZheNing Hu via GitGitGadget wrote:
> > +-w::
> > +--to-branch::
> > +     Rather than checking out a commit to work on it, checkout out
> > +     to the unique branch on it. If there are multiple branches on
> > +     the commit, the checkout will fail.
> > +
>
> Did you mean unique branch that contains the commit?
>

I mean the unique branch which points to the commit.

> --
> An old man doll... just what I always wanted! - Clara

--
ZheNing Hu

Re: [PATCH 2/2] checkout: introduce "--to-branch" option

[ZheNing Hu]

Junio C Hamano <gitster@pobox.com> 于2021年12月11日周六 06:14写道：
>
> "ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: ZheNing Hu <adlternative@gmail.com>
> >
> > When we want checkout to a branch (e.g. dev1) which reference
> > to a commit, but sometimes we only remember the tag (e.g. v1.1)
> > on it, we will use `git checkout v1.1` to find the commit first,
> > git will be in the state of deatching HEAD, so we have to search the
> > branches on the commit and checkout the branch we perfer. This will
> > be a bit cumbersome.
> >
> > Introduce "--to-branch" option, `git checkout --to-branch <tag>`
> > and `git checkout --to-branch <commit>` will search all branches
> > and find a unique branch reference to the commit (or the commit which
> > the tag reference to) and checkout to it. If the commit have more
> > than one branches, it will report error "here are more than one
> > branch on commit".
>
> Sorry, but the above explanation does not make any sense to me.
>
> It is unclear if you mean "dev1" exactly point at the commit tagged
> as v1.1, or you want the branch "dev1" that is a descedanant of
> v1.1.  Without telling that to the reader, the above explanation is
> useless.
>

I meant the former.

> And whether you meant the former or the latter, neither use case does
> not make much sense.
>
> First, suppose you meant "checkout --to-branch v1.1" to find a
> branch whose tip exactly points at v1.1.  You instead check out
> "dev1" branch, and work on it to advance its history.  When you are
> done, you may go to another branch and work on something else.
>
> But then what?  When you need another topic that also needs to be
> later merge-able to v1.1, "checkout --to-branch v1.1" no longer will
> be able to find "dev1", because, well, you have already used it to
> build something else.
>
> So, "--to-branch v1.1" that finds and checks out a branch whose tip
> exactly points at v1.1 would be pretty useless.
>

Well, I didn't consider what you said before. I just want to find a shortcut
for "oid -> branches" and "tag -> branches". And I can quickly use it to
switch branches.

> So let's correct the unwritten assumption and say "--to-branch v1.1"
> finds a branch that is descendant of the tag.  It is like I have
> maint-2.33 branch to prepare for v2.33.1, v2.33.2,... maintenance
> releases and being able to find maint-2.33 by saying v2.33.2 (or
> v2.33.1) _might_ be convenient.
>
> But that would only be true if there is only one single branch per
> family of tags (in the above example, v2.33.* tags).  You cannot use
> the workflow where many topic branches run in parallel, and get
> merged to the integration branch(es) only after they are ready,
> because you need bugfix-1-for-v2.33, bugfix-2-for-v2.33,... branches
> all forked from v2.33.0 (or a commit with a later tag in the v2.33.*
> family) to cook these independent fixes that are destined for the
> maint-2.33 integration branch, so you cannot uniquely find maint-2.33
> by saying v2.33.0 or v2.33.1 or whatever.
>

Well, in the case of many branches pointing to one commit, this "--to-branch"
is not very useful.

> I also sense that the first paragraph of the proposed log message
> for this commit hints that the user needs a bit more studying of
> existing tools.  When we know v1.1 but do not know if we already
> have branches that are based on it, we DO NOT do "git checkout v1.1".
> Instead the first thing we would do is "git branch --contains v1.1"
> (add "--no-merged main" to exclude the branches that have already
> graduated to 'main').
>

"git branch --contains v1.1" can find all branches whose history contains the
 commit tagged as v1.1. So what if "git checkout --contains v1.1"?

If there is only one branch, checkout to it; if there are multiple branches,
it will degenerate into "git branch --contains v1.1" to show these branches to
the user. Of course this feature is not very consistent with my
original intention...

> So, for this partcular topic, what I would recommend is *not* jump
> in and add a new feature, but to study what's available and build a
> workflow around the existing features.
>
>

Thanks.
--
ZheNing Hu

Re: [PATCH 2/2] checkout: introduce "--to-branch" option

[Junio C Hamano]

ZheNing Hu <adlternative@gmail.com> writes:

>> It is unclear if you mean "dev1" exactly point at the commit tagged
>> as v1.1, or you want the branch "dev1" that is a descedanant of
>> v1.1.  Without telling that to the reader, the above explanation is
>> useless.
>>
>
> I meant the former.
>
>> And whether you meant the former or the latter, neither use case does
>> not make much sense.
>> ...
>> So, "--to-branch v1.1" that finds and checks out a branch whose tip
>> exactly points at v1.1 would be pretty useless.
> ...
> "git branch --contains v1.1" can find all branches whose history contains the
>  commit tagged as v1.1. So what if "git checkout --contains v1.1"?

I already said, whether you meant "the only branch that points
exactly at" or "the only branch that contains", the feature does not
make sense.  Forcing users to keep only a single branch that either
points at a given tag is simply impossible and also useless.  Once
the branch gains even a single commit, it will no loger be pointing
at the tag, so "let's prepare a branch pointing at v1.1 just in case
when I want to start working from there" would not be a good
workflow to begin with.  Forcing users to keep only a single branch
that contains a given tag would encourage even a worse workflow to
throw in unrelated things, whose only commonality is that they all
want to fork from a single tag, into a single branch.

IOW, there is nothing we want to add to "git checkout/switch" for
this topic.  "git checkout --contains $tag" smells like a solution
looking for a problem.

Re: [PATCH 2/2] checkout: introduce "--to-branch" option

[Ævar Arnfjörð Bjarmason]

On Sun, Dec 12 2021, Junio C Hamano wrote:

> ZheNing Hu <adlternative@gmail.com> writes:
>
>>> It is unclear if you mean "dev1" exactly point at the commit tagged
>>> as v1.1, or you want the branch "dev1" that is a descedanant of
>>> v1.1.  Without telling that to the reader, the above explanation is
>>> useless.
>>>
>>
>> I meant the former.
>>
>>> And whether you meant the former or the latter, neither use case does
>>> not make much sense.
>>> ...
>>> So, "--to-branch v1.1" that finds and checks out a branch whose tip
>>> exactly points at v1.1 would be pretty useless.
>> ...
>> "git branch --contains v1.1" can find all branches whose history contains the
>>  commit tagged as v1.1. So what if "git checkout --contains v1.1"?
>
> I already said, whether you meant "the only branch that points
> exactly at" or "the only branch that contains", the feature does not
> make sense.  Forcing users to keep only a single branch that either
> points at a given tag is simply impossible and also useless.  Once
> the branch gains even a single commit, it will no loger be pointing
> at the tag, so "let's prepare a branch pointing at v1.1 just in case
> when I want to start working from there" would not be a good
> workflow to begin with.  Forcing users to keep only a single branch
> that contains a given tag would encourage even a worse workflow to
> throw in unrelated things, whose only commonality is that they all
> want to fork from a single tag, into a single branch.
>
> IOW, there is nothing we want to add to "git checkout/switch" for
> this topic.  "git checkout --contains $tag" smells like a solution
> looking for a problem.

I don't see how it's fundimentally different than the DWIM logic of
taking "<name>" and seeing that there's only one "refs/heads/<name>",
and giving up in other cases where we get ambiguous reference names that
we can't resolve.

I.e. I think this is probably useful for some, it's a common workflow to
have a 1=1 relationship like this, and if it's 1=many we can just handle
it as we do with ambiguous ref, or ambiguous short OIDs for that matter.

But as I noted upthread I really don't see this making sense as a
per-command thing, as opposed to some extension of the "peel"
syntax. I.e. we should (or at least expose it as such to the user)
interpret that sort of argument/newx syntax before "git checkout" et al
get to it, so from a UX perspective you could feed a ^{oid2ref} (or
whatever it would be called) to rev-parse, checkout etc. etc.

Re: [PATCH 2/2] checkout: introduce "--to-branch" option

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> I don't see how it's fundimentally different than the DWIM logic of
> taking "<name>" and seeing that there's only one "refs/heads/<name>",
> and giving up in other cases where we get ambiguous reference names that
> we can't resolve.

In that example, once you obtained a local branch whose name is
<name>, it is obvious how you would work with that.  Your next "git
checkout <name>" would work on the local one, and only the initial
one does something magical.

Which makes quite a lot of sense.

There is no similarity in it with "--to-branch <tag>" that is being
discussed here.

Re: [PATCH 2/2] checkout: introduce "--to-branch" option

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> I don't see how it's fundimentally different than the DWIM logic of
>> taking "<name>" and seeing that there's only one "refs/heads/<name>",
>> and giving up in other cases where we get ambiguous reference names that
>> we can't resolve.
>
> In that example, once you obtained a local branch whose name is
> <name>, it is obvious how you would work with that.  Your next "git
> checkout <name>" would work on the local one, and only the initial
> one does something magical.
>
> Which makes quite a lot of sense.
>
> There is no similarity in it with "--to-branch <tag>" that is being
> discussed here.

Another thing you seem to be ignoring is that it requires you to
prepare in advance and keep one (and only one) dormant branch for
the <tag> you will use the "--to-branch <tag>" with, because it
finds the branch whose tip exactly points at the tag.

You can use the command exactly once for the <tag> and check out
that branch, but then what?  Once you start working on that branch,
you would by definition no longer have a suitable branch that can be
used to with "--to-branch <tag>", because (1) it was the only one
that pointed at <tag> or you'd have gotten an error, and (2) you
have moved the tip of the branch so it no longer points at the tag.
And you'd expect the user to say "git branch <next-branch> <tag>" in
preparation for the next time you might want to use "--to-branch
<tag>" and keep that pristine?

I do not think of any similar counterpart in "teach checkout <name>
to dwim and fork from a remote-tracking branch from a remote, if
there is no other remote with a branch with that name" DWIMming to
these nonsense.