Re: [PATCH v2 03/23] contrib/subtree: Teach add to store repository & branch in .gittrees

[Paul Campbell <pcampbell@kemitix.net>]

On Mon, Mar 11, 2013 at 3:24 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Paul Campbell <pcampbell@kemitix.net> writes:
>
>> From: Matt Hoffman <matt.hoffman@quantumretail.com>
>>
>> The repository and branch of a subtree added with the add command is
>> stored in the .gittrees file.
>>
>> Signed-off-by: Paul Campbell <pcampbell@kemitix.net>
>> ---
>>  contrib/subtree/git-subtree.sh | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
>> index caf4988..7b70251 100755
>> --- a/contrib/subtree/git-subtree.sh
>> +++ b/contrib/subtree/git-subtree.sh
>> @@ -528,6 +528,14 @@ cmd_add_repository()
>>       revs=FETCH_HEAD
>>       set -- $revs
>>       cmd_add_commit "$@"
>> +
>> +  # now add it to our list of repos
>> +  git config -f .gittrees --unset subtree.$dir.url
>> +  git config -f .gittrees --add subtree.$dir.url $repository
>> +  git config -f .gittrees --unset subtree.$dir.path
>> +  git config -f .gittrees --add subtree.$dir.path $dir
>> +  git config -f .gittrees --unset subtree.$dir.branch
>> +  git config -f .gittrees --add subtree.$dir.branch $refspec
>
> Existing code in the function this touches seem to be written
> carefully to allow $IFS whitespace in $dir, but this change butchers
> it, it seems.
>
> Also, where does $refspec come from?  When this is called from
> cmd_add_repository(), there is an assignment to the variable, but it
> is not all clear.  As git-subtree declares it won't work with
> anything but bash, I think things like this should take advantage of
> being written for bash by using "local" and passing arguments
> explicitly instead of relying on global variables, which POSIX shell
> scripts cannot afford to do but bash scripts can.
>
>>  }
>>
>>  cmd_add_commit()

$refspec gets assigned in the third line of the function. The function
has only one caller in cmd_add() inside an if clause that requires
there be two parameters.

Personally I'd rather move git-subtree to be more portable and less
dependant on bash. I've sent some patches earlier to remove that
dependancy. As far as I could see there wasn't really anything that
was bash-only.

Reroll of this patch coming shortly allowing for $IFS whitespace.

-- 
Paul [W] Campbell