Re: [PATCH v4 2/3] sumodule--helper: fix submodule--helper clone usage and check argc count

From: Jacob Keller <jacob.keller@gmail.com>
Date: Feb 26, 2016, 22:08 UTC
Message-ID: <CA+P7+xq8T8BsjfH9hrwULZiyTutt+6-qxqBZbZFHk490my_ofQ@mail.gmail.com>
In-Reply-To: <CAGZ79kbD7_CYo9KT185oTxrjqch-otwfG0mWqAV2QiwrC-ch9Q@mail.gmail.com>

On Fri, Feb 26, 2016 at 11:31 AM, Stefan Beller <sbeller@google.com> wrote:

Show 18 quoted lines

> On Fri, Feb 26, 2016 at 11:18 AM, Jacob Keller <jacob.e.keller@intel.com> wrote:
>> From: Jacob Keller <jacob.keller@gmail.com>
>> -               git submodule--helper clone ${GIT_QUIET:+--quiet} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" "$reference" "$depth" || exit
>> +               git submodule--helper clone ${GIT_QUIET:+--quiet} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${depth:+"$depth"} || exit
>
> By having this additional fix (i.e. no '--depth', '<empty string>' is
> passed to the
> submodule helper, we can improve the submodule helper further
> in clone_submodule we can drop the double check for `depth` and `reference`
> (as well as `gitdir`, that double check is unneeded as of now already),
> by just checking for the pointer to be non  NULL and not further checking
> the dereferenced pointer.
>
> That can go in either squashed into this commit or on top of it, either is fine.
>
> That said:
> Reviewed-by: Stefan Beller <sbeller@google.com>
>

To be clear, what this *actually* does is prevent passing

""

when depth or reference are empty. It never passed "--depth" "" together, or "--reference" "". It *does* pass --prefix "" sometimes, but not always, from what I could tell when using print debug statements while running the submodule tests.

I am not sure if it ever passes "--depth" "" in any case, but I don't believe so.

What this final change is needed for is that without it, after parse_options, argc is equal 2.

Thanks, Jake