Re: [PATCH 1/6] builtin/submodule: allow cloning with different ref storage format

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2348 bytes --]

On Wed, Aug 07, 2024 at 03:55:57PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > -update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference <repository>] [--depth <depth>] [--recursive] [--jobs <n>] [--[no-]single-branch] [--filter <filter-spec>] [--] [<path>...]::
> > +update [--init] [--remote] [-N|--no-fetch] [--[no-]recommend-shallow] [-f|--force] [--checkout|--rebase|--merge] [--reference <repository>] [--ref-format <format>] [--depth <depth>] [--recursive] [--jobs <n>] [--[no-]single-branch] [--filter <filter-spec>] [--] [<path>...]::
> >  +
> >  --
> >  Update the registered submodules to match what the superproject
> > @@ -185,6 +185,9 @@ submodule with the `--init` option.
> >  If `--recursive` is specified, this command will recurse into the
> >  registered submodules, and update any nested submodules within.
> >  
> > +If `--ref-format <format>`  is specified, the ref storage format of newly
> > +cloned submodules will be set accordingly.
> > +
> >  If `--filter <filter-spec>` is specified, the given partial clone filter will be
> >  applied to the submodule. See linkgit:git-rev-list[1] for details on filter
> >  specifications.
> 
> Presumably, if the named submodule has already been initialized, we
> are not converting its ref backend with --ref-format=<format> option
> when "git submodule update --ref-format=<format>" is run.  Would it
> make sense to say it is an error to give it without "--init", I
> wonder.  If so, we probably would need to see if other existing
> options like "--filter" also need a similar sanity check, if not
> already done.

Well, even when "--init" was given it is not sure whether the ref
storage format will actually end up being used, because that option only
tells us to initialize uninitialized submodules. So if the submodule was
initialized already, then the ref storage format won't be used.

We probably could add such a sanity check. But as you say, other options
like "--filter", "--depth", "--reference" and "--recommend-shallow"
don't have that check, either, so it would feel a bit like opening a can
of worms. So personally, I'd rather defer this to another day where we
then implement the check for all of these options.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]