fetch unshallow fails to honor dry-run

fetch unshallow fails to honor dry-run

[Tim Janik]

Hey all,

git fetch --dry-run modifies the repository if --unshallow is passed:

$ git --version
git version 2.1.4
$ git fetch --dry-run --unshallow
remote: Counting objects: 30603, done.
remote: Compressing objects: 100% (6843/6843), done.
remote: Total 30603 (delta 24564), reused 29164 (delta 23386)
Receiving objects: 100% (30603/30603), 5.42 MiB | 0 bytes/s, done.
Resolving deltas: 100% (24564/24564), completed with 317 local objects.
remote: Counting objects: 7, done.
remote: Compressing objects: 100% (7/7), done.
remote: Total 7 (delta 0), reused 6 (delta 0)
Unpacking objects: 100% (7/7), done.

I actually tried --dry-run --unshallow in order to find a way to detect in a script if the current git repository is shallow or not.
Better suggestions to find this out are very welcome.


-- 
Yours sincerely,
Tim Janik

https://testbit.eu/timj/
Free software author and speaker.

Re: fetch unshallow fails to honor dry-run

[Jeff King]

[+cc Duy for shallow expertise]

On Sun, Oct 25, 2015 at 10:16:00AM +0100, Tim Janik wrote:

> git fetch --dry-run modifies the repository if --unshallow is passed:
> 
> $ git --version
> git version 2.1.4
> $ git fetch --dry-run --unshallow
> remote: Counting objects: 30603, done.
> remote: Compressing objects: 100% (6843/6843), done.
> remote: Total 30603 (delta 24564), reused 29164 (delta 23386)
> Receiving objects: 100% (30603/30603), 5.42 MiB | 0 bytes/s, done.
> Resolving deltas: 100% (24564/24564), completed with 317 local objects.
> remote: Counting objects: 7, done.
> remote: Compressing objects: 100% (7/7), done.
> remote: Total 7 (delta 0), reused 6 (delta 0)
> Unpacking objects: 100% (7/7), done.

Hmm. I think that is because --dry-run is effectively "transfer the
objects, but do not update refs". So by fetching the objects, we've
effectively deepened the repository, whether we update the refs or not.

That being said, I suspect nobody has really thought hard about the
interaction of these two flags. And while obviously we update the
object database with a dry-run fetch, I can see the reasoning that we
should not touch the .git/shallow file, even if we have the objects.

Naively, something like this patch might help, but I have no idea if it
causes other problems.

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 4ce4fa0..24aa331 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -870,7 +870,7 @@ static struct transport *prepare_transport(struct remote *remote)
 		set_option(transport, TRANS_OPT_KEEP, "yes");
 	if (depth)
 		set_option(transport, TRANS_OPT_DEPTH, depth);
-	if (update_shallow)
+	if (update_shallow && !dry_run)
 		set_option(transport, TRANS_OPT_UPDATE_SHALLOW, "yes");
 	return transport;
 }

> I actually tried --dry-run --unshallow in order to find a way to
> detect in a script if the current git repository is shallow or not.
> Better suggestions to find this out are very welcome.

You can look for .git/shallow. I don't know if we've documented that
anywhere as a public interface, but I think it should be safe to rely
on.

-Peff

Re: fetch unshallow fails to honor dry-run

[Duy Nguyen]

Thanks for including me. I thought I marked this for reply later but I did not..

On Mon, Nov 9, 2015 at 8:24 PM, Jeff King <peff@peff.net> wrote:
> [+cc Duy for shallow expertise]
>
> On Sun, Oct 25, 2015 at 10:16:00AM +0100, Tim Janik wrote:
>
>> git fetch --dry-run modifies the repository if --unshallow is passed:
>>
>> $ git --version
>> git version 2.1.4
>> $ git fetch --dry-run --unshallow
>> remote: Counting objects: 30603, done.
>> remote: Compressing objects: 100% (6843/6843), done.
>> remote: Total 30603 (delta 24564), reused 29164 (delta 23386)
>> Receiving objects: 100% (30603/30603), 5.42 MiB | 0 bytes/s, done.
>> Resolving deltas: 100% (24564/24564), completed with 317 local objects.
>> remote: Counting objects: 7, done.
>> remote: Compressing objects: 100% (7/7), done.
>> remote: Total 7 (delta 0), reused 6 (delta 0)
>> Unpacking objects: 100% (7/7), done.
>
> Hmm. I think that is because --dry-run is effectively "transfer the
> objects, but do not update refs". So by fetching the objects, we've
> effectively deepened the repository, whether we update the refs or not.
>
> That being said, I suspect nobody has really thought hard about the
> interaction of these two flags. And while obviously we update the
> object database with a dry-run fetch, I can see the reasoning that we
> should not touch the .git/shallow file, even if we have the objects.
>
> Naively, something like this patch might help, but I have no idea if it
> causes other problems.
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 4ce4fa0..24aa331 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -870,7 +870,7 @@ static struct transport *prepare_transport(struct remote *remote)
>                 set_option(transport, TRANS_OPT_KEEP, "yes");
>         if (depth)
>                 set_option(transport, TRANS_OPT_DEPTH, depth);
> -       if (update_shallow)
> +       if (update_shallow && !dry_run)
>                 set_option(transport, TRANS_OPT_UPDATE_SHALLOW, "yes");
>         return transport;
>  }

The key is not let shallow.c:update_shallow() do anything to
.git/shallow. --depth (or --unshallow) does not check this
UPDATE_SHALLOW flag and can still change .git/shallow. Other than
that, I don't think it can cause and bad effects. It's probably best
to pass this dry-run flag to transport.c
-- 
Duy