Re: [PATCH 1/2] fetch, upload-pack: add --no-shallow for infinite depth

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Nguyễn Thái Ngọc Duy  <pclouds@xxxxxxxxx> writes:

> The user can do --depth=2147483647 (*) for infinite depth now. But
> it's hard to remember. Any other numbers larger than the longest
> commit chain in the repository would also do, but some guessing may
> be involved. Make easy-to-remember --no-shallow an alias for
> --depth=2147483647.

I think "no shallow" makes sense in a much more important way than
"infinite depth", and this patch is a good idea for a reason
entirely different from the justification your log message makes ;-)

We explain that "clone --depth=<number>" is a way to end up with a
partial repository that contains only the recent history, and while
this may give users a smaller repository, in return the result will
be a "shallow repository" with certain limitations (i.e. fetching or
cloning from such a repository will be refused).

We also explain that the reason to use --depth=<number> is to grab
some history behind what you originally acquired into your "shallow
repository" so that you have deeper history than your original
"shallow repository".

A reader who does not know how this shallowness is implemented
cannot be blamed if she thinks the shallowness is a permanent
attribute of a repository, and once a repository is tainted by
shallowness, it cannot be wiped away by deepening it, because
nowhere in the documentation we say the "shallowness" will go away
once you grabbed enough number of older commits with it.

Calling the option "--no-shallow" (or even better, "--unshallow",
meaning "make it a repository that is no longer shallow") makes it
crystal clear that the option is about wiping away the shallowness.
Of course, the result has to contain an untruncted history, but that
is a mere side effect and an implementation detail from the end
user's point of view.

> Make upload-pack recognize this special number as infinite depth. The
> effect is essentially the same as before, except that upload-pack is
> more efficient because it does not have to traverse to the bottom
> any more. The chance of a user actually wanting exactly 2147483647
> commits depth, not infinite, on a repository with a history that long,
> is probably too small to consider.
>
> (*) This is the largest positive number a 32-bit signed integer can
>     contain. JGit and older C Git store depth as "int" so both are OK
>     with this number. Dulwich does not support shallow clone.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
>  Documentation/fetch-options.txt     |  4 ++++
>  Documentation/git-fetch-pack.txt    |  2 ++
>  Documentation/technical/shallow.txt |  3 +++
>  builtin/fetch.c                     | 15 ++++++++++++++-
>  commit.h                            |  3 +++
>  t/t5500-fetch-pack.sh               | 16 ++++++++++++++++
>  upload-pack.c                       | 13 ++++++++++---
>  7 files changed, 52 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
> index 6e98bdf..012d1b2 100644
> --- a/Documentation/fetch-options.txt
> +++ b/Documentation/fetch-options.txt
> @@ -13,6 +13,10 @@
>  	to the specified number of commits from the tip of each remote
>  	branch history. Tags for the deepened commits are not fetched.
>  
> +--no-shallow::
> +	Deepen to the roots of the repository's history (i.e. the
> +	result repository is no longer shallow).
> +

Mentioning both is a good thing, but I think "no longer shallow" is
more important aspect of this operation than "deepen to the roots".

> diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
> index 6322e8a..6a6e672 100755
> --- a/t/t5500-fetch-pack.sh
> +++ b/t/t5500-fetch-pack.sh
> @@ -264,6 +264,22 @@ test_expect_success 'clone shallow object count' '
>  	grep "^count: 52" count.shallow
>  '
>  
> +test_expect_success 'fetch --depth --no-shallow' '
> +	(
> +		cd shallow &&
> +		test_must_fail git fetch --depth=1 --no-shallow
> +	)
> +'
> +
> +test_expect_success 'infinite deepening (full repo)' '
> +	(
> +		cd shallow &&
> +		git fetch --no-shallow &&
> +		git fsck --full &&
> +		! test -f .git/shallow

This looks as if fsck is what removes .git/shallow but I do not
think that is what is being tested...

> diff --git a/upload-pack.c b/upload-pack.c
> index 6142421..88f0029 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -670,10 +670,17 @@ static void receive_needs(void)
>  	if (depth == 0 && shallows.nr == 0)
>  		return;
>  	if (depth > 0) {
> -		struct commit_list *result, *backup;
> +		struct commit_list *result = NULL, *backup = NULL;
>  		int i;
> -		backup = result = get_shallow_commits(&want_obj, depth,
> -			SHALLOW, NOT_SHALLOW);
> +		if (depth == INFINITE_DEPTH)
> +			for (i = 0; i < shallows.nr; i++) {
> +				struct object *object = shallows.objects[i].item;
> +				object->flags |= NOT_SHALLOW;
> +			}
> +		else
> +			backup = result =
> +				get_shallow_commits(&want_obj, depth,
> +						    SHALLOW, NOT_SHALLOW);

Nice.  Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]