Re: [PATCH v2] clone: add a --no-tags option to clone without tags

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Apr 25, 2017, 22:45 UTC
Message-ID: <20170425224521.GM28740@aiede.svl.corp.google.com>
In-Reply-To: <20170419143831.6868-1-avarab@gmail.com>

Hi,

Ævar Arnfjörð Bjarmason wrote:

Show 5 quoted lines

> Add a --no-tags option to "git clone" to clone without tags. Currently
> there's no easy way to clone a repository and end up with just a
> "master" branch via --single-branch, or track all branches and no
> tags. Now --no-tags can be added to "git clone" with or without
> --single-branch to clone a repository without tags.

Now I've read the discussion from v1, so you can see my thoughts evolving in real time. :)

The above feels a bit misleading when it says "there's no easy way to clone a repository and end up with just a 'master' branch". --single-branch does exactly that. Some annotated tags *pointing to its history* come along for the ride, but what harm are they doing?

In other words, I think the commit message needs a bit more detail about the use case, to say why omitting those tags is useful. The use case is probably sane but it is not explained. A side effect (and my main motivation) is that this would make it crystal clear to people looking at the patch in history that it is talking about tags that are part of "master"'s history, not tags pointing elsewhere.

> Before this the only way of doing this was either by manually tweaking
> the config in a fresh repository:

Usually commit messages refer to the state of things without some patch using the present tense --- e.g. "Without this patch, this --no-tags option can be emulated by (1) manually tweaking the config in a fresh repository, or (2) by setting tagOpt=--no-tags after cloning and deleting any existing tags".

[...]

Show 7 quoted lines

> Which of course was also subtly buggy if --branch was pointed at a
> tag, leaving the user in a detached head:
>
>     git clone --single-branch --branch v2.12.0 git@github.com:git/git.git &&
>     cd git &&
>     git config remote.origin.tagOpt --no-tags &&
>     git tag -l | xargs git tag -d

At this point I lose the trail of thought. I don't think it's important to understanding the patch.

Show 7 quoted lines

> Now all this complexity becomes the much simpler:
>
>     git clone --single-branch --no-tags git@github.com:git/git.git
>
> Or in the case of cloning a single tag "branch":
>
>     git clone --single-branch --branch v2.12.0 --no-tags git@github.com:git/git.git

Nice.

[...]

Show 15 quoted lines

>  Documentation/git-clone.txt | 14 ++++++++-
>  builtin/clone.c             | 13 ++++++--
>  t/t5612-clone-refspec.sh    | 73 +++++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 95 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
> index 30052cce49..57b3f478ed 100644
> --- a/Documentation/git-clone.txt
> +++ b/Documentation/git-clone.txt
> @@ -13,7 +13,7 @@ SYNOPSIS
>  	  [-l] [-s] [--no-hardlinks] [-q] [-n] [--bare] [--mirror]
>  	  [-o <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>]
>  	  [--dissociate] [--separate-git-dir <git dir>]
> -	  [--depth <depth>] [--[no-]single-branch]
> +	  [--depth <depth>] [--[no-]single-branch] [--no-tags]

Can I pass --tags to negate a previous --no-tags?

[...]

Show 8 quoted lines

> +--no-tags::
> +	Don't clone any tags, and set
> +	`remote.<remote>.tagOpt=--no-tags` in the config, ensuring
> +	that future `git pull` and `git fetch` operations won't follow
> +	any tags. Subsequent explicit tag fetches will still work,
> +	(see linkgit:git-fetch[1]).
> ++
> +Can be used in conjunction with `--single-branch` to clone & maintain

nit: s/&/and/

[...]

> +test_expect_success 'clone with --no-tags' '
> +	(
> +		cd dir_all_no_tags && git fetch &&
> +		git for-each-ref refs/tags >../actual

nit: this would be easier to read with the 'cd' and 'git fetch' on separate lines.

[...]

> +test_expect_success '--single-branch while HEAD pointing at master and --no-tags' '
> +	(
> +		cd dir_master_no_tags && git fetch &&

Likewise.

> +		git for-each-ref refs/remotes/origin |
> +		sed -e "/HEAD$/d" \
> +		    -e "s|/remotes/origin/|/heads/|" >../actual

Can $/ be expanded by the shell?

The rest looks sensible.

Thanks and hope that helps, Jonathan