Re: [PATCHv18 00/11] Expose

From: Stefan Beller <sbeller@google.com>
Date: Feb 25, 2016, 23:25 UTC
Message-ID: <CAGZ79kYUkgbHSXYmOXRKhju1QQwiw9EU+VoO-AMZL_nMCf0NNg@mail.gmail.com>
In-Reply-To: <20160225231904.GT28749@google.com>

On Thu, Feb 25, 2016 at 3:19 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 23 quoted lines

> Stefan Beller wrote:
>
>> --- a/builtin/submodule--helper.c
>> +++ b/builtin/submodule--helper.c
>> @@ -465,14 +465,14 @@ static int update_clone(int argc, const char **argv, const char *prefix)
>>               NULL
>>       };
>>
>> -     argc = parse_options(argc, argv, prefix, module_update_clone_options,
>> +     argc = parse_options(argc, argv, suc.prefix, module_update_clone_options,
>>                            git_submodule_helper_usage, 0);
>
> I would have expected this to use 'parse_options(argc, argv, prefix, ...' since
> I wouldn't expect a command-specific --prefix= parameter to affect the
> interpretation of relative filenames in other parameters.
>
> Now that I look around more, it seems that other submodule--helper subcommands
> have the same strange behavior of overwriting the 'prefix' var.  So I take
> back my suggestion of using a different variable --- that can be addressed in a
> separate patch that deals with them all at once.
>
> Sorry to flip-flop like this,
> Jonathan

I plan to fix that up in all the submodule--helper commands once this long running series is in, but for now we want to keep it consistently awkward?

The way all submodule helper commands are currently "working" is

    (in git submodule.h:)
    wt_prefix=$(git rev-parse --show=prefix)
    cd_to_toplevel
    git submodule--helper <command> --prefix $wt_prefix

which makes the command a bit awkward with having the prefix in the signature and the prefix as an option. the prefix as given in the signature ought to be empty for now and we always rely on the prefix option.

I plan to replace that to be

    git -C $wt_prefix submodule--helper <command>

which then doesn't carry a prefix option, but can rely on the prefix from its function signature.

So I flip-flop that change of 'prefix' back to the way all submodule helper operate for now and then send a cleanup once this series is done.

Thanks, Stefan