Re: Teach git-fetch(1) to use a quarantine directory

From: Jonathan Tan <jonathantanmy@google.com>
Date: Jul 17, 2023, 23:47 UTC
Message-ID: <20230717234704.1906465-1-jonathantanmy@google.com>
In-Reply-To: <87edl6bx7o.fsf@iotcl.com>

Toon Claes <toon@iotcl.com> writes:

> Hi,
> 
> I've been looking into making git-fetch(1) to use a quarantine
> directory, but I'm a bit stuck on direction.

Thanks for looking into this!

Show 6 quoted lines

> I took git-receive-pack(1) as an example how it uses a quarantine
> directory. It seems it sets the environment variables
> $GIT_OBJECT_DIRECTORY and $GIT_ALTERNATE_OBJECT_DIRECTORIES so the real
> object db is used as an alternative, and a temporary is set as the
> default. Then a sub-process is spawned to uses these. In case of
> git-receive-pack(1), it calls git-unpack-objects(1).

From a reading of the source code, it may call git-unpack-objects or git-index-pack, I think. (unpack() in builtin/receive-pack.c)

> At the moment git-fetch(1) does not spawn any similar subprocess, so if
> we want to take the same approach to use the quarantine, we'll need to
> split up that command.

It calls get_pack() in fetch-pack.c, which actually may call one of the two commands above in the same way. (The calling may happen in various ways. IIRC in fetch protocol v2, get_pack() is invoked directly by git- fetch, whereas in earlier protocol versions, it may be the remote helper that calls git-fetch-pack that invokes this function.)

> But then we run into another problem as well. git-fetch(1) updates
> references, and that is something that's not allowed when using a tmp
> object directory.

execute_commands() in builtin/receive-pack.c has this:

          /*                                                                                                                          
           * Now we'll start writing out refs, which means the objects need                                                           
           * to be in their final positions so that other processes can see them.                                                     
           */                                                                                                                         
          if (tmp_objdir_migrate(tmp_objdir) < 0) {

which seems to be a solution to the same problem. I think Taylor wrote something similar [1].

[1] https://lore.kernel.org/git/ZLVK5nzVZU48uvYE@nand.local/

Show 6 quoted lines

> As far as I can tell from the code, fetching packs and updating refs is
> heavily intertwined, so I'm not sure this approach is the best way
> forward. So a few questions:
> 
> 1) Does it even make sense to make use git-fetch(1) use a quarantine
>    directory?

I don't know off-hand if this will work, but this sounds promising.

> 2) When making git-fetch(1) use a quarantine directory, what is the
>    recommended way to achieve this? Is this by calling a subprocess?
>    Maybe git-fetch-pack(1)?

I also don't know off-hand if this will work, but replicating what git- receive-pack does makes sense to me.