git merge <tag> behavior
Hi,
While trying to reproduce/understand the problems[1][2] I was facing when using Google's Git repo tool[3], I've found minor problems in Git:
1) there's no mention of the git merge <tag> behavior in git-merge.1
When asking Git to merge a tag (such as a signed tag or annotated tag), it will always create a merge commit even if fast-forward was possible. It's like having --no-ff present on the command line.
It's a difference from the default behavior described in git-merge.1[4]. It should be documented as an exception of "FAST-FORWARD MERGE" section and "--ff" option description.
2) git merge <tag> VS git merge <object-id>
If <tag> is an object (not a lightweight/reference tag), git merge <tag> will by default create a merge commit with the tag message. Additionally, the signature check will be reported as comment, for example:
Merge tag 'v1.12.2' into branch-v1.12.2
repo 1.12.2
# gpg: Signature made Fri Mar 1 18:36:42 2013 CET using DSA key ID 920F5C65
# gpg: Good signature from "Repo Maintainer <[email protected]>"
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg: There is no indication that the signature belongs to the owner.
# Primary key fingerprint: 8BB9 AD79 3E8E 6153 AF0F 9A44 1653 0D5E 920F 5C65But, if you use the tag object-id instead of its name, for example using git merge git show-ref <tag>, the tag is not recognized and the signature is not checked. Git still create a merge commit, but doesn't prepare a commit message with the tag message and the signature:
Merge commit 'ac22c7ae2e652f63366b65ee23122292d3564fff' into
branch-ac22c7ae2e652f63366b65ee23122292d3564fff
It would be great to have Git using the tag message and check the signature.
3) Merge options can't be overridden.
If I modify .git/config to set a merge option, for example forcing fast-forward merge, this option cannot be overridden on command line:
Example 1:
$ cat .git/config:
[branch "master"]
mergeoptions = --ff-only $ git merge --no-ff <tag>
fatal: You cannot combine --no-ff with --ff-onlyExample 2:
$ cat .git/config:
[merge]
ff = only $ git merge --no-ff <tag>
fatal: You cannot combine --no-ff with --ff-onlySetting the merge options in config should overridden by command line.
Regards.
[1] issue 135: repo: repo sync should force fast-forward merge
https://code.google.com/p/git-repo/issues/detail?id=135
[2] Issue 136: repo: repo sync should use the tag name instead of object identifier of the tag
https://code.google.com/p/git-repo/issues/detail?id=136
[3] git-repo - repo - The multiple repository tool
http://code.google.com/p/git-repo/
[4] git-merge(1) Manual Page
https://www.kernel.org/pub/software/scm/git/docs/git-merge.html
Signature · 2 lines
Yann Droneaud OPTEYA
Quoted 9 lines · Yann Droneaud
1) there's no mention of the git merge <tag> behavior in git-merge.1
When asking Git to merge a tag (such as a signed tag or annotated tag), it will always create a merge commit even if fast-forward was possible. It's like having --no-ff present on the command line.
It's a difference from the default behavior described in git-merge.1[4]. It should be documented as an exception of "FAST-FORWARD MERGE" section and "--ff" option description.
Yes; we welcome documentation patches.
Quoted 6 lines
2) git merge <tag> VS git merge <object-id>
If <tag> is an object (not a lightweight/reference tag), git merge <tag>
...
But, if you use the tag object-id instead of its name, for example using git mergegit show-ref <tag>,
"git show-ref <tag>" gives you something like
572a535454612a046e7dd7404dcca94d6243c788 refs/tags/v1.8.2
which is an invalid thing to merge with. Perhaps you meant
git merge $(git rev-parse v1.12.2)
Quoted 5 lines
signature is not checked. Git still create a merge commit, but doesn't prepare a commit message with the tag message and the signature:
It would be great to have Git using the tag message and check the signature.
Perhaps, but if you feed the $(git rev-parse v1.12.2) to merge, your subject will not be able to say "Merge tag 'v1.12.2'" in the first place, so I do not think you would want to encourage such usage in the first place.
Re* git merge <tag> behavior
Quoted 9 lines · Junio C Hamano
Quoted 4 lines
2) git merge <tag> VS git merge <object-id>
...
It would be great to have Git using the tag message and check the signature.Perhaps, but if you feed the $(git rev-parse v1.12.2) to merge, your subject will not be able to say "Merge tag 'v1.12.2'" in the first place, so I do not think you would want to encourage such usage in the first place.
A patch to do so may look like this. You would probably want to also do read_sha1_file(desc->obj->sha1) here and parse out the header line "tag " to replace the "remote" to recover the symbolic tag name, but I didn't bother in this illustration.
builtin/merge.c +13 -0
@@ -516,6 +516,19 @@ static void merge_name(const char *remote, struct strbuf *msg)strbuf_release (&line); goto cleanup; } + +if (remote_head->util) { +struct merge_remote_desc *desc; + desc =merge_remote_util (remote_head); +if (desc && desc->obj && desc->obj->type ==OBJ_TAG ) { +strbuf_addf (msg,"%s\t\t%s '%s'\n" , +sha1_to_hex (desc->obj->sha1), +typename (desc->obj->type), + remote); + goto cleanup; + } + } +strbuf_addf (msg,"%s\t\tcommit '%s'\n" ,sha1_to_hex (remote_head->object.sha1), remote); cleanup:
cover Merging an annotated tag object
This is a patch I posted as an illustration, and then have been carrying in my tree a while, with some tests.
Junio C Hamano (3):
merge: a random object may not necssarily be a commit
t6200: use test_config/test_unconfig
t6200: test message for merging of an annotated tag
builtin/merge.c | 13 ++++++ t/t6200-fmt-merge-msg.sh | 100 ++++++++++++++++++++++++++++++++--------------- 2 files changed, 81 insertions(+), 32 deletions(-)
Signature · 1 line
1.8.2-480-g064f421
PATCH 1/3 merge: a random object may not necssarily be a commit
The user could have said "git merge $(git rev-parse v1.0.0)"; we shouldn't mark it as "Merge commit '15999998fb...'" as the merge name, even though such an invocation might be crazy.
We could even read the "tag " header from the tag object and replace the object name the user gave us, but let's not lose the information by doing so, at least not yet.
--- builtin/merge.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
builtin/merge.c +13 -0
@@ -516,6 +516,19 @@ static void merge_name(const char *remote, struct strbuf *msg)strbuf_release (&line); goto cleanup; } + +if (remote_head->util) { +struct merge_remote_desc *desc; + desc =merge_remote_util (remote_head); +if (desc && desc->obj && desc->obj->type ==OBJ_TAG ) { +strbuf_addf (msg,"%s\t\t%s '%s'\n" , +sha1_to_hex (desc->obj->sha1), +typename (desc->obj->type), + remote); + goto cleanup; + } + } +strbuf_addf (msg,"%s\t\tcommit '%s'\n" ,sha1_to_hex (remote_head->object.sha1), remote); cleanup:
Signature · 1 line
1.8.2-480-g064f421
Re: [PATCH 1/3] merge: a random object may not necssarily be a commit
Hi,
Quoted 9 lines · Junio C Hamano
The user could have said "git merge $(git rev-parse v1.0.0)"; we shouldn't mark it as "Merge commit '
15999998fb...'" as the merge name, even though such an invocation might be crazy.We could even read the "tag " header from the tag object and replace the object name the user gave us, but let's not lose the information by doing so, at least not yet.
Signed-off-by Junio C Hamano <[email protected]>
Thanks for the patch.
I gave it a try and found the behavior rather good.
Merging a tag object by its name or by its object-id are now using the same behavor: it is more consistent.
PS: there's a typo in the commit title :)
Regards.
Signature · 2 lines
Yann Droneaud OPTEYA
Re: [PATCH 1/3] merge: a random object may not necssarily be a commit
In reply to Junio C HamanoQuoted 26 lines · Junio C Hamano
The user could have said "git merge $(git rev-parse v1.0.0)"; we shouldn't mark it as "Merge commit '
15999998fb...'" as the merge name, even though such an invocation might be crazy.We could even read the "tag " header from the tag object and replace the object name the user gave us, but let's not lose the information by doing so, at least not yet.
Signed-off-by Junio C Hamano <[email protected]>--- builtin/merge.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)builtin/merge.c +5
-0@@ -516,6 +516,19 @@ static void merge_name(const char *remote, struct strbuf *msg)strbuf_release (&line); goto cleanup; } + +if (remote_head->util) { +struct merge_remote_desc *desc; + desc =merge_remote_util (remote_head); +if (desc && desc->obj && desc->obj->type ==OBJ_TAG ) {
builtin/merge.c static void merge_name(const char *remote, struct strbuf *msg) · line 533
Quoted 10 lines
builtin/merge.c +8
-0+strbuf_addf (msg,"%s\t\t%s '%s'\n" , +sha1_to_hex (desc->obj->sha1), +typename (desc->obj->type), + remote); + goto cleanup; + } + } +strbuf_addf (msg,"%s\t\tcommit '%s'\n" ,sha1_to_hex (remote_head->object.sha1), remote);
I guess there is no other object type besides OBJ_TAG and OBJ_COMMIT that would yield something we could merge, but it feels weird that you check only for OBJ_TAG here, and otherwise still say "commit". Would the intent be more clear if it just said:
if (desc && desc->obj && desc->obj->type != OBJ_COMMIT) {
...?
-Peff
Re: [PATCH 1/3] merge: a random object may not necssarily be a commit
Quoted 44 lines · Jeff King
Quoted 26 lines · Junio C Hamano
The user could have said "git merge $(git rev-parse v1.0.0)"; we shouldn't mark it as "Merge commit '
15999998fb...'" as the merge name, even though such an invocation might be crazy.We could even read the "tag " header from the tag object and replace the object name the user gave us, but let's not lose the information by doing so, at least not yet.
Signed-off-by Junio C Hamano <[email protected]>--- builtin/merge.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)builtin/merge.c +5
-0@@ -516,6 +516,19 @@ static void merge_name(const char *remote, struct strbuf *msg)strbuf_release (&line); goto cleanup; } + +if (remote_head->util) { +struct merge_remote_desc *desc; + desc =merge_remote_util (remote_head); +if (desc && desc->obj && desc->obj->type ==OBJ_TAG ) {builtin/merge.c static void merge_name(const char *remote, struct strbuf *msg) · line 533
Quoted 10 lines
builtin/merge.c +8
-0+strbuf_addf (msg,"%s\t\t%s '%s'\n" , +sha1_to_hex (desc->obj->sha1), +typename (desc->obj->type), + remote); + goto cleanup; + } + } +strbuf_addf (msg,"%s\t\tcommit '%s'\n" ,sha1_to_hex (remote_head->object.sha1), remote);I guess there is no other object type besides
OBJ_TAGandOBJ_COMMITthat would yield something we could merge, but it feels weird that you check only forOBJ_TAGhere, and otherwise still say "commit". Would the intent be more clear if it just said:
Quoted 4 lines
if (desc && desc->obj && desc->obj->type != OBJ_COMMIT) { ...?
I suspect not.
The point of the added code is that it knows we want to special case merging a tag object, and it wants to keep any other case behaving the same as before.
Re: [PATCH 1/3] merge: a random object may not necssarily be a commit
Quoted 23 lines · Junio C Hamano
Quoted 23 lines
Quoted 4 lines
builtin/merge.c +4
-0+if (remote_head->util) { +struct merge_remote_desc *desc; + desc =merge_remote_util (remote_head); +if (desc && desc->obj && desc->obj->type ==OBJ_TAG ) {builtin/merge.c static void merge_name(const char *remote, struct strbuf *msg) · line 533
Quoted 10 lines
builtin/merge.c +8
-0+strbuf_addf (msg,"%s\t\t%s '%s'\n" , +sha1_to_hex (desc->obj->sha1), +typename (desc->obj->type), + remote); + goto cleanup; + } + } +strbuf_addf (msg,"%s\t\tcommit '%s'\n" ,sha1_to_hex (remote_head->object.sha1), remote);I guess there is no other object type besides
OBJ_TAGandOBJ_COMMITthat would yield something we could merge, but it feels weird that you check only forOBJ_TAGhere, and otherwise still say "commit". Would the intent be more clear if it just said:if (desc && desc->obj && desc->obj->type != OBJ_COMMIT) { ...
Quoted 7 lines
Quoted 1 line
?
I suspect not.
The point of the added code is that it knows we want to special case merging a tag object, and it wants to keep any other case behaving the same as before.
Ah. I read it as "if we have a random object, we do not want to just say "commit X", because it is not a commit.
-Peff
PATCH 2/3 t6200: use test_config/test_unconfig
In reply to Junio C HamanoThe tests were already well protected from previous ones by running "git config --unset" on variables early they do not want to see, but it is easier to make sure they start from a clean state by using more modern test_config/test_unconfig helper functions.
It turns out that the last test depended on the merge.summary configuration previous one leaves behind. Set it explicitly in it.
--- t/t6200-fmt-merge-msg.sh | 61 +++++++++++++++++++++++------------------------- 1 file changed, 29 insertions(+), 32 deletions(-)
t/t6200-fmt-merge-msg.sh +29 -32
@@ -112,8 +112,8 @@ test_expect_success '[merge] summary/log configuration' ' Common #1 EOF - git config merge.log true && - test_might_fail git config --unset-all merge.summary && + test_config merge.log true && + test_unconfig merge.summary && git checkout master && test_tick &&
@@ -121,8 +121,8 @@ test_expect_success '[merge] summary/log configuration' ' git fmt-merge-msg <.git/FETCH_HEAD >actual1 && - test_might_fail git config --unset-all merge.log && - git config merge.summary true && + test_unconfig merge.log && + test_config merge.summary true && git checkout master && test_tick &&
@@ -134,11 +134,6 @@ test_expect_success '[merge] summary/log configuration' ' test_cmp expected actual2 ' -test_expect_success 'setup: clear [merge] configuration' ' - test_might_fail git config --unset-all merge.log && - test_might_fail git config --unset-all merge.summary -' - test_expect_success 'setup FETCH_HEAD' ' git checkout master && test_tick &&
@@ -248,14 +243,14 @@ test_expect_success 'fmt-merge-msg -m' ' Common #1 EOF - test_might_fail git config --unset merge.log && - test_might_fail git config --unset merge.summary && + test_unconfig merge.log && + test_unconfig merge.summary && git checkout master && git fetch "$(pwd)" left && git fmt-merge-msg -m "Sync with left" <.git/FETCH_HEAD >actual && git fmt-merge-msg --log -m "Sync with left" \ <.git/FETCH_HEAD >actual.log && - git config merge.log true && + test_config merge.log true && git fmt-merge-msg -m "Sync with left" \ <.git/FETCH_HEAD >actual.log-config && git fmt-merge-msg --no-log -m "Sync with left" \
@@ -290,29 +285,29 @@ test_expect_success 'setup: expected shortlog for two branches' ' ' test_expect_success 'shortlog for two branches' ' - git config merge.log true && - test_might_fail git config --unset-all merge.summary && + test_config merge.log true && + test_unconfig merge.summary && git checkout master && test_tick && git fetch . left right && git fmt-merge-msg <.git/FETCH_HEAD >actual1 && - test_might_fail git config --unset-all merge.log && - git config merge.summary true && + test_unconfig merge.log && + test_config merge.summary true && git checkout master && test_tick && git fetch . left right && git fmt-merge-msg <.git/FETCH_HEAD >actual2 && - git config merge.log yes && - test_might_fail git config --unset-all merge.summary && + test_config merge.log yes && + test_unconfig merge.summary && git checkout master && test_tick && git fetch . left right && git fmt-merge-msg <.git/FETCH_HEAD >actual3 && - test_might_fail git config --unset-all merge.log && - git config merge.summary yes && + test_unconfig merge.log && + test_config merge.summary yes && git checkout master && test_tick && git fetch . left right &&
@@ -325,8 +320,8 @@ test_expect_success 'shortlog for two branches' ' ' test_expect_success 'merge-msg -F' ' - test_might_fail git config --unset-all merge.log && - git config merge.summary yes && + test_unconfig merge.log && + test_config merge.summary yes && git checkout master && test_tick && git fetch . left right &&
@@ -335,8 +330,8 @@ test_expect_success 'merge-msg -F' ' ' test_expect_success 'merge-msg -F in subdirectory' ' - test_might_fail git config --unset-all merge.log && - git config merge.summary yes && + test_unconfig merge.log && + test_config merge.summary yes && git checkout master && test_tick && git fetch . left right &&
@@ -350,8 +345,8 @@ test_expect_success 'merge-msg -F in subdirectory' ' ' test_expect_success 'merge-msg with nothing to merge' ' - test_might_fail git config --unset-all merge.log && - git config merge.summary yes && + test_unconfig merge.log && + test_config merge.summary yes && >empty &&
@@ -376,8 +371,8 @@ test_expect_success 'merge-msg tag' ' Common #1 EOF - test_might_fail git config --unset-all merge.log && - git config merge.summary yes && + test_unconfig merge.log && + test_config merge.summary yes && git checkout master && test_tick &&
@@ -406,8 +401,8 @@ test_expect_success 'merge-msg two tags' ' Common #1 EOF - test_might_fail git config --unset-all merge.log && - git config merge.summary yes && + test_unconfig merge.log && + test_config merge.summary yes && git checkout master && test_tick &&
@@ -436,8 +431,8 @@ test_expect_success 'merge-msg tag and branch' ' Common #1 EOF - test_might_fail git config --unset-all merge.log && - git config merge.summary yes && + test_unconfig merge.log && + test_config merge.summary yes && git checkout master && test_tick &&
@@ -464,6 +459,8 @@ test_expect_success 'merge-msg lots of commits' ' echo " ..." } >expected && + test_config merge.summary yes && + git checkout master && test_tick && git fetch . long &&
Signature · 1 line
1.8.2-480-g064f421
PATCH 3/3 t6200: test message for merging of an annotated tag
In reply to Junio C Hamano--- t/t6200-fmt-merge-msg.sh | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
t/t6200-fmt-merge-msg.sh +39 -0
@@ -469,4 +469,43 @@ test_expect_success 'merge-msg lots of commits' ' test_cmp expected actual ' +test_expect_success 'merge-msg with "merging" an annotated tag' ' + test_config merge.log true && + + git checkout master^0 && + git commit --allow-empty -m "One step ahead" && + git tag -a -m "An annotated one" annote HEAD && + + git checkout master && + git fetch . annote && + + git fmt-merge-msg <.git/FETCH_HEAD >actual && + { + cat <<-\EOF + Merge tag '\''annote'\'' + + An annotated one + + * tag '\''annote'\'': + One step ahead + EOF + } >expected && + test_cmp expected actual && + + test_when_finished "git reset --hard" && + annote=$(git rev-parse annote) && + git merge --no-commit $annote && + { + cat <<-EOF + Merge tag '\''$annote'\'' + + An annotated one + + * tag '\''$annote'\'': + One step ahead + EOF + } >expected && + test_cmp expected .git/MERGE_MSG +' + test_done
Signature · 1 line
1.8.2-480-g064f421
PATCH 1/1 Documentation: merging a tag is a special case
In reply to Junio C HamanoWhen asking Git to merge a tag (such as a signed tag or annotated tag), it will always create a merge commit even if fast-forward was possible. It's like having --no-ff present on the command line.
It's a difference from the default behavior described in git-merge.txt. It should be documented as an exception of "FAST-FORWARD MERGE" section and "--ff" option description.
--- Documentation/git-merge.txt | 9 +++++++++ Documentation/merge-options.txt | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-)
Documentation/git-merge.txt +9 -0
@@ -170,6 +170,15 @@ happens: If you tried a merge which resulted in complex conflicts and want to start over, you can recover with `git merge --abort`. +MERGING TAG +----------- + +When merging a tag (annotated or signed), Git will create a merge commit +even if a fast-forward merge is possible (see above). +The commit message template will be created from the tag message. +Additionally, the signature check will be reported as a comment +if the tag was signed. See also linkgit:git-tag[1]. + HOW CONFLICTS ARE PRESENTED ---------------------------
Documentation/merge-options.txt +1 -1
@@ -26,7 +26,7 @@ set to `no` at the beginning of them. --ff:: When the merge resolves as a fast-forward, only update the branch pointer, without creating a merge commit. This is the default - behavior. + behavior (except when merging a tag). --no-ff:: Create a merge commit even when the merge resolves as a
Signature · 1 line
1.7.11.7
Re: [PATCH] Documentation: merging a tag is a special case
Quoted 3 lines · Yann Droneaud
When asking Git to merge a tag (such as a signed tag or annotated tag), it will always create a merge commit even if fast-forward was possible. It's like having --no-ff present on the command line.
Thanks. This looks good, modulo some nitpicks.
[...]
Documentation/git-merge.txt happens: · line 176
Quoted 10 lines
Documentation/git-merge.txt +4
-0@@ -170,6 +170,15 @@ happens: If you tried a merge which resulted in complex conflicts and want to start over, you can recover with `git merge --abort`. +MERGING TAG +----------- + +When merging a tag (annotated or signed), Git will create a merge commit
How about something like "When merging an annotated or signed tag" or "When merging an annotated (and possibly signed) tag"? The above text can be misread as meaning "When merging any tag, no matter whether it is annotated or signed", which is needlessly confusing for people who don't know about unannotated tags.
[...]
Documentation/merge-options.txt set to `no` at the beginning of them. · line 29
Quoted 8 lines
Documentation/merge-options.txt +1
-1@@ -26,7 +26,7 @@ set to `no` at the beginning of them. --ff:: When the merge resolves as a fast-forward, only update the branch pointer, without creating a merge commit. This is the default - behavior. + behavior (except when merging a tag).
s/a tag/an annotated tag/ here as well.
By the way, what about the possibility of dropping this implicit --no-ff? I think Linus could get used to passing --no-ff explicitly when responding to pull requests. I could go either way on it.
It is certainly useful to document the current state before considering changing it, so with the tweaks mentioned above,
Re: [PATCH] Documentation: merging a tag is a special case
In reply to Yann DroneaudQuoted 21 lines · Yann Droneaud
When asking Git to merge a tag (such as a signed tag or annotated tag), it will always create a merge commit even if fast-forward was possible. It's like having --no-ff present on the command line.
It's a difference from the default behavior described in git-merge.txt. It should be documented as an exception of "FAST-FORWARD MERGE" section and "--ff" option description.
Signed-off-by Yann Droneaud <[email protected]>--- Documentation/git-merge.txt | 9 +++++++++ Documentation/merge-options.txt | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-)Documentation/git-merge.txt +0
-0@@ -170,6 +170,15 @@ happens: If you tried a merge which resulted in complex conflicts and want to start over, you can recover with `git merge --abort`.
Documentation/git-merge.txt happens: · line 181
Quoted 10 lines
Documentation/git-merge.txt +9
-0+MERGING TAG +----------- + +When merging a tag (annotated or signed), Git will create a merge commit +even if a fast-forward merge is possible (see above). +The commit message template will be created from the tag message. +Additionally, the signature check will be reported as a comment +if the tag was signed. See also linkgit:git-tag[1]. +
It would make it more helpful to readers to describe how _not_ to create such a merge commit if it is unwanted, and how the request to merge a tag interacts with --ff-only option.
Documentation/merge-options.txt set to `no` at the beginning of them. · line 29
Quoted 13 lines
Documentation/git-merge.txt +0
-0HOW CONFLICTS ARE PRESENTED ---------------------------Documentation/merge-options.txt +1
-1@@ -26,7 +26,7 @@ set to `no` at the beginning of them. --ff:: When the merge resolves as a fast-forward, only update the branch pointer, without creating a merge commit. This is the default - behavior. + behavior (except when merging a tag).
With this update, the reader will be left wondering what would be the default when she asks Git to merge a tag, no?
Quoted 2 lines
Documentation/merge-options.txt +0
-0--no-ff:: Create a merge commit even when the merge resolves as a
Re: [PATCH] Documentation: merging a tag is a special case
Quoted 12 lines · Junio C Hamano
Review
Quoted 7 lines
(quoted diff) +7
-0+MERGING TAG +----------- + +When merging a tag (annotated or signed), Git will create a merge commit +... +if the tag was signed. See also linkgit:git-tag[1]. +It would make it more helpful to readers to describe how _not_ to create such a merge commit if it is unwanted, and how the request to merge a tag interacts with --ff-only option.
Quoted 9 lines
Review set to `no` at the beginning of them. · line 29
Quoted 6 lines
(quoted diff) +1
-1@@ -26,7 +26,7 @@ set to `no` at the beginning of them. --ff:: When the merge resolves as a fast-forward, only update the branch pointer, without creating a merge commit. This is the default - behavior. + behavior (except when merging a tag).With this update, the reader will be left wondering what would be the default when she asks Git to merge a tag, no?
Taking Jonathan's input and the above into account, perhaps we can do something like this on top of the posted patch?
Documentation/git-merge.txt | 26 +++++++++++++++++++++----- Documentation/merge-options.txt | 5 +++-- 2 files changed, 24 insertions(+), 7 deletions(-)
Documentation/git-merge.txt +21 -5
@@ -173,11 +173,27 @@ want to start over, you can recover with `git merge --abort`. MERGING TAG ----------- -When merging a tag (annotated or signed), Git will create a merge commit -even if a fast-forward merge is possible (see above). -The commit message template will be created from the tag message. -Additionally, the signature check will be reported as a comment -if the tag was signed. See also linkgit:git-tag[1]. +When merging an annotated (and possibly signed) tag, Git always +creates a merge commit even if a fast-forward merge is possible, and +the commit message template is prepared with the tag message. +Additionally, the signature check is reported as a comment +if the tag is signed. See also linkgit:git-tag[1]. + +Consequently a request `git merge --ff-only v1.2.3` to merge such a +tag would fail. + +When you want to just integrate with the work leading to the commit +that happens to be tagged, e.g. synchronizing with an upstream +release point, you may not want to make an unnecessary merge commit +especially when you do not have any work on your own. In such a +case, you can "unwrap" the tag yourself before feeding it to `git +merge`, e.g. + +--- +git fetch origin +git merge [--ff-only] v1.2.3^0 +--- + HOW CONFLICTS ARE PRESENTED ---------------------------
Documentation/merge-options.txt +3 -2
@@ -26,11 +26,12 @@ set to `no` at the beginning of them. --ff:: When the merge resolves as a fast-forward, only update the branch pointer, without creating a merge commit. This is the default - behavior (except when merging a tag). + behavior. --no-ff:: Create a merge commit even when the merge resolves as a - fast-forward. + fast-forward. This is the default behaviour when merging an + annotated (and possibly signed) tag. --ff-only:: Refuse to merge and exit with a non-zero status unless the
Re: [PATCH] Documentation: merging a tag is a special case
Quoted 4 lines · Junio C Hamano
Documentation/git-merge.txt +4
-0+Consequently a request `git merge --ff-only v1.2.3` to merge such a +tag would fail. + +When you want to just integrate with the work leading to the commit
Documentation/git-merge.txt want to start over, you can recover with `git merge --abort`. · line 195
Quoted 10 lines
Documentation/git-merge.txt +10
-0+that happens to be tagged, e.g. synchronizing with an upstream +release point, you may not want to make an unnecessary merge commit +especially when you do not have any work on your own. In such a +case, you can "unwrap" the tag yourself before feeding it to `git +merge`, e.g. + +--- +git fetch origin +git merge [--ff-only] v1.2.3^0 +---
Nice and clear, but doesn't this contradict b5c9f1c1b0ed (merge: do not create a signed tag merge under --ff-only option, 2012-02-05)?
Re: [PATCH] Documentation: merging a tag is a special case
Quoted 2 lines · Jonathan Nieder
Nice and clear, but doesn't this contradict
b5c9f1c1b0ed(merge: do not create a signed tag merge under --ff-only option, 2012-02-05)?
It does X-<. Here is a replacement.
The "--ff-only v1.2.3 will fail" can be left unsaid because it would fail (and succeed) under the same condition "-ff-only v1.2.3^0" would.
Documentation/git-merge.txt | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)
Documentation/git-merge.txt +7 -8
@@ -179,19 +179,18 @@ the commit message template is prepared with the tag message. Additionally, the signature check is reported as a comment if the tag is signed. See also linkgit:git-tag[1]. -Consequently a request `git merge --ff-only v1.2.3` to merge such a -tag would fail. - When you want to just integrate with the work leading to the commit that happens to be tagged, e.g. synchronizing with an upstream -release point, you may not want to make an unnecessary merge commit -especially when you do not have any work on your own. In such a -case, you can "unwrap" the tag yourself before feeding it to `git -merge`, e.g. +release point, you may not want to make an unnecessary merge commit. + +In such a case, you can "unwrap" the tag yourself before feeding it +to `git merge`, or pass `--ff-only` when you do not have any work on +your own. e.g. --- git fetch origin -git merge [--ff-only] v1.2.3^0 +git merge v1.2.3^0 +git merge --ff-only v1.2.3 ---
Re: [PATCH] Documentation: merging a tag is a special case
Hi,
Just a little change I made on my own.
The other part are definitely better than my version, so I propose to merge all the patches in the thread with you as author, putting Jonathan Nieder and myself as reviewers.
Regards
Documentation/git-merge.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Documentation/git-merge.txt +2 -2
@@ -176,8 +176,8 @@ MERGING TAG When merging an annotated (and possibly signed) tag, Git always creates a merge commit even if a fast-forward merge is possible, and the commit message template is prepared with the tag message. -Additionally, the signature check is reported as a comment -if the tag is signed. See also linkgit:git-tag[1]. +Additionally, if the tag is signed, the signature check is reported +as a comment in the message template. See also linkgit:git-tag[1]. When you want to just integrate with the work leading to the commit that happens to be tagged, e.g. synchronizing with an upstream
Signature · 1 line
1.7.11.7
PATCH v2 1/1 Documentation: merging a tag is a special case
From: Junio C Hamano <[email protected]>
When asking Git to merge a tag (such as a signed tag or annotated tag), it will always create a merge commit even if fast-forward was possible. It's like having --no-ff present on the command line.
It's a difference from the default behavior described in git-merge.txt. It should be documented as an exception of "FAST-FORWARD MERGE" section and "--ff" option description.
--- Documentation/git-merge.txt | 24 ++++++++++++++++++++++++ Documentation/merge-options.txt | 3 ++- 2 files changed, 26 insertions(+), 1 deletion(-)
Documentation/git-merge.txt +24 -0
@@ -170,6 +170,30 @@ happens: If you tried a merge which resulted in complex conflicts and want to start over, you can recover with `git merge --abort`. +MERGING TAG +----------- + +When merging an annotated (and possibly signed) tag, Git always +creates a merge commit even if a fast-forward merge is possible, and +the commit message template is prepared with the tag message. +Additionally, if the tag is signed, the signature check is reported +as a comment in the message template. See also linkgit:git-tag[1]. + +When you want to just integrate with the work leading to the commit +that happens to be tagged, e.g. synchronizing with an upstream +release point, you may not want to make an unnecessary merge commit. + +In such a case, you can "unwrap" the tag yourself before feeding it +to `git merge`, or pass `--ff-only` when you do not have any work on +your own. e.g. + +--- +git fetch origin +git merge v1.2.3^0 +git merge --ff-only v1.2.3 +--- + + HOW CONFLICTS ARE PRESENTED ---------------------------
Documentation/merge-options.txt +2 -1
@@ -30,7 +30,8 @@ set to `no` at the beginning of them. --no-ff:: Create a merge commit even when the merge resolves as a - fast-forward. + fast-forward. This is the default behaviour when merging an + annotated (and possibly signed) tag. --ff-only:: Refuse to merge and exit with a non-zero status unless the
Signature · 1 line
1.7.11.7
Re: [PATCH v2] Documentation: merging a tag is a special case
Quoted 1 line · Yann Droneaud
Reviewed-by Jonathan Nieder <[email protected]>
Yes, I think this is in good shape now.
Re: [PATCH] Documentation: merging a tag is a special case
In reply to Junio C HamanoQuoted 1 line · Junio C Hamano
Here is a replacement.
Looks good. Thanks for taking care of this.
Hi,
Quoted 6 lines · Junio C Hamano
Quoted 1 line · Yann Droneaud
1) there's no mention of the git merge <tag> behavior in git-merge.1
Yes; we welcome documentation patches.
Sent.
Quoted 6 lines
Quoted 6 lines
2) git merge <tag> VS git merge <object-id>
If <tag> is an object (not a lightweight/reference tag), git merge <tag>
...
But, if you use the tag object-id instead of its name, for example using git mergegit parse-rev <tag>,
[EDIT]
Quoted 10 lines
Quoted 5 lines
signature is not checked. Git still create a merge commit, but doesn't prepare a commit message with the tag message and the signature:
It would be great to have Git using the tag message and check the signature.
Perhaps, but if you feed the $(git rev-parse v1.12.2) to merge, your subject will not be able to say "Merge tag 'v1.12.2'" in the first place, so I do not think you would want to encourage such usage in the first place.
I think if someone want to merge the tag object-id instead of the tag, the commit subject/message should probably not make a reference to the tag.
The only use case for such tag merging by commit-id would be to get consistent behavior in case of tag deletion. The named tag could be recreated to point to another point in time. So when looking at the merge commit message and searching for the tag (by name) could be misleading.
PS: and what about my third issue ?
Regards.
Signature · 2 lines
Yann Droneaud OPTEYA
Hi,
Quoted 22 lines · Yann Droneaud
Quoted 6 lines
Quoted 6 lines
2) git merge <tag> VS git merge <object-id>
If <tag> is an object (not a lightweight/reference tag), git merge <tag>
...
But, if you use the tag object-id instead of its name, for example using git mergegit parse-rev <tag>,[EDIT]
Quoted 10 lines
Quoted 5 lines
signature is not checked. Git still create a merge commit, but doesn't prepare a commit message with the tag message and the signature:
It would be great to have Git using the tag message and check the signature.
Perhaps, but if you feed the $(git rev-parse v1.12.2) to merge, your subject will not be able to say "Merge tag 'v1.12.2'" in the first place, so I do not think you would want to encourage such usage in the first place.
I think if someone want to merge the tag object-id instead of the tag, the commit subject/message should probably not make a reference to the tag.
Quoted 6 lines
The only use case for such tag merging by commit-id would be to get consistent behavior in case of tag deletion. The named tag could be recreated to point to another point in time. So when looking at the merge commit message and searching for the tag (by name) could be misleading.
But but do not take those remarks as a feature request.
I was just asking for clarification/comment on the behavior difference between merging tag/tag object-id.
Regards
Signature · 2 lines
Yann Droneaud OPTEYA
Quoted 3 lines · Yann Droneaud
But but do not take those remarks as a feature request.
I was just asking for clarification/comment on the behavior difference between merging tag/tag object-id.
If you are asking why things are as they are, the answer is simply because "git merge $(git rev-parse v1.2.3)" was not even considered while adding the support to pull signed tags.
We did find the use case for "git merge v1.2.3" interesting and important enough to give it a proper support with defined semantics. "git merge $(git rev-parse v1.2.3)" may behave differently but it was not because we found the use case for it important and designed a behaviour that is different from merging the tag by name that suits that use case.
It is just we didn't even think giving the bare object name to name an annotated or signed tag on the command line is interesting, and the command does whatever the implementation happens to do to such an input.
I think I sent out a "how about this" patch. Have you tried it?
PATCH 1/1 t7600: merge tag shoud create a merge commit
In reply to Yann DroneaudFrom: Yann Droneaud <[email protected]>
This test ensures a merge commit is always created when merging an annotated (signed) tag without --ff-only option.
---
Here's a proposition for a test tath check the creation of a merge commit when merging a tag.
It's not in final shape: the line
EDITOR=false test_must_fail git merge signed
should failed, but doesn't: the commit merge is created with the default message, just like --no-edit was given.
I'm making a mistake somewhere, since the EDITOR=false trick works, it's even used in the next test, for --no-edit testing.
Regards.
t/t7600-merge.sh | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)
t/t7600-merge.sh +21 -1
@@ -697,10 +697,30 @@ test_expect_success 'merge --no-ff --edit' ' test_cmp actual expected ' +test_expect_failure GPG 'merge tag should create a merge commit' ' + git reset --hard c0 && + git commit --allow-empty -m "A newer commit" && + git tag -f -s -m "A newer commit" signed && + git reset --hard c0 && + + git merge signed && + git rev-parse signed^0 >expect && + git rev-parse HEAD^0 >actual && + ! test_cmp actual expect && + + git reset --hard c0 && + git commit --allow-empty -m "An other newer commit" && + git tag -f -s -m "An other newer commit" signed && + git reset --hard c0 && + + EDITOR=false test_must_fail git merge signed && + verify_head "$c0" +' + test_expect_success GPG 'merge --ff-only tag' ' git reset --hard c0 && git commit --allow-empty -m "A newer commit" && - git tag -s -m "A newer commit" signed && + git tag -f -s -m "A newer commit" signed && git reset --hard c0 && git merge --ff-only signed &&
Signature · 1 line
1.7.11.7
Quoted 6 lines · Yann Droneaud <ydroneaud <at> opteya.com> writes:
3) Merge options can't be overridden.
If I modify .git/config to set a merge option, for example forcing fast-forward merge, this option cannot be overridden on command line:
(git merge --no-ff-only --no-ff) should work. The --no-ff-only overrides the --ff-only configuration setting, and the --no-ff ensures that the merge is not a fast-forward.
Hi,
Le 21.03.2013 21:31, Max Nanasy a écrit :
Quoted 5 lines
Quoted 3 lines · Yann Droneaud <ydroneaud <at> opteya.com> writes:
3) Merge options can't be overridden.
Quoted 8 lines
Quoted 3 lines
If I modify .git/config to set a merge option, for example forcing fast-forward merge, this option cannot be overridden on command line:
(git merge --no-ff-only --no-ff) should work. The --no-ff-only overrides the --ff-only configuration setting, and the --no-ff ensures that the merge is not a fast-forward.
Thanks. I wasn't aware of the --no-ff-only option and thought --no-ff would be the opposite of --ff-only, or at least disable it given the order of the options.
Please find a patch to document option --no-ff-only
Documentation/merge-options.txt | 4 ++++ 1 file changed, 4 insertions(+)
(quoted diff) +0 -0
b/Documentation/merge-options.txt
index 0bcbe0a..20a31cf 100644
Documentation/merge-options.txt +4 -0
@@ -37,6 +37,10 @@ set to `no` at the beginning of them. current `HEAD` is already up-to-date or the merge can be resolved as a fast-forward. +--no-ff-only:: + Disable `--ff-only` behavior, eg. allows creation of merge commit. + This is the default behavior. + --log[=<n>]:: --no-log:: In addition to branch names, populate the log message with
PATCH 1/1 t7600: test merge configuration override
Set the configuration variable 'merge.ff' to either 'only' or 'no' and check that this configuration can be overridden on command line.
Additionally, test for currently not tested option '--no-ff-only'
--- t/t7600-merge.sh | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
t/t7600-merge.sh +26 -0
@@ -254,6 +254,32 @@ test_expect_success 'merges with merge.ff=only' ' verify_head $c3 ' +test_expect_success 'merges with merge.ff=only and --no-ff-only' ' + git reset --hard c1 && + test_tick && + test_when_finished "git config --unset merge.ff" && + git config merge.ff only && + test_must_fail git merge --no-ff c2 && + git merge --no-ff-only c2 && + + git reset --hard c1 && + git merge --no-ff-only --no-ff c2 +' + +test_expect_success 'merges with merge.ff=no and --ff' ' + git reset --hard c0 && + test_tick && + test_when_finished "git config --unset merge.ff" && + git config merge.ff no && + test_must_fail git merge --ff-only c1 && + git merge --ff c1 && + verify_head $c1 && + + git reset --hard c0 && + git merge --ff --ff-only c1 && + verify_head $c1 +' + test_expect_success 'merge c0 with c1 (no-commit)' ' git reset --hard c0 && git merge --no-commit c1 &&
Signature · 1 line
1.7.11.7
Re: [PATCH] t7600: test merge configuration override
t/t7600-merge.sh test_expect_success 'merges with merge.ff=only' ' · line 261
Quoted 5 lines · Yann Droneaud
t/t7600-merge.sh +5
-0+test_expect_success 'merges with merge.ff=only and --no-ff-only' ' + git reset --hard c1 && + test_tick && + test_when_finished "git config --unset merge.ff" && + git config merge.ff only &&
I see this was copied from existing tests, but we should use "test_config" these days. It would be a good approach to first do a preparatory patch to convert the existing ones to use "test_config" and then to redo this patch using "test_config" on top of it.
cover Use test_config
Please find some patches to use test_config/test_unconfig
Instead of using construct such as:
test_when_finished "git config --unset <key>"
git config <key> <value>
uses
test_config <key> <value>
The latter takes care of removing <key> at the end of the test.
Additionally, instead of
git config <key> ""
or
git config --unset <key>
uses
test_unconfig <key>
The latter doesn't failed if <key> is not defined.
Patch "t7600: use test_config to set/unset git config variables" is more important than the other and must be carefully reviewed regarded to the --no-log --no-ff behavior.
Others patches are fairly simple.
Testsuite results are the same after the patches. Tested against master, 7b592fadf1e23b10b913e0771b9f711770597266
Yann Droneaud (15):
t4018: remove test_config implementation
t7810: remove test_config implementation
t7811: remove test_config implementation
t3400: use test_config to set/unset git config variables
t4304: use test_config to set/unset git config variables
t4034: use test_config/test_unconfig to set/unset git config
variables
t4202: use test_config/test_unconfig to set/unset git config
variables
t5520: use test_config to set/unset git config variables
t5541: use test_config to set/unset git config variables
t7500: use test_config to set/unset git config variables
t7502: use test_config to set/unset git config variables
t7508: use test_config to set/unset git config variables
t7600: use test_config to set/unset git config variables
t9500: use test_config to set/unset git config variables
t7502: remove clear_config
t/t3400-rebase.sh | 3 +- t/t3404-rebase-interactive.sh | 3 +- t/t4018-diff-funcname.sh | 5 --- t/t4034-diff-words.sh | 7 ++-- t/t4202-log.sh | 28 +++++----------- t/t5520-pull.sh | 12 +++---- t/t5541-http-push.sh | 3 +- t/t7500-commit.sh | 6 ++-- t/t7502-commit.sh | 40 +++++------------------ t/t7508-status.sh | 46 +++++++++----------------- t/t7600-merge.sh | 60 +++++++++++++++------------------- t/t7810-grep.sh | 5 --- t/t7811-grep-open.sh | 5 --- t/t9500-gitweb-standalone-no-errors.sh | 3 +- 14 files changed, 72 insertions(+), 154 deletions(-)
Signature · 1 line
1.7.11.7
PATCH 1/15 t4018: remove test_config implementation
test_config is provided by the test library,
a private version is not needed.
--- t/t4018-diff-funcname.sh | 5 ----- 1 file changed, 5 deletions(-)
t/t4018-diff-funcname.sh +0 -5
@@ -93,11 +93,6 @@ sed -e ' s/song;/song();/ ' <Beer.perl >Beer-correct.perl -test_config () { - git config "$1" "$2" && - test_when_finished "git config --unset $1" -} - test_expect_funcname () { lang=${2-java} test_expect_code 1 git diff --no-index -U1 \
Signature · 1 line
1.7.11.7
PATCH 2/15 t7810: remove test_config implementation
In reply to Yann Droneaudtest_config is provided by the test library,
a private version is not needed.
--- t/t7810-grep.sh | 5 ----- 1 file changed, 5 deletions(-)
t/t7810-grep.sh +0 -5
@@ -1084,11 +1084,6 @@ test_expect_success 'grep -E pattern with grep.patternType=fixed' ' test_cmp expected actual ' -test_config() { - git config "$1" "$2" && - test_when_finished "git config --unset $1" -} - cat >expected <<EOF hello.c<RED>:<RESET>int main(int argc, const char **argv) hello.c<RED>-<RESET>{
Signature · 1 line
1.7.11.7
PATCH 3/15 t7811: remove test_config implementation
In reply to Yann Droneaudtest_config is provided by the test library,
a private version is not needed.
--- t/t7811-grep-open.sh | 5 ----- 1 file changed, 5 deletions(-)
t/t7811-grep-open.sh +0 -5
@@ -125,11 +125,6 @@ test_expect_success 'modified file' ' test_cmp empty out ' -test_config() { - git config "$1" "$2" && - test_when_finished "git config --unset $1" -} - test_expect_success 'copes with color settings' ' rm -f actual && echo grep.h >expect &&
Signature · 1 line
1.7.11.7
PATCH 4/15 t3400: use test_config to set/unset git config variables
In reply to Yann DroneaudInstead of using construct such as:
test_when_finished "git config --unset <key>"
git config <key> <value>
uses
test_config <key> <value>
The latter takes care of removing <key> at the end of the test.
--- t/t3400-rebase.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
t/t3400-rebase.sh +1 -2
@@ -138,8 +138,7 @@ test_expect_success 'rebase a single mode change' ' ' test_expect_success 'rebase is not broken by diff.renames' ' - git config diff.renames copies && - test_when_finished "git config --unset diff.renames" && + test_config diff.renames copies && git checkout filemove && GIT_TRACE=1 git rebase force-3way '
Signature · 1 line
1.7.11.7
PATCH 5/15 t4304: use test_config to set/unset git config variables
In reply to Yann DroneaudInstead of using construct such as:
test_when_finished "git config --unset <key>"
git config <key> <value>
uses
test_config <key> <value>
The latter takes care of removing <key> at the end of the test.
Tests are modified to assume correct (default) configuration at entry, and to reset the modified configuration variables at the end.
--- t/t3404-rebase-interactive.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
t/t3404-rebase-interactive.sh +1 -2
@@ -937,8 +937,7 @@ test_expect_success 'rebase --edit-todo can be used to modify todo' ' test_expect_success 'rebase -i respects core.commentchar' ' git reset --hard && git checkout E^0 && - git config core.commentchar "\\" && - test_when_finished "git config --unset core.commentchar" && + test_config core.commentchar "\\" && write_script remove-all-but-first.sh <<-\EOF && sed -e "2,\$s/^/\\\\/" "$1" >"$1.tmp" && mv "$1.tmp" "$1"
Signature · 1 line
1.7.11.7
PATCH 6/15 t4034: use test_config/test_unconfig to set/unset git config variables
In reply to Yann DroneaudInstead of using construct such as:
test_when_finished "git config --unset <key>"
git config <key> <value>
uses
test_config <key> <value>
The latter takes care of removing <key> at the end of the test.
Additionally, instead of
git config <key> ""
or
git config --unset <key>
uses
test_unconfig <key>
The latter doesn't failed if <key> is not defined.
--- t/t4034-diff-words.sh | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
t/t4034-diff-words.sh +3 -4
@@ -230,7 +230,7 @@ test_expect_success '.gitattributes override config' ' ' test_expect_success 'setup: remove diff driver regex' ' - test_might_fail git config --unset diff.testdriver.wordRegex + test_unconfig diff.testdriver.wordRegex ' test_expect_success 'use configured regex' '
@@ -335,8 +335,7 @@ test_expect_success 'word-diff with diff.sbe' ' c EOF - test_when_finished "git config --unset diff.suppress-blank-empty" && - git config diff.suppress-blank-empty true && + test_config diff.suppress-blank-empty true && word_diff --word-diff=plain '
@@ -368,7 +367,7 @@ test_expect_success 'setup history with two files' ' test_expect_success 'wordRegex for the first file does not apply to the second' ' echo "*.tex diff=tex" >.gitattributes && - git config diff.tex.wordRegex "[a-z]+|." && + test_config diff.tex.wordRegex "[a-z]+|." && cat >expect <<-\EOF && diff --git a/a.tex b/a.tex --- a/a.tex
Signature · 1 line
1.7.11.7
PATCH 7/15 t4202: use test_config/test_unconfig to set/unset git config variables
In reply to Yann DroneaudInstead of using construct such as:
test_when_finished "git config --unset <key>"
git config <key> <value>
uses
test_config <key> <value>
The latter takes care of removing <key> at the end of the test.
Additionally, instead of
git config <key> ""
or
git config --unset <key>
uses
test_unconfig <key>
The latter doesn't failed if <key> is not defined.
Tests are modified to assume correct (default) configuration at entry, and to reset the modified configuration variables at the end.
--- t/t4202-log.sh | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-)
t/t4202-log.sh +8 -20
@@ -419,8 +419,6 @@ test_expect_success 'log --graph with merge' ' ' test_expect_success 'log.decorate configuration' ' - test_might_fail git config --unset-all log.decorate && - git log --oneline >expect.none && git log --oneline --decorate >expect.short && git log --oneline --decorate=full >expect.full &&
@@ -429,8 +427,7 @@ test_expect_success 'log.decorate configuration' ' git log --oneline >actual && test_cmp expect.short actual && - git config --unset-all log.decorate && - git config log.decorate true && + test_config log.decorate true && git log --oneline >actual && test_cmp expect.short actual && git log --oneline --decorate=full >actual &&
@@ -438,8 +435,7 @@ test_expect_success 'log.decorate configuration' ' git log --oneline --decorate=no >actual && test_cmp expect.none actual && - git config --unset-all log.decorate && - git config log.decorate no && + test_config log.decorate no && git log --oneline >actual && test_cmp expect.none actual && git log --oneline --decorate >actual &&
@@ -447,8 +443,7 @@ test_expect_success 'log.decorate configuration' ' git log --oneline --decorate=full >actual && test_cmp expect.full actual && - git config --unset-all log.decorate && - git config log.decorate 1 && + test_config log.decorate 1 && git log --oneline >actual && test_cmp expect.short actual && git log --oneline --decorate=full >actual &&
@@ -456,8 +451,7 @@ test_expect_success 'log.decorate configuration' ' git log --oneline --decorate=no >actual && test_cmp expect.none actual && - git config --unset-all log.decorate && - git config log.decorate short && + test_config log.decorate short && git log --oneline >actual && test_cmp expect.short actual && git log --oneline --no-decorate >actual &&
@@ -465,8 +459,7 @@ test_expect_success 'log.decorate configuration' ' git log --oneline --decorate=full >actual && test_cmp expect.full actual && - git config --unset-all log.decorate && - git config log.decorate full && + test_config log.decorate full && git log --oneline >actual && test_cmp expect.full actual && git log --oneline --no-decorate >actual &&
@@ -474,16 +467,15 @@ test_expect_success 'log.decorate configuration' ' git log --oneline --decorate >actual && test_cmp expect.short actual - git config --unset-all log.decorate && + test_unconfig log.decorate && git log --pretty=raw >expect.raw && - git config log.decorate full && + test_config log.decorate full && git log --pretty=raw >actual && test_cmp expect.raw actual ' test_expect_success 'reflog is expected format' ' - test_might_fail git config --remove-section log && git log -g --abbrev-commit --pretty=oneline >expect && git reflog >actual && test_cmp expect actual
@@ -496,10 +488,6 @@ test_expect_success 'whatchanged is expected format' ' ' test_expect_success 'log.abbrevCommit configuration' ' - test_when_finished "git config --unset log.abbrevCommit" && - - test_might_fail git config --unset log.abbrevCommit && - git log --abbrev-commit >expect.log.abbrev && git log --no-abbrev-commit >expect.log.full && git log --pretty=raw >expect.log.raw &&
@@ -508,7 +496,7 @@ test_expect_success 'log.abbrevCommit configuration' ' git whatchanged --abbrev-commit >expect.whatchanged.abbrev && git whatchanged --no-abbrev-commit >expect.whatchanged.full && - git config log.abbrevCommit true && + test_config log.abbrevCommit true && git log >actual && test_cmp expect.log.abbrev actual &&
Signature · 1 line
1.7.11.7
PATCH 8/15 t5520: use test_config to set/unset git config variables
In reply to Yann DroneaudInstead of using construct such as:
test_when_finished "git config --unset <key>"
git config <key> <value>
uses
test_config <key> <value>
The latter takes care of removing <key> at the end of the test.
--- t/t5520-pull.sh | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)
t/t5520-pull.sh +4 -8
@@ -96,8 +96,7 @@ test_expect_success '--rebase' ' ' test_expect_success 'pull.rebase' ' git reset --hard before-rebase && - git config --bool pull.rebase true && - test_when_finished "git config --unset pull.rebase" && + test_config pull.rebase true && git pull . copy && test $(git rev-parse HEAD^) = $(git rev-parse copy) && test new = $(git show HEAD:file2)
@@ -105,8 +104,7 @@ test_expect_success 'pull.rebase' ' test_expect_success 'branch.to-rebase.rebase' ' git reset --hard before-rebase && - git config --bool branch.to-rebase.rebase true && - test_when_finished "git config --unset branch.to-rebase.rebase" && + test_config branch.to-rebase.rebase true && git pull . copy && test $(git rev-parse HEAD^) = $(git rev-parse copy) && test new = $(git show HEAD:file2)
@@ -114,10 +112,8 @@ test_expect_success 'branch.to-rebase.rebase' ' test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' git reset --hard before-rebase && - git config --bool pull.rebase true && - test_when_finished "git config --unset pull.rebase" && - git config --bool branch.to-rebase.rebase false && - test_when_finished "git config --unset branch.to-rebase.rebase" && + test_config pull.rebase true && + test_config branch.to-rebase.rebase false && git pull . copy && test $(git rev-parse HEAD^) != $(git rev-parse copy) && test new = $(git show HEAD:file2)
Signature · 1 line
1.7.11.7
PATCH 9/15 t5541: use test_config to set/unset git config variables
In reply to Yann DroneaudInstead of using construct such as:
test_when_finished "git config --unset <key>"
git config <key> <value>
uses
test_config <key> <value>
The latter takes care of removing <key> at the end of the test.
--- t/t5541-http-push.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
t/t5541-http-push.sh +1 -2
@@ -181,8 +181,7 @@ test_expect_success 'push (chunked)' ' git checkout master && test_commit commit path3 && HEAD=$(git rev-parse --verify HEAD) && - git config http.postbuffer 4 && - test_when_finished "git config --unset http.postbuffer" && + test_config http.postbuffer 4 && git push -v -v origin $BRANCH 2>err && grep "POST git-receive-pack (chunked)" err && (cd "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git &&
Signature · 1 line
1.7.11.7
PATCH 10/15 t7500: use test_config to set/unset git config variables
In reply to Yann DroneaudInstead of using construct such as:
test_when_finished "git config --unset <key>"
git config <key> <value>
uses
test_config <key> <value>
The latter takes care of removing <key> at the end of the test.
--- t/t7500-commit.sh | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
t/t7500-commit.sh +2 -4
@@ -36,8 +36,7 @@ test_expect_success 'nonexistent template file should return error' ' ' test_expect_success 'nonexistent template file in config should return error' ' - git config commit.template "$PWD"/notexist && - test_when_finished "git config --unset commit.template" && + test_config commit.template "$PWD"/notexist && ( GIT_EDITOR="echo hello >\"\$1\"" && export GIT_EDITOR &&
@@ -93,14 +92,13 @@ test_expect_success '-t option should be short for --template' ' test_expect_success 'config-specified template should commit' ' echo "new template" > "$TEMPLATE" && - git config commit.template "$TEMPLATE" && + test_config commit.template "$TEMPLATE" && echo "more content" >> foo && git add foo && ( test_set_editor "$TEST_DIRECTORY"/t7500/add-content && git commit ) && - git config --unset commit.template && commit_msg_is "new templatecommit message" '
Signature · 1 line
1.7.11.7
PATCH 11/15 t7502: use test_config to set/unset git config variables
In reply to Yann DroneaudInstead of using construct such as:
test_when_finished "git config --unset <key>"
git config <key> <value>
uses
test_config <key> <value>
The latter takes care of removing <key> at the end of the test.
--- t/t7502-commit.sh | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
t/t7502-commit.sh +3 -5
@@ -171,10 +171,9 @@ test_expect_success 'verbose' ' test_expect_success 'verbose respects diff config' ' - git config color.diff always && + test_config color.diff always && git status -v >actual && - grep "\[1mdiff --git" actual && - git config --unset color.diff + grep "\[1mdiff --git" actual ' mesg_with_comment_and_newlines='
@@ -534,8 +533,7 @@ use_template="-t template" try_commit_status_combo test_expect_success 'commit --status with custom comment character' ' - test_when_finished "git config --unset core.commentchar" && - git config core.commentchar ";" && + test_config core.commentchar ";" && try_commit --status && test_i18ngrep "^; Changes to be committed:" .git/COMMIT_EDITMSG '
Signature · 1 line
1.7.11.7
PATCH 12/15 t7508: use test_config to set/unset git config variables
In reply to Yann DroneaudInstead of using construct such as:
test_when_finished "git config --unset <key>"
git config <key> <value>
uses
test_config <key> <value>
The latter takes care of removing <key> at the end of the test.
--- t/t7508-status.sh | 46 ++++++++++++++++------------------------------ 1 file changed, 16 insertions(+), 30 deletions(-)
t/t7508-status.sh +16 -30
@@ -131,8 +131,7 @@ cat >expect <<\EOF EOF test_expect_success 'status (advice.statusHints false)' ' - test_when_finished "git config --unset advice.statusHints" && - git config advice.statusHints false && + test_config advice.statusHints false && git status >output && test_i18ncmp expect output
@@ -332,8 +331,7 @@ test_expect_success 'status -uno' ' ' test_expect_success 'status (status.showUntrackedFiles no)' ' - git config status.showuntrackedfiles no - test_when_finished "git config --unset status.showuntrackedfiles" && + test_config status.showuntrackedfiles no && git status >output && test_i18ncmp expect output '
@@ -348,12 +346,11 @@ cat >expect <<EOF # # Untracked files not listed EOF -git config advice.statusHints false test_expect_success 'status -uno (advice.statusHints false)' ' + test_config advice.statusHints false && git status -uno >output && test_i18ncmp expect output ' -git config --unset advice.statusHints cat >expect << EOF M dir1/modified
@@ -400,8 +397,7 @@ test_expect_success 'status -unormal' ' ' test_expect_success 'status (status.showUntrackedFiles normal)' ' - git config status.showuntrackedfiles normal - test_when_finished "git config --unset status.showuntrackedfiles" && + test_config status.showuntrackedfiles normal git status >output && test_i18ncmp expect output '
@@ -459,8 +455,7 @@ test_expect_success 'status -uall' ' ' test_expect_success 'status (status.showUntrackedFiles all)' ' - git config status.showuntrackedfiles all - test_when_finished "git config --unset status.showuntrackedfiles" && + test_config status.showuntrackedfiles all git status >output && test_i18ncmp expect output '
@@ -485,10 +480,9 @@ test_expect_success 'status -s -uall' ' test_cmp expect output ' test_expect_success 'status -s (status.showUntrackedFiles all)' ' - git config status.showuntrackedfiles all + test_config status.showuntrackedfiles all && git status -s >output && rm -rf dir3 && - git config --unset status.showuntrackedfiles && test_cmp expect output '
@@ -588,15 +582,13 @@ cat >expect <<\EOF EOF test_expect_success 'status with color.ui' ' - git config color.ui always && - test_when_finished "git config --unset color.ui" && + test_config color.ui always && git status | test_decode_color >output && test_i18ncmp expect output ' test_expect_success 'status with color.status' ' - git config color.status always && - test_when_finished "git config --unset color.status" && + test_config color.status always && git status | test_decode_color >output && test_i18ncmp expect output '
@@ -720,8 +712,7 @@ EOF test_expect_success 'status without relative paths' ' - git config status.relativePaths false && - test_when_finished "git config --unset status.relativePaths" && + test_config status.relativePaths false && (cd dir1 && git status) >output && test_i18ncmp expect output
@@ -740,8 +731,7 @@ EOF test_expect_success 'status -s without relative paths' ' - git config status.relativePaths false && - test_when_finished "git config --unset status.relativePaths" && + test_config status.relativePaths false && (cd dir1 && git status -s) >output && test_cmp expect output
@@ -1038,15 +1028,14 @@ test_expect_success '--ignore-submodules=untracked suppresses submodules with un ' test_expect_success '.gitmodules ignore=untracked suppresses submodules with untracked content' ' - git config diff.ignoreSubmodules dirty && + test_config diff.ignoreSubmodules dirty && git status >output && test_i18ncmp expect output && git config --add -f .gitmodules submodule.subname.ignore untracked && git config --add -f .gitmodules submodule.subname.path sm && git status >output && test_i18ncmp expect output && - git config -f .gitmodules --remove-section submodule.subname && - git config --unset diff.ignoreSubmodules + git config -f .gitmodules --remove-section submodule.subname ' test_expect_success '.git/config ignore=untracked suppresses submodules with untracked content' '
@@ -1066,15 +1055,14 @@ test_expect_success '--ignore-submodules=dirty suppresses submodules with untrac ' test_expect_success '.gitmodules ignore=dirty suppresses submodules with untracked content' ' - git config diff.ignoreSubmodules dirty && + test_config diff.ignoreSubmodules dirty && git status >output && ! test -s actual && git config --add -f .gitmodules submodule.subname.ignore dirty && git config --add -f .gitmodules submodule.subname.path sm && git status >output && test_i18ncmp expect output && - git config -f .gitmodules --remove-section submodule.subname && - git config --unset diff.ignoreSubmodules + git config -f .gitmodules --remove-section submodule.subname ' test_expect_success '.git/config ignore=dirty suppresses submodules with untracked content' '
@@ -1291,15 +1279,13 @@ cat > expect << EOF EOF test_expect_success "status (core.commentchar with submodule summary)" ' - test_when_finished "git config --unset core.commentchar" && - git config core.commentchar ";" && + test_config core.commentchar ";" && git status >output && test_i18ncmp expect output ' test_expect_success "status (core.commentchar with two chars with submodule summary)" ' - test_when_finished "git config --unset core.commentchar" && - git config core.commentchar ";;" && + test_config core.commentchar ";;" && git status >output && test_i18ncmp expect output '
Signature · 1 line
1.7.11.7
PATCH 13/15 t7600: use test_config to set/unset git config variables
In reply to Yann DroneaudInstead of using construct such as:
test_when_finished "git config --unset <key>"
git config <key> <value>
uses
test_config <key> <value>
The latter takes care of removing <key> at the end of the test.
Tests are modified to assume default configuration at entry, and to reset the modified configuration variables at the end.
Test 'merge log message' was relying on the presence of option --no-ff in the configuration. With the option, git show -s --pretty=format:%b HEAD produces an empty line and without the option, it produces an empty file. The test is modified to check with and without --no-ff option.
--- t/t7600-merge.sh | 60 ++++++++++++++++++++++++-------------------------------- 1 file changed, 26 insertions(+), 34 deletions(-)
t/t7600-merge.sh +26 -34
@@ -56,7 +56,8 @@ create_merge_msgs () { echo && git log --no-merges ^HEAD c2 c3 } >squash.1-5-9 && - echo >msg.nolog && + : >msg.nologff && + echo >msg.nolognoff && { echo "* tag 'c3':" && echo " commit 3" &&
@@ -244,8 +245,7 @@ test_expect_success 'merges with --ff-only' ' test_expect_success 'merges with merge.ff=only' ' git reset --hard c1 && test_tick && - test_when_finished "git config --unset merge.ff" && - git config merge.ff only && + test_config merge.ff "only" && test_must_fail git merge c2 && test_must_fail git merge c3 && test_must_fail git merge c2 c3 &&
@@ -336,7 +336,7 @@ test_debug 'git log --graph --decorate --oneline --all' test_expect_success 'merge c1 with c2 (no-commit in config)' ' git reset --hard c1 && - git config branch.master.mergeoptions "--no-commit" && + test_config branch.master.mergeoptions "--no-commit" && git merge c2 && verify_merge file result.1-5 && verify_head $c1 &&
@@ -346,12 +346,11 @@ test_expect_success 'merge c1 with c2 (no-commit in config)' ' test_debug 'git log --graph --decorate --oneline --all' test_expect_success 'merge c1 with c2 (log in config)' ' - git config branch.master.mergeoptions "" && git reset --hard c1 && git merge --log c2 && git show -s --pretty=tformat:%s%n%b >expect && - git config branch.master.mergeoptions --log && + test_config branch.master.mergeoptions "--log" && git reset --hard c1 && git merge c2 && git show -s --pretty=tformat:%s%n%b >actual &&
@@ -360,17 +359,12 @@ test_expect_success 'merge c1 with c2 (log in config)' ' ' test_expect_success 'merge c1 with c2 (log in config gets overridden)' ' - test_when_finished "git config --remove-section branch.master" && - test_when_finished "git config --remove-section merge" && - test_might_fail git config --remove-section branch.master && - test_might_fail git config --remove-section merge && - git reset --hard c1 && git merge c2 && git show -s --pretty=tformat:%s%n%b >expect && - git config branch.master.mergeoptions "--no-log" && - git config merge.log true && + test_config branch.master.mergeoptions "--no-log" && + test_config merge.log "true" && git reset --hard c1 && git merge c2 && git show -s --pretty=tformat:%s%n%b >actual &&
@@ -380,7 +374,7 @@ test_expect_success 'merge c1 with c2 (log in config gets overridden)' ' test_expect_success 'merge c1 with c2 (squash in config)' ' git reset --hard c1 && - git config branch.master.mergeoptions "--squash" && + test_config branch.master.mergeoptions "--squash" && git merge c2 && verify_merge file result.1-5 && verify_head $c1 &&
@@ -392,7 +386,7 @@ test_debug 'git log --graph --decorate --oneline --all' test_expect_success 'override config option -n with --summary' ' git reset --hard c1 && - git config branch.master.mergeoptions "-n" && + test_config branch.master.mergeoptions "-n" && test_tick && git merge --summary c2 >diffstat.txt && verify_merge file result.1-5 msg.1-5 &&
@@ -406,7 +400,7 @@ test_expect_success 'override config option -n with --summary' ' test_expect_success 'override config option -n with --stat' ' git reset --hard c1 && - git config branch.master.mergeoptions "-n" && + test_config branch.master.mergeoptions "-n" && test_tick && git merge --stat c2 >diffstat.txt && verify_merge file result.1-5 msg.1-5 &&
@@ -422,7 +416,7 @@ test_debug 'git log --graph --decorate --oneline --all' test_expect_success 'override config option --stat' ' git reset --hard c1 && - git config branch.master.mergeoptions "--stat" && + test_config branch.master.mergeoptions "--stat" && test_tick && git merge -n c2 >diffstat.txt && verify_merge file result.1-5 msg.1-5 &&
@@ -438,7 +432,7 @@ test_debug 'git log --graph --decorate --oneline --all' test_expect_success 'merge c1 with c2 (override --no-commit)' ' git reset --hard c1 && - git config branch.master.mergeoptions "--no-commit" && + test_config branch.master.mergeoptions "--no-commit" && test_tick && git merge --commit c2 && verify_merge file result.1-5 msg.1-5 &&
@@ -449,7 +443,7 @@ test_debug 'git log --graph --decorate --oneline --all' test_expect_success 'merge c1 with c2 (override --squash)' ' git reset --hard c1 && - git config branch.master.mergeoptions "--squash" && + test_config branch.master.mergeoptions "--squash" && test_tick && git merge --no-squash c2 && verify_merge file result.1-5 msg.1-5 &&
@@ -460,7 +454,6 @@ test_debug 'git log --graph --decorate --oneline --all' test_expect_success 'merge c0 with c1 (no-ff)' ' git reset --hard c0 && - git config branch.master.mergeoptions "" && test_tick && git merge --no-ff c1 && verify_merge file result.1 &&
@@ -471,10 +464,9 @@ test_debug 'git log --graph --decorate --oneline --all' test_expect_success 'merge c0 with c1 (merge.ff=false)' ' git reset --hard c0 && - git config merge.ff false && + test_config merge.ff "false" && test_tick && git merge c1 && - git config --remove-section merge && verify_merge file result.1 && verify_parents $c0 $c1 '
@@ -482,22 +474,19 @@ test_debug 'git log --graph --decorate --oneline --all' test_expect_success 'combine branch.master.mergeoptions with merge.ff' ' git reset --hard c0 && - git config branch.master.mergeoptions --ff && - git config merge.ff false && + test_config branch.master.mergeoptions "--ff" && + test_config merge.ff "false" && test_tick && git merge c1 && - git config --remove-section "branch.master" && - git config --remove-section "merge" && verify_merge file result.1 && verify_parents "$c0" ' test_expect_success 'tolerate unknown values for merge.ff' ' git reset --hard c0 && - git config merge.ff something-new && + test_config merge.ff "something-new" && test_tick && git merge c1 2>message && - git config --remove-section "merge" && verify_head "$c1" && test_cmp empty message '
@@ -515,7 +504,7 @@ test_expect_success 'combining --ff-only and --no-ff is refused' ' test_expect_success 'merge c0 with c1 (ff overrides no-ff)' ' git reset --hard c0 && - git config branch.master.mergeoptions "--no-ff" && + test_config branch.master.mergeoptions "--no-ff" && git merge --ff c1 && verify_merge file result.1 && verify_head $c1
@@ -525,14 +514,20 @@ test_expect_success 'merge log message' ' git reset --hard c0 && git merge --no-log c2 && git show -s --pretty=format:%b HEAD >msg.act && - test_cmp msg.nolog msg.act && + test_cmp msg.nologff msg.act && + + git reset --hard c0 && + test_config branch.master.mergeoptions "--no-ff" && + git merge --no-log c2 && + git show -s --pretty=format:%b HEAD >msg.act && + test_cmp msg.nolognoff msg.act && git merge --log c3 && git show -s --pretty=format:%b HEAD >msg.act && test_cmp msg.log msg.act && git reset --hard HEAD^ && - git config merge.log yes && + test_config merge.log "yes" && git merge c3 && git show -s --pretty=format:%b HEAD >msg.act && test_cmp msg.log msg.act
@@ -542,7 +537,6 @@ test_debug 'git log --graph --decorate --oneline --all' test_expect_success 'merge c1 with c0, c2, c0, and c1' ' git reset --hard c1 && - git config branch.master.mergeoptions "" && test_tick && git merge c0 c2 c0 c1 && verify_merge file result.1-5 &&
@@ -553,7 +547,6 @@ test_debug 'git log --graph --decorate --oneline --all' test_expect_success 'merge c1 with c0, c2, c0, and c1' ' git reset --hard c1 && - git config branch.master.mergeoptions "" && test_tick && git merge c0 c2 c0 c1 && verify_merge file result.1-5 &&
@@ -564,7 +557,6 @@ test_debug 'git log --graph --decorate --oneline --all' test_expect_success 'merge c1 with c1 and c2' ' git reset --hard c1 && - git config branch.master.mergeoptions "" && test_tick && git merge c1 c2 && verify_merge file result.1-5 &&
Signature · 1 line
1.7.11.7
PATCH 14/15 t9500: use test_config to set/unset git config variables
In reply to Yann DroneaudInstead of using construct such as:
test_when_finished "git config --unset <key>"
git config <key> <value>
uses
test_config <key> <value>
The latter takes care of removing <key> at the end of the test.
--- t/t9500-gitweb-standalone-no-errors.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
t/t9500-gitweb-standalone-no-errors.sh +1 -2
@@ -539,8 +539,7 @@ test_expect_success \ test_when_finished "GIT_COMMITTER_NAME=\"C O Mitter\"" && echo "ISO-8859-1" >> file && git add file && - git config i18n.commitencoding ISO-8859-1 && - test_when_finished "git config --unset i18n.commitencoding" && + test_config i18n.commitencoding ISO-8859-1 && git commit -F "$TEST_DIRECTORY"/t3900/ISO8859-1.txt && gitweb_run "p=.git;a=commit"'
Signature · 1 line
1.7.11.7
PATCH 15/15 t7502: remove clear_config
In reply to Yann DroneaudUsing test_config ensure the configuration variable are removed at the end of the test, there's no need to remove variable at the beginning of the test.
--- t/t7502-commit.sh | 32 ++++++-------------------------- 1 file changed, 6 insertions(+), 26 deletions(-)
t/t7502-commit.sh +6 -26
@@ -434,16 +434,6 @@ EOF echo '## Custom template' >template -clear_config () { - ( - git config --unset-all "$1" - case $? in - 0|5) exit 0 ;; - *) exit 1 ;; - esac - ) -} - try_commit () { git reset --hard && echo >>negative &&
@@ -459,67 +449,57 @@ try_commit () { try_commit_status_combo () { test_expect_success 'commit' ' - clear_config commit.status && try_commit "" && test_i18ngrep "^# Changes to be committed:" .git/COMMIT_EDITMSG ' test_expect_success 'commit' ' - clear_config commit.status && try_commit "" && test_i18ngrep "^# Changes to be committed:" .git/COMMIT_EDITMSG ' test_expect_success 'commit --status' ' - clear_config commit.status && try_commit --status && test_i18ngrep "^# Changes to be committed:" .git/COMMIT_EDITMSG ' test_expect_success 'commit --no-status' ' - clear_config commit.status && try_commit --no-status && test_i18ngrep ! "^# Changes to be committed:" .git/COMMIT_EDITMSG ' test_expect_success 'commit with commit.status = yes' ' - clear_config commit.status && - git config commit.status yes && + test_config commit.status yes && try_commit "" && test_i18ngrep "^# Changes to be committed:" .git/COMMIT_EDITMSG ' test_expect_success 'commit with commit.status = no' ' - clear_config commit.status && - git config commit.status no && + test_config commit.status no && try_commit "" && test_i18ngrep ! "^# Changes to be committed:" .git/COMMIT_EDITMSG ' test_expect_success 'commit --status with commit.status = yes' ' - clear_config commit.status && - git config commit.status yes && + test_config commit.status yes && try_commit --status && test_i18ngrep "^# Changes to be committed:" .git/COMMIT_EDITMSG ' test_expect_success 'commit --no-status with commit.status = yes' ' - clear_config commit.status && - git config commit.status yes && + test_config commit.status yes && try_commit --no-status && test_i18ngrep ! "^# Changes to be committed:" .git/COMMIT_EDITMSG ' test_expect_success 'commit --status with commit.status = no' ' - clear_config commit.status && - git config commit.status no && + test_config commit.status no && try_commit --status && test_i18ngrep "^# Changes to be committed:" .git/COMMIT_EDITMSG ' test_expect_success 'commit --no-status with commit.status = no' ' - clear_config commit.status && - git config commit.status no && + test_config commit.status no && try_commit --no-status && test_i18ngrep ! "^# Changes to be committed:" .git/COMMIT_EDITMSG '
Signature · 1 line
1.7.11.7
Re: [PATCH 15/15] t7502: remove clear_config
Quoted 5 lines · Yann Droneaud
Using
test_configensure the configuration variable are removed at the end of the test, there's no need to remove variable at the beginning of the test.Signed-off-by Yann Droneaud <[email protected]>
This is a good change in the longer term, but there must not be any other topic in-flight that adds new tests that modify configuration in a persistent way, which your previous patch based on 'master' wouldn't have addressed, for this to be a safe change (I assume that you already have checked that).
Otherwise these apparently redundant unsets need to be left as "belt and suspenders" safety. The same for the change to 4202 in [PATCH 07/15].
Thanks.
Re: [PATCH 00/15] Use test_config
In reply to Yann DroneaudQuoted 1 line · Yann Droneaud
Tested against master,
7b592fadf1e23b10b913e0771b9f711770597266
Is this because I suggested you to clean things up while you were touching in a vicinity of something that could use this clean-up?
If so, please first clean _that_ script in a patch, and then add the change you wanted to do in another patch, as a single two-patch series, without touching anything else that is not related to that change. The patch to t7600 is the one that needs to become two patches, one to clean up and the other to add tests for --no-ff.
The rest, as a separate "only cleaning up, doing nothing else" series, are fine as a follow-up, but please make sure that they do not touch anything in-flight (one easy way to check is to see "git diff --name-only maint pu -- t/"). I would prefer to see "clean-up only" changes that introduce unnecessary conflicts with other real features and fixes held off until the dust settles.
Quoted 4 lines
Yann Droneaud (15):
t4018: removetest_configimplementation
t7810: removetest_configimplementation
t7811: removetest_configimplementation
We already have equivalents of these in-flight.
Quoted 1 line
t7600: use
test_configto set/unset git config variables
Needs to be split into two.
Re: [PATCH 00/15] Use test_config
Hi,
Le 27.03.2013 16:05, Junio C Hamano a écrit :
Quoted 7 lines
Quoted 1 line · Yann Droneaud
Tested against master,
7b592fadf1e23b10b913e0771b9f711770597266Is this because I suggested you to clean things up while you were touching in a vicinity of something that could use this clean-up?
Yes, grep'ing shows others usage of the test_config pattern. I patched them all.
Quoted 6 lines
If so, please first clean _that_ script in a patch, and then add the change you wanted to do in another patch, as a single two-patch series, without touching anything else that is not related to that change. The patch to t7600 is the one that needs to become two patches, one to clean up and the other to add tests for --no-ff.
Actually the initial patch adding test for --no-ff-only is not part of this series.
Patch against t7600 has a special note about a strange behavor found while testing test_config "anyware", that's why there's somes line added to the test and a note in the commit message.
I was waiting for your opinion on this change in the test, but more, on the difference of behavior exhibited in the patched test "merge log message":
git merge --no-log git show -s --pretty=format:%b HEAD
vs
git merge --no-ff --no-log git show -s --pretty=format:%b HEAD
First produce an empty file, while the second produce an empty line.
This was revealed by changing test "merge c0 with c1 (ff overrides no-ff)
- git config branch.master.mergeoptions "--no-ff" &&
- test_config branch.master.mergeoptions "--no-ff" &&
I could split this patch in a first patch that add the behavor test to "merge log message" test, than I could rebase the patch series against.
And later, submit my proposition for new tests in t7600 regarding --no-ff-only and tags.
Quoted 7 lines
The rest, as a separate "only cleaning up, doing nothing else" series, are fine as a follow-up, but please make sure that they do not touch anything in-flight (one easy way to check is to see "git diff --name-only maint pu -- t/"). I would prefer to see "clean-up only" changes that introduce unnecessary conflicts with other real features and fixes held off until the dust settles.
It's a good advice that fit perfectly in Documentation/SubmittingPatches.
Regards.
Signature · 2 lines
Yann Droneaud OPTEYA
Quoted 15 lines · Yann Droneaud
Thanks. I wasn't aware of the --no-ff-only option and thought --no-ff would be the opposite of --ff-only, or at least disable it given the order of the options.
Please find a patch to document option --no-ff-only
Documentation/merge-options.txt | 4 ++++ 1 file changed, 4 insertions(+)Documentation/merge-options.txt +0
-0
Documentation/merge-options.txt set to `no` at the beginning of them. · line 43
Quoted 7 lines
Documentation/merge-options.txt +4
-0@@ -37,6 +37,10 @@ set to `no` at the beginning of them. current `HEAD` is already up-to-date or the merge can be resolved as a fast-forward. +--no-ff-only:: + Disable `--ff-only` behavior, eg. allows creation of merge commit. + This is the default behavior. +
We should follow the usual
--option::
--no-option::
description for bothconvention for this one, before or after fixing the existing --ff/--no-ff description.
Quoted 3 lines
Documentation/merge-options.txt +0
-0--log[=<n>]:: --no-log:: In addition to branch names, populate the log message with
PATCH 1/1 t7600: merge tag shoud create a merge commit
In reply to Yann DroneaudFrom: Yann Droneaud <[email protected]>
This test ensures a merge commit is always created when merging an annotated (signed) tag without --ff-only option.
---
Here's a proposition for a test tath check the creation of a merge commit when merging a tag.
It's not in final shape: the line
EDITOR=false test_must_fail git merge signed
should failed, but doesn't: the commit merge is created with the default message, just like --no-edit was given.
I'm making a mistake somewhere, since the EDITOR=false trick works, it's even used in the next test, for --no-edit testing.
Regards.
t/t7600-merge.sh | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)
t/t7600-merge.sh +21 -1
@@ -697,10 +697,30 @@ test_expect_success 'merge --no-ff --edit' ' test_cmp actual expected ' +test_expect_failure GPG 'merge tag should create a merge commit' ' + git reset --hard c0 && + git commit --allow-empty -m "A newer commit" && + git tag -f -s -m "A newer commit" signed && + git reset --hard c0 && + + git merge signed && + git rev-parse signed^0 >expect && + git rev-parse HEAD^0 >actual && + ! test_cmp actual expect && + + git reset --hard c0 && + git commit --allow-empty -m "An other newer commit" && + git tag -f -s -m "An other newer commit" signed && + git reset --hard c0 && + + EDITOR=false test_must_fail git merge signed && + verify_head "$c0" +' + test_expect_success GPG 'merge --ff-only tag' ' git reset --hard c0 && git commit --allow-empty -m "A newer commit" && - git tag -s -m "A newer commit" signed && + git tag -f -s -m "A newer commit" signed && git reset --hard c0 && git merge --ff-only signed &&
Signature · 1 line
1.7.11.7
Re: [PATCH] t7600: merge tag shoud create a merge commit
Quoted 6 lines · [email protected] writes:
From: Yann Droneaud <[email protected]>
This test ensures a merge commit is always created when merging an annotated (signed) tag without --ff-only option.
Signed-off-by Yann Droneaud <[email protected]>
Quoted 8 lines
---
Here's a proposition for a test tath check the creation of a merge commit when merging a tag.
It's not in final shape: the line
EDITOR=false
test_must_failgit merge signed
Because test_must_fail is a shell function, single-shot environment assignment like this should not be used.
Re: [PATCH] t7600: merge tag shoud create a merge commit
Le 22.03.2013 15:48, Junio C Hamano a écrit :
Quoted 7 lines
Quoted 4 lines
It's not in final shape: the line
EDITOR=false
test_must_failgit merge signedBecause
test_must_failis a shell function, single-shot environment assignment like this should not be used.
It's used throughout the test. The test 'merge --no-edit tag should skip editor' is using it.
Before posting my half useful test, I used "EDITOR=false test_must_fail set" in --verbose mode to find if EDITOR was correctly defined passed test_must_fail, and it was.
So it's still not clear why it's failing at failing. And it's making me angry.
Regards.
Signature · 2 lines
Yann Droneaud OPTEYA
Re: [PATCH] t7600: merge tag shoud create a merge commit
Quoted 11 lines · Yann Droneaud
Le 22.03.2013 15:48, Junio C Hamano a écrit :
Quoted 7 lines
Quoted 4 lines
It's not in final shape: the line
EDITOR=false
test_must_failgit merge signedBecause
test_must_failis a shell function, single-shot environment assignment like this should not be used.It's used throughout the test. The test 'merge --no-edit tag should skip editor' is using it.
It's OK to do:
SINGLE_SHOT=foo some_real_command
and it's OK to do:
some_fun args
but it's not OK to do:
SINGLE_SHOT=foo some_function args
Because some POSIX shells do not create a new environment for the function (and SINGLE_SHOT will persist after the call, polluting the environment).
Quoted 3 lines
Before posting my half useful test, I used "EDITOR=false
test_must_failset" in --verbose mode to find if EDITOR was correctly defined passedtest_must_fail, and it was.
I do not think there is a shell that does not set it; it is only that some shells do not _unset_ it.
-Peff