Re: [PATCH 3/4] submodule--helper list: lose the extra prefix option

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 25, 2016, 16:02 UTC
Message-ID: <xmqqegay5yw0.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqqmvpn5awo.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 25 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>> The usual early machinery of Git is to change the directory to
>> the top level of the working tree and pass the actual path inside
>> the working tree as `prefix` to the command being run.
>>
>> This is the case both for commands written in C (where the
>> prefix is passed into the command in a function parameter) as
>> well as in git-submodule.sh where the setup code runs...
>>
>> Adhere to Gits standard of passing the relative path inside the
>> working tree by passing it via -C.
>
> While -C _also_ works, I do not think it is "standard" in any sense.
> What made you think so?  -C is a way to tell Git to chdir there
> before doing anything else, without even adjusting the command line
> arguments that might be paths, and "chdir there to go to top" may
> (or may not--I haven't thought things thru) have the same effect as
> passing the prefix into functions, that is merely true as a side
> effect, I would think.
>
> So this change may not be wrong per-se, but if the lossage of prefix
> is the final goal (as opposed to an approach to gain other benefits,
> e.g. "now we do not have to use prefix, we can simplify these other
> things"), I do not know if it is worth it.

I actually do not care too deeply either way, as I understand that the long term goal is to have "git submodule" itself rewritten in C, so that places that currently call submodule--helper would become an internal call.

The way all the subcommand written in C works is

 - The start-up sequence does the repository discovery, which
   involves crawling up to the top-level of the working tree, and
   compute "prefix", where the end-user was when the command was
   invoked;

 - The subcommand implementation is called with this "prefix";

 - When the subcommand implementation interprets the command line
   arguments and option arguments, it prefixes the "prefix" as
   needed.  If, for example, "git grep -f patterns" is invoked
   inside "sub/" subdirectory, when the command line and option
   arguments are processed, the process is already at the top level
   of the working tree, so it needs to read the patterns from
   "sub/patterns" file.  "git ls-files 'Makefil*'" invoked inside
   "sub/" subdirectory needs to limit the output to those that match
   not "Makefile", but "sub/Makefil*".

The hope of doing an incremental rewrite of the whole thing by enriching submodule--helper is that the bulk of the code there will be reusable when the entirety of "git submodule" is rewritten in C, so they need to take the "prefix" the same way, whether the caller is calling from "git-submodule.sh" script via submodule--helper, or the eventual C implementation of "git submodule" is making direct calls to them. As long as the correct "prefix" is passed to the routines that are driven via submodule--helper, it does not matter and I do not care how it is done.

The current code of "git submodule" whose higher parts are still in shell would would:

 - The start-up sequence in shell does the cd_to_toplevel and finds
   the prefix;

 - "git submodule--helper list --prefix=$prefix $other_args" is
   called; as this is called from the top-level of the working tree,
   internally its "prefix" is empty, but $other_args must be
   interpreted relative to the $prefix passed with --prefix option.

If we instead call "git -C "$prefix" submodule--helper list $other_args", then

 - This command first does another chdir(2) back to $prefix;

 - The start-up sequence of "submodule--helper" does the usual
   repository discovery again, which involves crawling up to the
   top-level of the working tree, and compute "prefix".  This
   happens to match what -C "$prefix" gave the command.

Making calls to submodule--helper via "git -C" feels a little bit roundabout because of this "caller tells to chdir, and then it has to again chdir back up" only to rediscover the same information.

Again, I actually do not care too deeply either way, though, as long as the correct "prefix" is passed to the routines that are driven via submodule--helper, which would assure that the C code you write today will be reusable when "git submodule" as a whole is redone in C.

Thanks.