Re: [PATCH v2] builtin/clone: teach git-clone(1) the --revision= option

> On Fri, Nov 29, 2024 at 01:00:42PM +0100, Toon Claes wrote:
> > The git-clone(1) command has the option `--branch` that allows the user
> > to select the branch they want HEAD to point to. In a non-bare
> > repository this also checks out that branch.
> > 
> > Option `--branch` also accepts a tag. When a tag name is provided, the
> > commit this tag points to is checked out and HEAD is detached. Thus
> > `--branch` can be used to clone a repository and check out a ref kept
> > under `refs/heads` or `refs/tags`. But some other refs might be in use
> > as well. For example Git forges might use refs like `refs/pull/<id>` and
> > `refs/merge-requests/<id>` to track pull/merge requests. These refs
> > cannot selected upon git-clone(1).
> 
> s/cannot/cannot be/
> 
> > Add option `--revision` to git-clone(1). This option accepts a fully
> > qualified reference, or a raw commit hash. This enables the user to
> > clone and checkout any revision they want. `--revision` can be used in
> 
> s/checkout/check out/
> 
> Does this have to be a raw commit hash, or do we also accept an
> arbitrary committish like a tag that peels down to a commit?
> 
> > conjunction with `--depth` to do a minimal clone that only contains the
> > sources for a single revision. This can be useful for automated tests.
> 
> It's implicity, but automated tests in this context probably means CI
> systems.
> 
> > This type of shallow clone could also be achieved with the following set
> > of commands:
> > 
> >     git init the-repo
> >     cd ./the-repo
> >     git remote add origin <url>
> >     git fetch --depth=1 origin <commit-id>
> >     git checkout <commit-id>
> > 
> > Unfortunately, this approach uses git-fetch(1) instead of git-clone(1),
> > and only on git-clone(1) the bundle URIs advertised by the server are
> > used. By adding this option `--revision` to git-clone(1) allows us to
> > get the same end result, while benefiting from bundle URIs if advertised
> > by the server.
> 
> I'd claim that this is not only about enabling bundle URIs, but also
> about making this easier to work with in the first place. The above is
> rather on the complex side even though the use case at hand is not all
> that esoteric.
> 
> > diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
> > index 7acb4cb17618c6cbee5d6ebe41a53be03ebfaa6c..2a3f6d9deae4b817db50d8c4e555a6f33b8296f1 100644
> > --- a/Documentation/git-clone.txt
> > +++ b/Documentation/git-clone.txt
> > @@ -218,6 +218,13 @@ objects from the source repository into a pack in the cloned repository.
> >  	`--branch` can also take tags and detaches the `HEAD` at that commit
> >  	in the resulting repository.
> >  
> > +`--revision` _<rev>_::
> > +	This clones the given revision, and that revision only. The argument can
> > +	be a symbolic ref name (e.g. `refs/heads/main`), or a raw commit hash.
> 
> Nit: while I know what you want to say with "symbolic ref name", I think
> it's a bit awkwardly worded because a reader may confuse it with an
> actual symbolic ref. I would just drop the "symbolic".
> 
> We should also clarify whether this accepts tags that peel down to a
> commit.
> 
> > +	Unless the revision points to a branch (i.e. ref starting with
> > +	`refs/heads/`), the HEAD is detached.
> 
> Okay, makes sense. When cloning a tag or committish we wouldn't know how
> to name the branch, so we instead put us into detached HEAD mode.
> 
> I could see an argument that we should do this unconditionally even when
> cloning a branch to simplify the UX by always doing the same thing, no
> matter what the user has passed.