[PATCH] git-pull.sh: Fix call to git-merge for new command format

[PATCH] git-pull.sh: Fix call to git-merge for new command format

[Horst H. von Brand]

Signed-off-by: Horst H. von Brand <vonbrand@inf.utfsm.cl>
---
 git-pull.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index bfeb4a0..a875809 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -218,5 +218,5 @@ merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
 test true = "$rebase" &&
 	exec git-rebase $diffstat $strategy_args --onto $merge_head \
 	${oldremoteref:-$merge_head}
-exec git-merge $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args \
-	"$merge_name" HEAD $merge_head $verbosity
+exec git-merge  $verbosity -m $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args \
+	"$merge_name" $merge_head
-- 
1.6.6.rc0.114.gc8648

Re: [PATCH] git-pull.sh: Fix call to git-merge for new command format

[Junio C Hamano]

"Horst H. von Brand" <vonbrand@inf.utfsm.cl> writes:

> Signed-off-by: Horst H. von Brand <vonbrand@inf.utfsm.cl>
> ---
>  git-pull.sh |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/git-pull.sh b/git-pull.sh
> index bfeb4a0..a875809 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -218,5 +218,5 @@ merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
>  test true = "$rebase" &&
>  	exec git-rebase $diffstat $strategy_args --onto $merge_head \
>  	${oldremoteref:-$merge_head}
> -exec git-merge $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args \
> -	"$merge_name" HEAD $merge_head $verbosity
> +exec git-merge  $verbosity -m $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args \
> +	"$merge_name" $merge_head
> -- 
> 1.6.6.rc0.114.gc8648

Heh, embarrasing.

But I think you wanted to have -m immediately before "$merge_name", no?

Re: [PATCH] git-pull.sh: Fix call to git-merge for new command format

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 02.12.2009 00:54:
> "Horst H. von Brand" <vonbrand@inf.utfsm.cl> writes:
> 
>> Signed-off-by: Horst H. von Brand <vonbrand@inf.utfsm.cl>
>> ---
>>  git-pull.sh |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/git-pull.sh b/git-pull.sh
>> index bfeb4a0..a875809 100755
>> --- a/git-pull.sh
>> +++ b/git-pull.sh
>> @@ -218,5 +218,5 @@ merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
>>  test true = "$rebase" &&
>>  	exec git-rebase $diffstat $strategy_args --onto $merge_head \
>>  	${oldremoteref:-$merge_head}
>> -exec git-merge $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args \
>> -	"$merge_name" HEAD $merge_head $verbosity
>> +exec git-merge  $verbosity -m $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args \
>> +	"$merge_name" $merge_head
>> -- 
>> 1.6.6.rc0.114.gc8648
> 
> Heh, embarrasing.
> 
> But I think you wanted to have -m immediately before "$merge_name", no?

This made me wonder a bit: Do we have a policy regarding the use of
"git-command" vs. "git command" in git shell scripts such as this one?
Of course, having been called through git, the dashed versions are in
the PATH. But I see a mix here ("git fmt-merge-msg" vs. "git-merge") and
in other scripts, which may potentially (in broken setups) lead to parts
of git from different installs being called. I would think the dashed
form is even more efficient (fewer lookups)?

Michael

Re: [PATCH] git-pull.sh: Fix call to git-merge for new command format

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> This made me wonder a bit: Do we have a policy regarding the use of
> "git-command" vs. "git command" in git shell scripts such as this one?

Yes.  Anything that sets GIT_EXEC_PATH correctly can use git-foo form.

Re: [PATCH] git-pull.sh: Fix call to git-merge for new command format

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 02.12.2009 17:45:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> This made me wonder a bit: Do we have a policy regarding the use of
>> "git-command" vs. "git command" in git shell scripts such as this one?
> 
> Yes.  Anything that sets GIT_EXEC_PATH correctly can use git-foo form.

I know they can. That was in the part you snipped ;)

The questions is: Should they? Should we avoid mixing both forms in one
script?

Michael

Re: [PATCH] git-pull.sh: Fix call to git-merge for new command format

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

>> Yes.  Anything that sets GIT_EXEC_PATH correctly can use git-foo form.
>
> I know they can. That was in the part you snipped ;)

You asked about the presense of "a policy", and you got an answer.

> The questions is: Should they? Should we avoid mixing both forms in one
> script?

Should we avoid it?  Yes but not very enthusiastically.  We should make
sure that new invocations anybody adds use dashless form, but I would
recommend against a "let's remove use of dashed form" patch _unless_ you
find a time when the project is really quiet and there is nothing else
going on.

The whole point of GIT_EXEC_PATH trick is to allow continued use of the
dashed form, so that we do not have to suffer from code churn and patches
to implement real changes do not have to crash with such clean-ups.

As we'll be changing "git pull", we should use dashless form in the
vicinity of the real change (which is only one line) while at it, like
this.

-- >8 --
From: Horst H. von Brand <vonbrand@inf.utfsm.cl>
Date: Tue, 1 Dec 2009 19:44:11 -0300
Subject: [PATCH] git-pull.sh: Fix call to git-merge for new command format

Now "git merge <msg> HEAD" is officially deprecated, we should
clean our own use as well.

Signed-off-by: Horst H. von Brand <vonbrand@inf.utfsm.cl>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-pull.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index bfeb4a0..fcf6c81 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -216,7 +216,7 @@ fi
 
 merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
 test true = "$rebase" &&
-	exec git-rebase $diffstat $strategy_args --onto $merge_head \
+	exec git rebase $diffstat $strategy_args --onto $merge_head \
 	${oldremoteref:-$merge_head}
-exec git-merge $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args \
-	"$merge_name" HEAD $merge_head $verbosity
+exec git merge $verbosity $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args \
+	-m "$merge_name" $merge_head

Re: [PATCH] git-pull.sh: Fix call to git-merge for new command format

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 02.12.2009 18:49:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>>> Yes.  Anything that sets GIT_EXEC_PATH correctly can use git-foo form.
>>
>> I know they can. That was in the part you snipped ;)
> 
> You asked about the presense of "a policy", and you got an answer.

I guess that was a language issue (on both sides) then, since "can"
could be "is able to" as well as "is allowed to", and I read your answer
in the former sense; the latter makes it a policy.

>> The questions is: Should they? Should we avoid mixing both forms in one
>> script?
> 
> Should we avoid it?  Yes but not very enthusiastically.  We should make
> sure that new invocations anybody adds use dashless form, but I would
> recommend against a "let's remove use of dashed form" patch _unless_ you
> find a time when the project is really quiet and there is nothing else
> going on.

OK, that's all I wanted to know. Thanks.

Michael