threads / patch / 21814

patchgit-pull.sh: Fix call to git-merge for new command format

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

## tl;dr

7 messages between Dec 1, 2009 and Dec 3, 2009. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Horst H. von Brand· Dec 1, 2009, 22:44 UTC · lore
Signed-off-by: Horst H. von Brand <vonbrand@inf.utfsm.cl>
---
 git-pull.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Show changes to git-pull.sh +2 −2
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
Junio C Hamano· Dec 1, 2009, 23:54 UTC · re: Horst H. von Brand · lore

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

"Horst H. von Brand" <vonbrand@inf.utfsm.cl> writes:
Show 19 quoted lines
> 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?
Michael J Gruber· Dec 2, 2009, 10:02 UTC · re: Junio C Hamano · lore

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

Junio C Hamano venit, vidit, dixit 02.12.2009 00:54:
Show 25 quoted lines
> "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
Junio C Hamano· Dec 2, 2009, 16:45 UTC · re: Michael J Gruber · lore

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

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.
Michael J Gruber· Dec 2, 2009, 17:29 UTC · re: Junio C Hamano · lore

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

Junio C Hamano venit, vidit, dixit 02.12.2009 17:45:
Show 6 quoted lines
> 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
Junio C Hamano· Dec 2, 2009, 17:49 UTC · re: Michael J Gruber · lore

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

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(-)
Show changes to git-pull.sh +3 −3
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
Michael J Gruber· Dec 3, 2009, 08:10 UTC · re: Junio C Hamano · lore

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

Junio C Hamano venit, vidit, dixit 02.12.2009 18:49:
Show 7 quoted lines
> 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.

Show 8 quoted lines
>> 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

← back to recent threads