Re: [PATCH 2/5] t4108: remove git command upstream of pipe
On Wed, Oct 23, 2019 at 09:32:26AM -0400, Eric Sunshine wrote:
> On Wed, Oct 23, 2019 at 8:04 AM Denton Liu wrote:
> > Before, the output of `git diff HEAD` would always be piped to
> > sanitize_conflicted_diff(). However, since the Git command was upstream
> > of the pipe, in case the Git command fails, the return code would be
> > lost. Rewrite into separate statements so that the return code is no
> > longer lost.
> >
> > Since only the command `git diff HEAD` was being piped to
> > sanitize_conflicted_diff(), move the command into the function and rename
> > it to print_sanitized_diff().
> >
> > Signed-off-by: Denton Liu
> > ---
> > diff --git a/t/t4108-apply-threeway.sh b/t/t4108-apply-threeway.sh
> > @@ -4,11 +4,12 @@ test_description='git apply --3way'
> > -sanitize_conflicted_diff () {
> > +print_sanitized_diff () {
> > + git diff HEAD >diff.raw &&
> > sed -e '
> > /^index /d
> > s/^\(+[<>][<>][<>][<>]*\) .*/\1/
> > - '
> > + ' diff.raw
> > }
>
> Nit: By hard-coding "HEAD" in this function, you lose the flexibility
> of the original. An alternative would have been to accept the ref
> against which to diff as an argument to this function:
>
> print_sanitized_diff () {
> git diff "$@" >diff.raw &&
> ...
>
> Or, better yet, keep the original design and pass the diff in as the
> shell function's input, so a caller would say:
>
> git diff HEAD >diff.raw &&
> sanitize_conflicted_diff expect.diff &&
>
> However, not necessarily worth a re-roll if we never expect anyone to
> pass anything other than "HEAD".
Since it doesn't really make sense to commmit conflicts, I decided to
hardcode it to be a diff against HEAD and the worktree since that's the
only sensible place where the conflict should live.
Speaking of conflicts, I dropped the "conflicted" part of the old
function name. I think that removes a lot of clarity so I'll reroll
renaming the function to print_sanitized_conflicted_diff() or something
like that.
[PATCH 1/5] t4108: replace create_file with test_write_lines
Since the locally defined create_file() duplicates the functionality of
the test_write_lines() helper function, remove create_file() and replace
all instances with test_write_lines(). While we're at it, move
redirection operators to the end of the command which is the more
conventional place to put it.
Signed-off-by: Denton Liu
---
t/t4108-apply-threeway.sh | 27 ++-
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/t/t4108-apply-threeway.sh b/t/t4108-apply-threeway.sh
index fa5d4efb89..b109ecbd9f 100755
--- a/t/t4108-apply-threeway.sh
+++ b/t/t4108-apply-threeway.sh
@@ -4,13 +4,6 @@ test_description='git apply --3way'
. ./test-lib.sh
-create_file () {
- for i
- do
- echo "$i"
- done
-}
-
sanitize_conflicted_diff () {
sed -e '
/^index /d
@@ -20,7 +13,7 @@ sanitize_conflicted_diff () {
test_expect_success setup '
test_tick &&
- create_file >one 1 2 3 4 5 6 7 &&
+ test_write_lines 1 2 3 4 5 6 7 >one &&
cat one >two &&
git add one two &&
git commit -m initial &&
@@ -28,13 +21,13 @@ test_expect_success setup '
git branch side &&
test_tick &&
- create_file >one 1 two 3 4 5 six 7 &&
- create_file >two 1 two 3 4 5 6 7 &&
+ test_write_lines 1 two 3 4 5 six 7 >one &&
+ test_write_lines 1 two 3 4 5 6 7 >two &&
git commit -a -m master &&
git checkout side &&
- create_file >one 1 2 3 4 five 6 7 &&
- create_file >two 1 2 3 4 five 6 7 &&
+ test_write_lines 1 2 3 4 five 6 7 >one &&
+ test_write_lines 1 2 3 4 five 6 7 >two &&
git commit -a -m side &&
git checkout master
@@ -87,7 +80,7 @@ test_expect_success 'apply with --3way with rerere enabled' '
test_must_fail git merge --no-commit side &&
# Manually resolve and record the resolution
- create_file 1 two 3 4 five six 7 >one &&
+ test_write_lines 1 two 3 4 five six 7 >one &&
git rerere &&
cat one >expect &&
@@ -104,14 +97,14 @@ test_expect_success 'apply -3 with add/add conflict setup'
'
git reset --hard &&
git checkout -b adder &&
- create_file 1 2 3 4 5 6 7 >three &&
- create_file 1 2 3 4 5 6 7 >four &&
+ test_write_lines 1 2 3 4 5 6 7 >three &&
+ test_write_lines 1 2 3 4 5 6 7 >four &&
git add three four &&
git commit -m "add three and four" &&
git checkout -b another adder^ &&
- create_file 1 2 3 4 5 6 7 >three &&
- create_file 1 2 3 four 5 6 7 >four &&
+ test_write_lines 1 2 3 4 5 6 7 >three &&
+ test_write_lines 1 2 3 four 5 6 7 >four &&
git add three four &&
git commit -m "add three and four" &&
--
2.24.0.rc0.197.g0926ab8072
[PATCH 3/5] t4108: use `test_config` instead of `git config`
Since `git config` leaves the configurations set even after the test case completes, use `test_config` instead so that the configurations are reset once the test case finishes. Signed-off-by: Denton Liu --- t/t4108-apply-threeway.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t4108-apply-threeway.sh b/t/t4108-apply-threeway.sh index 49739ce8b4..3615256492 100755 --- a/t/t4108-apply-threeway.sh +++ b/t/t4108-apply-threeway.sh @@ -70,7 +70,7 @@ test_expect_success 'apply with --3way' ' ' test_expect_success 'apply with --3way with rerere enabled' ' - git config rerere.enabled true && + test_config rerere.enabled true && # Merging side should be similar to applying this patch git diff ...side >P.diff && -- 2.24.0.rc0.197.g0926ab8072
[PATCH 2/5] t4108: remove git command upstream of pipe
Before, the output of `git diff HEAD` would always be piped to
sanitize_conflicted_diff(). However, since the Git command was upstream
of the pipe, in case the Git command fails, the return code would be
lost. Rewrite into separate statements so that the return code is no
longer lost.
Since only the command `git diff HEAD` was being piped to
sanitize_conflicted_diff(), move the command into the function and rename
it to print_sanitized_diff().
Signed-off-by: Denton Liu
---
t/t4108-apply-threeway.sh | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/t/t4108-apply-threeway.sh b/t/t4108-apply-threeway.sh
index b109ecbd9f..49739ce8b4 100755
--- a/t/t4108-apply-threeway.sh
+++ b/t/t4108-apply-threeway.sh
@@ -4,11 +4,12 @@ test_description='git apply --3way'
. ./test-lib.sh
-sanitize_conflicted_diff () {
+print_sanitized_diff () {
+ git diff HEAD >diff.raw &&
sed -e '
/^index /d
s/^\(+[<>][<>][<>][<>]*\) .*/\1/
- '
+ ' diff.raw
}
test_expect_success setup '
@@ -54,14 +55,14 @@ test_expect_success 'apply with --3way' '
git checkout master^0 &&
test_must_fail git merge --no-commit side &&
git ls-files -s >expect.ls &&
- git diff HEAD | sanitize_conflicted_diff >expect.diff &&
+ print_sanitized_diff >expect.diff &&
# should fail to apply
git reset --hard &&
git checkout master^0 &&
test_must_fail git apply --index --3way P.diff &&
git ls-files -s >actual.ls &&
- git diff HEAD | sanitize_conflicted_diff >actual.diff &&
+ print_sanitized_diff >actual.diff &&
# The result should resemble the corresponding merge
test_cmp expect.ls actual.ls &&
@@ -114,7 +115,7 @@ test_expect_success 'apply -3 with add/add conflict setup' '
git checkout adder^0 &&
test_must_fail git merge --no-commit another &&
git ls-files -s >expect.ls &&
- git diff HEAD | sanitize_conflicted_diff >expect.diff
+ print_sanitized_diff >expect.diff
'
test_expect_success 'apply -3 with add/add conflict' '
@@ -124,7 +125,7 @@ test_expect_success 'apply -3 with add/add conflict' '
test_must_fail git apply --index --3way P.diff &&
# ... and leave conflicts in the index and in the working tree
git ls-files -s >actual.ls &&
- git diff HEAD | sanitize_conflicted_diff >actual.diff &&
+ print_sanitized_diff >actual.diff &&
# The result should resemble the corresponding merge
test_cmp expect.ls actual.ls &&
--
2.24.0.rc0.197.g0926ab8072
[PATCH 0/5] apply: fix merge.conflictStyle bug in --3way
This fixes a bug where even if `merge.conflictStyle = diff3`, running `git apply --3way` would not output the base. Patches 1-3 are general cleanup for t4108, 4 demonstrates the bug and 5 actually fixes it. Denton Liu (5): t4108: replace create_file with test_write_lines t4108: remove git command upstream of pipe t4108: use `test_config` instead of `git config` t4108: demonstrate bug in apply apply: respect merge.conflictStyle in --3way apply.c | 2 +- t/t4108-apply-threeway.sh | 96 +-- 2 files changed, 53 insertions(+), 45 deletions(-) -- 2.24.0.rc0.197.g0926ab8072
[PATCH 4/5] t4108: demonstrate bug in apply
Currently, apply does not respect the merge.conflictStyle setting.
Demonstrate this by making the 'apply with --3way' test case generic and
extending it to show that the configuration of
merge.conflictStyle = diff3 causes a breakage.
Change print_sanitized_diff() to also sanitize `|||` conflict markers.
Signed-off-by: Denton Liu
---
t/t4108-apply-threeway.sh | 58 ---
1 file changed, 36 insertions(+), 22 deletions(-)
diff --git a/t/t4108-apply-threeway.sh b/t/t4108-apply-threeway.sh
index 3615256492..84347fc178 100755
--- a/t/t4108-apply-threeway.sh
+++ b/t/t4108-apply-threeway.sh
@@ -8,7 +8,7 @@ print_sanitized_diff () {
git diff HEAD >diff.raw &&
sed -e '
/^index /d
- s/^\(+[<>][<>][<>][<>]*\) .*/\1/
+ s/^\(+[<>|][<>|][<>|][<>|]*\) .*/\1/
' diff.raw
}
@@ -46,28 +46,42 @@ test_expect_success 'apply without --3way' '
git diff-index --exit-code --cached HEAD
'
-test_expect_success 'apply with --3way' '
- # Merging side should be similar to applying this patch
- git diff ...side >P.diff &&
-
- # The corresponding conflicted merge
- git reset --hard &&
- git checkout master^0 &&
- test_must_fail git merge --no-commit side &&
- git ls-files -s >expect.ls &&
- print_sanitized_diff >expect.diff &&
-
- # should fail to apply
- git reset --hard &&
- git checkout master^0 &&
- test_must_fail git apply --index --3way P.diff &&
- git ls-files -s >actual.ls &&
- print_sanitized_diff >actual.diff &&
+test_apply_with_3way () {
+ status="$1" &&
+ shift &&
+ description="$1" &&
+ shift &&
+ preamble="$1" &&
+ shift &&
+
+ test_expect_$status "apply with --3way ($description)" "
+ $preamble &&
+
+ # Merging side should be similar to applying this patch
+ git diff ...side >P.diff &&
+
+ # The corresponding conflicted merge
+ git reset --hard &&
+ git checkout master^0 &&
+ test_must_fail git merge --no-commit side &&
+ git ls-files -s >expect.ls &&
+ print_sanitized_diff >expect.diff &&
+
+ # should fail to apply
+ git reset --hard &&
+ git checkout master^0 &&
+ test_must_fail git apply --index --3way P.diff &&
+ git ls-files -s >actual.ls &&
+ print_sanitized_diff >actual.diff &&
+
+ # The result should resemble the corresponding merge
+ test_cmp expect.ls actual.ls &&
+ test_cmp expect.diff actual.diff
+ "
+}
- # The result should resemble the corresponding merge
- test_cmp expect.ls actual.ls &&
- test_cmp expect.diff actual.diff
-'
+test_apply_with_3way success default true
+test_apply_with_3way failure 'merge.conflictStyle = diff3' 'test_config
merge.conflictStyle diff3'
test_expect_success 'apply with --3way with rerere enabled' '
test_config rerere.enabled true &&
--
2.24.0.rc0.197.g0926ab8072
[PATCH 5/5] apply: respect merge.conflictStyle in --3way
Before, when doing a 3-way merge, the merge.conflictStyle option was not
respected and the "merge" style was always used, even if "diff3" was
specified.
Call git_xmerge_config() at the end of git_apply_config() so that the
merge.conflictStyle config is read.
Signed-off-by: Denton Liu
---
apply.c | 2 +-
t/t4108-apply-threeway.sh | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/apply.c b/apply.c
index f8a046a6a5..b9291f5f7b 100644
--- a/apply.c
+++ b/apply.c
@@ -32,7 +32,7 @@ static void git_apply_config(void)
{
git_config_get_string_const("apply.whitespace",
&apply_default_whitespace);
git_config_get_string_const("apply.ignorewhitespace",
&apply_default_ignorewhitespace);
- git_config(git_default_config, NULL);
+ git_config(git_xmerge_config, NULL);
}
static int parse_whitespace_option(struct apply_state *state, const char
*option)
diff --git a/t/t4108-apply-threeway.sh b/t/t4108-apply-threeway.sh
index 84347fc178..0e4eeac083 100755
--- a/t/t4108-apply-threeway.sh
+++ b/t/t4108-apply-threeway.sh
@@ -81,7 +81,7 @@ test_apply_with_3way () {
}
test_apply_with_3way success default true
-test_apply_with_3way failure 'merge.conflictStyle = diff3' 'test_config
merge.conflictStyle diff3'
+test_apply_with_3way success 'merge.conflictStyle = diff3' 'test_config
merge.conflictStyle diff3'
test_expect_success 'apply with --3way with rerere enabled' '
test_config rerere.enabled true &&
--
2.24.0.rc0.197.g0926ab8072
Re: [BUG] "--show-current-patch" return a mail instead of a patch
On Wed, Oct 23, 2019 at 10:15:15AM +, Jerome Pouiller wrote: > On Wednesday 23 October 2019 10:55:03 CEST Denton Liu wrote: > > I am currently have a WIP patchset that will print the location of the > > failed patch file (.git/rebase-apply/patch) in the case of a failure as > > well as the line number. Will this be sufficient for your purposes? > > It would be a clear improvement (the perfection would be to be able to > use mergetool with git-am :) ). You should be able to do that with the --3way (-3) flag. > > Thank you, > > -- > Jérôme Pouiller >
Re: [BUG] "--show-current-patch" return a mail instead of a patch
Hi Jerome,
On Wed, Oct 23, 2019 at 08:49:41AM +, Jerome Pouiller wrote:
> On Wednesday 23 October 2019 04:24:58 CEST Junio C Hamano wrote:
> > Jerome Pouiller writes:
> > > I try to use "git am" to apply a patch sent using "git send-email". This
> > > patch does not apply properly. I try to use "git am --show-current-patch"
> > > to understand the problem. However, since original mail is encoded in
> > > quoted-
> > > printable, data returned by --show-current-patch is not a valid patch.
> >
> > I agree that --show-current-patch is a misdesigned feature. We'd be
> > doing a better service to our users if we documented that the patch
> > and log message are found at .git/rebase-apply/{patch,msg} instead
> > of trying to hide the path.
> >
> > Unfortunately, it is likely that those who added that feature have
> > built their tooling around it to depend on its output being the full
> > e-mail message "am" was fed (and split by "git mailsplit"). So I do
> > not think we will be changing the output to the patch file only.
> >
> > But even then, the documentation can be fixed without any backward
> > compatibility issues. Perhaps like this?
> >
> > Documentation/git-am.txt | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
> > index 6f6c34b0f4..f63b70325c 100644
> > --- a/Documentation/git-am.txt
> > +++ b/Documentation/git-am.txt
> > @@ -172,7 +172,7 @@ default. You can use `--no-utf8` to override this.
> > untouched.
> >
> > --show-current-patch::
> > - Show the patch being applied when "git am" is stopped because
> > + Show the entire e-mail message "git am" has stopped at, because
> > of conflicts.
>
> I agree with you: I think that manpage and/or output of "git am" should
> mention ".git/rebase-apply/patch" (that is exactly what I was looking
> for).
I am currently have a WIP patchset that will print the location of the
failed patch file (.git/rebase-apply/patch) in the case of a failure as
well as the line number. Will this be sufficient for your purposes?
Thanks,
Denton
>
> Maybe documentation of --show-current-patch should be clarified with a
> note like "This option is mainly for internal purpose. If you want to
> get current patch, rely on .git/rebase-apply/patch".
>
>
> --
> Jérôme Pouiller
>
[PATCH v3 12/14] t5520: replace subshell cat comparison with test_cmp
We currently have many instances of `test = $(cat )` and
`test $(cat ) = `. In the case where this fails, it will be
difficult for a developer to debug since the output will be masked.
Replace these instances with invocations of test_cmp().
This change was done with the following GNU sed expressions:
s/\(\s*\)test \([^=]*\)= "$(cat \([^)]*\))"/\1echo \2>expect
\&\&\n\1test_cmp expect \3/
s/\(\s*\)test "$(cat \([^)]*\))" = \([^&]*\)\( &&\)\?$/\1echo \3
>expect \&\&\n\1test_cmp expect \2\4/
A future patch will clean up situations where we have multiple duplicate
statements within a test case. This is done to keep this patch purely
mechanical.
Signed-off-by: Denton Liu
---
t/t5520-pull.sh | 105
1 file changed, 70 insertions(+), 35 deletions(-)
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 8ddf89e550..c9e4eec004 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -15,8 +15,10 @@ test_pull_autostash () {
git add new_file &&
git pull "$@" . copy &&
test_cmp_rev HEAD^ copy &&
- test "$(cat new_file)" = dirty &&
- test "$(cat file)" = "modified again"
+ echo dirty >expect &&
+ test_cmp expect new_file &&
+ echo "modified again" >expect &&
+ test_cmp expect file
}
test_pull_autostash_fail () {
@@ -110,9 +112,11 @@ test_expect_success 'test . as a remote' '
echo updated >file &&
git commit -a -m updated &&
git checkout copy &&
- test "$(cat file)" = file &&
+ echo file >expect &&
+ test_cmp expect file &&
git pull &&
- test "$(cat file)" = updated &&
+ echo updated >expect &&
+ test_cmp expect file &&
git reflog -1 >reflog.actual &&
sed "s/^[0-9a-f][0-9a-f]*/OBJID/" reflog.actual >reflog.fuzzy &&
echo "OBJID HEAD@{0}: pull: Fast-forward" >reflog.expected &&
@@ -125,9 +129,11 @@ test_expect_success 'the default remote . should not break
explicit pull' '
git commit -a -m modified &&
git checkout copy &&
git reset --hard HEAD^ &&
- test "$(cat file)" = file &&
+ echo file >expect &&
+ test_cmp expect file &&
git pull . second &&
- test "$(cat file)" = modified &&
+ echo modified >expect &&
+ test_cmp expect file &&
git reflog -1 >reflog.actual &&
sed "s/^[0-9a-f][0-9a-f]*/OBJID/" reflog.actual >reflog.fuzzy &&
echo "OBJID HEAD@{0}: pull . second: Fast-forward" >reflog.expected &&
@@ -137,10 +143,12 @@ test_expect_success 'the default remote . should not
break explicit pull' '
test_expect_success 'fail if wildcard spec does not match any refs' '
git checkout -b test copy^ &&
test_when_finished "git checkout -f copy && git branch -D test" &&
- test "$(cat file)" = file &&
+ echo file >expect &&
+ test_cmp expect file &&
test_must_fail git pull . "refs/nonexisting1/*:refs/nonexisting2/*"
2>err &&
test_i18ngrep "no candidates for merging" err &&
- test "$(cat file)" = file
+ echo file >expect &&
+ test_cmp expect file
'
test_expect_success 'fail if no branches specified with non-default remote' '
@@ -148,11 +156,13 @@ test_expect_success 'fail if no branches specified with
non-default remote' '
test_when_finished "git remote remove test_remote" &&
git checkout -b test copy^ &&
test_when_finished "git checkout -f copy && git branch -D test" &&
- test "$(cat file)" = file &&
+ echo file >expect &&
+ test_cmp expect file &&
test_config branch.test.remote origin &&
test_must_fail git pull test_remote 2>err &&
test_i18ngrep "specify a branch on the command line" err &&
- test "$(cat file)" = file
+ echo file >expect &&
+ test_cmp expect file
'
test_expect_success 'fail if not on a branch' '
@@ -160,10 +170,12 @@ test_expect_success 'fail if not on a branch' '
test_when_finished "git remote remove origin" &&
git check
[PATCH v3 13/14] t5520: remove redundant lines in test cases
In the previous patches, the mechanical application of changes left some duplicate statements in the test case which were not strictly incorrect but were redundant and possibly misleading. Remove these duplicate statements so that it is clear that the intent behind the tests are that the content of the file stays the same throughout the whole test case. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 8 1 file changed, 8 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index c9e4eec004..ef3dbc201a 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -147,7 +147,6 @@ test_expect_success 'fail if wildcard spec does not match any refs' ' test_cmp expect file && test_must_fail git pull . "refs/nonexisting1/*:refs/nonexisting2/*" 2>err && test_i18ngrep "no candidates for merging" err && - echo file >expect && test_cmp expect file ' @@ -161,7 +160,6 @@ test_expect_success 'fail if no branches specified with non-default remote' ' test_config branch.test.remote origin && test_must_fail git pull test_remote 2>err && test_i18ngrep "specify a branch on the command line" err && - echo file >expect && test_cmp expect file ' @@ -174,7 +172,6 @@ test_expect_success 'fail if not on a branch' ' test_cmp expect file && test_must_fail git pull 2>err && test_i18ngrep "not currently on a branch" err && - echo file >expect && test_cmp expect file ' @@ -188,7 +185,6 @@ test_expect_success 'fail if no configuration for current branch' ' test_cmp expect file && test_must_fail git pull 2>err && test_i18ngrep "no tracking information" err && - echo file >expect && test_cmp expect file ' @@ -202,7 +198,6 @@ test_expect_success 'pull --all: fail if no configuration for current branch' ' test_cmp expect file && test_must_fail git pull --all 2>err && test_i18ngrep "There is no tracking information" err && - echo file >expect && test_cmp expect file ' @@ -215,7 +210,6 @@ test_expect_success 'fail if upstream branch does not exist' ' test_cmp expect file && test_must_fail git pull 2>err && test_i18ngrep "no such ref was fetched" err && - echo file >expect && test_cmp expect file ' @@ -685,10 +679,8 @@ test_expect_success 'pull --rebase fails on unborn branch with staged changes' ' git ls-files >actual && test_cmp expect actual && test_must_fail git pull --rebase .. master 2>err && - echo staged-file >expect && git ls-files >actual && test_cmp expect actual && - echo staged-file >expect && git show :staged-file >actual && test_cmp expect actual && test_i18ngrep "unborn branch with changes added to the index" err -- 2.24.0.rc0.197.g0926ab8072
[PATCH v3 09/14] t5520: use test_cmp_rev where possible
In case an invocation of `git rev-list` fails within the subshell, the failure will be masked. Remove the subshell and use test_cmp_rev() so that failures can be discovered. This change was done with the following sed expressions: s/test "$(git rev-parse.* \([^)]*\))" = "$(git rev-parse \([^)]*\))"/test_cmp_rev \1 \2/ s/test \([^ ]*\) = "$(git rev-parse.* \([^)]*\))"/test_cmp_rev \1 \2/ s/test "$(git rev-parse.* \([^)]*\))" != "$(git rev-parse.* \([^)]*\))"/test_cmp_rev ! \1 \2/ s/test \([^ ]*\) != "$(git rev-parse.* \([^)]*\))"/test_cmp_rev ! \1 \2/ Signed-off-by: Denton Liu --- t/t5520-pull.sh | 50 - 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 18225d8430..1af6ea06ee 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -230,7 +230,7 @@ test_expect_success 'fast-forwards working tree if branch head is updated' ' git pull . second:third 2>err && test_i18ngrep "fetch updated the current branch head" err && test "$(cat file)" = modified && - test "$(git rev-parse third)" = "$(git rev-parse second)" + test_cmp_rev third second ' test_expect_success 'fast-forward fails with conflicting work tree' ' @@ -241,7 +241,7 @@ test_expect_success 'fast-forward fails with conflicting work tree' ' test_must_fail git pull . second:third 2>err && test_i18ngrep "Cannot fast-forward your working tree" err && test "$(cat file)" = conflict && - test "$(git rev-parse third)" = "$(git rev-parse second)" + test_cmp_rev third second ' test_expect_success '--rebase' ' @@ -254,7 +254,7 @@ test_expect_success '--rebase' ' git commit -m "new file" && git tag before-rebase && git pull --rebase . copy && - test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && + test_cmp_rev HEAD^ copy && test new = "$(git show HEAD:file2)" ' @@ -266,7 +266,7 @@ test_expect_success '--rebase fast forward' ' git checkout to-rebase && git pull --rebase . ff && - test "$(git rev-parse HEAD)" = "$(git rev-parse ff)" && + test_cmp_rev HEAD ff && # The above only validates the result. Did we actually bypass rebase? git reflog -1 >reflog.actual && @@ -290,7 +290,7 @@ test_expect_success '--rebase --autostash fast forward' ' git checkout behind && echo dirty >file && git pull --rebase --autostash . to-rebase-ff && - test "$(git rev-parse HEAD)" = "$(git rev-parse to-rebase-ff)" + test_cmp_rev HEAD to-rebase-ff ' test_expect_success '--rebase with conflicts shows advice' ' @@ -328,7 +328,7 @@ test_expect_success 'failed --rebase shows advice' ' test_expect_success '--rebase fails with multiple branches' ' git reset --hard before-rebase && test_must_fail git pull --rebase . copy master 2>err && - test "$(git rev-parse HEAD)" = "$(git rev-parse before-rebase)" && + test_cmp_rev HEAD before-rebase && test_i18ngrep "Cannot rebase onto multiple branches" err && test modified = "$(git show HEAD:file)" ' @@ -380,7 +380,7 @@ test_expect_success 'pull.rebase' ' git reset --hard before-rebase && test_config pull.rebase true && git pull . copy && - test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && + test_cmp_rev HEAD^ copy && test new = "$(git show HEAD:file2)" ' @@ -398,7 +398,7 @@ test_expect_success 'branch.to-rebase.rebase' ' git reset --hard before-rebase && test_config branch.to-rebase.rebase true && git pull . copy && - test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && + test_cmp_rev HEAD^ copy && test new = "$(git show HEAD:file2)" ' @@ -407,14 +407,14 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' test_config pull.rebase true && test_config branch.to-rebase.rebase false && git pull . copy && - test "$(git rev-parse HEAD^)" !=
[PATCH v3 10/14] t5520: test single-line files by git with test_cmp
In case an invocation of a Git command fails within the subshell, the failure will be masked. Replace the subshell with a file-redirection and a call to test_cmp. This change was done with the following GNU sed expressions: s/\(\s*\)test \([^ ]*\) = "$(\(git [^)]*\))"/\1echo \2 >expect \&\&\n\1\3 >actual \&\&\n\1test_cmp expect actual/ s/\(\s*\)test "$(\(git [^)]*\))" = \([^ ]*\)/\1echo \3 >expect \&\&\n\1\2 >actual \&\&\n\1test_cmp expect actual/ A future patch will clean up situations where we have multiple duplicate statements within a test case. This is done to keep this patch purely mechanical. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 64 - 1 file changed, 48 insertions(+), 16 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 1af6ea06ee..8b7e7ae55d 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -255,7 +255,9 @@ test_expect_success '--rebase' ' git tag before-rebase && git pull --rebase . copy && test_cmp_rev HEAD^ copy && - test new = "$(git show HEAD:file2)" + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual ' test_expect_success '--rebase fast forward' ' @@ -330,7 +332,9 @@ test_expect_success '--rebase fails with multiple branches' ' test_must_fail git pull --rebase . copy master 2>err && test_cmp_rev HEAD before-rebase && test_i18ngrep "Cannot rebase onto multiple branches" err && - test modified = "$(git show HEAD:file)" + echo modified >expect && + git show HEAD:file >actual && + test_cmp expect actual ' test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' ' @@ -381,7 +385,9 @@ test_expect_success 'pull.rebase' ' test_config pull.rebase true && git pull . copy && test_cmp_rev HEAD^ copy && - test new = "$(git show HEAD:file2)" + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual ' test_expect_success 'pull --autostash & pull.rebase=true' ' @@ -399,7 +405,9 @@ test_expect_success 'branch.to-rebase.rebase' ' test_config branch.to-rebase.rebase true && git pull . copy && test_cmp_rev HEAD^ copy && - test new = "$(git show HEAD:file2)" + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual ' test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' @@ -408,14 +416,18 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' test_config branch.to-rebase.rebase false && git pull . copy && test_cmp_rev ! HEAD^ copy && - test new = "$(git show HEAD:file2)" + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual ' test_expect_success 'pull --rebase warns on --verify-signatures' ' git reset --hard before-rebase && git pull --rebase --verify-signatures . copy 2>err && test_cmp_rev HEAD^ copy && - test new = "$(git show HEAD:file2)" && + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual && test_i18ngrep "ignoring --verify-signatures for rebase" err ' @@ -423,7 +435,9 @@ test_expect_success 'pull --rebase does not warn on --no-verify-signatures' ' git reset --hard before-rebase && git pull --rebase --no-verify-signatures . copy 2>err && test_cmp_rev HEAD^ copy && - test new = "$(git show HEAD:file2)" && + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual && test_i18ngrep ! "verify-signatures" err ' @@ -445,7 +459,9 @@ test_expect_success 'pull.rebase=false create a new merge commit' ' git pull . copy && test_cmp_rev HEAD^1 before-preserve-rebase && test_cmp_rev HEAD^2 copy && - test file3 = "$(git show HEAD:file3.t)" + echo file3 >expect && + git show HEAD:file3.t >actual && + test_cmp expect actual ' test_expect_success 'p
[PATCH v3 08/14] t5520: replace test -{n,z} with test-lib functions
When wrapping a Git command in a subshell within another command, we throw away the Git command's exit code. In case the Git command fails, we would like to know about it rather than the failure being silent. Extract Git commands so that their exit codes are not lost. Instead of using `test -n` or `test -z`, replace them respectively with invocations of test_file_not_empty() and test_must_be_empty() so that we get better debugging information in the case of a failure. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 0ca4867e96..18225d8430 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -206,15 +206,18 @@ test_expect_success 'fail if the index has unresolved entries' ' test_when_finished "git checkout -f copy && git branch -D third" && test "$(cat file)" = file && test_commit modified2 file && - test -z "$(git ls-files -u)" && + git ls-files -u >unmerged && + test_must_be_empty unmerged && test_must_fail git pull . second && - test -n "$(git ls-files -u)" && + git ls-files -u >unmerged && + test_file_not_empty unmerged && cp file expected && test_must_fail git pull . second 2>err && test_i18ngrep "Pulling is not possible because you have unmerged files." err && test_cmp expected file && git add file && - test -z "$(git ls-files -u)" && + git ls-files -u >unmerged && + test_must_be_empty unmerged && test_must_fail git pull . second 2>err && test_i18ngrep "You have not concluded your merge" err && test_cmp expected file @@ -667,7 +670,8 @@ test_expect_success 'git pull --rebase detects upstreamed changes' ' ( cd dst && git pull --rebase && - test -z "$(git ls-files -u)" + git ls-files -u >untracked && + test_must_be_empty untracked ) ' -- 2.24.0.rc0.197.g0926ab8072
[PATCH v3 14/14] t5520: replace `! git` with `test_must_fail git`
Currently, if a Git command fails in an unexpected way, such as a segfault, it will be masked and ignored. Replace the ! with test_must_fail so that only expected failures pass. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index ef3dbc201a..602d996a33 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -537,7 +537,7 @@ test_expect_success 'pull --rebase=i' ' test_expect_success 'pull.rebase=invalid fails' ' git reset --hard before-preserve-rebase && test_config pull.rebase invalid && - ! git pull . copy + test_must_fail git pull . copy ' test_expect_success '--rebase=false create a new merge commit' ' @@ -572,7 +572,7 @@ test_expect_success REBASE_P \ test_expect_success '--rebase=invalid fails' ' git reset --hard before-preserve-rebase && - ! git pull --rebase=invalid . copy + test_must_fail git pull --rebase=invalid . copy ' test_expect_success '--rebase overrides pull.rebase=preserve and flattens keep-merge' ' -- 2.24.0.rc0.197.g0926ab8072
[PATCH v3 06/14] t5520: remove spaces after redirect operator
The style for tests in Git is to have the redirect operator attached to the filename with no spaces. Fix test cases where this is not the case. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 004d5884cd..7bb9031140 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -243,10 +243,10 @@ test_expect_success 'fast-forward fails with conflicting work tree' ' test_expect_success '--rebase' ' git branch to-rebase && - echo modified again > file && + echo modified again >file && git commit -m file file && git checkout to-rebase && - echo new > file2 && + echo new >file2 && git add file2 && git commit -m "new file" && git tag before-rebase && @@ -542,10 +542,10 @@ test_expect_success '--rebase with rebased upstream' ' git checkout copy && git tag copy-orig && git reset --hard HEAD^ && - echo conflicting modification > file && + echo conflicting modification >file && git commit -m conflict file && git checkout to-rebase && - echo file > file2 && + echo file >file2 && git commit -m to-rebase file2 && git tag to-rebase-orig && git pull --rebase me copy && @@ -591,7 +591,7 @@ test_expect_success 'pull --rebase dies early with dirty working directory' ' test_config branch.to-rebase.remote me && test_config branch.to-rebase.merge refs/heads/copy && test_config branch.to-rebase.rebase true && - echo dirty >> file && + echo dirty >>file && git add file && test_must_fail git pull && test "$COPY" = "$(git rev-parse --verify me/copy)" && -- 2.24.0.rc0.197.g0926ab8072
[PATCH v3 07/14] t5520: use test_line_count where possible
Instead of rolling our own functionality to test the number of lines a command outputs, use test_line_count() which provides better debugging information in the case of a failure. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 7bb9031140..0ca4867e96 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -699,7 +699,8 @@ test_expect_success 'git pull --rebase does not reapply old patches' ' ( cd dst && test_must_fail git pull --rebase && - test 1 = $(find .git/rebase-apply -name "000*" | wc -l) + find .git/rebase-apply -name "000*" >patches && + test_line_count = 1 patches ) ' -- 2.24.0.rc0.197.g0926ab8072
[PATCH v3 11/14] t5520: don't put git in upstream of pipe
Before, if the invocation of git failed, it would be masked by the pipe since only the return code of the last element of a pipe is used. Rewrite the test to put the Git command on its own line so its return code is not masked. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 8b7e7ae55d..8ddf89e550 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -668,7 +668,8 @@ test_expect_success 'pull --rebase fails on corrupt HEAD' ' ( cd corrupt && test_commit one && - obj=$(git rev-parse --verify HEAD | sed "s#^..#&/#") && + git rev-parse --verify HEAD >head && + obj=$(sed "s#^..#&/#" head) && rm -f .git/objects/$obj && test_must_fail git pull --rebase ) -- 2.24.0.rc0.197.g0926ab8072
[PATCH v3 05/14] t5520: replace test -f with test-lib functions
Although `test -f` has the same functionality as test_path_is_file(), in the case where test_path_is_file() fails, we get much better debugging information. Replace `test -f` with test_path_is_file() so that future developers will have a better experience debugging these test cases. Also, in the case of `! test -f`, not only should that path not be a file, it shouldn't exist at all so replace it with test_path_is_missing(). Signed-off-by: Denton Liu --- t/t5520-pull.sh | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 55560ce3cd..004d5884cd 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -39,8 +39,8 @@ test_expect_success 'pulling into void' ' cd cloned && git pull .. ) && - test -f file && - test -f cloned/file && + test_path_is_file file && + test_path_is_file cloned/file && test_cmp file cloned/file ' @@ -50,8 +50,8 @@ test_expect_success 'pulling into void using master:master' ' cd cloned-uho && git pull .. master:master ) && - test -f file && - test -f cloned-uho/file && + test_path_is_file file && + test_path_is_file cloned-uho/file && test_cmp file cloned-uho/file ' @@ -99,7 +99,7 @@ test_expect_success 'pulling into void must not create an octopus' ' ( cd cloned-octopus && test_must_fail git pull .. master master && - ! test -f file + test_path_is_missing file ) ' -- 2.24.0.rc0.197.g0926ab8072
[PATCH v3 03/14] t5520: use sq for test case names
The usual convention is for test case names to be written between single-quotes. Change all double-quoted test case names to single-quotes except for two test case names that use variables within. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 51d6ce8aec..a3de2e19b6 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -408,7 +408,7 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' test new = "$(git show HEAD:file2)" ' -test_expect_success "pull --rebase warns on --verify-signatures" ' +test_expect_success 'pull --rebase warns on --verify-signatures' ' git reset --hard before-rebase && git pull --rebase --verify-signatures . copy 2>err && test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && @@ -416,7 +416,7 @@ test_expect_success "pull --rebase warns on --verify-signatures" ' test_i18ngrep "ignoring --verify-signatures for rebase" err ' -test_expect_success "pull --rebase does not warn on --no-verify-signatures" ' +test_expect_success 'pull --rebase does not warn on --no-verify-signatures' ' git reset --hard before-rebase && git pull --rebase --no-verify-signatures . copy 2>err && test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && -- 2.24.0.rc0.197.g0926ab8072
[PATCH v3 04/14] t5520: let sed open its own input
We were using a redirection operator to feed input into sed. However,
since sed is capable of opening its own files, make sed open its own
files instead of redirecting input into it.
Signed-off-by: Denton Liu
---
t/t5520-pull.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index a3de2e19b6..55560ce3cd 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -5,7 +5,7 @@ test_description='pulling into void'
. ./test-lib.sh
modify () {
- sed -e "$1" <"$2" >"$2.x" &&
+ sed -e "$1" "$2" >"$2.x" &&
mv "$2.x" "$2"
}
--
2.24.0.rc0.197.g0926ab8072
[PATCH v3 00/14] t5520: various test cleanup
Like earlier patchsets, I want to implement a feature that involves
modifications to the test suite. Since that feature will probably take a
while to polish up, however, let's clean up the test suite in a separate
patchset first so it's not blocked by the feature work.
1/15 is a general improvement to test_rev_cmp() that will be used later
in the series.
Changes since v2:
* Drop 't7408: replace `test_must_fail test_path_is_file`' since it's
not a rabbit hole we want to go into right now
* Fix the output of `test_cmp_rev !` when revs are actually equal
* Rebase against the latest master since this topic hasn't been picked
up yet
Changes since v1:
* Incorporate Eric's feedback
Denton Liu (14):
t: teach test_cmp_rev to accept ! for not-equals
t5520: improve test style
t5520: use sq for test case names
t5520: let sed open its own input
t5520: replace test -f with test-lib functions
t5520: remove spaces after redirect operator
t5520: use test_line_count where possible
t5520: replace test -{n,z} with test-lib functions
t5520: use test_cmp_rev where possible
t5520: test single-line files by git with test_cmp
t5520: don't put git in upstream of pipe
t5520: replace subshell cat comparison with test_cmp
t5520: remove redundant lines in test cases
t5520: replace `! git` with `test_must_fail git`
t/t2400-worktree-add.sh | 4 +-
t/t3400-rebase.sh | 2 +-
t/t3421-rebase-topology-linear.sh | 6 +-
t/t3430-rebase-merges.sh| 2 +-
t/t3432-rebase-fast-forward.sh | 2 +-
t/t3501-revert-cherry-pick.sh | 2 +-
t/t3508-cherry-pick-many-commits.sh | 2 +-
t/t5520-pull.sh | 343 +---
t/test-lib-functions.sh | 22 +-
9 files changed, 234 insertions(+), 151 deletions(-)
Range-diff against v2:
1: 987fee4652 < -: -- t7408: replace `test_must_fail
test_path_is_file`
2: 417e808466 ! 1: 9a96f113e7 t: teach test_cmp_rev to accept ! for
not-equals
@@ t/test-lib-functions.sh: test_cmp_rev () {
- if test "$r1" != "$r2"
+ if test "$r1" "$inverted_op" "$r2"
then
++ local comp_out
++ if "x$inverted_op" = 'x='
++ then
++ comp_out='the same'
++ else
++ comp_out='different'
++ fi
cat >&4 <<-EOF
- error: two revisions point to different objects:
+- error: two revisions point to different objects:
++ error: two revisions point to $comp_out objects:
+ '$1': $r1
+ '$2': $r2
+ EOF
3: 0a56980857 = 2: dfc86a8d9b t5520: improve test style
4: dfa89ba1cb = 3: a1071038f5 t5520: use sq for test case names
5: 9fac3dff83 = 4: 0af3f5027b t5520: let sed open its own input
6: c6ca45eb17 = 5: b696ff0a67 t5520: replace test -f with test-lib functions
7: 830a8212ae = 6: d2e49fd990 t5520: remove spaces after redirect operator
8: 3d982230be = 7: fcfc3226f8 t5520: use test_line_count where possible
9: 2bca4f046d = 8: 86dafc7b54 t5520: replace test -{n,z} with test-lib
functions
10: 1a54db1d5c = 9: bf9b5023a3 t5520: use test_cmp_rev where possible
11: 52cf4f0d0f = 10: bfabf8ceff t5520: test single-line files by git with
test_cmp
12: 0cfabb201c = 11: 56bcbf3047 t5520: don't put git in upstream of pipe
13: b2d0ce21c8 = 12: e9d50b8bb0 t5520: replace subshell cat comparison with
test_cmp
14: 5aac40a029 = 13: 9db0fc2156 t5520: remove redundant lines in test cases
15: 2c0d3ac416 = 14: a721d5f119 t5520: replace `! git` with `test_must_fail
git`
--
2.24.0.rc0.197.g0926ab8072
[PATCH v3 01/14] t: teach test_cmp_rev to accept ! for not-equals
Currently, in the case where we are using test_cmp_rev() to report
not-equals, we write `! test_cmp_rev`. However, since test_cmp_rev()
contains
r1=$(git rev-parse --verify "$1") &&
r2=$(git rev-parse --verify "$2") &&
In the case where `git rev-parse` segfaults and dies unexpectedly, the
failure will be ignored.
Rewrite test_cmp_rev() to optionally accept "!" as the first argument to
do a not-equals comparison. Rewrite `! test_cmp_rev` to `test_cmp_rev !`
in all tests to take advantage of this new functionality.
Signed-off-by: Denton Liu
---
t/t2400-worktree-add.sh | 4 ++--
t/t3400-rebase.sh | 2 +-
t/t3421-rebase-topology-linear.sh | 6 +++---
t/t3430-rebase-merges.sh| 2 +-
t/t3432-rebase-fast-forward.sh | 2 +-
t/t3501-revert-cherry-pick.sh | 2 +-
t/t3508-cherry-pick-many-commits.sh | 2 +-
t/test-lib-functions.sh | 22 +++---
8 files changed, 29 insertions(+), 13 deletions(-)
diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index e819ba741e..52d476979b 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -438,7 +438,7 @@ test_expect_success 'git worktree add does not match
remote' '
cd foo &&
test_must_fail git config "branch.foo.remote" &&
test_must_fail git config "branch.foo.merge" &&
- ! test_cmp_rev refs/remotes/repo_a/foo refs/heads/foo
+ test_cmp_rev ! refs/remotes/repo_a/foo refs/heads/foo
)
'
@@ -483,7 +483,7 @@ test_expect_success 'git worktree --no-guess-remote option
overrides config' '
cd foo &&
test_must_fail git config "branch.foo.remote" &&
test_must_fail git config "branch.foo.merge" &&
- ! test_cmp_rev refs/remotes/repo_a/foo refs/heads/foo
+ test_cmp_rev ! refs/remotes/repo_a/foo refs/heads/foo
)
'
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index ab18ac5f28..f267f6cd54 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -64,7 +64,7 @@ test_expect_success 'rebase sets ORIG_HEAD to pre-rebase
state' '
pre="$(git rev-parse --verify HEAD)" &&
git rebase master &&
test_cmp_rev "$pre" ORIG_HEAD &&
- ! test_cmp_rev "$pre" HEAD
+ test_cmp_rev ! "$pre" HEAD
'
test_expect_success 'rebase, with and specified as
:/quuxery' '
diff --git a/t/t3421-rebase-topology-linear.sh
b/t/t3421-rebase-topology-linear.sh
index b847064f91..325072b0a3 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -61,7 +61,7 @@ test_run_rebase () {
test_expect_$result "rebase $* -f rewrites even if upstream is an
ancestor" "
reset_rebase &&
git rebase $* -f b e &&
- ! test_cmp_rev e HEAD &&
+ test_cmp_rev ! e HEAD &&
test_cmp_rev b HEAD~2 &&
test_linear_range 'd e' b..
"
@@ -78,7 +78,7 @@ test_run_rebase () {
test_expect_$result "rebase $* -f rewrites even if remote upstream is
an ancestor" "
reset_rebase &&
git rebase $* -f branch-b branch-e &&
- ! test_cmp_rev branch-e origin/branch-e &&
+ test_cmp_rev ! branch-e origin/branch-e &&
test_cmp_rev branch-b HEAD~2 &&
test_linear_range 'd e' branch-b..
"
@@ -368,7 +368,7 @@ test_run_rebase () {
test_expect_$result "rebase $* -f --root on linear history causes
re-write" "
reset_rebase &&
git rebase $* -f --root c &&
- ! test_cmp_rev a HEAD~2 &&
+ test_cmp_rev ! a HEAD~2 &&
test_linear_range 'a b c' HEAD
"
}
diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index 9efcf4808a..abbdc26b1b 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -346,7 +346,7 @@ test_expect_success 'A root commit can be a cousin, treat
it that way' '
git merge --allow-unrelated-histories khnum &&
test_tick &&
git rebase -f -r HEAD^ &&
- ! test_cmp_rev HEAD^2 khnum &&
+ test_cmp_rev ! HEAD^2 khnum &&
test_cmp_graph HEAD^.. <<-\EOF &&
* Merge branch '\''khnum'\'' into asherah
|\
diff --git a/t/t3432-rebase-fast-forward.sh b/t/t
[PATCH v3 02/14] t5520: improve test style
Improve the test style by removing leading and trailing empty lines within test cases. Also, reformat multi-line subshells to conform to the existing style. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 88 + 1 file changed, 45 insertions(+), 43 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index cf4cc32fd0..51d6ce8aec 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -538,7 +538,6 @@ test_expect_success '--rebase overrides pull.rebase=preserve and flattens keep-m ' test_expect_success '--rebase with rebased upstream' ' - git remote add -f me . && git checkout copy && git tag copy-orig && @@ -552,7 +551,6 @@ test_expect_success '--rebase with rebased upstream' ' git pull --rebase me copy && test "conflicting modification" = "$(cat file)" && test file = "$(cat file2)" - ' test_expect_success '--rebase -f with rebased upstream' ' @@ -564,14 +562,12 @@ test_expect_success '--rebase -f with rebased upstream' ' ' test_expect_success '--rebase with rebased default upstream' ' - git update-ref refs/remotes/me/copy copy-orig && git checkout --track -b to-rebase2 me/copy && git reset --hard to-rebase-orig && git pull --rebase && test "conflicting modification" = "$(cat file)" && test file = "$(cat file2)" - ' test_expect_success 'rebased upstream + fetch + pull --rebase' ' @@ -588,7 +584,6 @@ test_expect_success 'rebased upstream + fetch + pull --rebase' ' ' test_expect_success 'pull --rebase dies early with dirty working directory' ' - git checkout to-rebase && git update-ref refs/remotes/me/copy copy^ && COPY="$(git rev-parse --verify me/copy)" && @@ -603,16 +598,16 @@ test_expect_success 'pull --rebase dies early with dirty working directory' ' git checkout HEAD -- file && git pull && test "$COPY" != "$(git rev-parse --verify me/copy)" - ' test_expect_success 'pull --rebase works on branch yet to be born' ' git rev-parse master >expect && mkdir empty_repo && - (cd empty_repo && -git init && -git pull --rebase .. master && -git rev-parse HEAD >../actual + ( + cd empty_repo && + git init && + git pull --rebase .. master && + git rev-parse HEAD >../actual ) && test_cmp expect actual ' @@ -646,58 +641,65 @@ test_expect_success 'pull --rebase fails on corrupt HEAD' ' test_expect_success 'setup for detecting upstreamed changes' ' mkdir src && - (cd src && -git init && -printf "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n" > stuff && -git add stuff && -git commit -m "Initial revision" + ( + cd src && + git init && + printf "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n" > stuff && + git add stuff && + git commit -m "Initial revision" ) && git clone src dst && - (cd src && -modify s/5/43/ stuff && -git commit -a -m "5->43" && -modify s/6/42/ stuff && -git commit -a -m "Make it bigger" + ( + cd src && + modify s/5/43/ stuff && + git commit -a -m "5->43" && + modify s/6/42/ stuff && + git commit -a -m "Make it bigger" ) && - (cd dst && -modify s/5/43/ stuff && -git commit -a -m "Independent discovery of 5->43" + ( + cd dst && + modify s/5/43/ stuff && + git commit -a -m "Independent discovery of 5->43" ) ' test_expect_success 'git pull --rebase detects upstreamed changes' ' - (cd dst && -git pull --rebase && -test -z "$(git ls-files -u)" + ( + cd dst && + git pull --rebase && + test -z "$(git ls-files -u)" ) ' test_expect_success 'setup for avoiding reapplying old patches'
[PATCH] t7419: change test_must_fail to ! for grep
According to t/README, test_must_fail() should only be used to test for failure in Git commands. Replace the invocations of `test_must_fail grep` with `! grep`. Signed-off-by: Denton Liu --- *sigh* Here's another cleanup patch for 'dl/submodule-set-branch'. It's inspired by Eric Sunshine's comments on the t5520 patchset from earlier. It's definitely not urgent, though, and can wait for v2.25.0. t/t7419-submodule-set-branch.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t7419-submodule-set-branch.sh b/t/t7419-submodule-set-branch.sh index c4b370ea85..fd25f786a3 100755 --- a/t/t7419-submodule-set-branch.sh +++ b/t/t7419-submodule-set-branch.sh @@ -34,7 +34,7 @@ test_expect_success 'submodule config cache setup' ' test_expect_success 'ensure submodule branch is unset' ' (cd super && - test_must_fail grep branch .gitmodules + ! grep branch .gitmodules ) ' @@ -54,7 +54,7 @@ test_expect_success 'test submodule set-branch --branch' ' test_expect_success 'test submodule set-branch --default' ' (cd super && git submodule set-branch --default submodule && - test_must_fail grep branch .gitmodules && + ! grep branch .gitmodules && git submodule update --remote && cat <<-\EOF >expect && a @@ -80,7 +80,7 @@ test_expect_success 'test submodule set-branch -b' ' test_expect_success 'test submodule set-branch -d' ' (cd super && git submodule set-branch -d submodule && - test_must_fail grep branch .gitmodules && + ! grep branch .gitmodules && git submodule update --remote && cat <<-\EOF >expect && a -- 2.24.0.rc0.197.g0926ab8072
Re: [RFC PATCH 1/7] Makefile: alphabetically sort += lists
Hi Johannes, On Mon, Oct 21, 2019 at 08:44:40PM +0200, Johannes Schindelin wrote: > Hi Junio, > > On Fri, 18 Oct 2019, Junio C Hamano wrote: > > > Denton Liu writes: > > > > > There are many += lists in the Makefile and, over time, they have gotten > > > slightly out of order, alphabetically. Alphabetically sort all += lists > > > to bring them back in order. > > > ... > > > > Hmm. I like the general thrust, but ... > > > > > LIB_OBJS += combine-diff.o > > > -LIB_OBJS += commit.o > > > LIB_OBJS += commit-graph.o > > > LIB_OBJS += commit-reach.o > > > +LIB_OBJS += commit.o > > > > ... I do not particularly see this change (there may be similar > > ones) desirable. I'd find it it be much more natural to sort > > "commit-anything" after "commit", and that is true with or without > > the common extension ".o" added to these entries. > > > > In short, flipping these entries because '.' sorts later than '-' is > > making the result look "less sorted", at least to me. > > The problem with this argument is that it disagrees with ASCII, as `-` > has code 0x2d while `.` has code 0x2e, i.e. it is lexicographically > _larger_. > > So Denton's patch does the correct thing. I actually agree with Junio on this one. Without the prefixes, "commit" would go before "commit-graph" so I think it would make more sense to order with the prefixes removed instead of taking the naive ordering by just sorting each block. Thanks, Denton > > Ciao, > Dscho
[PATCH] rebase: hide --preserve-merges option
Since --preserve-merges has been deprecated in favour of
--rebase-merges, mark this option as hidden so it no longer shows up in
the usage and completions.
Signed-off-by: Denton Liu
---
This patch continues the deprecation effort and can be based on top of
`js/rebase-deprecate-preserve-merges`.
builtin/rebase.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 21ac10f739..0d63651d95 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1099,9 +1099,10 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
N_("let the user edit the list of commits to rebase"),
PARSE_OPT_NOARG | PARSE_OPT_NONEG,
parse_opt_interactive },
- OPT_SET_INT('p', "preserve-merges", &options.type,
- N_("(DEPRECATED) try to recreate merges instead of "
- "ignoring them"), REBASE_PRESERVE_MERGES),
+ OPT_SET_INT_F('p', "preserve-merges", &options.type,
+ N_("(DEPRECATED) try to recreate merges instead
of "
+"ignoring them"),
+ REBASE_PRESERVE_MERGES, PARSE_OPT_HIDDEN),
OPT_BOOL(0, "rerere-autoupdate",
&options.allow_rerere_autoupdate,
N_("allow rerere to update index with resolved "
--
2.23.0
Re: [PATCH v2 00/15] t5520: various test cleanup
Sorry for the double-send. public-inbox and MARC both showed that
only 3 messages got through the in the first send so I sent it again.
Seems to have gotten through find the second time around.
On Fri, Oct 18, 2019 at 03:04:03PM -0700, Denton Liu wrote:
> Like earlier patchsets, I want to implement a feature that involves
> modifications to the test suite. Since that feature will probably take a
> while to polish up, however, let's clean up the test suite in a separate
> patchset first so it's not blocked by the feature work.
>
> 1/15 is a cleanup to an unrelated test that I found while addressing
> some of Eric's comments.
>
> 2/15 is a general improvement to test_rev_cmp() that will be used later
> in the series.
>
> Changes since v1:
>
> * Incorporate Eric's feedback
>
> Denton Liu (15):
> t7408: replace `test_must_fail test_path_is_file`
> t: teach test_cmp_rev to accept ! for not-equals
> t5520: improve test style
> t5520: use sq for test case names
> t5520: let sed open its own input
> t5520: replace test -f with test-lib functions
> t5520: remove spaces after redirect operator
> t5520: use test_line_count where possible
> t5520: replace test -{n,z} with test-lib functions
> t5520: use test_cmp_rev where possible
> t5520: test single-line files by git with test_cmp
> t5520: don't put git in upstream of pipe
> t5520: replace subshell cat comparison with test_cmp
> t5520: remove redundant lines in test cases
> t5520: replace `! git` with `test_must_fail git`
>
> t/t2400-worktree-add.sh | 4 +-
> t/t3400-rebase.sh | 2 +-
> t/t3421-rebase-topology-linear.sh | 6 +-
> t/t3430-rebase-merges.sh| 2 +-
> t/t3432-rebase-fast-forward.sh | 2 +-
> t/t3501-revert-cherry-pick.sh | 2 +-
> t/t3508-cherry-pick-many-commits.sh | 2 +-
> t/t5520-pull.sh | 343 +---
> t/t7408-submodule-reference.sh | 2 +-
> t/test-lib-functions.sh | 13 +-
> 10 files changed, 227 insertions(+), 151 deletions(-)
>
> Range-diff against v1:
> -: -- > 1: 987fee4652 t7408: replace `test_must_fail
> test_path_is_file`
> -: -- > 2: 417e808466 t: teach test_cmp_rev to accept ! for
> not-equals
> 1: 0bc54dd330 = 3: 0a56980857 t5520: improve test style
> 2: a5dee82ecc = 4: dfa89ba1cb t5520: use sq for test case names
> 3: 58cc2fcda3 = 5: 9fac3dff83 t5520: let sed open its own input
> 4: d2946208d3 ! 6: c6ca45eb17 t5520: replace test -f with
> test_path_is_file
> @@ Metadata
> Author: Denton Liu
>
> ## Commit message ##
> -t5520: replace test -f with test_path_is_file
> +t5520: replace test -f with test-lib functions
>
> Although `test -f` has the same functionality as
> test_path_is_file(), in
> the case where test_path_is_file() fails, we get much better
> debugging
> -information. Replace `test -f` with test_path_is_file so that future
> -developers will have a better experience debugging these test cases.
> +information.
> +
> +Replace `test -f` with test_path_is_file() so that future developers
> +will have a better experience debugging these test cases.
> +
> +Also, in the case of `! test -f`, not only should that path not be a
> +file, it shouldn't exist at all so replace it with
> +test_path_is_missing().
>
> Signed-off-by: Denton Liu
>
> @@ t/t5520-pull.sh: test_expect_success 'pulling into void must not
> create an octop
> cd cloned-octopus &&
> test_must_fail git pull .. master master &&
> -! test -f file
> -+test_must_fail test_path_is_file file
> ++test_path_is_missing file
> )
> '
>
> 5: 5b4c1dd291 = 7: 830a8212ae t5520: remove spaces after redirect operator
> 6: 26fea15950 = 8: 3d982230be t5520: use test_line_count where possible
> 7: 3fc0354c9c ! 9: 2bca4f046d t5520: replace test -{n,z} with test-lib
> functions
> @@ Metadata
> ## Commit message ##
> t5520: replace test -{n,z} with test-lib functions
>
> +When wrapping a Git command in a subshell within another command, we
> +throw away the Git command's exit code. In case the Git command
> fails,
> +we would like to know about it rather than the failure being silent.
> +Extract Git commands so that their exit codes are not lost.
> +
&g
[PATCH v2 00/15] t5520: various test cleanup
Like earlier patchsets, I want to implement a feature that involves
modifications to the test suite. Since that feature will probably take a
while to polish up, however, let's clean up the test suite in a separate
patchset first so it's not blocked by the feature work.
1/15 is a cleanup to an unrelated test that I found while addressing
some of Eric's comments.
2/15 is a general improvement to test_rev_cmp() that will be used later
in the series.
Changes since v1:
* Incorporate Eric's feedback
Denton Liu (15):
t7408: replace `test_must_fail test_path_is_file`
t: teach test_cmp_rev to accept ! for not-equals
t5520: improve test style
t5520: use sq for test case names
t5520: let sed open its own input
t5520: replace test -f with test-lib functions
t5520: remove spaces after redirect operator
t5520: use test_line_count where possible
t5520: replace test -{n,z} with test-lib functions
t5520: use test_cmp_rev where possible
t5520: test single-line files by git with test_cmp
t5520: don't put git in upstream of pipe
t5520: replace subshell cat comparison with test_cmp
t5520: remove redundant lines in test cases
t5520: replace `! git` with `test_must_fail git`
t/t2400-worktree-add.sh | 4 +-
t/t3400-rebase.sh | 2 +-
t/t3421-rebase-topology-linear.sh | 6 +-
t/t3430-rebase-merges.sh| 2 +-
t/t3432-rebase-fast-forward.sh | 2 +-
t/t3501-revert-cherry-pick.sh | 2 +-
t/t3508-cherry-pick-many-commits.sh | 2 +-
t/t5520-pull.sh | 343 +---
t/t7408-submodule-reference.sh | 2 +-
t/test-lib-functions.sh | 13 +-
10 files changed, 227 insertions(+), 151 deletions(-)
Range-diff against v1:
-: -- > 1: 987fee4652 t7408: replace `test_must_fail
test_path_is_file`
-: -- > 2: 417e808466 t: teach test_cmp_rev to accept ! for
not-equals
1: 0bc54dd330 = 3: 0a56980857 t5520: improve test style
2: a5dee82ecc = 4: dfa89ba1cb t5520: use sq for test case names
3: 58cc2fcda3 = 5: 9fac3dff83 t5520: let sed open its own input
4: d2946208d3 ! 6: c6ca45eb17 t5520: replace test -f with test_path_is_file
@@ Metadata
Author: Denton Liu
## Commit message ##
-t5520: replace test -f with test_path_is_file
+t5520: replace test -f with test-lib functions
Although `test -f` has the same functionality as test_path_is_file(),
in
the case where test_path_is_file() fails, we get much better debugging
-information. Replace `test -f` with test_path_is_file so that future
-developers will have a better experience debugging these test cases.
+information.
+
+Replace `test -f` with test_path_is_file() so that future developers
+will have a better experience debugging these test cases.
+
+Also, in the case of `! test -f`, not only should that path not be a
+file, it shouldn't exist at all so replace it with
+test_path_is_missing().
Signed-off-by: Denton Liu
@@ t/t5520-pull.sh: test_expect_success 'pulling into void must not create
an octop
cd cloned-octopus &&
test_must_fail git pull .. master master &&
- ! test -f file
-+ test_must_fail test_path_is_file file
++ test_path_is_missing file
)
'
5: 5b4c1dd291 = 7: 830a8212ae t5520: remove spaces after redirect operator
6: 26fea15950 = 8: 3d982230be t5520: use test_line_count where possible
7: 3fc0354c9c ! 9: 2bca4f046d t5520: replace test -{n,z} with test-lib
functions
@@ Metadata
## Commit message ##
t5520: replace test -{n,z} with test-lib functions
+When wrapping a Git command in a subshell within another command, we
+throw away the Git command's exit code. In case the Git command fails,
+we would like to know about it rather than the failure being silent.
+Extract Git commands so that their exit codes are not lost.
+
Instead of using `test -n` or `test -z`, replace them respectively with
-invocations of test_file_not_empty() and test_must_be_empty().
+invocations of test_file_not_empty() and test_must_be_empty() so that
we
+get better debugging information in the case of a failure.
Signed-off-by: Denton Liu
8: f4eb80c9ac ! 10: 1a54db1d5c t5520: use test_cmp_rev where possible
@@ Commit message
s/test "$(git rev-parse.* \([^)]*\))" = "$(git rev-parse
\([^)]*\))"/test_cmp_rev \1 \2/
s/test \([^ ]*\) = "$(git rev-parse.* \([^)]*\))"/test_cmp_rev
\1 \2/
-s/test "$(git rev-parse.* \([^)]*\))" != "$(git rev-parse.*
\([^)]*\))"/test_must_fail te
[PATCH v2 03/15] t5520: improve test style
Improve the test style by removing leading and trailing empty lines within test cases. Also, reformat multi-line subshells to conform to the existing style. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 88 + 1 file changed, 45 insertions(+), 43 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index cf4cc32fd0..51d6ce8aec 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -538,7 +538,6 @@ test_expect_success '--rebase overrides pull.rebase=preserve and flattens keep-m ' test_expect_success '--rebase with rebased upstream' ' - git remote add -f me . && git checkout copy && git tag copy-orig && @@ -552,7 +551,6 @@ test_expect_success '--rebase with rebased upstream' ' git pull --rebase me copy && test "conflicting modification" = "$(cat file)" && test file = "$(cat file2)" - ' test_expect_success '--rebase -f with rebased upstream' ' @@ -564,14 +562,12 @@ test_expect_success '--rebase -f with rebased upstream' ' ' test_expect_success '--rebase with rebased default upstream' ' - git update-ref refs/remotes/me/copy copy-orig && git checkout --track -b to-rebase2 me/copy && git reset --hard to-rebase-orig && git pull --rebase && test "conflicting modification" = "$(cat file)" && test file = "$(cat file2)" - ' test_expect_success 'rebased upstream + fetch + pull --rebase' ' @@ -588,7 +584,6 @@ test_expect_success 'rebased upstream + fetch + pull --rebase' ' ' test_expect_success 'pull --rebase dies early with dirty working directory' ' - git checkout to-rebase && git update-ref refs/remotes/me/copy copy^ && COPY="$(git rev-parse --verify me/copy)" && @@ -603,16 +598,16 @@ test_expect_success 'pull --rebase dies early with dirty working directory' ' git checkout HEAD -- file && git pull && test "$COPY" != "$(git rev-parse --verify me/copy)" - ' test_expect_success 'pull --rebase works on branch yet to be born' ' git rev-parse master >expect && mkdir empty_repo && - (cd empty_repo && -git init && -git pull --rebase .. master && -git rev-parse HEAD >../actual + ( + cd empty_repo && + git init && + git pull --rebase .. master && + git rev-parse HEAD >../actual ) && test_cmp expect actual ' @@ -646,58 +641,65 @@ test_expect_success 'pull --rebase fails on corrupt HEAD' ' test_expect_success 'setup for detecting upstreamed changes' ' mkdir src && - (cd src && -git init && -printf "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n" > stuff && -git add stuff && -git commit -m "Initial revision" + ( + cd src && + git init && + printf "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n" > stuff && + git add stuff && + git commit -m "Initial revision" ) && git clone src dst && - (cd src && -modify s/5/43/ stuff && -git commit -a -m "5->43" && -modify s/6/42/ stuff && -git commit -a -m "Make it bigger" + ( + cd src && + modify s/5/43/ stuff && + git commit -a -m "5->43" && + modify s/6/42/ stuff && + git commit -a -m "Make it bigger" ) && - (cd dst && -modify s/5/43/ stuff && -git commit -a -m "Independent discovery of 5->43" + ( + cd dst && + modify s/5/43/ stuff && + git commit -a -m "Independent discovery of 5->43" ) ' test_expect_success 'git pull --rebase detects upstreamed changes' ' - (cd dst && -git pull --rebase && -test -z "$(git ls-files -u)" + ( + cd dst && + git pull --rebase && + test -z "$(git ls-files -u)" ) ' test_expect_success 'setup for avoiding reapplying old patches'
[PATCH v2 10/15] t5520: use test_cmp_rev where possible
In case an invocation of `git rev-list` fails within the subshell, the failure will be masked. Remove the subshell and use test_cmp_rev() so that failures can be discovered. This change was done with the following sed expressions: s/test "$(git rev-parse.* \([^)]*\))" = "$(git rev-parse \([^)]*\))"/test_cmp_rev \1 \2/ s/test \([^ ]*\) = "$(git rev-parse.* \([^)]*\))"/test_cmp_rev \1 \2/ s/test "$(git rev-parse.* \([^)]*\))" != "$(git rev-parse.* \([^)]*\))"/test_cmp_rev ! \1 \2/ s/test \([^ ]*\) != "$(git rev-parse.* \([^)]*\))"/test_cmp_rev ! \1 \2/ Signed-off-by: Denton Liu --- t/t5520-pull.sh | 50 - 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 18225d8430..1af6ea06ee 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -230,7 +230,7 @@ test_expect_success 'fast-forwards working tree if branch head is updated' ' git pull . second:third 2>err && test_i18ngrep "fetch updated the current branch head" err && test "$(cat file)" = modified && - test "$(git rev-parse third)" = "$(git rev-parse second)" + test_cmp_rev third second ' test_expect_success 'fast-forward fails with conflicting work tree' ' @@ -241,7 +241,7 @@ test_expect_success 'fast-forward fails with conflicting work tree' ' test_must_fail git pull . second:third 2>err && test_i18ngrep "Cannot fast-forward your working tree" err && test "$(cat file)" = conflict && - test "$(git rev-parse third)" = "$(git rev-parse second)" + test_cmp_rev third second ' test_expect_success '--rebase' ' @@ -254,7 +254,7 @@ test_expect_success '--rebase' ' git commit -m "new file" && git tag before-rebase && git pull --rebase . copy && - test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && + test_cmp_rev HEAD^ copy && test new = "$(git show HEAD:file2)" ' @@ -266,7 +266,7 @@ test_expect_success '--rebase fast forward' ' git checkout to-rebase && git pull --rebase . ff && - test "$(git rev-parse HEAD)" = "$(git rev-parse ff)" && + test_cmp_rev HEAD ff && # The above only validates the result. Did we actually bypass rebase? git reflog -1 >reflog.actual && @@ -290,7 +290,7 @@ test_expect_success '--rebase --autostash fast forward' ' git checkout behind && echo dirty >file && git pull --rebase --autostash . to-rebase-ff && - test "$(git rev-parse HEAD)" = "$(git rev-parse to-rebase-ff)" + test_cmp_rev HEAD to-rebase-ff ' test_expect_success '--rebase with conflicts shows advice' ' @@ -328,7 +328,7 @@ test_expect_success 'failed --rebase shows advice' ' test_expect_success '--rebase fails with multiple branches' ' git reset --hard before-rebase && test_must_fail git pull --rebase . copy master 2>err && - test "$(git rev-parse HEAD)" = "$(git rev-parse before-rebase)" && + test_cmp_rev HEAD before-rebase && test_i18ngrep "Cannot rebase onto multiple branches" err && test modified = "$(git show HEAD:file)" ' @@ -380,7 +380,7 @@ test_expect_success 'pull.rebase' ' git reset --hard before-rebase && test_config pull.rebase true && git pull . copy && - test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && + test_cmp_rev HEAD^ copy && test new = "$(git show HEAD:file2)" ' @@ -398,7 +398,7 @@ test_expect_success 'branch.to-rebase.rebase' ' git reset --hard before-rebase && test_config branch.to-rebase.rebase true && git pull . copy && - test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && + test_cmp_rev HEAD^ copy && test new = "$(git show HEAD:file2)" ' @@ -407,14 +407,14 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' test_config pull.rebase true && test_config branch.to-rebase.rebase false && git pull . copy && - test "$(git rev-parse HEAD^)" !=
[PATCH v2 11/15] t5520: test single-line files by git with test_cmp
In case an invocation of a Git command fails within the subshell, the failure will be masked. Replace the subshell with a file-redirection and a call to test_cmp. This change was done with the following GNU sed expressions: s/\(\s*\)test \([^ ]*\) = "$(\(git [^)]*\))"/\1echo \2 >expect \&\&\n\1\3 >actual \&\&\n\1test_cmp expect actual/ s/\(\s*\)test "$(\(git [^)]*\))" = \([^ ]*\)/\1echo \3 >expect \&\&\n\1\2 >actual \&\&\n\1test_cmp expect actual/ A future patch will clean up situations where we have multiple duplicate statements within a test case. This is done to keep this patch purely mechanical. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 64 - 1 file changed, 48 insertions(+), 16 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 1af6ea06ee..8b7e7ae55d 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -255,7 +255,9 @@ test_expect_success '--rebase' ' git tag before-rebase && git pull --rebase . copy && test_cmp_rev HEAD^ copy && - test new = "$(git show HEAD:file2)" + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual ' test_expect_success '--rebase fast forward' ' @@ -330,7 +332,9 @@ test_expect_success '--rebase fails with multiple branches' ' test_must_fail git pull --rebase . copy master 2>err && test_cmp_rev HEAD before-rebase && test_i18ngrep "Cannot rebase onto multiple branches" err && - test modified = "$(git show HEAD:file)" + echo modified >expect && + git show HEAD:file >actual && + test_cmp expect actual ' test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' ' @@ -381,7 +385,9 @@ test_expect_success 'pull.rebase' ' test_config pull.rebase true && git pull . copy && test_cmp_rev HEAD^ copy && - test new = "$(git show HEAD:file2)" + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual ' test_expect_success 'pull --autostash & pull.rebase=true' ' @@ -399,7 +405,9 @@ test_expect_success 'branch.to-rebase.rebase' ' test_config branch.to-rebase.rebase true && git pull . copy && test_cmp_rev HEAD^ copy && - test new = "$(git show HEAD:file2)" + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual ' test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' @@ -408,14 +416,18 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' test_config branch.to-rebase.rebase false && git pull . copy && test_cmp_rev ! HEAD^ copy && - test new = "$(git show HEAD:file2)" + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual ' test_expect_success 'pull --rebase warns on --verify-signatures' ' git reset --hard before-rebase && git pull --rebase --verify-signatures . copy 2>err && test_cmp_rev HEAD^ copy && - test new = "$(git show HEAD:file2)" && + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual && test_i18ngrep "ignoring --verify-signatures for rebase" err ' @@ -423,7 +435,9 @@ test_expect_success 'pull --rebase does not warn on --no-verify-signatures' ' git reset --hard before-rebase && git pull --rebase --no-verify-signatures . copy 2>err && test_cmp_rev HEAD^ copy && - test new = "$(git show HEAD:file2)" && + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual && test_i18ngrep ! "verify-signatures" err ' @@ -445,7 +459,9 @@ test_expect_success 'pull.rebase=false create a new merge commit' ' git pull . copy && test_cmp_rev HEAD^1 before-preserve-rebase && test_cmp_rev HEAD^2 copy && - test file3 = "$(git show HEAD:file3.t)" + echo file3 >expect && + git show HEAD:file3.t >actual && + test_cmp expect actual ' test_expect_success 'p
[PATCH v2 15/15] t5520: replace `! git` with `test_must_fail git`
Currently, if a Git command fails in an unexpected way, such as a segfault, it will be masked and ignored. Replace the ! with test_must_fail so that only expected failures pass. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index ef3dbc201a..602d996a33 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -537,7 +537,7 @@ test_expect_success 'pull --rebase=i' ' test_expect_success 'pull.rebase=invalid fails' ' git reset --hard before-preserve-rebase && test_config pull.rebase invalid && - ! git pull . copy + test_must_fail git pull . copy ' test_expect_success '--rebase=false create a new merge commit' ' @@ -572,7 +572,7 @@ test_expect_success REBASE_P \ test_expect_success '--rebase=invalid fails' ' git reset --hard before-preserve-rebase && - ! git pull --rebase=invalid . copy + test_must_fail git pull --rebase=invalid . copy ' test_expect_success '--rebase overrides pull.rebase=preserve and flattens keep-merge' ' -- 2.23.0.897.g0a19638b1e
[PATCH v2 07/15] t5520: remove spaces after redirect operator
The style for tests in Git is to have the redirect operator attached to the filename with no spaces. Fix test cases where this is not the case. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 004d5884cd..7bb9031140 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -243,10 +243,10 @@ test_expect_success 'fast-forward fails with conflicting work tree' ' test_expect_success '--rebase' ' git branch to-rebase && - echo modified again > file && + echo modified again >file && git commit -m file file && git checkout to-rebase && - echo new > file2 && + echo new >file2 && git add file2 && git commit -m "new file" && git tag before-rebase && @@ -542,10 +542,10 @@ test_expect_success '--rebase with rebased upstream' ' git checkout copy && git tag copy-orig && git reset --hard HEAD^ && - echo conflicting modification > file && + echo conflicting modification >file && git commit -m conflict file && git checkout to-rebase && - echo file > file2 && + echo file >file2 && git commit -m to-rebase file2 && git tag to-rebase-orig && git pull --rebase me copy && @@ -591,7 +591,7 @@ test_expect_success 'pull --rebase dies early with dirty working directory' ' test_config branch.to-rebase.remote me && test_config branch.to-rebase.merge refs/heads/copy && test_config branch.to-rebase.rebase true && - echo dirty >> file && + echo dirty >>file && git add file && test_must_fail git pull && test "$COPY" = "$(git rev-parse --verify me/copy)" && -- 2.23.0.897.g0a19638b1e
[PATCH v2 12/15] t5520: don't put git in upstream of pipe
Before, if the invocation of git failed, it would be masked by the pipe since only the return code of the last element of a pipe is used. Rewrite the test to put the Git command on its own line so its return code is not masked. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 8b7e7ae55d..8ddf89e550 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -668,7 +668,8 @@ test_expect_success 'pull --rebase fails on corrupt HEAD' ' ( cd corrupt && test_commit one && - obj=$(git rev-parse --verify HEAD | sed "s#^..#&/#") && + git rev-parse --verify HEAD >head && + obj=$(sed "s#^..#&/#" head) && rm -f .git/objects/$obj && test_must_fail git pull --rebase ) -- 2.23.0.897.g0a19638b1e
[PATCH v2 13/15] t5520: replace subshell cat comparison with test_cmp
We currently have many instances of `test = $(cat )` and
`test $(cat ) = `. In the case where this fails, it will be
difficult for a developer to debug since the output will be masked.
Replace these instances with invocations of test_cmp().
This change was done with the following GNU sed expressions:
s/\(\s*\)test \([^=]*\)= "$(cat \([^)]*\))"/\1echo \2>expect
\&\&\n\1test_cmp expect \3/
s/\(\s*\)test "$(cat \([^)]*\))" = \([^&]*\)\( &&\)\?$/\1echo \3
>expect \&\&\n\1test_cmp expect \2\4/
A future patch will clean up situations where we have multiple duplicate
statements within a test case. This is done to keep this patch purely
mechanical.
Signed-off-by: Denton Liu
---
t/t5520-pull.sh | 105
1 file changed, 70 insertions(+), 35 deletions(-)
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 8ddf89e550..c9e4eec004 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -15,8 +15,10 @@ test_pull_autostash () {
git add new_file &&
git pull "$@" . copy &&
test_cmp_rev HEAD^ copy &&
- test "$(cat new_file)" = dirty &&
- test "$(cat file)" = "modified again"
+ echo dirty >expect &&
+ test_cmp expect new_file &&
+ echo "modified again" >expect &&
+ test_cmp expect file
}
test_pull_autostash_fail () {
@@ -110,9 +112,11 @@ test_expect_success 'test . as a remote' '
echo updated >file &&
git commit -a -m updated &&
git checkout copy &&
- test "$(cat file)" = file &&
+ echo file >expect &&
+ test_cmp expect file &&
git pull &&
- test "$(cat file)" = updated &&
+ echo updated >expect &&
+ test_cmp expect file &&
git reflog -1 >reflog.actual &&
sed "s/^[0-9a-f][0-9a-f]*/OBJID/" reflog.actual >reflog.fuzzy &&
echo "OBJID HEAD@{0}: pull: Fast-forward" >reflog.expected &&
@@ -125,9 +129,11 @@ test_expect_success 'the default remote . should not break
explicit pull' '
git commit -a -m modified &&
git checkout copy &&
git reset --hard HEAD^ &&
- test "$(cat file)" = file &&
+ echo file >expect &&
+ test_cmp expect file &&
git pull . second &&
- test "$(cat file)" = modified &&
+ echo modified >expect &&
+ test_cmp expect file &&
git reflog -1 >reflog.actual &&
sed "s/^[0-9a-f][0-9a-f]*/OBJID/" reflog.actual >reflog.fuzzy &&
echo "OBJID HEAD@{0}: pull . second: Fast-forward" >reflog.expected &&
@@ -137,10 +143,12 @@ test_expect_success 'the default remote . should not
break explicit pull' '
test_expect_success 'fail if wildcard spec does not match any refs' '
git checkout -b test copy^ &&
test_when_finished "git checkout -f copy && git branch -D test" &&
- test "$(cat file)" = file &&
+ echo file >expect &&
+ test_cmp expect file &&
test_must_fail git pull . "refs/nonexisting1/*:refs/nonexisting2/*"
2>err &&
test_i18ngrep "no candidates for merging" err &&
- test "$(cat file)" = file
+ echo file >expect &&
+ test_cmp expect file
'
test_expect_success 'fail if no branches specified with non-default remote' '
@@ -148,11 +156,13 @@ test_expect_success 'fail if no branches specified with
non-default remote' '
test_when_finished "git remote remove test_remote" &&
git checkout -b test copy^ &&
test_when_finished "git checkout -f copy && git branch -D test" &&
- test "$(cat file)" = file &&
+ echo file >expect &&
+ test_cmp expect file &&
test_config branch.test.remote origin &&
test_must_fail git pull test_remote 2>err &&
test_i18ngrep "specify a branch on the command line" err &&
- test "$(cat file)" = file
+ echo file >expect &&
+ test_cmp expect file
'
test_expect_success 'fail if not on a branch' '
@@ -160,10 +170,12 @@ test_expect_success 'fail if not on a branch' '
test_when_finished "git remote remove origin" &&
git check
[PATCH v2 05/15] t5520: let sed open its own input
We were using a redirection operator to feed input into sed. However,
since sed is capable of opening its own files, make sed open its own
files instead of redirecting input into it.
Signed-off-by: Denton Liu
---
t/t5520-pull.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index a3de2e19b6..55560ce3cd 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -5,7 +5,7 @@ test_description='pulling into void'
. ./test-lib.sh
modify () {
- sed -e "$1" <"$2" >"$2.x" &&
+ sed -e "$1" "$2" >"$2.x" &&
mv "$2.x" "$2"
}
--
2.23.0.897.g0a19638b1e
[PATCH v2 08/15] t5520: use test_line_count where possible
Instead of rolling our own functionality to test the number of lines a command outputs, use test_line_count() which provides better debugging information in the case of a failure. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 7bb9031140..0ca4867e96 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -699,7 +699,8 @@ test_expect_success 'git pull --rebase does not reapply old patches' ' ( cd dst && test_must_fail git pull --rebase && - test 1 = $(find .git/rebase-apply -name "000*" | wc -l) + find .git/rebase-apply -name "000*" >patches && + test_line_count = 1 patches ) ' -- 2.23.0.897.g0a19638b1e
[PATCH v2 06/15] t5520: replace test -f with test-lib functions
Although `test -f` has the same functionality as test_path_is_file(), in the case where test_path_is_file() fails, we get much better debugging information. Replace `test -f` with test_path_is_file() so that future developers will have a better experience debugging these test cases. Also, in the case of `! test -f`, not only should that path not be a file, it shouldn't exist at all so replace it with test_path_is_missing(). Signed-off-by: Denton Liu --- t/t5520-pull.sh | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 55560ce3cd..004d5884cd 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -39,8 +39,8 @@ test_expect_success 'pulling into void' ' cd cloned && git pull .. ) && - test -f file && - test -f cloned/file && + test_path_is_file file && + test_path_is_file cloned/file && test_cmp file cloned/file ' @@ -50,8 +50,8 @@ test_expect_success 'pulling into void using master:master' ' cd cloned-uho && git pull .. master:master ) && - test -f file && - test -f cloned-uho/file && + test_path_is_file file && + test_path_is_file cloned-uho/file && test_cmp file cloned-uho/file ' @@ -99,7 +99,7 @@ test_expect_success 'pulling into void must not create an octopus' ' ( cd cloned-octopus && test_must_fail git pull .. master master && - ! test -f file + test_path_is_missing file ) ' -- 2.23.0.897.g0a19638b1e
[PATCH v2 04/15] t5520: use sq for test case names
The usual convention is for test case names to be written between single-quotes. Change all double-quoted test case names to single-quotes except for two test case names that use variables within. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 51d6ce8aec..a3de2e19b6 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -408,7 +408,7 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' test new = "$(git show HEAD:file2)" ' -test_expect_success "pull --rebase warns on --verify-signatures" ' +test_expect_success 'pull --rebase warns on --verify-signatures' ' git reset --hard before-rebase && git pull --rebase --verify-signatures . copy 2>err && test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && @@ -416,7 +416,7 @@ test_expect_success "pull --rebase warns on --verify-signatures" ' test_i18ngrep "ignoring --verify-signatures for rebase" err ' -test_expect_success "pull --rebase does not warn on --no-verify-signatures" ' +test_expect_success 'pull --rebase does not warn on --no-verify-signatures' ' git reset --hard before-rebase && git pull --rebase --no-verify-signatures . copy 2>err && test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && -- 2.23.0.897.g0a19638b1e
[PATCH v2 02/15] t: teach test_cmp_rev to accept ! for not-equals
Currently, in the case where we are using test_cmp_rev() to report
not-equals, we write `! test_cmp_rev`. However, since test_cmp_rev()
contains
r1=$(git rev-parse --verify "$1") &&
r2=$(git rev-parse --verify "$2") &&
In the case where `git rev-parse` segfaults and dies unexpectedly, the
failure will be ignored.
Rewrite test_cmp_rev() to optionally accept "!" as the first argument to
do a not-equals comparison. Rewrite `! test_cmp_rev` to `test_cmp_rev !`
in all tests to take advantage of this new functionality.
Signed-off-by: Denton Liu
---
t/t2400-worktree-add.sh | 4 ++--
t/t3400-rebase.sh | 2 +-
t/t3421-rebase-topology-linear.sh | 6 +++---
t/t3430-rebase-merges.sh| 2 +-
t/t3432-rebase-fast-forward.sh | 2 +-
t/t3501-revert-cherry-pick.sh | 2 +-
t/t3508-cherry-pick-many-commits.sh | 2 +-
t/test-lib-functions.sh | 13 +++--
8 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index e819ba741e..52d476979b 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -438,7 +438,7 @@ test_expect_success 'git worktree add does not match
remote' '
cd foo &&
test_must_fail git config "branch.foo.remote" &&
test_must_fail git config "branch.foo.merge" &&
- ! test_cmp_rev refs/remotes/repo_a/foo refs/heads/foo
+ test_cmp_rev ! refs/remotes/repo_a/foo refs/heads/foo
)
'
@@ -483,7 +483,7 @@ test_expect_success 'git worktree --no-guess-remote option
overrides config' '
cd foo &&
test_must_fail git config "branch.foo.remote" &&
test_must_fail git config "branch.foo.merge" &&
- ! test_cmp_rev refs/remotes/repo_a/foo refs/heads/foo
+ test_cmp_rev ! refs/remotes/repo_a/foo refs/heads/foo
)
'
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index ab18ac5f28..f267f6cd54 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -64,7 +64,7 @@ test_expect_success 'rebase sets ORIG_HEAD to pre-rebase
state' '
pre="$(git rev-parse --verify HEAD)" &&
git rebase master &&
test_cmp_rev "$pre" ORIG_HEAD &&
- ! test_cmp_rev "$pre" HEAD
+ test_cmp_rev ! "$pre" HEAD
'
test_expect_success 'rebase, with and specified as
:/quuxery' '
diff --git a/t/t3421-rebase-topology-linear.sh
b/t/t3421-rebase-topology-linear.sh
index b847064f91..325072b0a3 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -61,7 +61,7 @@ test_run_rebase () {
test_expect_$result "rebase $* -f rewrites even if upstream is an
ancestor" "
reset_rebase &&
git rebase $* -f b e &&
- ! test_cmp_rev e HEAD &&
+ test_cmp_rev ! e HEAD &&
test_cmp_rev b HEAD~2 &&
test_linear_range 'd e' b..
"
@@ -78,7 +78,7 @@ test_run_rebase () {
test_expect_$result "rebase $* -f rewrites even if remote upstream is
an ancestor" "
reset_rebase &&
git rebase $* -f branch-b branch-e &&
- ! test_cmp_rev branch-e origin/branch-e &&
+ test_cmp_rev ! branch-e origin/branch-e &&
test_cmp_rev branch-b HEAD~2 &&
test_linear_range 'd e' branch-b..
"
@@ -368,7 +368,7 @@ test_run_rebase () {
test_expect_$result "rebase $* -f --root on linear history causes
re-write" "
reset_rebase &&
git rebase $* -f --root c &&
- ! test_cmp_rev a HEAD~2 &&
+ test_cmp_rev ! a HEAD~2 &&
test_linear_range 'a b c' HEAD
"
}
diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index 9efcf4808a..abbdc26b1b 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -346,7 +346,7 @@ test_expect_success 'A root commit can be a cousin, treat
it that way' '
git merge --allow-unrelated-histories khnum &&
test_tick &&
git rebase -f -r HEAD^ &&
- ! test_cmp_rev HEAD^2 khnum &&
+ test_cmp_rev ! HEAD^2 khnum &&
test_cmp_graph HEAD^.. <<-\EOF &&
* Merge branch '\''khnum'\'' into asherah
|\
diff --git a/t/t3432-rebase-fast-forward.sh b/t/t3432-re
[PATCH v2 01/15] t7408: replace `test_must_fail test_path_is_file`
According to t/README, test_must_fail() should only be used to test for failure in git commands. Replace the only invocation of `test_must_fail test_path_is_file` with `test_path_is_missing` since in this test case, the path should not exist at all. Signed-off-by: Denton Liu --- t/t7408-submodule-reference.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh index 34ac28c056..9e62d43cac 100755 --- a/t/t7408-submodule-reference.sh +++ b/t/t7408-submodule-reference.sh @@ -123,7 +123,7 @@ test_expect_success 'missing submodule alternate fails clone and submodule updat test_must_fail git submodule update --init && # and we have no alternates: test_must_fail test_alternate_is_used .git/modules/sub/objects/info/alternates sub && - test_must_fail test_path_is_file sub/file1 + test_path_is_missing sub/file1 ) ' -- 2.23.0.897.g0a19638b1e
[PATCH v2 14/15] t5520: remove redundant lines in test cases
In the previous patches, the mechanical application of changes left some duplicate statements in the test case which were not strictly incorrect but were redundant and possibly misleading. Remove these duplicate statements so that it is clear that the intent behind the tests are that the content of the file stays the same throughout the whole test case. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 8 1 file changed, 8 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index c9e4eec004..ef3dbc201a 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -147,7 +147,6 @@ test_expect_success 'fail if wildcard spec does not match any refs' ' test_cmp expect file && test_must_fail git pull . "refs/nonexisting1/*:refs/nonexisting2/*" 2>err && test_i18ngrep "no candidates for merging" err && - echo file >expect && test_cmp expect file ' @@ -161,7 +160,6 @@ test_expect_success 'fail if no branches specified with non-default remote' ' test_config branch.test.remote origin && test_must_fail git pull test_remote 2>err && test_i18ngrep "specify a branch on the command line" err && - echo file >expect && test_cmp expect file ' @@ -174,7 +172,6 @@ test_expect_success 'fail if not on a branch' ' test_cmp expect file && test_must_fail git pull 2>err && test_i18ngrep "not currently on a branch" err && - echo file >expect && test_cmp expect file ' @@ -188,7 +185,6 @@ test_expect_success 'fail if no configuration for current branch' ' test_cmp expect file && test_must_fail git pull 2>err && test_i18ngrep "no tracking information" err && - echo file >expect && test_cmp expect file ' @@ -202,7 +198,6 @@ test_expect_success 'pull --all: fail if no configuration for current branch' ' test_cmp expect file && test_must_fail git pull --all 2>err && test_i18ngrep "There is no tracking information" err && - echo file >expect && test_cmp expect file ' @@ -215,7 +210,6 @@ test_expect_success 'fail if upstream branch does not exist' ' test_cmp expect file && test_must_fail git pull 2>err && test_i18ngrep "no such ref was fetched" err && - echo file >expect && test_cmp expect file ' @@ -685,10 +679,8 @@ test_expect_success 'pull --rebase fails on unborn branch with staged changes' ' git ls-files >actual && test_cmp expect actual && test_must_fail git pull --rebase .. master 2>err && - echo staged-file >expect && git ls-files >actual && test_cmp expect actual && - echo staged-file >expect && git show :staged-file >actual && test_cmp expect actual && test_i18ngrep "unborn branch with changes added to the index" err -- 2.23.0.897.g0a19638b1e
[PATCH v2 09/15] t5520: replace test -{n,z} with test-lib functions
When wrapping a Git command in a subshell within another command, we throw away the Git command's exit code. In case the Git command fails, we would like to know about it rather than the failure being silent. Extract Git commands so that their exit codes are not lost. Instead of using `test -n` or `test -z`, replace them respectively with invocations of test_file_not_empty() and test_must_be_empty() so that we get better debugging information in the case of a failure. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 0ca4867e96..18225d8430 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -206,15 +206,18 @@ test_expect_success 'fail if the index has unresolved entries' ' test_when_finished "git checkout -f copy && git branch -D third" && test "$(cat file)" = file && test_commit modified2 file && - test -z "$(git ls-files -u)" && + git ls-files -u >unmerged && + test_must_be_empty unmerged && test_must_fail git pull . second && - test -n "$(git ls-files -u)" && + git ls-files -u >unmerged && + test_file_not_empty unmerged && cp file expected && test_must_fail git pull . second 2>err && test_i18ngrep "Pulling is not possible because you have unmerged files." err && test_cmp expected file && git add file && - test -z "$(git ls-files -u)" && + git ls-files -u >unmerged && + test_must_be_empty unmerged && test_must_fail git pull . second 2>err && test_i18ngrep "You have not concluded your merge" err && test_cmp expected file @@ -667,7 +670,8 @@ test_expect_success 'git pull --rebase detects upstreamed changes' ' ( cd dst && git pull --rebase && - test -z "$(git ls-files -u)" + git ls-files -u >untracked && + test_must_be_empty untracked ) ' -- 2.23.0.897.g0a19638b1e
Re: [PATCH 08/12] t5520: use test_cmp_rev where possible
Hi Eric, Thanks for the reviews. I have no idea how you always get to my patches so quickly but I appreciate the prompt reviews whenever I send a test-related patchset in. I've fixed up the other concerns you had and I'll send a v2 later but I wanted to address this one. On Thu, Oct 17, 2019 at 07:41:44PM -0400, Eric Sunshine wrote: > On Thu, Oct 17, 2019 at 7:17 PM Denton Liu wrote: > > In case an invocation of `git rev-list` fails within the subshell, the > > failure will be masked. Remove the subshell and use test_cmp_rev() so > > that failures can be discovered. > > > > Signed-off-by: Denton Liu > > --- > > diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh > > @@ -597,10 +597,10 @@ test_expect_success 'pull --rebase dies early with > > dirty working directory' ' > > test_must_fail git pull && > > - test "$COPY" = "$(git rev-parse --verify me/copy)" && > > + test_cmp_rev "$COPY" me/copy && > > This transformation doesn't look correct. COPY already holds the > result of a git-rev-parse invocation: > > COPY="$(git rev-parse --verify me/copy)" && > > so passing it to test_cmp_rev() -- which applies its own git-rev-parse > invocation -- doesn't make sense. I'll annotate the entire test case with my comments: # so we'll be testing that pull --rebase dies early test_expect_success 'pull --rebase dies early with dirty working directory' ' git checkout to-rebase && git update-ref refs/remotes/me/copy copy^ && COPY="$(git rev-parse --verify me/copy)" && git rebase --onto $COPY copy && # according to git log --graph, we have this currently: # # $ git log --pretty=oneline --graph to-rebase copy me/copy # * 9366795adb50ef6eb482b610b37cb1fb6edbd3d0 (HEAD -> to-rebase) to-rebase # * b3dcf50eea47c4ad85faabb0fb74eded71cc829f file3 # * faf459539411b4557cf735232a3746be073177a9 new file # | * f340b1b8932f7b1e016c06867cbdc3f637eeea2d (copy) conflict # |/ # * 5e86d50a28977ebee5ea378f81591ea558149272 (me/second, me/copy, second) modified # * d25cf184567afad1dc1e018b2b7d793bc1bd2dc1 original test_config branch.to-rebase.remote me && test_config branch.to-rebase.merge refs/heads/copy && test_config branch.to-rebase.rebase true && # we make the tree dirty here echo dirty >>file && git add file && # and over here, the pull should fail test_must_fail git pull && # but since we're testing that it dies early, we want to # make sure that the remote ref doesn't change, which is # why it should still be equal test_cmp_rev "$COPY" me/copy && git checkout HEAD -- file && # but over here, the pull succeeds... git pull && # so as a result, the remote ref should now be updated test_cmp_rev ! "$COPY" me/copy ' So after grokking the test case, it seems like the the transformation is indeed correct. Maybe we can replace the last line with test_cmp_rev copy me/copy but I think I'll leave it unless you have any strong opinions. Thanks, Denton
[PATCH 4.5/12] t5520: replace test -f with test-lib functions
Although `test -f` has the same functionality as test_path_is_file(), in the case where test_path_is_file() fails, we get much better debugging information. Replace `test -f` with test_path_is_file() so that future developers will have a better experience debugging these test cases. Also, in the case of `! test -f`, replace it with test_path_is_missing(). Signed-off-by: Denton Liu --- I just realised that test_path_is_missing() is a much better replacement than `test_must_fail test_path_is_file`. Please use this patch instead of 04/12. t/t5520-pull.sh | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 55560ce3cd..004d5884cd 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -39,8 +39,8 @@ test_expect_success 'pulling into void' ' cd cloned && git pull .. ) && - test -f file && - test -f cloned/file && + test_path_is_file file && + test_path_is_file cloned/file && test_cmp file cloned/file ' @@ -50,8 +50,8 @@ test_expect_success 'pulling into void using master:master' ' cd cloned-uho && git pull .. master:master ) && - test -f file && - test -f cloned-uho/file && + test_path_is_file file && + test_path_is_file cloned-uho/file && test_cmp file cloned-uho/file ' @@ -99,7 +99,7 @@ test_expect_success 'pulling into void must not create an octopus' ' ( cd cloned-octopus && test_must_fail git pull .. master master && - ! test -f file + test_path_is_missing file ) ' -- 2.23.0.897.g0a19638b1e
[PATCH 00/12] t5520: various test cleanup
Like earlier patchsets, I want to implement a feature that involves
modifications to the test suite. Since that feature will probably take a
while to polish up, however, let's clean up the test suite in a separate
patchset first so it's not blocked by the feature work.
Denton Liu (12):
t5520: improve test style
t5520: use sq for test case names
t5520: let sed open its own input
t5520: replace test -f with test_path_is_file
t5520: remove spaces after redirect operator
t5520: use test_line_count where possible
t5520: replace test -{n,z} with test-lib functions
t5520: use test_cmp_rev where possible
t5520: test single-line files by git with test_cmp
t5520: don't put git in upstream of pipe
t5520: replace subshell cat comparison with test_cmp
t5520: replace `! git` with `test_must_fail git`
t/t5520-pull.sh | 351 +---
1 file changed, 213 insertions(+), 138 deletions(-)
--
2.23.0.897.g0a19638b1e
[PATCH 01/12] t5520: improve test style
Improve the test style by removing leading and trailing empty lines within test cases. Also, reformat multi-line subshells to conform to the existing style. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 88 + 1 file changed, 45 insertions(+), 43 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index cf4cc32fd0..51d6ce8aec 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -538,7 +538,6 @@ test_expect_success '--rebase overrides pull.rebase=preserve and flattens keep-m ' test_expect_success '--rebase with rebased upstream' ' - git remote add -f me . && git checkout copy && git tag copy-orig && @@ -552,7 +551,6 @@ test_expect_success '--rebase with rebased upstream' ' git pull --rebase me copy && test "conflicting modification" = "$(cat file)" && test file = "$(cat file2)" - ' test_expect_success '--rebase -f with rebased upstream' ' @@ -564,14 +562,12 @@ test_expect_success '--rebase -f with rebased upstream' ' ' test_expect_success '--rebase with rebased default upstream' ' - git update-ref refs/remotes/me/copy copy-orig && git checkout --track -b to-rebase2 me/copy && git reset --hard to-rebase-orig && git pull --rebase && test "conflicting modification" = "$(cat file)" && test file = "$(cat file2)" - ' test_expect_success 'rebased upstream + fetch + pull --rebase' ' @@ -588,7 +584,6 @@ test_expect_success 'rebased upstream + fetch + pull --rebase' ' ' test_expect_success 'pull --rebase dies early with dirty working directory' ' - git checkout to-rebase && git update-ref refs/remotes/me/copy copy^ && COPY="$(git rev-parse --verify me/copy)" && @@ -603,16 +598,16 @@ test_expect_success 'pull --rebase dies early with dirty working directory' ' git checkout HEAD -- file && git pull && test "$COPY" != "$(git rev-parse --verify me/copy)" - ' test_expect_success 'pull --rebase works on branch yet to be born' ' git rev-parse master >expect && mkdir empty_repo && - (cd empty_repo && -git init && -git pull --rebase .. master && -git rev-parse HEAD >../actual + ( + cd empty_repo && + git init && + git pull --rebase .. master && + git rev-parse HEAD >../actual ) && test_cmp expect actual ' @@ -646,58 +641,65 @@ test_expect_success 'pull --rebase fails on corrupt HEAD' ' test_expect_success 'setup for detecting upstreamed changes' ' mkdir src && - (cd src && -git init && -printf "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n" > stuff && -git add stuff && -git commit -m "Initial revision" + ( + cd src && + git init && + printf "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n" > stuff && + git add stuff && + git commit -m "Initial revision" ) && git clone src dst && - (cd src && -modify s/5/43/ stuff && -git commit -a -m "5->43" && -modify s/6/42/ stuff && -git commit -a -m "Make it bigger" + ( + cd src && + modify s/5/43/ stuff && + git commit -a -m "5->43" && + modify s/6/42/ stuff && + git commit -a -m "Make it bigger" ) && - (cd dst && -modify s/5/43/ stuff && -git commit -a -m "Independent discovery of 5->43" + ( + cd dst && + modify s/5/43/ stuff && + git commit -a -m "Independent discovery of 5->43" ) ' test_expect_success 'git pull --rebase detects upstreamed changes' ' - (cd dst && -git pull --rebase && -test -z "$(git ls-files -u)" + ( + cd dst && + git pull --rebase && + test -z "$(git ls-files -u)" ) ' test_expect_success 'setup for avoiding reapplying old patches'
[PATCH 05/12] t5520: remove spaces after redirect operator
The style for tests in Git is to have the redirect operator attached to the filename with no spaces. Fix test cases where this is not the case. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 5ab5ec508a..68b8822db2 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -243,10 +243,10 @@ test_expect_success 'fast-forward fails with conflicting work tree' ' test_expect_success '--rebase' ' git branch to-rebase && - echo modified again > file && + echo modified again >file && git commit -m file file && git checkout to-rebase && - echo new > file2 && + echo new >file2 && git add file2 && git commit -m "new file" && git tag before-rebase && @@ -542,10 +542,10 @@ test_expect_success '--rebase with rebased upstream' ' git checkout copy && git tag copy-orig && git reset --hard HEAD^ && - echo conflicting modification > file && + echo conflicting modification >file && git commit -m conflict file && git checkout to-rebase && - echo file > file2 && + echo file >file2 && git commit -m to-rebase file2 && git tag to-rebase-orig && git pull --rebase me copy && @@ -591,7 +591,7 @@ test_expect_success 'pull --rebase dies early with dirty working directory' ' test_config branch.to-rebase.remote me && test_config branch.to-rebase.merge refs/heads/copy && test_config branch.to-rebase.rebase true && - echo dirty >> file && + echo dirty >>file && git add file && test_must_fail git pull && test "$COPY" = "$(git rev-parse --verify me/copy)" && -- 2.23.0.897.g0a19638b1e
[PATCH 03/12] t5520: let sed open its own input
We were using a redirection operator to feed input into sed. However,
since sed is capable of opening its own files, make sed open its own
files instead of redirecting input into it.
Signed-off-by: Denton Liu
---
t/t5520-pull.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index a3de2e19b6..55560ce3cd 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -5,7 +5,7 @@ test_description='pulling into void'
. ./test-lib.sh
modify () {
- sed -e "$1" <"$2" >"$2.x" &&
+ sed -e "$1" "$2" >"$2.x" &&
mv "$2.x" "$2"
}
--
2.23.0.897.g0a19638b1e
[PATCH 04/12] t5520: replace test -f with test_path_is_file
Although `test -f` has the same functionality as test_path_is_file(), in the case where test_path_is_file() fails, we get much better debugging information. Replace `test -f` with test_path_is_file so that future developers will have a better experience debugging these test cases. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 55560ce3cd..5ab5ec508a 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -39,8 +39,8 @@ test_expect_success 'pulling into void' ' cd cloned && git pull .. ) && - test -f file && - test -f cloned/file && + test_path_is_file file && + test_path_is_file cloned/file && test_cmp file cloned/file ' @@ -50,8 +50,8 @@ test_expect_success 'pulling into void using master:master' ' cd cloned-uho && git pull .. master:master ) && - test -f file && - test -f cloned-uho/file && + test_path_is_file file && + test_path_is_file cloned-uho/file && test_cmp file cloned-uho/file ' @@ -99,7 +99,7 @@ test_expect_success 'pulling into void must not create an octopus' ' ( cd cloned-octopus && test_must_fail git pull .. master master && - ! test -f file + test_must_fail test_path_is_file file ) ' -- 2.23.0.897.g0a19638b1e
[PATCH 11/12] t5520: replace subshell cat comparison with test_cmp
We currently have many instances of `test = $(cat )` and
`test $(cat ) = `. In the case where this fails, it will be
difficult for a developer to debug since the output will be masked.
Replace these instances with invocations of test_cmp().
This change was done with the following GNU sed expressions:
s/\(\s*\)test \([^=]*\)= "$(cat \([^)]*\))"/\1echo \2>expect
\&\&\n\1test_cmp expect \3/
s/\(\s*\)test "$(cat \([^)]*\))" = \([^&]*\)\( &&\)\?$/\1echo \3
>expect \&\&\n\1test_cmp expect \2\4/
Signed-off-by: Denton Liu
---
t/t5520-pull.sh | 105
1 file changed, 70 insertions(+), 35 deletions(-)
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index e611d414c8..c6425d20ab 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -15,8 +15,10 @@ test_pull_autostash () {
git add new_file &&
git pull "$@" . copy &&
test_cmp_rev HEAD^ copy &&
- test "$(cat new_file)" = dirty &&
- test "$(cat file)" = "modified again"
+ echo dirty >expect &&
+ test_cmp expect new_file &&
+ echo "modified again" >expect &&
+ test_cmp expect file
}
test_pull_autostash_fail () {
@@ -110,9 +112,11 @@ test_expect_success 'test . as a remote' '
echo updated >file &&
git commit -a -m updated &&
git checkout copy &&
- test "$(cat file)" = file &&
+ echo file >expect &&
+ test_cmp expect file &&
git pull &&
- test "$(cat file)" = updated &&
+ echo updated >expect &&
+ test_cmp expect file &&
git reflog -1 >reflog.actual &&
sed "s/^[0-9a-f][0-9a-f]*/OBJID/" reflog.actual >reflog.fuzzy &&
echo "OBJID HEAD@{0}: pull: Fast-forward" >reflog.expected &&
@@ -125,9 +129,11 @@ test_expect_success 'the default remote . should not break
explicit pull' '
git commit -a -m modified &&
git checkout copy &&
git reset --hard HEAD^ &&
- test "$(cat file)" = file &&
+ echo file >expect &&
+ test_cmp expect file &&
git pull . second &&
- test "$(cat file)" = modified &&
+ echo modified >expect &&
+ test_cmp expect file &&
git reflog -1 >reflog.actual &&
sed "s/^[0-9a-f][0-9a-f]*/OBJID/" reflog.actual >reflog.fuzzy &&
echo "OBJID HEAD@{0}: pull . second: Fast-forward" >reflog.expected &&
@@ -137,10 +143,12 @@ test_expect_success 'the default remote . should not
break explicit pull' '
test_expect_success 'fail if wildcard spec does not match any refs' '
git checkout -b test copy^ &&
test_when_finished "git checkout -f copy && git branch -D test" &&
- test "$(cat file)" = file &&
+ echo file >expect &&
+ test_cmp expect file &&
test_must_fail git pull . "refs/nonexisting1/*:refs/nonexisting2/*"
2>err &&
test_i18ngrep "no candidates for merging" err &&
- test "$(cat file)" = file
+ echo file >expect &&
+ test_cmp expect file
'
test_expect_success 'fail if no branches specified with non-default remote' '
@@ -148,11 +156,13 @@ test_expect_success 'fail if no branches specified with
non-default remote' '
test_when_finished "git remote remove test_remote" &&
git checkout -b test copy^ &&
test_when_finished "git checkout -f copy && git branch -D test" &&
- test "$(cat file)" = file &&
+ echo file >expect &&
+ test_cmp expect file &&
test_config branch.test.remote origin &&
test_must_fail git pull test_remote 2>err &&
test_i18ngrep "specify a branch on the command line" err &&
- test "$(cat file)" = file
+ echo file >expect &&
+ test_cmp expect file
'
test_expect_success 'fail if not on a branch' '
@@ -160,10 +170,12 @@ test_expect_success 'fail if not on a branch' '
test_when_finished "git remote remove origin" &&
git checkout HEAD^ &&
test_when_finished "git checkout -f copy" &&
- test "$(cat file)" = file &&
+ echo fil
[PATCH 10/12] t5520: don't put git in upstream of pipe
Before, if the invocation of git failed, it would be masked by the pipe since only the return code of the last element of a pipe is used. Rewrite the test to put the Git command on its own line so its return code is not masked. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 0eebab7d86..e611d414c8 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -668,7 +668,8 @@ test_expect_success 'pull --rebase fails on corrupt HEAD' ' ( cd corrupt && test_commit one && - obj=$(git rev-parse --verify HEAD | sed "s#^..#&/#") && + git rev-parse --verify HEAD >head && + obj=$(sed "s#^..#&/#" head) && rm -f .git/objects/$obj && test_must_fail git pull --rebase ) -- 2.23.0.897.g0a19638b1e
[PATCH 02/12] t5520: use sq for test case names
The usual convention is for test case names to be written between single-quotes. Change all double-quoted test case names to single-quotes except for two test case names that use variables within. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 51d6ce8aec..a3de2e19b6 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -408,7 +408,7 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' test new = "$(git show HEAD:file2)" ' -test_expect_success "pull --rebase warns on --verify-signatures" ' +test_expect_success 'pull --rebase warns on --verify-signatures' ' git reset --hard before-rebase && git pull --rebase --verify-signatures . copy 2>err && test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && @@ -416,7 +416,7 @@ test_expect_success "pull --rebase warns on --verify-signatures" ' test_i18ngrep "ignoring --verify-signatures for rebase" err ' -test_expect_success "pull --rebase does not warn on --no-verify-signatures" ' +test_expect_success 'pull --rebase does not warn on --no-verify-signatures' ' git reset --hard before-rebase && git pull --rebase --no-verify-signatures . copy 2>err && test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && -- 2.23.0.897.g0a19638b1e
[PATCH 12/12] t5520: replace `! git` with `test_must_fail git`
Currently, if a Git command fails in an unexpected way, such as a segfault, it will be masked and ignored. Replace the ! with test_must_fail so that only expected failures pass. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index c6425d20ab..0c187b025b 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -543,7 +543,7 @@ test_expect_success 'pull --rebase=i' ' test_expect_success 'pull.rebase=invalid fails' ' git reset --hard before-preserve-rebase && test_config pull.rebase invalid && - ! git pull . copy + test_must_fail git pull . copy ' test_expect_success '--rebase=false create a new merge commit' ' @@ -578,7 +578,7 @@ test_expect_success REBASE_P \ test_expect_success '--rebase=invalid fails' ' git reset --hard before-preserve-rebase && - ! git pull --rebase=invalid . copy + test_must_fail git pull --rebase=invalid . copy ' test_expect_success '--rebase overrides pull.rebase=preserve and flattens keep-merge' ' -- 2.23.0.897.g0a19638b1e
[PATCH 08/12] t5520: use test_cmp_rev where possible
In case an invocation of `git rev-list` fails within the subshell, the failure will be masked. Remove the subshell and use test_cmp_rev() so that failures can be discovered. This change was done with the following sed expressions: s/test "$(git rev-parse.* \([^)]*\))" = "$(git rev-parse \([^)]*\))"/test_cmp_rev \1 \2/ s/test \([^ ]*\) = "$(git rev-parse.* \([^)]*\))"/test_cmp_rev \1 \2/ s/test "$(git rev-parse.* \([^)]*\))" != "$(git rev-parse.* \([^)]*\))"/test_must_fail test_cmp_rev \1 \2/ s/test \([^ ]*\) != "$(git rev-parse.* \([^)]*\))"/test_must_fail test_cmp_rev \1 \2/ Signed-off-by: Denton Liu --- t/t5520-pull.sh | 50 - 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index c7797b13e6..f11fadc075 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -230,7 +230,7 @@ test_expect_success 'fast-forwards working tree if branch head is updated' ' git pull . second:third 2>err && test_i18ngrep "fetch updated the current branch head" err && test "$(cat file)" = modified && - test "$(git rev-parse third)" = "$(git rev-parse second)" + test_cmp_rev third second ' test_expect_success 'fast-forward fails with conflicting work tree' ' @@ -241,7 +241,7 @@ test_expect_success 'fast-forward fails with conflicting work tree' ' test_must_fail git pull . second:third 2>err && test_i18ngrep "Cannot fast-forward your working tree" err && test "$(cat file)" = conflict && - test "$(git rev-parse third)" = "$(git rev-parse second)" + test_cmp_rev third second ' test_expect_success '--rebase' ' @@ -254,7 +254,7 @@ test_expect_success '--rebase' ' git commit -m "new file" && git tag before-rebase && git pull --rebase . copy && - test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && + test_cmp_rev HEAD^ copy && test new = "$(git show HEAD:file2)" ' @@ -266,7 +266,7 @@ test_expect_success '--rebase fast forward' ' git checkout to-rebase && git pull --rebase . ff && - test "$(git rev-parse HEAD)" = "$(git rev-parse ff)" && + test_cmp_rev HEAD ff && # The above only validates the result. Did we actually bypass rebase? git reflog -1 >reflog.actual && @@ -290,7 +290,7 @@ test_expect_success '--rebase --autostash fast forward' ' git checkout behind && echo dirty >file && git pull --rebase --autostash . to-rebase-ff && - test "$(git rev-parse HEAD)" = "$(git rev-parse to-rebase-ff)" + test_cmp_rev HEAD to-rebase-ff ' test_expect_success '--rebase with conflicts shows advice' ' @@ -328,7 +328,7 @@ test_expect_success 'failed --rebase shows advice' ' test_expect_success '--rebase fails with multiple branches' ' git reset --hard before-rebase && test_must_fail git pull --rebase . copy master 2>err && - test "$(git rev-parse HEAD)" = "$(git rev-parse before-rebase)" && + test_cmp_rev HEAD before-rebase && test_i18ngrep "Cannot rebase onto multiple branches" err && test modified = "$(git show HEAD:file)" ' @@ -380,7 +380,7 @@ test_expect_success 'pull.rebase' ' git reset --hard before-rebase && test_config pull.rebase true && git pull . copy && - test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && + test_cmp_rev HEAD^ copy && test new = "$(git show HEAD:file2)" ' @@ -398,7 +398,7 @@ test_expect_success 'branch.to-rebase.rebase' ' git reset --hard before-rebase && test_config branch.to-rebase.rebase true && git pull . copy && - test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && + test_cmp_rev HEAD^ copy && test new = "$(git show HEAD:file2)" ' @@ -407,14 +407,14 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' test_config pull.rebase true && test_config branch.to-rebase.rebase false && git pull . copy && - test "$(g
[PATCH 09/12] t5520: test single-line files by git with test_cmp
In case an invocation of a Git command fails within the subshell, the failure will be masked. Replace the subshell with a file-redirection and a call to test_cmp. This change was done with the following GNU sed expressions: s/\(\s*\)test \([^ ]*\) = "$(\(git [^)]*\))"/\1echo \2 >expect \&\&\n\1\3 >actual \&\&\n\1test_cmp expect actual/ s/\(\s*\)test "$(\(git [^)]*\))" = \([^ ]*\)/\1echo \3 >expect \&\&\n\1\2 >actual \&\&\n\1test_cmp expect actual/ Signed-off-by: Denton Liu --- t/t5520-pull.sh | 64 - 1 file changed, 48 insertions(+), 16 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index f11fadc075..0eebab7d86 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -255,7 +255,9 @@ test_expect_success '--rebase' ' git tag before-rebase && git pull --rebase . copy && test_cmp_rev HEAD^ copy && - test new = "$(git show HEAD:file2)" + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual ' test_expect_success '--rebase fast forward' ' @@ -330,7 +332,9 @@ test_expect_success '--rebase fails with multiple branches' ' test_must_fail git pull --rebase . copy master 2>err && test_cmp_rev HEAD before-rebase && test_i18ngrep "Cannot rebase onto multiple branches" err && - test modified = "$(git show HEAD:file)" + echo modified >expect && + git show HEAD:file >actual && + test_cmp expect actual ' test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' ' @@ -381,7 +385,9 @@ test_expect_success 'pull.rebase' ' test_config pull.rebase true && git pull . copy && test_cmp_rev HEAD^ copy && - test new = "$(git show HEAD:file2)" + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual ' test_expect_success 'pull --autostash & pull.rebase=true' ' @@ -399,7 +405,9 @@ test_expect_success 'branch.to-rebase.rebase' ' test_config branch.to-rebase.rebase true && git pull . copy && test_cmp_rev HEAD^ copy && - test new = "$(git show HEAD:file2)" + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual ' test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' @@ -408,14 +416,18 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' test_config branch.to-rebase.rebase false && git pull . copy && test_must_fail test_cmp_rev HEAD^ copy && - test new = "$(git show HEAD:file2)" + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual ' test_expect_success 'pull --rebase warns on --verify-signatures' ' git reset --hard before-rebase && git pull --rebase --verify-signatures . copy 2>err && test_cmp_rev HEAD^ copy && - test new = "$(git show HEAD:file2)" && + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual && test_i18ngrep "ignoring --verify-signatures for rebase" err ' @@ -423,7 +435,9 @@ test_expect_success 'pull --rebase does not warn on --no-verify-signatures' ' git reset --hard before-rebase && git pull --rebase --no-verify-signatures . copy 2>err && test_cmp_rev HEAD^ copy && - test new = "$(git show HEAD:file2)" && + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual && test_i18ngrep ! "verify-signatures" err ' @@ -445,7 +459,9 @@ test_expect_success 'pull.rebase=false create a new merge commit' ' git pull . copy && test_cmp_rev HEAD^1 before-preserve-rebase && test_cmp_rev HEAD^2 copy && - test file3 = "$(git show HEAD:file3.t)" + echo file3 >expect && + git show HEAD:file3.t >actual && + test_cmp expect actual ' test_expect_success 'pull.rebase=true flattens keep-merge' ' @@ -453,7 +469,9 @@ test_expect_success 'pull.rebase=true flattens keep-merge' ' test_conf
[PATCH 07/12] t5520: replace test -{n,z} with test-lib functions
Instead of using `test -n` or `test -z`, replace them respectively with invocations of test_file_not_empty() and test_must_be_empty(). Signed-off-by: Denton Liu --- t/t5520-pull.sh | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 9452779f40..c7797b13e6 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -206,15 +206,18 @@ test_expect_success 'fail if the index has unresolved entries' ' test_when_finished "git checkout -f copy && git branch -D third" && test "$(cat file)" = file && test_commit modified2 file && - test -z "$(git ls-files -u)" && + git ls-files -u >unmerged && + test_must_be_empty unmerged && test_must_fail git pull . second && - test -n "$(git ls-files -u)" && + git ls-files -u >unmerged && + test_file_not_empty unmerged && cp file expected && test_must_fail git pull . second 2>err && test_i18ngrep "Pulling is not possible because you have unmerged files." err && test_cmp expected file && git add file && - test -z "$(git ls-files -u)" && + git ls-files -u >unmerged && + test_must_be_empty unmerged && test_must_fail git pull . second 2>err && test_i18ngrep "You have not concluded your merge" err && test_cmp expected file @@ -667,7 +670,8 @@ test_expect_success 'git pull --rebase detects upstreamed changes' ' ( cd dst && git pull --rebase && - test -z "$(git ls-files -u)" + git ls-files -u >untracked && + test_must_be_empty untracked ) ' -- 2.23.0.897.g0a19638b1e
[PATCH 06/12] t5520: use test_line_count where possible
Instead of rolling our own functionality to test the number of lines a command outputs, use test_line_count() which provides better debugging information in the case of a failure. Signed-off-by: Denton Liu --- t/t5520-pull.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 68b8822db2..9452779f40 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -699,7 +699,8 @@ test_expect_success 'git pull --rebase does not reapply old patches' ' ( cd dst && test_must_fail git pull --rebase && - test 1 = $(find .git/rebase-apply -name "000*" | wc -l) + find .git/rebase-apply -name "000*" >patches && + test_line_count = 1 patches ) ' -- 2.23.0.897.g0a19638b1e
[RFC PATCH 6/7] autostash.c: undefine USE_THE_INDEX_COMPATIBILITY_MACROS
Since autostash.c was recently introduced, we should avoid
USE_THE_INDEX_COMPATIBILITY_MACROS since we are trying to move away from
this in the rest of the codebase. Rewrite the autostash code to not need
it and remove its definition.
Signed-off-by: Denton Liu
---
autostash.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/autostash.c b/autostash.c
index 722cf78b12..0a1f00d2e5 100644
--- a/autostash.c
+++ b/autostash.c
@@ -1,5 +1,3 @@
-#define USE_THE_INDEX_COMPATIBILITY_MACROS
-
#include "git-compat-util.h"
#include "autostash.h"
#include "cache-tree.h"
@@ -46,7 +44,7 @@ int reset_head(struct object_id *oid, const char *action,
if (switch_to_branch && !starts_with(switch_to_branch, "refs/"))
BUG("Not a fully qualified branch: '%s'", switch_to_branch);
- if (!refs_only && hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0) {
+ if (!refs_only && repo_hold_locked_index(the_repository, &lock,
LOCK_REPORT_ON_ERROR) < 0) {
ret = -1;
goto leave_reset_head;
}
@@ -157,8 +155,8 @@ void perform_autostash(const char *path)
struct lock_file lock_file = LOCK_INIT;
int fd;
- fd = hold_locked_index(&lock_file, 0);
- refresh_cache(REFRESH_QUIET);
+ fd = repo_hold_locked_index(the_repository, &lock_file, 0);
+ refresh_index(the_repository->index, REFRESH_QUIET, NULL, NULL, NULL);
if (0 <= fd)
repo_update_index_if_able(the_repository, &lock_file);
rollback_lock_file(&lock_file);
--
2.23.0.897.g0a19638b1e
[RFC PATCH 1/7] Makefile: alphabetically sort += lists
There are many += lists in the Makefile and, over time, they have gotten slightly out of order, alphabetically. Alphabetically sort all += lists to bring them back in order. Signed-off-by: Denton Liu --- Makefile | 75 1 file changed, 38 insertions(+), 37 deletions(-) diff --git a/Makefile b/Makefile index de60c8e7aa..268a273df5 100644 --- a/Makefile +++ b/Makefile @@ -604,12 +604,12 @@ unexport CDPATH SCRIPT_SH += git-bisect.sh SCRIPT_SH += git-difftool--helper.sh SCRIPT_SH += git-filter-branch.sh +SCRIPT_SH += git-legacy-stash.sh SCRIPT_SH += git-merge-octopus.sh SCRIPT_SH += git-merge-one-file.sh SCRIPT_SH += git-merge-resolve.sh SCRIPT_SH += git-mergetool.sh SCRIPT_SH += git-quiltimport.sh -SCRIPT_SH += git-legacy-stash.sh SCRIPT_SH += git-request-pull.sh SCRIPT_SH += git-submodule.sh SCRIPT_SH += git-web--browse.sh @@ -617,8 +617,8 @@ SCRIPT_SH += git-web--browse.sh SCRIPT_LIB += git-mergetool--lib SCRIPT_LIB += git-parse-remote SCRIPT_LIB += git-rebase--preserve-merges -SCRIPT_LIB += git-sh-setup SCRIPT_LIB += git-sh-i18n +SCRIPT_LIB += git-sh-setup SCRIPT_PERL += git-add--interactive.perl SCRIPT_PERL += git-archimport.perl @@ -686,9 +686,9 @@ PROGRAM_OBJS += daemon.o PROGRAM_OBJS += fast-import.o PROGRAM_OBJS += http-backend.o PROGRAM_OBJS += imap-send.o +PROGRAM_OBJS += remote-testsvn.o PROGRAM_OBJS += sh-i18n--envsubst.o PROGRAM_OBJS += shell.o -PROGRAM_OBJS += remote-testsvn.o # Binary suffix, set to .exe for Windows builds X = @@ -709,9 +709,9 @@ TEST_BUILTINS_OBJS += test-dump-untracked-cache.o TEST_BUILTINS_OBJS += test-example-decorate.o TEST_BUILTINS_OBJS += test-genrandom.o TEST_BUILTINS_OBJS += test-genzeros.o +TEST_BUILTINS_OBJS += test-hash-speed.o TEST_BUILTINS_OBJS += test-hash.o TEST_BUILTINS_OBJS += test-hashmap.o -TEST_BUILTINS_OBJS += test-hash-speed.o TEST_BUILTINS_OBJS += test-index-version.o TEST_BUILTINS_OBJS += test-json-writer.o TEST_BUILTINS_OBJS += test-lazy-init-name-hash.o @@ -735,8 +735,8 @@ TEST_BUILTINS_OBJS += test-revision-walking.o TEST_BUILTINS_OBJS += test-run-command.o TEST_BUILTINS_OBJS += test-scrap-cache-tree.o TEST_BUILTINS_OBJS += test-serve-v2.o -TEST_BUILTINS_OBJS += test-sha1.o TEST_BUILTINS_OBJS += test-sha1-array.o +TEST_BUILTINS_OBJS += test-sha1.o TEST_BUILTINS_OBJS += test-sha256.o TEST_BUILTINS_OBJS += test-sigchain.o TEST_BUILTINS_OBJS += test-strcmp-offset.o @@ -746,10 +746,10 @@ TEST_BUILTINS_OBJS += test-submodule-nested-repo-config.o TEST_BUILTINS_OBJS += test-subprocess.o TEST_BUILTINS_OBJS += test-trace2.o TEST_BUILTINS_OBJS += test-urlmatch-normalization.o -TEST_BUILTINS_OBJS += test-xml-encode.o TEST_BUILTINS_OBJS += test-wildmatch.o TEST_BUILTINS_OBJS += test-windows-named-pipe.o TEST_BUILTINS_OBJS += test-write-cache.o +TEST_BUILTINS_OBJS += test-xml-encode.o # Do not add more tests here unless they have extra dependencies. Add # them in TEST_BUILTINS_OBJS above. @@ -786,10 +786,10 @@ OTHER_PROGRAMS = git$X # what test wrappers are needed and 'install' will install, in bindir BINDIR_PROGRAMS_NEED_X += git -BINDIR_PROGRAMS_NEED_X += git-upload-pack BINDIR_PROGRAMS_NEED_X += git-receive-pack -BINDIR_PROGRAMS_NEED_X += git-upload-archive BINDIR_PROGRAMS_NEED_X += git-shell +BINDIR_PROGRAMS_NEED_X += git-upload-archive +BINDIR_PROGRAMS_NEED_X += git-upload-pack BINDIR_PROGRAMS_NO_X += git-cvsserver @@ -827,11 +827,12 @@ LIB_OBJS += advice.o LIB_OBJS += alias.o LIB_OBJS += alloc.o LIB_OBJS += apply.o -LIB_OBJS += archive.o LIB_OBJS += archive-tar.o LIB_OBJS += archive-zip.o +LIB_OBJS += archive.o LIB_OBJS += argv-array.o LIB_OBJS += attr.o +LIB_OBJS += autostash.o LIB_OBJS += base85.o LIB_OBJS += bisect.o LIB_OBJS += blame.o @@ -845,9 +846,9 @@ LIB_OBJS += checkout.o LIB_OBJS += color.o LIB_OBJS += column.o LIB_OBJS += combine-diff.o -LIB_OBJS += commit.o LIB_OBJS += commit-graph.o LIB_OBJS += commit-reach.o +LIB_OBJS += commit.o LIB_OBJS += compat/obstack.o LIB_OBJS += compat/terminal.o LIB_OBJS += config.o @@ -861,17 +862,17 @@ LIB_OBJS += ctype.o LIB_OBJS += date.o LIB_OBJS += decorate.o LIB_OBJS += delta-islands.o +LIB_OBJS += diff-delta.o +LIB_OBJS += diff-lib.o +LIB_OBJS += diff-no-index.o +LIB_OBJS += diff.o LIB_OBJS += diffcore-break.o LIB_OBJS += diffcore-delta.o LIB_OBJS += diffcore-order.o LIB_OBJS += diffcore-pickaxe.o LIB_OBJS += diffcore-rename.o -LIB_OBJS += diff-delta.o -LIB_OBJS += diff-lib.o -LIB_OBJS += diff-no-index.o -LIB_OBJS += diff.o -LIB_OBJS += dir.o LIB_OBJS += dir-iterator.o +LIB_OBJS += dir.o LIB_OBJS += editor.o LIB_OBJS += entry.o LIB_OBJS += environment.o @@ -889,7 +890,6 @@ LIB_OBJS += gpg-interface.o LIB_OBJS += graph.o LIB_OBJS += grep.o LIB_OBJS += hashmap.o -LIB_OBJS += linear-assignment.o LIB_OBJS += help.o LIB_OBJS += hex.o LIB_OBJS += ident.o @@ -899,9 +899,10 @@ LIB_OBJS += kwset.o LIB_OBJS += levenshtein.o LIB_OBJS += line-log.o
[RFC PATCH 5/7] autostash: extract perform_autostash() from rebase
Continue the process of lib-ifying the autostash code. In a future
commit, this will be used to implement `--autostash` in other builtins.
This patch is best viewed with `--color-moved` and
`--color-moved-ws=allow-indentation-change`.
Signed-off-by: Denton Liu
---
autostash.c | 46 ++
autostash.h | 1 +
builtin/rebase.c | 44 +---
3 files changed, 48 insertions(+), 43 deletions(-)
diff --git a/autostash.c b/autostash.c
index eb58e0c8a4..722cf78b12 100644
--- a/autostash.c
+++ b/autostash.c
@@ -12,6 +12,7 @@
#include "tree-walk.h"
#include "tree.h"
#include "unpack-trees.h"
+#include "wt-status.h"
int read_one(const char *path, struct strbuf *buf)
{
@@ -150,6 +151,51 @@ int reset_head(struct object_id *oid, const char *action,
return ret;
}
+void perform_autostash(const char *path)
+{
+ struct strbuf buf = STRBUF_INIT;
+ struct lock_file lock_file = LOCK_INIT;
+ int fd;
+
+ fd = hold_locked_index(&lock_file, 0);
+ refresh_cache(REFRESH_QUIET);
+ if (0 <= fd)
+ repo_update_index_if_able(the_repository, &lock_file);
+ rollback_lock_file(&lock_file);
+
+ if (has_unstaged_changes(the_repository, 1) ||
+ has_uncommitted_changes(the_repository, 1)) {
+ struct child_process stash = CHILD_PROCESS_INIT;
+ struct object_id oid;
+
+ argv_array_pushl(&stash.args,
+"stash", "create", "autostash", NULL);
+ stash.git_cmd = 1;
+ stash.no_stdin = 1;
+ if (capture_command(&stash, &buf, GIT_MAX_HEXSZ))
+ die(_("Cannot autostash"));
+ strbuf_trim_trailing_newline(&buf);
+ if (get_oid(buf.buf, &oid))
+ die(_("Unexpected stash response: '%s'"),
+ buf.buf);
+ strbuf_reset(&buf);
+ strbuf_add_unique_abbrev(&buf, &oid, DEFAULT_ABBREV);
+
+ if (safe_create_leading_directories_const(path))
+ die(_("Could not create directory for '%s'"),
+ path);
+ write_file(path, "%s", oid_to_hex(&oid));
+ printf(_("Created autostash: %s\n"), buf.buf);
+ if (reset_head(NULL, "reset --hard",
+ NULL, RESET_HEAD_HARD, NULL, NULL) < 0)
+ die(_("could not reset --hard"));
+
+ if (discard_index(the_repository->index) < 0 ||
+ repo_read_index(the_repository) < 0)
+ die(_("could not read index"));
+ }
+}
+
int apply_autostash(const char *path)
{
struct strbuf autostash = STRBUF_INIT;
diff --git a/autostash.h b/autostash.h
index 1406638166..e08ccb9881 100644
--- a/autostash.h
+++ b/autostash.h
@@ -18,6 +18,7 @@ int reset_head(struct object_id *oid, const char *action,
const char *switch_to_branch, unsigned flags,
const char *reflog_orig_head, const char *reflog_head);
+void perform_autostash(const char *path);
int apply_autostash(const char *path);
#endif
diff --git a/builtin/rebase.c b/builtin/rebase.c
index c3165896cc..c4decdfb5b 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1797,49 +1797,7 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
die(_("could not read index"));
if (options.autostash) {
- struct lock_file lock_file = LOCK_INIT;
- int fd;
-
- fd = hold_locked_index(&lock_file, 0);
- refresh_cache(REFRESH_QUIET);
- if (0 <= fd)
- repo_update_index_if_able(the_repository, &lock_file);
- rollback_lock_file(&lock_file);
-
- if (has_unstaged_changes(the_repository, 1) ||
- has_uncommitted_changes(the_repository, 1)) {
- const char *autostash =
- state_dir_path("autostash", &options);
- struct child_process stash = CHILD_PROCESS_INIT;
- struct object_id oid;
-
- argv_array_pushl(&stash.args,
-"stash", "create", "autostash", NULL);
- stash.git_cmd = 1;
- stash.no_stdin = 1;
- strbuf_reset(&buf);
- if (capture_command(&stash, &buf, GIT_MAX_HEXSZ))
- die(_("Cannot autostash"));
-
[RFC PATCH 0/7] merge: learn --autostash
Alban reported[1] that he expected merge to have an --autostash option, just like rebase. Since there's not really any reason why merge can't have it, let's implement it in this patchset. The actual logic isn't too bad. That change can be found in the last patch. The remainder is refactoring so that the change can be made easily. One thing I would like some attention on is if apply_autostash() is called in the right place. It's called somewhere above close_object_store() and also in the --abort case, just like in rebase, but I'm not quite sure if that's right. Since this is an RFC, we're missing tests and documentation but I've tested it and it seems to work reasonably well. [1]: https://github.com/gitgitgadget/git/issues/394 Denton Liu (7): Makefile: alphabetically sort += lists autostash: extract read_one() from rebase autostash: extract apply_autostash() from rebase autostash: extract reset_head() from rebase autostash: extract perform_autostash() from rebase autostash.c: undefine USE_THE_INDEX_COMPATIBILITY_MACROS merge: teach --autostash option Makefile | 75 +++ autostash.c | 239 ++ autostash.h | 24 + builtin/merge.c | 13 +++ builtin/pull.c | 9 +- builtin/rebase.c | 240 +-- t/t5520-pull.sh | 8 -- 7 files changed, 323 insertions(+), 285 deletions(-) create mode 100644 autostash.c create mode 100644 autostash.h -- 2.23.0.897.g0a19638b1e
[RFC PATCH 3/7] autostash: extract apply_autostash() from rebase
Continue the process of lib-ifying the autostash code. In a future
commit, this will be used to implement `--autostash` in other builtins.
This patch is best viewed with `--color-moved` and
`--color-moved-ws=allow-indentation-change`.
Signed-off-by: Denton Liu
---
autostash.c | 46 +
autostash.h | 2 ++
builtin/rebase.c | 49 ++--
3 files changed, 50 insertions(+), 47 deletions(-)
diff --git a/autostash.c b/autostash.c
index a6898e0fda..62ec7a7c80 100644
--- a/autostash.c
+++ b/autostash.c
@@ -1,6 +1,8 @@
#include "git-compat-util.h"
#include "autostash.h"
+#include "dir.h"
#include "gettext.h"
+#include "run-command.h"
#include "strbuf.h"
int read_one(const char *path, struct strbuf *buf)
@@ -10,3 +12,47 @@ int read_one(const char *path, struct strbuf *buf)
strbuf_trim_trailing_newline(buf);
return 0;
}
+
+int apply_autostash(const char *path)
+{
+ struct strbuf autostash = STRBUF_INIT;
+ struct child_process stash_apply = CHILD_PROCESS_INIT;
+
+ if (!file_exists(path))
+ return 0;
+
+ if (read_one(path, &autostash))
+ return error(_("Could not read '%s'"), path);
+ /* Ensure that the hash is not mistaken for a number */
+ strbuf_addstr(&autostash, "^0");
+ argv_array_pushl(&stash_apply.args,
+"stash", "apply", autostash.buf, NULL);
+ stash_apply.git_cmd = 1;
+ stash_apply.no_stderr = stash_apply.no_stdout =
+ stash_apply.no_stdin = 1;
+ if (!run_command(&stash_apply))
+ printf(_("Applied autostash.\n"));
+ else {
+ struct argv_array args = ARGV_ARRAY_INIT;
+ int res = 0;
+
+ argv_array_pushl(&args,
+"stash", "store", "-m", "autostash", "-q",
+autostash.buf, NULL);
+ if (run_command_v_opt(args.argv, RUN_GIT_CMD))
+ res = error(_("Cannot store %s"), autostash.buf);
+ argv_array_clear(&args);
+ strbuf_release(&autostash);
+ if (res)
+ return res;
+
+ fprintf(stderr,
+ _("Applying autostash resulted in conflicts.\n"
+ "Your changes are safe in the stash.\n"
+ "You can run \"git stash pop\" or \"git stash drop\" "
+ "at any time.\n"));
+ }
+
+ strbuf_release(&autostash);
+ return 0;
+}
diff --git a/autostash.h b/autostash.h
index 4a8f504f12..5f4e4bd22c 100644
--- a/autostash.h
+++ b/autostash.h
@@ -6,4 +6,6 @@
/* Read one file, then strip line endings */
int read_one(const char *path, struct strbuf *buf);
+int apply_autostash(const char *path);
+
#endif
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 9fd7de6b2f..661928d427 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -682,51 +682,6 @@ static int rebase_write_basic_state(struct rebase_options
*opts)
return 0;
}
-static int apply_autostash(struct rebase_options *opts)
-{
- const char *path = state_dir_path("autostash", opts);
- struct strbuf autostash = STRBUF_INIT;
- struct child_process stash_apply = CHILD_PROCESS_INIT;
-
- if (!file_exists(path))
- return 0;
-
- if (read_one(path, &autostash))
- return error(_("Could not read '%s'"), path);
- /* Ensure that the hash is not mistaken for a number */
- strbuf_addstr(&autostash, "^0");
- argv_array_pushl(&stash_apply.args,
-"stash", "apply", autostash.buf, NULL);
- stash_apply.git_cmd = 1;
- stash_apply.no_stderr = stash_apply.no_stdout =
- stash_apply.no_stdin = 1;
- if (!run_command(&stash_apply))
- printf(_("Applied autostash.\n"));
- else {
- struct argv_array args = ARGV_ARRAY_INIT;
- int res = 0;
-
- argv_array_pushl(&args,
-"stash", "store", "-m", "autostash", "-q",
-autostash.buf, NULL);
- if (run_command_v_opt(args.argv, RUN_GIT_CMD))
- res = error(_("Cannot store %s"), autostash.buf);
- argv_array_clear(&args);
- strbuf_release(&autostash);
- if (res)
- return res;
-
- fprin
[RFC PATCH 7/7] merge: teach --autostash option
In rebase, one can pass the `--autostash` option to cause the worktree
to be automatically stashed before continuing with the rebase. This
option is missing in merge, however.
Implement the `--autostash` option and corresponding `merge.autoStash`
option in merge which stashes before merging and then pops after.
Reported-by: Alban Gruin
Signed-off-by: Denton Liu
---
builtin/merge.c | 13 +
builtin/pull.c | 9 +
t/t5520-pull.sh | 8
3 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 062e911441..d1a5eaad0d 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -40,6 +40,7 @@
#include "branch.h"
#include "commit-reach.h"
#include "wt-status.h"
+#include "autostash.h"
#define DEFAULT_TWOHEAD (1<<0)
#define DEFAULT_OCTOPUS (1<<1)
@@ -58,6 +59,8 @@ static const char * const builtin_merge_usage[] = {
NULL
};
+static GIT_PATH_FUNC(merge_autostash, "MERGE_AUTOSTASH")
+
static int show_diffstat = 1, shortlog_len = -1, squash;
static int option_commit = -1;
static int option_edit = -1;
@@ -81,6 +84,7 @@ static int show_progress = -1;
static int default_to_upstream = 1;
static int signoff;
static const char *sign_commit;
+static int autostash;
static int no_verify;
static struct strategy all_strategy[] = {
@@ -285,6 +289,8 @@ static struct option builtin_merge_options[] = {
OPT_SET_INT(0, "progress", &show_progress, N_("force progress
reporting"), 1),
{ OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"),
N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
+ OPT_BOOL(0, "autostash", &autostash,
+ N_("automatically stash/stash pop before and after")),
OPT_BOOL(0, "overwrite-ignore", &overwrite_ignore, N_("update ignored
files (default)")),
OPT_BOOL(0, "signoff", &signoff, N_("add Signed-off-by:")),
OPT_BOOL(0, "no-verify", &no_verify, N_("bypass pre-merge-commit and
commit-msg hooks")),
@@ -440,6 +446,7 @@ static void finish(struct commit *head_commit,
strbuf_addf(&reflog_message, "%s: %s",
getenv("GIT_REFLOG_ACTION"), msg);
}
+ apply_autostash(merge_autostash());
if (squash) {
squash_message(head_commit, remoteheads);
} else {
@@ -631,6 +638,9 @@ static int git_merge_config(const char *k, const char *v,
void *cb)
} else if (!strcmp(k, "commit.gpgsign")) {
sign_commit = git_config_bool(k, v) ? "" : NULL;
return 0;
+ } else if (!strcmp(k, "merge.autostash")) {
+ autostash = git_config_bool(k, v);
+ return 0;
}
status = fmt_merge_msg_config(k, v, cb);
@@ -724,6 +734,8 @@ static int try_merge_strategy(const char *strategy, struct
commit_list *common,
for (j = common; j; j = j->next)
commit_list_insert(j->item, &reversed);
+ if (autostash)
+ perform_autostash(merge_autostash());
hold_locked_index(&lock, LOCK_DIE_ON_ERROR);
clean = merge_recursive(&o, head,
remoteheads->item, reversed, &result);
@@ -1288,6 +1300,7 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
/* Invoke 'git reset --merge' */
ret = cmd_reset(nargc, nargv, prefix);
+ apply_autostash(merge_autostash());
goto done;
}
diff --git a/builtin/pull.c b/builtin/pull.c
index d25ff13a60..ee186781ae 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -183,7 +183,7 @@ static struct option pull_options[] = {
N_("verify that the named commit has a valid GPG signature"),
PARSE_OPT_NOARG),
OPT_BOOL(0, "autostash", &opt_autostash,
- N_("automatically stash/stash pop before and after rebase")),
+ N_("automatically stash/stash pop before and after")),
OPT_PASSTHRU_ARGV('s', "strategy", &opt_strategies, N_("strategy"),
N_("merge strategy to use"),
0),
@@ -671,6 +671,10 @@ static int run_merge(void)
argv_array_pushv(&args, opt_strategy_opts.argv);
if (opt_gpg_sign)
argv_array_push(&args, opt_gpg_sign);
+ if (opt_autostash == 0)
+ argv_array_push(&args, "--no-autostash");
+ else if (opt_autostash == 1)
+ argv_array_push(&args, "--autostash");
[RFC PATCH 4/7] autostash: extract reset_head() from rebase
Begin the process of lib-ifying the autostash code. In a future commit,
This patch is best viewed with `--color-moved` and
`--color-moved-ws=allow-indentation-change`.
this will be used to implement `--autostash` in other builtins.
Signed-off-by: Denton Liu
---
autostash.c | 137 +++
autostash.h | 12 +
builtin/rebase.c | 137 ---
3 files changed, 149 insertions(+), 137 deletions(-)
diff --git a/autostash.c b/autostash.c
index 62ec7a7c80..eb58e0c8a4 100644
--- a/autostash.c
+++ b/autostash.c
@@ -1,9 +1,17 @@
+#define USE_THE_INDEX_COMPATIBILITY_MACROS
+
#include "git-compat-util.h"
#include "autostash.h"
+#include "cache-tree.h"
#include "dir.h"
#include "gettext.h"
+#include "lockfile.h"
+#include "refs.h"
#include "run-command.h"
#include "strbuf.h"
+#include "tree-walk.h"
+#include "tree.h"
+#include "unpack-trees.h"
int read_one(const char *path, struct strbuf *buf)
{
@@ -13,6 +21,135 @@ int read_one(const char *path, struct strbuf *buf)
return 0;
}
+int reset_head(struct object_id *oid, const char *action,
+ const char *switch_to_branch, unsigned flags,
+ const char *reflog_orig_head, const char *reflog_head)
+{
+ unsigned detach_head = flags & RESET_HEAD_DETACH;
+ unsigned reset_hard = flags & RESET_HEAD_HARD;
+ unsigned run_hook = flags & RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
+ unsigned refs_only = flags & RESET_HEAD_REFS_ONLY;
+ unsigned update_orig_head = flags & RESET_ORIG_HEAD;
+ struct object_id head_oid;
+ struct tree_desc desc[2] = { { NULL }, { NULL } };
+ struct lock_file lock = LOCK_INIT;
+ struct unpack_trees_options unpack_tree_opts;
+ struct tree *tree;
+ const char *reflog_action;
+ struct strbuf msg = STRBUF_INIT;
+ size_t prefix_len;
+ struct object_id *orig = NULL, oid_orig,
+ *old_orig = NULL, oid_old_orig;
+ int ret = 0, nr = 0;
+
+ if (switch_to_branch && !starts_with(switch_to_branch, "refs/"))
+ BUG("Not a fully qualified branch: '%s'", switch_to_branch);
+
+ if (!refs_only && hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0) {
+ ret = -1;
+ goto leave_reset_head;
+ }
+
+ if ((!oid || !reset_hard) && get_oid("HEAD", &head_oid)) {
+ ret = error(_("could not determine HEAD revision"));
+ goto leave_reset_head;
+ }
+
+ if (!oid)
+ oid = &head_oid;
+
+ if (refs_only)
+ goto reset_head_refs;
+
+ memset(&unpack_tree_opts, 0, sizeof(unpack_tree_opts));
+ setup_unpack_trees_porcelain(&unpack_tree_opts, action);
+ unpack_tree_opts.head_idx = 1;
+ unpack_tree_opts.src_index = the_repository->index;
+ unpack_tree_opts.dst_index = the_repository->index;
+ unpack_tree_opts.fn = reset_hard ? oneway_merge : twoway_merge;
+ unpack_tree_opts.update = 1;
+ unpack_tree_opts.merge = 1;
+ if (!detach_head)
+ unpack_tree_opts.reset = 1;
+
+ if (repo_read_index_unmerged(the_repository) < 0) {
+ ret = error(_("could not read index"));
+ goto leave_reset_head;
+ }
+
+ if (!reset_hard && !fill_tree_descriptor(the_repository, &desc[nr++],
&head_oid)) {
+ ret = error(_("failed to find tree of %s"),
+ oid_to_hex(&head_oid));
+ goto leave_reset_head;
+ }
+
+ if (!fill_tree_descriptor(the_repository, &desc[nr++], oid)) {
+ ret = error(_("failed to find tree of %s"), oid_to_hex(oid));
+ goto leave_reset_head;
+ }
+
+ if (unpack_trees(nr, desc, &unpack_tree_opts)) {
+ ret = -1;
+ goto leave_reset_head;
+ }
+
+ tree = parse_tree_indirect(oid);
+ prime_cache_tree(the_repository, the_repository->index, tree);
+
+ if (write_locked_index(the_repository->index, &lock, COMMIT_LOCK) < 0) {
+ ret = error(_("could not write index"));
+ goto leave_reset_head;
+ }
+
+reset_head_refs:
+ reflog_action = getenv(GIT_REFLOG_ACTION_ENVIRONMENT);
+ strbuf_addf(&msg, "%s: ", reflog_action ? reflog_action : "rebase");
+ prefix_len = msg.len;
+
+ if (update_orig_head) {
+ if (!get_oid("ORIG_HEAD", &oid_old_orig))
+ old_orig = &oid_old_orig;
+ if (!get_oid("HEAD", &oid_orig)) {
+
[RFC PATCH 2/7] autostash: extract read_one() from rebase
Begin the process of lib-ifying the autostash code. In a future commit,
this will be used to implement `--autostash` in other builtins.
This patch is best viewed with `--color-moved` and
`--color-moved-ws=allow-indentation-change`.
Signed-off-by: Denton Liu
---
autostash.c | 12
autostash.h | 9 +
builtin/rebase.c | 10 +-
3 files changed, 22 insertions(+), 9 deletions(-)
create mode 100644 autostash.c
create mode 100644 autostash.h
diff --git a/autostash.c b/autostash.c
new file mode 100644
index 00..a6898e0fda
--- /dev/null
+++ b/autostash.c
@@ -0,0 +1,12 @@
+#include "git-compat-util.h"
+#include "autostash.h"
+#include "gettext.h"
+#include "strbuf.h"
+
+int read_one(const char *path, struct strbuf *buf)
+{
+ if (strbuf_read_file(buf, path, 0) < 0)
+ return error_errno(_("could not read '%s'"), path);
+ strbuf_trim_trailing_newline(buf);
+ return 0;
+}
diff --git a/autostash.h b/autostash.h
new file mode 100644
index 00..4a8f504f12
--- /dev/null
+++ b/autostash.h
@@ -0,0 +1,9 @@
+#ifndef AUTOSTASH_H
+#define AUTOSTASH_H
+
+#include "strbuf.h"
+
+/* Read one file, then strip line endings */
+int read_one(const char *path, struct strbuf *buf);
+
+#endif
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 4a20582e72..9fd7de6b2f 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -27,6 +27,7 @@
#include "branch.h"
#include "sequencer.h"
#include "rebase-interactive.h"
+#include "autostash.h"
static char const * const builtin_rebase_usage[] = {
N_("git rebase [-i] [options] [--exec ] "
@@ -561,15 +562,6 @@ static const char *state_dir_path(const char *filename,
struct rebase_options *o
return path.buf;
}
-/* Read one file, then strip line endings */
-static int read_one(const char *path, struct strbuf *buf)
-{
- if (strbuf_read_file(buf, path, 0) < 0)
- return error_errno(_("could not read '%s'"), path);
- strbuf_trim_trailing_newline(buf);
- return 0;
-}
-
/* Initialize the rebase options from the state directory. */
static int read_basic_state(struct rebase_options *opts)
{
--
2.23.0.897.g0a19638b1e
[PATCH v6 3/3] format-patch: teach --cover-from-description option
Before, when format-patch generated a cover letter, only the body would
be populated with a branch's description while the subject would be
populated with placeholder text. However, users may want to have the
subject of their cover letter automatically populated in the same way.
Teach format-patch to accept the `--cover-from-description` option and
corresponding `format.coverFromDescription` config, allowing users to
populate different parts of the cover letter (including the subject
now).
Signed-off-by: Denton Liu
Signed-off-by: Junio C Hamano
---
Documentation/config/format.txt| 6 +
Documentation/git-format-patch.txt | 22
builtin/log.c | 95
t/t4014-format-patch.sh| 172 +
t/t9902-completion.sh | 5 +-
5 files changed, 279 insertions(+), 21 deletions(-)
diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
index cb629fa769..735dfcf827 100644
--- a/Documentation/config/format.txt
+++ b/Documentation/config/format.txt
@@ -36,6 +36,12 @@ format.subjectPrefix::
The default for format-patch is to output files with the '[PATCH]'
subject prefix. Use this variable to change that prefix.
+format.coverFromDescription::
+ The default mode for format-patch to determine which parts of
+ the cover letter will be populated using the branch's
+ description. See the `--cover-from-description` option in
+ linkgit:git-format-patch[1].
+
format.signature::
The default for format-patch is to output a signature containing
the Git version number. Use this variable to change that default.
diff --git a/Documentation/git-format-patch.txt
b/Documentation/git-format-patch.txt
index 0ac56f4b70..6800e1ab9a 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -19,6 +19,7 @@ SYNOPSIS
[--start-number ] [--numbered-files]
[--in-reply-to=] [--suffix=.]
[--ignore-if-in-upstream]
+ [--cover-from-description=]
[--rfc] [--subject-prefix=]
[(--reroll-count|-v) ]
[--to=] [--cc=]
@@ -171,6 +172,26 @@ will want to ensure that threading is disabled for `git
send-email`.
patches being generated, and any patch that matches is
ignored.
+--cover-from-description=::
+ Controls which parts of the cover letter will be automatically
+ populated using the branch's description.
++
+If `` is `message` or `default`, the cover letter subject will be
+populated with placeholder text. The body of the cover letter will be
+populated with the branch's description. This is the default mode when
+no configuration nor command line option is specified.
++
+If `` is `subject`, the first paragraph of the branch description will
+populate the cover letter subject. The remainder of the description will
+populate the body of the cover letter.
++
+If `` is `auto`, if the first paragraph of the branch description
+is greater than 100 bytes, then the mode will be `message`, otherwise
+`subject` will be used.
++
+If `` is `none`, both the cover letter subject and body will be
+populated with placeholder text.
+
--subject-prefix=::
Instead of the standard '[PATCH]' prefix in the subject
line, instead use '[]'. This
@@ -347,6 +368,7 @@ with configuration variables.
signOff = true
outputDirectory =
coverLetter = auto
+ coverFromDescription = auto
diff --git a/builtin/log.c b/builtin/log.c
index d212a8305d..04be559bd2 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -37,6 +37,7 @@
#include "range-diff.h"
#define MAIL_DEFAULT_WRAP 72
+#define COVER_FROM_AUTO_MAX_SUBJECT_LEN 100
/* Set a default date-time format for git log ("log.date" config variable) */
static const char *default_date_mode = NULL;
@@ -777,6 +778,13 @@ enum thread_level {
THREAD_DEEP
};
+enum cover_from_description {
+ COVER_FROM_NONE,
+ COVER_FROM_MESSAGE,
+ COVER_FROM_SUBJECT,
+ COVER_FROM_AUTO
+};
+
static enum thread_level thread;
static int do_signoff;
static int base_auto;
@@ -785,6 +793,23 @@ static const char *signature = git_version_string;
static const char *signature_file;
static enum cover_setting config_cover_letter;
static const char *config_output_directory;
+static enum cover_from_description cover_from_description_mode =
COVER_FROM_MESSAGE;
+
+static enum cover_from_description parse_cover_from_description(const char
*arg)
+{
+ if (!arg || !strcmp(arg, "default"))
+ return COVER_FROM_MESSAGE;
+ else if (!strcmp(arg, "none"))
+ return COVER_FROM_NONE;
+ else if (!strcmp(arg, "message"))
+ return COVER_FROM_MESSAGE;
+ else
[PATCH v6 0/3] format-patch: learn --infer-cover-subject option (also t4014 cleanup)
Currently, format-patch only puts "*** SUBJECT HERE ***" when a cover
letter is generated. However, it is already smart enough to be able to
populate the cover letter with the branch description so there's no
reason why it cannot populate the subject as well.
Teach format-patch the `--infer-cover-subject` option and corresponding
`format.inferCoverSubject` configuration option which will read the
subject from the branch description using the same rules as for a commit
message (that is, it will expect a subject line followed by a blank
line).
This was based on patches 1-3 of an earlier patchset I sent[1].
Changes since v5:
* Remove speculation in log message of 1/3
* Rename pp_from_desc() to prepare_cover_text()
Changes since v4:
* Modify 1/3 to more closely reflect intent of the original author
* Incorporate Junio's suggestions into the documentation
* Extract branch desc logic into pp_from_desc()
* Fix broken tests
Changes since v3:
* Change --infer-cover-subject to --cover-from-description
* No more test cleanup patches (they were merged in
'dl/format-patch-doc-test-cleanup')
Changes since v2:
* Break 1/4 into many different patches (one per paragraph of the
original patch)
* Incorporate Eric's documentation/commit message suggestions
Changes since v1:
* Incorporate Eric's suggestions for cleanup in all patches
* Add patch 3/4 to make it clear what is the default value for
format.coverLetter (since format.inferCoverSubject was borrowed from
this config but it also did not state what the default value was)
* In 1/4, rename all instances of "expected" to "expect"
[1]: https://public-inbox.org/git/cover.1558492582.git.liu.den...@gmail.com/
Denton Liu (3):
format-patch: replace erroneous and condition
format-patch: use enum variables
format-patch: teach --cover-from-description option
Documentation/config/format.txt| 6 +
Documentation/git-format-patch.txt | 22
builtin/log.c | 125 +++--
t/t4014-format-patch.sh| 172 +
t/t9902-completion.sh | 5 +-
5 files changed, 296 insertions(+), 34 deletions(-)
Range-diff against v5:
1: f89e56545b ! 1: 9d41068e73 format-patch: change erroneous and condition
@@ Metadata
Author: Denton Liu
## Commit message ##
-format-patch: change erroneous and condition
+format-patch: replace erroneous and condition
Commit 30984ed2e9 (format-patch: support deep threading, 2009-02-19),
introduced the following lines:
@@ Commit message
thread = git_config_bool(var, value) && THREAD_SHALLOW;
Since git_config_bool() returns a bool, the trailing `&&
THREAD_SHALLOW`
-is a no-op.
-
-In Python, `x and y` is equivalent to `y if x else x`[1]. Since this
-seems to be a Python-ism that's mistakenly leaked into our code,
convert
-this to the equivalent C expression.
-
-[1]:
https://docs.python.org/3/reference/expressions.html#boolean-operations
-
-Signed-off-by: Denton Liu
-Signed-off-by: Junio C Hamano
+is a no-op. Replace this errorneous and condition with a ternary
+statement so that it is clear what the configured value is when a
+boolean is given.
## builtin/log.c ##
@@ builtin/log.c: static int git_format_config(const char *var, const char
*value, void *cb)
2: 1fe780b5a4 = 2: 821e706bae format-patch: use enum variables
3: d5cd34a44b ! 3: 42b4a60fd2 format-patch: teach --cover-from-description
option
@@ builtin/log.c: static void show_diffstat(struct rev_info *rev,
fprintf(rev->diffopt.file, "\n");
}
-+static void pp_from_desc(struct pretty_print_context *pp,
-+ const char *branch_name,
-+ struct strbuf *sb,
-+ const char *encoding,
-+ int need_8bit_cte)
++static void prepare_cover_text(struct pretty_print_context *pp,
++ const char *branch_name,
++ struct strbuf *sb,
++ const char *encoding,
++ int need_8bit_cte)
+{
+ const char *subject = "*** SUBJECT HERE ***";
+ const char *body = "*** BLURB HERE ***";
@@ builtin/log.c: static void make_cover_letter(struct rev_info *rev, int
use_stdou
- pp_title_line(&pp, &msg, &sb, encoding, need_8bit_cte);
- pp_remainder(&pp, &msg, &sb, 0);
- add_branch_description(&sb, branch_name);
-+ pp_from_desc(&pp, branch_name, &sb, encoding, need_8bit_cte);
++ prepare_cover_text(&pp, branch_name, &sb, encoding, need_8bit_cte);
fprintf(rev->diffopt.file, "%s\n", sb.buf);
strbuf_release(&sb);
--
2.23.0.17.gd2208d9060
[PATCH v6 2/3] format-patch: use enum variables
Before, `thread` and `config_cover_letter` were defined as ints even
though they behaved as enums. Define actual enums and change these
variables to use these new definitions.
Signed-off-by: Denton Liu
Signed-off-by: Junio C Hamano
---
builtin/log.c | 30 +-
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 351f4ffcfd..d212a8305d 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -764,24 +764,28 @@ static void add_header(const char *value)
item->string[len] = '\0';
}
-#define THREAD_SHALLOW 1
-#define THREAD_DEEP 2
-static int thread;
+enum cover_setting {
+ COVER_UNSET,
+ COVER_OFF,
+ COVER_ON,
+ COVER_AUTO
+};
+
+enum thread_level {
+ THREAD_UNSET,
+ THREAD_SHALLOW,
+ THREAD_DEEP
+};
+
+static enum thread_level thread;
static int do_signoff;
static int base_auto;
static char *from;
static const char *signature = git_version_string;
static const char *signature_file;
-static int config_cover_letter;
+static enum cover_setting config_cover_letter;
static const char *config_output_directory;
-enum {
- COVER_UNSET,
- COVER_OFF,
- COVER_ON,
- COVER_AUTO
-};
-
static int git_format_config(const char *var, const char *value, void *cb)
{
struct rev_info *rev = cb;
@@ -1248,9 +1252,9 @@ static int output_directory_callback(const struct option
*opt, const char *arg,
static int thread_callback(const struct option *opt, const char *arg, int
unset)
{
- int *thread = (int *)opt->value;
+ enum thread_level *thread = (enum thread_level *)opt->value;
if (unset)
- *thread = 0;
+ *thread = THREAD_UNSET;
else if (!arg || !strcmp(arg, "shallow"))
*thread = THREAD_SHALLOW;
else if (!strcmp(arg, "deep"))
--
2.23.0.17.gd2208d9060
[PATCH v6 1/3] format-patch: replace erroneous and condition
Commit 30984ed2e9 (format-patch: support deep threading, 2009-02-19),
introduced the following lines:
#define THREAD_SHALLOW 1
[...]
thread = git_config_bool(var, value) && THREAD_SHALLOW;
Since git_config_bool() returns a bool, the trailing `&& THREAD_SHALLOW`
is a no-op. Replace this errorneous and condition with a ternary
statement so that it is clear what the configured value is when a
boolean is given.
Signed-off-by: Denton Liu
---
builtin/log.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/log.c b/builtin/log.c
index 44b10b3415..351f4ffcfd 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -835,7 +835,7 @@ static int git_format_config(const char *var, const char
*value, void *cb)
thread = THREAD_SHALLOW;
return 0;
}
- thread = git_config_bool(var, value) && THREAD_SHALLOW;
+ thread = git_config_bool(var, value) ? THREAD_SHALLOW :
THREAD_UNSET;
return 0;
}
if (!strcmp(var, "format.signoff")) {
--
2.23.0.17.gd2208d9060
Re: [PATCH v5 1/3] format-patch: change erroneous and condition
Hi Junio,
Thanks for the feedback.
On Tue, Oct 15, 2019 at 11:16:35AM +0900, Junio C Hamano wrote:
> Denton Liu writes:
>
> > Since this
> > seems to be a Python-ism that's mistakenly leaked into our code, convert
>
> The conclusion is OK, but as the inventor of format-patch and a
> non-pythonista, I do not think the above claim is correct, and even
> if Thomas thought it was a good idea to follow Python style in
> 30984ed2 ("format-patch: support deep threading", 2009-02-19), which
> I doubt he did, I do not think there is much point in speculating.
I agree, I probably shouldn't be putting speculation in the log
messages. I'll change this for the next reroll.
>
> Both the log message and the patch text in the previous round were
> better than this round, I would have to say.
I'll probably keep the patch text, however. In the previous version, we
were implicitly relying on the value of THREAD_SHALLOW to be 1. This
seems a little bit flimsy to me since it's possible that the enum can be
changed in the future and it may invalidate that assumption.
I'll keep it explicit so that it's a little bit more robust and also, so
that it's more obvious to future readers what's going on.
Thanks,
Denton
>
> Thanks.
>
>
>
> > diff --git a/builtin/log.c b/builtin/log.c
> > index 44b10b3415..351f4ffcfd 100644
> > --- a/builtin/log.c
> > +++ b/builtin/log.c
> > @@ -835,7 +835,7 @@ static int git_format_config(const char *var, const
> > char *value, void *cb)
> > thread = THREAD_SHALLOW;
> > return 0;
> > }
> > - thread = git_config_bool(var, value) && THREAD_SHALLOW;
> > + thread = git_config_bool(var, value) ? THREAD_SHALLOW :
> > THREAD_UNSET;
> > return 0;
> > }
> > if (!strcmp(var, "format.signoff")) {
[PATCH v5 3/3] format-patch: teach --cover-from-description option
Before, when format-patch generated a cover letter, only the body would
be populated with a branch's description while the subject would be
populated with placeholder text. However, users may want to have the
subject of their cover letter automatically populated in the same way.
Teach format-patch to accept the `--cover-from-description` option and
corresponding `format.coverFromDescription` config, allowing users to
populate different parts of the cover letter (including the subject
now).
Signed-off-by: Denton Liu
---
Documentation/config/format.txt| 6 +
Documentation/git-format-patch.txt | 22
builtin/log.c | 95
t/t4014-format-patch.sh| 172 +
t/t9902-completion.sh | 5 +-
5 files changed, 279 insertions(+), 21 deletions(-)
diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
index cb629fa769..735dfcf827 100644
--- a/Documentation/config/format.txt
+++ b/Documentation/config/format.txt
@@ -36,6 +36,12 @@ format.subjectPrefix::
The default for format-patch is to output files with the '[PATCH]'
subject prefix. Use this variable to change that prefix.
+format.coverFromDescription::
+ The default mode for format-patch to determine which parts of
+ the cover letter will be populated using the branch's
+ description. See the `--cover-from-description` option in
+ linkgit:git-format-patch[1].
+
format.signature::
The default for format-patch is to output a signature containing
the Git version number. Use this variable to change that default.
diff --git a/Documentation/git-format-patch.txt
b/Documentation/git-format-patch.txt
index 0ac56f4b70..6800e1ab9a 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -19,6 +19,7 @@ SYNOPSIS
[--start-number ] [--numbered-files]
[--in-reply-to=] [--suffix=.]
[--ignore-if-in-upstream]
+ [--cover-from-description=]
[--rfc] [--subject-prefix=]
[(--reroll-count|-v) ]
[--to=] [--cc=]
@@ -171,6 +172,26 @@ will want to ensure that threading is disabled for `git
send-email`.
patches being generated, and any patch that matches is
ignored.
+--cover-from-description=::
+ Controls which parts of the cover letter will be automatically
+ populated using the branch's description.
++
+If `` is `message` or `default`, the cover letter subject will be
+populated with placeholder text. The body of the cover letter will be
+populated with the branch's description. This is the default mode when
+no configuration nor command line option is specified.
++
+If `` is `subject`, the first paragraph of the branch description will
+populate the cover letter subject. The remainder of the description will
+populate the body of the cover letter.
++
+If `` is `auto`, if the first paragraph of the branch description
+is greater than 100 bytes, then the mode will be `message`, otherwise
+`subject` will be used.
++
+If `` is `none`, both the cover letter subject and body will be
+populated with placeholder text.
+
--subject-prefix=::
Instead of the standard '[PATCH]' prefix in the subject
line, instead use '[]'. This
@@ -347,6 +368,7 @@ with configuration variables.
signOff = true
outputDirectory =
coverLetter = auto
+ coverFromDescription = auto
diff --git a/builtin/log.c b/builtin/log.c
index d212a8305d..af33fe9ffb 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -37,6 +37,7 @@
#include "range-diff.h"
#define MAIL_DEFAULT_WRAP 72
+#define COVER_FROM_AUTO_MAX_SUBJECT_LEN 100
/* Set a default date-time format for git log ("log.date" config variable) */
static const char *default_date_mode = NULL;
@@ -777,6 +778,13 @@ enum thread_level {
THREAD_DEEP
};
+enum cover_from_description {
+ COVER_FROM_NONE,
+ COVER_FROM_MESSAGE,
+ COVER_FROM_SUBJECT,
+ COVER_FROM_AUTO
+};
+
static enum thread_level thread;
static int do_signoff;
static int base_auto;
@@ -785,6 +793,23 @@ static const char *signature = git_version_string;
static const char *signature_file;
static enum cover_setting config_cover_letter;
static const char *config_output_directory;
+static enum cover_from_description cover_from_description_mode =
COVER_FROM_MESSAGE;
+
+static enum cover_from_description parse_cover_from_description(const char
*arg)
+{
+ if (!arg || !strcmp(arg, "default"))
+ return COVER_FROM_MESSAGE;
+ else if (!strcmp(arg, "none"))
+ return COVER_FROM_NONE;
+ else if (!strcmp(arg, "message"))
+ return COVER_FROM_MESSAGE;
+ else if (!strcmp(arg, "subject&qu
[PATCH v5 0/3] format-patch: learn --cover-from-description option
Currently, format-patch only puts "*** SUBJECT HERE ***" when a cover
letter is generated. However, it is already smart enough to be able to
populate the cover letter with the branch description so there's no
reason why it cannot populate the subject as well.
Teach format-patch the `--cover-from-description` option and
corresponding `format.coverFromDescription` configuration option which
will allow it to populate not only the body but the subject as well.
Changes since v4:
* Modify 1/3 to more closely reflect intent of the original author
* Incorporate Junio's suggestions into the documentation
* Extract branch desc logic into pp_from_desc()
* Fix broken tests
Changes since v3:
* Change --infer-cover-subject to --cover-from-description
* No more test cleanup patches (they were merged in
'dl/format-patch-doc-test-cleanup')
Changes since v2:
* Break 1/4 into many different patches (one per paragraph of the
original patch)
* Incorporate Eric's documentation/commit message suggestions
Changes since v1:
* Incorporate Eric's suggestions for cleanup in all patches
* Add patch 3/4 to make it clear what is the default value for
format.coverLetter (since format.inferCoverSubject was borrowed from
this config but it also did not state what the default value was)
* In 1/4, rename all instances of "expected" to "expect"
Denton Liu (3):
format-patch: change erroneous and condition
format-patch: use enum variables
format-patch: teach --cover-from-description option
Documentation/config/format.txt| 6 +
Documentation/git-format-patch.txt | 22
builtin/log.c | 125 +++--
t/t4014-format-patch.sh| 172 +
t/t9902-completion.sh | 5 +-
5 files changed, 296 insertions(+), 34 deletions(-)
Range-diff against v4:
1: 267bc00dc8 ! 1: 56fb230ad2 format-patch: remove erroneous and condition
@@ Metadata
Author: Denton Liu
## Commit message ##
-format-patch: remove erroneous and condition
+format-patch: change erroneous and condition
Commit 30984ed2e9 (format-patch: support deep threading, 2009-02-19),
introduced the following lines:
@@ Commit message
thread = git_config_bool(var, value) && THREAD_SHALLOW;
Since git_config_bool() returns a bool, the trailing `&&
THREAD_SHALLOW`
-is a no-op. Remove this erroneous and condition.
+is a no-op.
+
+In Python, `x and y` is equivalent to `y if x else x`[1]. Since this
+seems to be a Python-ism that's mistakenly leaked into our code,
convert
+this to the equivalent C expression.
+
+[1]:
https://docs.python.org/3/reference/expressions.html#boolean-operations
Signed-off-by: Denton Liu
-Signed-off-by: Junio C Hamano
## builtin/log.c ##
@@ builtin/log.c: static int git_format_config(const char *var, const char
*value, void *cb)
@@ builtin/log.c: static int git_format_config(const char *var, const char
*value,
return 0;
}
- thread = git_config_bool(var, value) && THREAD_SHALLOW;
-+ thread = git_config_bool(var, value);
++ thread = git_config_bool(var, value) ? THREAD_SHALLOW :
THREAD_UNSET;
return 0;
}
if (!strcmp(var, "format.signoff")) {
2: 638a5b40d2 ! 2: e2769092fa format-patch: use enum variables
@@ Commit message
variables to use these new definitions.
Signed-off-by: Denton Liu
-Signed-off-by: Junio C Hamano
## builtin/log.c ##
@@ builtin/log.c: static void add_header(const char *value)
3: 3289ce62bb ! 3: 315c308950 format-patch: teach --cover-from-description
option
@@ Commit message
populate different parts of the cover letter (including the subject
now).
-Signed-off-by: Denton Liu
-Signed-off-by: Junio C Hamano
-
## Documentation/config/format.txt ##
@@ Documentation/config/format.txt: format.subjectPrefix::
The default for format-patch is to output files with the '[PATCH]'
@@ Documentation/git-format-patch.txt: will want to ensure that threading
is disabl
++
+If `` is `message` or `default`, the cover letter subject will be
+populated with placeholder text. The body of the cover letter will be
-+populated with the branch's description.
++populated with the branch's description. This is the default mode when
++no configuration nor command line option is specified.
++
-+If `` is `subject`, the beginning of the branch description (up to
-+the first blank line) will populate the cover letter subject. The
-+remainder of the description will pop
[PATCH v5 2/3] format-patch: use enum variables
Before, `thread` and `config_cover_letter` were defined as ints even
though they behaved as enums. Define actual enums and change these
variables to use these new definitions.
Signed-off-by: Denton Liu
---
Hi Junio, I double-checked and made sure that there is no arithmetic
done on the new enums.
builtin/log.c | 30 +-
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 351f4ffcfd..d212a8305d 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -764,24 +764,28 @@ static void add_header(const char *value)
item->string[len] = '\0';
}
-#define THREAD_SHALLOW 1
-#define THREAD_DEEP 2
-static int thread;
+enum cover_setting {
+ COVER_UNSET,
+ COVER_OFF,
+ COVER_ON,
+ COVER_AUTO
+};
+
+enum thread_level {
+ THREAD_UNSET,
+ THREAD_SHALLOW,
+ THREAD_DEEP
+};
+
+static enum thread_level thread;
static int do_signoff;
static int base_auto;
static char *from;
static const char *signature = git_version_string;
static const char *signature_file;
-static int config_cover_letter;
+static enum cover_setting config_cover_letter;
static const char *config_output_directory;
-enum {
- COVER_UNSET,
- COVER_OFF,
- COVER_ON,
- COVER_AUTO
-};
-
static int git_format_config(const char *var, const char *value, void *cb)
{
struct rev_info *rev = cb;
@@ -1248,9 +1252,9 @@ static int output_directory_callback(const struct option
*opt, const char *arg,
static int thread_callback(const struct option *opt, const char *arg, int
unset)
{
- int *thread = (int *)opt->value;
+ enum thread_level *thread = (enum thread_level *)opt->value;
if (unset)
- *thread = 0;
+ *thread = THREAD_UNSET;
else if (!arg || !strcmp(arg, "shallow"))
*thread = THREAD_SHALLOW;
else if (!strcmp(arg, "deep"))
--
2.23.0.17.g315c308950
[PATCH v5 1/3] format-patch: change erroneous and condition
Commit 30984ed2e9 (format-patch: support deep threading, 2009-02-19),
introduced the following lines:
#define THREAD_SHALLOW 1
[...]
thread = git_config_bool(var, value) && THREAD_SHALLOW;
Since git_config_bool() returns a bool, the trailing `&& THREAD_SHALLOW`
is a no-op.
In Python, `x and y` is equivalent to `y if x else x`[1]. Since this
seems to be a Python-ism that's mistakenly leaked into our code, convert
this to the equivalent C expression.
[1]: https://docs.python.org/3/reference/expressions.html#boolean-operations
Signed-off-by: Denton Liu
---
builtin/log.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/log.c b/builtin/log.c
index 44b10b3415..351f4ffcfd 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -835,7 +835,7 @@ static int git_format_config(const char *var, const char
*value, void *cb)
thread = THREAD_SHALLOW;
return 0;
}
- thread = git_config_bool(var, value) && THREAD_SHALLOW;
+ thread = git_config_bool(var, value) ? THREAD_SHALLOW :
THREAD_UNSET;
return 0;
}
if (!strcmp(var, "format.signoff")) {
--
2.23.0.17.g315c308950
[PATCH v4 4/3] fixup! format-patch: teach --cover-from-description option
Signed-off-by: Denton Liu --- Documentation/git-format-patch.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 86114e4c22..4c652c97f5 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -368,7 +368,7 @@ with configuration variables. signOff = true outputDirectory = coverLetter = auto - inferCoverSubject = true + coverFromDescription = auto -- 2.23.0.746.g72fc0fc0b9
[PATCH v4 0/3] format-patch: learn --cover-from-description option
Hi all, since 'dl/format-patch-doc-test-cleanup' has calmed down and merged into 'master', I think now's a good time to revive this topic. I've incorporated Philip's --cover-letter-from-description idea (and shortened it so it's not so verbose) so now it should be a lot more general than the old --infer-cover-subject. Currently, format-patch only puts "*** SUBJECT HERE ***" when a cover letter is generated. However, it is already smart enough to be able to populate the cover letter with the branch description so there's no reason why it cannot populate the subject as well. Teach format-patch the `--cover-from-description` option and corresponding `format.coverFromDescripion` configuration option which will allow it to populate not only the body but the subject as well. Changes since v3: * Change --infer-cover-subject to --cover-from-description * No more test cleanup patches (they were merged in 'dl/format-patch-doc-test-cleanup') Changes since v2: * Break 1/4 into many different patches (one per paragraph of the original patch) * Incorporate Eric's documentation/commit message suggestions Changes since v1: * Incorporate Eric's suggestions for cleanup in all patches * Add patch 3/4 to make it clear what is the default value for format.coverLetter (since format.inferCoverSubject was borrowed from this config but it also did not state what the default value was) * In 1/4, rename all instances of "expected" to "expect" Denton Liu (3): format-patch: remove erroneous and condition format-patch: use enum variables format-patch: teach --cover-from-description option Documentation/config/format.txt| 6 + Documentation/git-format-patch.txt | 22 builtin/log.c | 114 +-- t/t4014-format-patch.sh| 172 + 4 files changed, 280 insertions(+), 34 deletions(-) -- 2.23.0.17.g7cce04acd6.dirty
[PATCH v4 2/3] format-patch: use enum variables
Before, `thread` and `config_cover_letter` were defined as ints even
though they behaved as enums. Define actual enums and change these
variables to use these new definitions.
Signed-off-by: Denton Liu
---
builtin/log.c | 30 +-
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 7d658cecef..f06f5d586b 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -764,24 +764,28 @@ static void add_header(const char *value)
item->string[len] = '\0';
}
-#define THREAD_SHALLOW 1
-#define THREAD_DEEP 2
-static int thread;
+enum cover_setting {
+ COVER_UNSET,
+ COVER_OFF,
+ COVER_ON,
+ COVER_AUTO
+};
+
+enum thread_level {
+ THREAD_UNSET,
+ THREAD_SHALLOW,
+ THREAD_DEEP
+};
+
+static enum thread_level thread;
static int do_signoff;
static int base_auto;
static char *from;
static const char *signature = git_version_string;
static const char *signature_file;
-static int config_cover_letter;
+static enum cover_setting config_cover_letter;
static const char *config_output_directory;
-enum {
- COVER_UNSET,
- COVER_OFF,
- COVER_ON,
- COVER_AUTO
-};
-
static int git_format_config(const char *var, const char *value, void *cb)
{
struct rev_info *rev = cb;
@@ -1248,9 +1252,9 @@ static int output_directory_callback(const struct option
*opt, const char *arg,
static int thread_callback(const struct option *opt, const char *arg, int
unset)
{
- int *thread = (int *)opt->value;
+ enum thread_level *thread = (enum thread_level *)opt->value;
if (unset)
- *thread = 0;
+ *thread = THREAD_UNSET;
else if (!arg || !strcmp(arg, "shallow"))
*thread = THREAD_SHALLOW;
else if (!strcmp(arg, "deep"))
--
2.23.0.17.g7cce04acd6.dirty
[PATCH v4 1/3] format-patch: remove erroneous and condition
Commit 30984ed2e9 (format-patch: support deep threading, 2009-02-19),
introduced the following lines:
#define THREAD_SHALLOW 1
[...]
thread = git_config_bool(var, value) && THREAD_SHALLOW;
Since git_config_bool() returns a bool, the trailing `&& THREAD_SHALLOW`
is a no-op. Remove this erroneous and condition.
Signed-off-by: Denton Liu
---
builtin/log.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/log.c b/builtin/log.c
index 44b10b3415..7d658cecef 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -835,7 +835,7 @@ static int git_format_config(const char *var, const char
*value, void *cb)
thread = THREAD_SHALLOW;
return 0;
}
- thread = git_config_bool(var, value) && THREAD_SHALLOW;
+ thread = git_config_bool(var, value);
return 0;
}
if (!strcmp(var, "format.signoff")) {
--
2.23.0.17.g7cce04acd6.dirty
[PATCH v4 3/3] format-patch: teach --cover-from-description option
Before, when format-patch generated a cover letter, only the body would
be populated with a branch's description while the subject would be
populated with placeholder text. However, users may want to have the
subject of their cover letter automatically populated in the same way.
Teach format-patch to accept the `--cover-from-description` option and
corresponding `format.coverFromDescription` config, allowing users to
populate different parts of the cover letter (including the subject
now).
Signed-off-by: Denton Liu
---
Documentation/config/format.txt| 6 +
Documentation/git-format-patch.txt | 22
builtin/log.c | 84 ++
t/t4014-format-patch.sh| 172 +
4 files changed, 263 insertions(+), 21 deletions(-)
diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
index cb629fa769..735dfcf827 100644
--- a/Documentation/config/format.txt
+++ b/Documentation/config/format.txt
@@ -36,6 +36,12 @@ format.subjectPrefix::
The default for format-patch is to output files with the '[PATCH]'
subject prefix. Use this variable to change that prefix.
+format.coverFromDescription::
+ The default mode for format-patch to determine which parts of
+ the cover letter will be populated using the branch's
+ description. See the `--cover-from-description` option in
+ linkgit:git-format-patch[1].
+
format.signature::
The default for format-patch is to output a signature containing
the Git version number. Use this variable to change that default.
diff --git a/Documentation/git-format-patch.txt
b/Documentation/git-format-patch.txt
index 0ac56f4b70..86114e4c22 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -19,6 +19,7 @@ SYNOPSIS
[--start-number ] [--numbered-files]
[--in-reply-to=] [--suffix=.]
[--ignore-if-in-upstream]
+ [--cover-from-description=]
[--rfc] [--subject-prefix=]
[(--reroll-count|-v) ]
[--to=] [--cc=]
@@ -171,6 +172,26 @@ will want to ensure that threading is disabled for `git
send-email`.
patches being generated, and any patch that matches is
ignored.
+--cover-from-description=::
+ Controls which parts of the cover letter will be automatically
+ populated using the branch's description.
++
+If `` is `message` or `default`, the cover letter subject will be
+populated with placeholder text. The body of the cover letter will be
+populated with the branch's description.
++
+If `` is `subject`, the beginning of the branch description (up to
+the first blank line) will populate the cover letter subject. The
+remainder of the description will populate the body of the cover
+letter.
++
+If `` is `auto`, if the beginning of the branch description (up to
+the first line) is greater than 100 characters then the mode will be
+`message`, otherwise `subject` will be used.
++
+If `` is `none`, both the cover letter subject and body will be
+populated with placeholder text.
+
--subject-prefix=::
Instead of the standard '[PATCH]' prefix in the subject
line, instead use '[]'. This
@@ -347,6 +368,7 @@ with configuration variables.
signOff = true
outputDirectory =
coverLetter = auto
+ inferCoverSubject = true
diff --git a/builtin/log.c b/builtin/log.c
index f06f5d586b..0cc8b59991 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -37,6 +37,7 @@
#include "range-diff.h"
#define MAIL_DEFAULT_WRAP 72
+#define COVER_FROM_AUTO_MAX_SUBJECT_LEN 100
/* Set a default date-time format for git log ("log.date" config variable) */
static const char *default_date_mode = NULL;
@@ -777,6 +778,13 @@ enum thread_level {
THREAD_DEEP
};
+enum cover_from_description {
+ COVER_FROM_NONE,
+ COVER_FROM_MESSAGE,
+ COVER_FROM_SUBJECT,
+ COVER_FROM_AUTO
+};
+
static enum thread_level thread;
static int do_signoff;
static int base_auto;
@@ -785,6 +793,23 @@ static const char *signature = git_version_string;
static const char *signature_file;
static enum cover_setting config_cover_letter;
static const char *config_output_directory;
+static enum cover_from_description cover_from_description_mode =
COVER_FROM_MESSAGE;
+
+static enum cover_from_description parse_cover_from_description(const char
*arg)
+{
+ if (!arg || !strcmp(arg, "default"))
+ return COVER_FROM_MESSAGE;
+ else if (!strcmp(arg, "none"))
+ return COVER_FROM_NONE;
+ else if (!strcmp(arg, "message"))
+ return COVER_FROM_MESSAGE;
+ else if (!strcmp(arg, "subject"))
+ return COVER_FROM_SUBJECT;
+ else if (!strcmp(arg, "auto&quo
Re: [PATCH 01/11] graph: automatically track visible width of `strbuf`
On Fri, Oct 11, 2019 at 10:42:20AM +0900, Junio C Hamano wrote:
> Denton Liu writes:
>
> > static int calculate_width(const struct strbuf *row)
> > {
> > int in_termcode = 0;
> > int width = 0;
> > int i;
> >
> > for (i = 0; i < row.len; i++) {
> > if (row.buf[i] == '\033')
> > in_termcode = 1;
> >
> > if (!in_termcode)
> > width++;
> > else if (row.buf[i] == 'm')
> > in_termcode = 0;
> > }
> > }
>
> Not every byte that is outside the escape sequence contributes to
> one display columns. You would want to take a look at utf8_width()
> for inspiration.
>
Heh, I guess you're right. Looking right below the definition of
utf8_width, I realised we have the utf8_strnwidth function. We should be
able to just call
utf8_strnwidth(row.buf, row.len, 1);
Re: [PATCH 01/11] graph: automatically track visible width of `strbuf`
Hi Dscho,
On Thu, Oct 10, 2019 at 11:07:35PM +0200, Johannes Schindelin wrote:
> Hi James,
>
> On Thu, 10 Oct 2019, James Coglan via GitGitGadget wrote:
>
> > From: James Coglan
> >
> > All the output functions in `graph.c` currently keep track of how many
> > printable chars they've written to the buffer, before calling
> > `graph_pad_horizontally()` to pad the line with spaces. Some functions
> > do this by incrementing a counter whenever they write to the buffer, and
> > others do it by encoding an assumption about how many chars are written,
> > as in:
> >
> > graph_pad_horizontally(graph, sb, graph->num_columns * 2);
> >
> > This adds a fair amount of noise to the functions' logic and is easily
> > broken if one forgets to increment the right counter or update the
> > calculations used for padding.
> >
> > To make this easier to use, I'm adding a `width` field to `strbuf` that
> > tracks the number of printing characters added after the line prefix.
>
> This is a big heavy-handed: adding a `width` field to `struct strbuf`
> and maintaining it _just_ for the purpose of `graph.c` puts an
> unnecssary load on every other `strbuf` user (of which there are a
> _lot_).
>
> So my obvious question is: what makes `width` different from `len`?
> Since we exclusively use ASCII characters for the graph part, we should
> be able to use the already-existing `len`, for free, no?
>From what I can gleam from looking at the code, `width` is different
from `len` because when we're printing with colours, there'll be a bunch
of termcodes that don't actually count for the width.
I think that we should either leave the `chars_written` variable as is
or maybe calculate it after the fact. Here's an untested and uncompiled
implementation of something that might do that:
static int calculate_width(const struct strbuf *row)
{
int in_termcode = 0;
int width = 0;
int i;
for (i = 0; i < row.len; i++) {
if (row.buf[i] == '\033')
in_termcode = 1;
if (!in_termcode)
width++;
else if (row.buf[i] == 'm')
in_termcode = 0;
}
}
If we include this, I'm not sure what kind of performance hit we might
take if the graph we're generating is particularly big, though.
>
> I could imagine that the `strbuf` might receive more than one line, but
> then we still would only need to remember the offset of the last newline
> character in that `strbuf`, no?
>
> Ciao,
> Johannes
[PATCH] pthread.h: manually align parameter lists
In previous patches, extern was mechanically removed from function
declarations without care to formatting, causing parameter lists to be
misaligned. Manually format changed sections such that the parameter
lists are realigned.
Viewing this patch with 'git diff -w' should produce no output.
Signed-off-by: Denton Liu
---
I missed a step in 'dl/compat-cleanup'. This patch can be applied on the
tip of that branch.
compat/win32/pthread.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
index f1cfe73de9..737983d00b 100644
--- a/compat/win32/pthread.h
+++ b/compat/win32/pthread.h
@@ -51,7 +51,7 @@ typedef struct {
} pthread_t;
int pthread_create(pthread_t *thread, const void *unused,
- void *(*start_routine)(void*), void *arg);
+ void *(*start_routine)(void*), void *arg);
/*
* To avoid the need of copying a struct, we use small macro wrapper to pass
--
2.23.0.746.g72fc0fc0b9
Re: [PATCH 11/11] graph: fix coloring of octopus dashes
On Thu, Oct 10, 2019 at 11:16:24AM -0700, Denton Liu wrote: > You can see the results of this by doing: > > $ git fetch https://github.com/Denton-L/git.git testing/graph-output > $ git diff FETCH_HEAD^2 t/t4214-log-graph-octopus.sh > > and the resulting diff is very pleasing imo. I guess it'd probably be nice if the result of that diff were viewable without the extra work of fetching everything, so here it is: diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh index 3ae8e51e50..40d27db674 100755 --- a/t/t4214-log-graph-octopus.sh +++ b/t/t4214-log-graph-octopus.sh @@ -26,15 +26,14 @@ test_expect_success 'set up merge history' ' test_expect_success 'log --graph with tricky octopus merge, no color' ' cat >expect.uncolored <<-\EOF && * left - | *---. octopus-merge - | |\ \ \ - |/ / / / + | *-. octopus-merge + |/|\ \ | | | * 4 | | * | 3 | | |/ - | * | 2 + | * / 2 | |/ - * | 1 + * / 1 |/ * initial EOF @@ -47,15 +46,14 @@ test_expect_success 'log --graph with tricky octopus merge with colors' ' test_config log.graphColors red,green,yellow,blue,magenta,cyan && cat >expect.colors <<-\EOF && * left - | *---. octopus-merge - | |\ \ \ - |/ / / / + | *-. octopus-merge + |/|\ \ | | | * 4 | | * | 3 | | |/ - | * | 2 + | * / 2 | |/ - * | 1 + * / 1 |/ * initial EOF @@ -74,9 +72,9 @@ test_expect_success 'log --graph with normal octopus merge, no color' ' | | | * 4 | | * | 3 | | |/ - | * | 2 + | * / 2 | |/ - * | 1 + * / 1 |/ * initial EOF @@ -92,9 +90,9 @@ test_expect_success 'log --graph with normal octopus merge with colors' ' | | | * 4 | | * | 3 | | |/ - | * | 2 + | * / 2 | |/ - * | 1 + * / 1 |/ * initial EOF @@ -112,9 +110,9 @@ test_expect_success 'log --graph with normal octopus merge and child, no color' | | | * 4 | | * | 3 | | |/ - | * | 2 + | * / 2 | |/ - * | 1 + * / 1 |/ * initial EOF @@ -123,7 +121,7 @@ test_expect_success 'log --graph with normal octopus merge and child, no color' test_cmp expect.uncolored actual ' -test_expect_failure 'log --graph with normal octopus and child merge with colors' ' +test_expect_success 'log --graph with normal octopus and child merge with colors' ' cat >expect.colors <<-\EOF && * after-merge *---. octopus-merge @@ -131,9 +129,9 @@ test_expect_failure 'log --graph with normal octopus and child merge with colors | | | * 4 | | * | 3 | | |/ - | * | 2 + | * / 2 | |/ - * | 1 + * / 1 |/ * initial EOF @@ -147,15 +145,14 @@ test_expect_success 'log --graph with tricky octopus merge and its child, no col cat >expect.uncolored <<-\EOF && * left | * after-merge - | *---. octopus-merge - | |\ \ \ - |/ / / / + | *-. octopus-merge + |/|\ \ | | | * 4 | | * | 3 | | |/ - | * | 2 + | * / 2 | |/ - * | 1 + * / 1 |/ * initial EOF @@ -164,20 +161,19 @@ test_expect_success 'log --graph with tricky octopus merge and its child, no col test_cmp expect.uncolored actual ' -test_expect_failure 'log --graph with tricky octopus merge and its child with colors' ' +test_expect_success 'log --graph with tricky octopus merge and its child with colors' ' test_config log.graphColors red,green,yellow,blue,magenta,cyan && cat >expect.colors <<-\EOF && * left | * after-merge - | *---. octopus-merge - | |\ \ \ - |/ / / / + | *-. octopus-merge + |/|\ \ | | | * 4 | | * | 3 | | |/ - | * | 2 + | * / 2 | |/ - * | 1 + * / 1 |/ * initial EOF @@ -209,7 +205,7 @@ test_expect_success 'log --graph with crossover in octopus merge, no color' ' test_cmp expect.uncolored actual ' -test_expect_failure 'log --graph with crossover in octopus merge with colors' ' +test_expect_success 'log --graph with crossover in octopus merge with colors' ' test_config log.graphColors red,green,yellow,blue,magenta,cyan &&a
Re: [PATCH 11/11] graph: fix coloring of octopus dashes
Hi James, Nicely done! This issue was bugging me for a while! On Thu, Oct 10, 2019 at 09:13:52AM -0700, James Coglan via GitGitGadget wrote: [...] > diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh > index 9ada687628..fbd485d83a 100755 > --- a/t/t4214-log-graph-octopus.sh > +++ b/t/t4214-log-graph-octopus.sh > @@ -42,23 +42,74 @@ test_expect_success 'set up merge history' ' > test_tick && > git merge -m octopus-merge 1 2 3 4 && > git checkout 1 -b L && > - test_commit left > + test_commit left && > + git checkout 4 -b R && > + test_commit right > ' > > test_expect_success 'log --graph with tricky octopus merge with colors' ' > test_config log.graphColors red,green,yellow,blue,magenta,cyan && > - git log --color=always --graph --date-order --pretty=tformat:%s --all > >actual.colors.raw && > + git log --color=always --graph --date-order --pretty=tformat:%s L merge > >actual.colors.raw && > test_decode_color actual.colors && > test_cmp expect.colors actual.colors > ' > > test_expect_success 'log --graph with tricky octopus merge, no color' ' > - git log --color=never --graph --date-order --pretty=tformat:%s --all > >actual.raw && > + git log --color=never --graph --date-order --pretty=tformat:%s L merge > >actual.raw && > sed "s/ *\$//" actual.raw >actual && > test_cmp expect.uncolored actual > ' > > -# Repeat the previous two tests with "normal" octopus merge (i.e., > +# Repeat the previous two tests with an octopus merge whose final parent > skews left > + > +test_expect_success 'log --graph with left-skewed final parent, no color' ' > + cat >expect.uncolored <<-\EOF && > + * right > + | *---. octopus-merge > + | |\ \ \ > + | |_|_|/ > + |/| | | > + * | | | 4 > + | | | * 3 > + | |_|/ > + |/| | > + | | * 2 > + | |/ > + |/| > + | * 1 > + |/ > + * initial > + EOF > + git log --color=never --graph --date-order --pretty=tformat:%s R merge > >actual.raw && > + sed "s/ *\$//" actual.raw >actual && > + test_cmp expect.uncolored actual > +' > + > +test_expect_success 'log --graph with left-skewed final parent with colors' ' > + cat >expect.colors <<-\EOF && > + * right > + | *---. > octopus-merge > + | |\ \ > \ > + | > |_|_|/ > + |/| | > | > + * | | | 4 > + | | | * 3 > + | > |_|/ > + |/| | > + | | * 2 > + | |/ > + |/| > + | * 1 > + |/ > + * initial > + EOF > + test_config log.graphColors red,green,yellow,blue,magenta,cyan && > + git log --color=always --graph --date-order --pretty=tformat:%s R merge > >actual.colors.raw && > + test_decode_color actual.colors && > + test_cmp expect.colors actual.colors > +' > + > +# Repeat the first two tests with "normal" octopus merge (i.e., > # without the first parent skewing to the "left" branch column). > > test_expect_success 'log --graph with normal octopus merge, no color' ' So, I decided to merge 'dl/octopus-graph-bug' with your branch and I couldn't be happier! I had to make a couple of minor tweaks to the existing test cases but most of them only involved flipping `test_expect_failure` to `test_expect_success`. You can see the results of this by doing: $ git fetch https://github.com/Denton-L/git.git testing/graph-output $ git diff FETCH_HEAD^2 t/t4214-log-graph-octopus.sh and the resulting diff is very pleasing imo. Junio, when you pick this topic up, that branch should contain the correct conflict resolution if that can help you out in any way. Anyway, thanks for the work, James! I'll give this patch my: Tested-by: Denton Liu > -- > gitgitgadget
Re: [PATCH 1/1] commit: add support to provide --coauthor
On Thu, Oct 10, 2019 at 01:49:03PM +0200, Johannes Schindelin wrote: > Hi, > > On Wed, 9 Oct 2019, Jeff King wrote: > > > On Wed, Oct 09, 2019 at 11:19:47AM +0900, Junio C Hamano wrote: > > > > > > I wonder how we are supposed to use this trailer in the Git project, > > > > in particular in combination with Signed-off-by. Should all > > > > (co)authors sign off as well? Or will Co-authored-by imply > > > > Signed-off-by? > > > > > > I think we have been happy with (1) a comment at the end of the log > > > message that says X worked together with Y and Z to produce this > > > patch, and (2) the trailer block that has S-o-b: from X, Y and Z, > > > without any need for Co-authored-by: trailer so far, and I do not > > > see any reason to change it in this project. > > > > One advantage to making a machine-readable version is that tools on the > > reading side can then count contributions, etc. For instance: > > > > https://github.com/git/git/commit/69f272b922df153c86db520bf9b6018a9808c2a6 > > > > shows all of the co-author avatars, and you can click through to their > > pages. Yep, this is the reason why I raised the suggestion[1] in the first place. Since special support for this trailer is implemented in GitHub (and GitLab too, as I recently learned), I think this could be considered a defacto standard for co-authored commits. > > FWIW I really like this. It bugged me ever since that GitMerge talk > (https://www.youtube.com/watch?v=usQgAy8YDVA) that we did not have any > standardized way to document co-authored commits. Yep, and this isn't the first time this has been brought up. I remember stumbling on this thread[2] a while back about someone asking for co-author functionality by default so it would be nice to have a standard way of doing it. Thanks, Denton [1]: https://github.com/gitgitgadget/git/issues/343 [2]: https://public-inbox.org/git/caovwq4i_hl7xgnxzrvu3osnsbntyxbg8vh6vzi4c1issrre...@mail.gmail.com/ > > > > If other projects wants to use such a footer, that's their choice, > > > but I am fairly negative to the idea to open the gate to unbounded > > > number of new options for new trailer lines. We do not even have > > > such options like --acked=, --reported=, for the > > > trailers that are actively used already (and to make sure nobody > > > misunderstands, I do not think it is a good idea to add such > > > individual options). > > > > Yeah, I'd agree that we should start first with a generic trailer line. > > There might be some advantage to building trailer-specific intelligence > > on top of that (for instance, it would be nice for coauthor trailers to > > expand names the way --author does). But that can come after, and might > > not even be in the form of specific command-line options. E.g., if the > > coauthor trailer could be marked in config as "this is an ident", then > > we we would know to expand it. And the same could apply to acked, > > reported, etc. > > Yep, and we have to start somewhere. I think this patch is a good start. > > FWIW I would not even mind introducing the synonym `--co-author` for > `--coauthor`. But that's just a very minor suggestion. > > Ciao, > Dscho
[PATCH] Makefile: respect $(V) in %.cocci.patch target
When the %.cocci.patch target was defined in 63f0a758a0 (add coccicheck make target, 2016-09-15), it included a mechanism to suppress the noisy output, similar to the $(QUIET_) family of variables. In the case where one wants to inspect the output hidden by $(QUIET_), one could define $(V) for verbose output. In the %.cocci.patch target, this was not implemented. Move the output suppression into the $(QUIET_SPATCH) variable which is used like the other $(QUIET_) variables. While we're at it, change the number of spaces printed from 5 to 4, like the other variables there. Signed-off-by: Denton Liu --- Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index c99361e719..ae45bfc429 100644 --- a/Makefile +++ b/Makefile @@ -1890,6 +1890,7 @@ ifndef V QUIET_SP = @echo ' ' SP $<; QUIET_HDR = @echo ' ' HDR $(<:hcc=h); QUIET_RC = @echo ' ' RC $@; + QUIET_SPATCH = @echo ' ' SPATCH $<; QUIET_SUBDIR0 = +@subdir= QUIET_SUBDIR1 = ;$(NO_SUBDIR) echo ' ' SUBDIR $$subdir; \ $(MAKE) $(PRINT_DIR) -C $$subdir @@ -2818,7 +2819,7 @@ FOUND_C_SOURCES = $(filter %.c,$(shell $(FIND_SOURCE_FILES))) COCCI_SOURCES = $(filter-out $(THIRD_PARTY_SOURCES),$(FOUND_C_SOURCES)) %.cocci.patch: %.cocci $(COCCI_SOURCES) - @echo '' SPATCH $<; \ + $(QUIET_SPATCH) \ if test $(SPATCH_BATCH_SIZE) = 0; then \ limit=; \ else \ -- 2.23.0.746.g72fc0fc0b9
[PATCH v2] t0000: cover GIT_SKIP_TESTS blindspots
Currently, the tests for GIT_SKIP_TESTS do not cover the situation where we skip an entire test suite. The tests also do not cover the situation where we have GIT_SKIP_TESTS defined but the test suite does not match. Add two test cases so we cover this blindspot. Signed-off-by: Denton Liu --- Changes since v1: * Added another test case to cover the blindspot where GIT_SKIP_TESTS is defined but the test suite does not match. Range-diff against v1: 1: 08273a0d0d ! 1: b5f3af8a63 t: cover GIT_SKIP_TESTS blindspot @@ Metadata Author: Denton Liu ## Commit message ## -t: cover GIT_SKIP_TESTS blindspot +t: cover GIT_SKIP_TESTS blindspots Currently, the tests for GIT_SKIP_TESTS do not cover the situation where -we skip an entire test suite. Add a test case so we cover this -blindspot. +we skip an entire test suite. The tests also do not cover the situation +where we have GIT_SKIP_TESTS defined but the test suite does not match. + +Add two test cases so we cover this blindspot. ## t/t-basic.sh ## @@ t/t-basic.sh: test_expect_success 'GIT_SKIP_TESTS sh pattern' " @@ t/t-basic.sh: test_expect_success 'GIT_SKIP_TESTS sh pattern' " + EOF + ) +" ++ ++test_expect_success 'GIT_SKIP_TESTS does not skip unmatched suite' " ++ ( ++ GIT_SKIP_TESTS='notgit' && export GIT_SKIP_TESTS && ++ run_sub_test_lib_test git-skip-tests-unmatched-suite \ ++ 'GIT_SKIP_TESTS does not skip unmatched suite' <<-\\EOF && ++ for i in 1 2 3 ++ do ++ test_expect_success \"passing test #\$i\" 'true' ++ done ++ test_done ++ EOF ++ check_sub_test_lib_test git-skip-tests-unmatched-suite <<-\\EOF ++ > ok 1 - passing test #1 ++ > ok 2 - passing test #2 ++ > ok 3 - passing test #3 ++ > # passed all 3 test(s) ++ > 1..3 ++ EOF ++ ) ++" + test_expect_success '--run basic' " run_sub_test_lib_test run-basic \ t/t-basic.sh | 38 ++ 1 file changed, 38 insertions(+) diff --git a/t/t-basic.sh b/t/t-basic.sh index 4c01f60dd3..4d3f7ba295 100755 --- a/t/t-basic.sh +++ b/t/t-basic.sh @@ -391,6 +391,44 @@ test_expect_success 'GIT_SKIP_TESTS sh pattern' " ) " +test_expect_success 'GIT_SKIP_TESTS entire suite' " + ( + GIT_SKIP_TESTS='git' && export GIT_SKIP_TESTS && + run_sub_test_lib_test git-skip-tests-entire-suite \ + 'GIT_SKIP_TESTS entire suite' <<-\\EOF && + for i in 1 2 3 + do + test_expect_success \"passing test #\$i\" 'true' + done + test_done + EOF + check_sub_test_lib_test git-skip-tests-entire-suite <<-\\EOF + > 1..0 # SKIP skip all tests in git + EOF + ) +" + +test_expect_success 'GIT_SKIP_TESTS does not skip unmatched suite' " + ( + GIT_SKIP_TESTS='notgit' && export GIT_SKIP_TESTS && + run_sub_test_lib_test git-skip-tests-unmatched-suite \ + 'GIT_SKIP_TESTS does not skip unmatched suite' <<-\\EOF && + for i in 1 2 3 + do + test_expect_success \"passing test #\$i\" 'true' + done + test_done + EOF + check_sub_test_lib_test git-skip-tests-unmatched-suite <<-\\EOF + > ok 1 - passing test #1 + > ok 2 - passing test #2 + > ok 3 - passing test #3 + > # passed all 3 test(s) + > 1..3 + EOF + ) +" + test_expect_success '--run basic' " run_sub_test_lib_test run-basic \ '--run basic' --run='1 3 5' <<-\\EOF && -- 2.23.0.248.g3a9dd8fb08
[PATCH] t4014: treat rev-list output as the expected value
In 6bd26f58ea (t4014: use test_line_count() where possible, 2019-08-27), we converted many test cases to take advantage of the test_line_count() function. In one conversion, we inverted the expected and actual value as tested by test_line_count(). Although functionally correct, if format-patch ever produced incorrect output, the debugging output would be a bunch of hashes which would be difficult to debug. Invert the expected and actual values provided to test_line_count() so that if format-patch produces incorrect output, the debugging output will be a list of human-readable files instead. Helped-by: SZEDER Gábor Signed-off-by: Denton Liu --- Thanks for point out my mistake, Szeder. This patch can be applied to the tip of 'dl/format-patch-doc-test-cleanup' and then we should base Bert's changes on top of this branch. t/t4014-format-patch.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index 83f52614d3..72b09896cf 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -1610,8 +1610,9 @@ test_expect_success 'format-patch format.outputDirectory option' ' test_config format.outputDirectory patches && rm -fr patches && git format-patch master..side && - git rev-list master..side >list && - test_line_count = $(ls patches | wc -l) list + count=$(git rev-list --count master..side) && + ls patches >list && + test_line_count = $count list ' test_expect_success 'format-patch -o overrides format.outputDirectory' ' -- 2.23.0.248.g3a9dd8fb08
Re: [PATCH 1/1] commit: add support to provide --coauthor
Welcome to the Git community, Toon!
Wow, I never realised that people actually read my braindump of issues
on GGG. Thanks for taking this on.
First some housekeeping, when formatting your patches, it's a good idea
to use `git format-patch --thread` so that your patches are grouped
together by thread.
On Tue, Oct 08, 2019 at 09:49:35AM +0200, Toon Claes wrote:
> Add support to provide the Co-author when committing. For each
> co-author provided with --coauthor=, a line is added at the
I know you're taking the name from my GGG issue but perhaps --co-author
would look better?
> bottom of the commit message, like this:
>
> Co-authored-by:
>
> It's a common practice use when pairing up with other people and both
> authors want to in the commit message.
>
> Co-authored-by: Zeger-Jan van de Weg
Nice ;)
> Signed-off-by: Toon Claes
> ---
> Documentation/git-commit.txt | 5
> builtin/commit.c | 7 ++
> sequencer.c | 44 ++--
> sequencer.h | 2 ++
> t/t7502-commit-porcelain.sh | 11 +
> 5 files changed, 57 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
> index afa7b75a23..c059944e38 100644
> --- a/Documentation/git-commit.txt
> +++ b/Documentation/git-commit.txt
> @@ -140,6 +140,11 @@ OPTIONS
> commit by that author (i.e. rev-list --all -i --author=);
> the commit author is then copied from the first such commit found.
>
> +--coauthor=::
> +Add a Co-authored-by line with the specified author. Specify the
This line is indented with spaces but it should be changed to a single
tab.
> + author using the standard `Co Artur `
> + format.
> +
> --date=::
> Override the author date used in the commit.
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index ae7aaf6dc6..feb423ed6f 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -110,6 +110,7 @@ static int config_commit_verbose = -1; /* unspecified */
> static int no_post_rewrite, allow_empty_message;
> static char *untracked_files_arg, *force_date, *ignore_submodule_arg,
> *ignored_arg;
> static char *sign_commit;
> +static struct string_list coauthors = STRING_LIST_INIT_NODUP;
>
> /*
> * The default commit message cleanup mode will remove the lines
> @@ -672,6 +673,7 @@ static int prepare_to_commit(const char *index_file,
> const char *prefix,
> int clean_message_contents = (cleanup_mode != COMMIT_MSG_CLEANUP_NONE);
> int old_display_comment_prefix;
> int merge_contains_scissors = 0;
> + int i;
>
> /* This checks and barfs if author is badly specified */
> determine_author_info(author_ident);
> @@ -803,6 +805,10 @@ static int prepare_to_commit(const char *index_file,
> const char *prefix,
> if (clean_message_contents)
> strbuf_stripspace(&sb, 0);
>
> + for (i = 0; i < coauthors.nr; i++) {
> + append_coauthor(&sb, coauthors.items[i].string);
> + }
> +
> if (signoff)
> append_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0);
>
> @@ -1504,6 +1510,7 @@ int cmd_commit(int argc, const char **argv, const char
> *prefix)
> OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use
> autosquash formatted message to squash specified commit")),
> OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit
> is authored by me now (used with -C/-c/--amend)")),
> OPT_BOOL('s', "signoff", &signoff, N_("add Signed-off-by:")),
> + OPT_STRING_LIST(0, "coauthor", &coauthors, N_("co-author"),
> N_("add Co-authored-by:")),
> OPT_FILENAME('t', "template", &template_file, N_("use specified
> template file")),
> OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),
> OPT_CLEANUP(&cleanup_arg),
> diff --git a/sequencer.c b/sequencer.c
> index d648aaf416..8958a22470 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -36,6 +36,7 @@
> #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
>
> static const char sign_off_header[] = "Signed-off-by: ";
> +static const char coauthor_header[] = "Co-authored-by: ";
> static const char cherry_picked_prefix[] = "(cherry picked from commit ";
>
> GIT_PATH_FUNC(git_path_commit_editmsg, "COMMIT_EDITMSG")
> @@ -4385,15 +4386,9 @@ int sequencer_pick_revisions(struct repository *r,
> return res;
> }
>
> -void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned
> flag)
> +static void append_footer(struct strbuf *msgbuf, struct strbuf* sob, size_t
> ignore_footer, size_t no_dup_sob)
The * for pointers should go with the name, not the type. Also, `sob` is
a misleading name since it means "Signed-off-by". In this case, we're
using it as a general footer (since it can include Co-author lines too)
so perhaps rename this to `footer`?
Finally, as a nit, can we mark sob as const?
> {
> -
[PATCH] git-rev-list.txt: prune options in synopsis
The synopsis section in git-rev-list.txt has grown to be a huge list that probably needs its own synopsis. Since the list is huge, users may be given the false impression that the list is complete, however it is not. It is missing many of the available options. Since the list of options in the synopsis is not only annoying but actively harmful, replace it with `[]` so users know to explicitly look through the documentation for further information. While we're at it, update the optional path notation so that it is more modern. Signed-off-by: Denton Liu --- I initially wrote a patch to document --children in the synopsis alongside --parents. However, I quickly realised that --children was only one of many missing options. Other options included: --since --after --until --before --show-notes --all-match --invert-grep --basic-regexp --perl-regexp --exclude --reflog --alternate-refs --single-worktree --boundary --progress --simplify-by-decoration ... stopped checking here so I decided it wasn't worth it to keep this list around if it's so incomplete when even the porcelain git-log.txt doesn't carry a huge list in the synopsis. I went through and manually checked each option to ensure that there was a corresponding option in either rev-list-options.txt or pretty-options.txt so we shouldn't lose any information by deleting this list. Documentation/git-rev-list.txt | 54 +- 1 file changed, 1 insertion(+), 53 deletions(-) diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt index 9392760b25..025c911436 100644 --- a/Documentation/git-rev-list.txt +++ b/Documentation/git-rev-list.txt @@ -9,59 +9,7 @@ git-rev-list - Lists commit objects in reverse chronological order SYNOPSIS [verse] -'git rev-list' [ --max-count= ] -[ --skip= ] -[ --max-age= ] -[ --min-age= ] -[ --sparse ] -[ --merges ] -[ --no-merges ] -[ --min-parents= ] -[ --no-min-parents ] -[ --max-parents= ] -[ --no-max-parents ] -[ --first-parent ] -[ --remove-empty ] -[ --full-history ] -[ --not ] -[ --all ] -[ --branches[=] ] -[ --tags[=] ] -[ --remotes[=] ] -[ --glob= ] -[ --ignore-missing ] -[ --stdin ] -[ --quiet ] -[ --topo-order ] -[ --parents ] -[ --timestamp ] -[ --left-right ] -[ --left-only ] -[ --right-only ] -[ --cherry-mark ] -[ --cherry-pick ] -[ --encoding= ] -[ --(author|committer|grep)= ] -[ --regexp-ignore-case | -i ] -[ --extended-regexp | -E ] -[ --fixed-strings | -F ] -[ --date=] -[ [ --objects | --objects-edge | --objects-edge-aggressive ] - [ --unpacked ] - [ --object-names | --no-object-names ] - [ --filter= [ --filter-print-omitted ] ] ] -[ --missing= ] -[ --pretty | --header ] -[ --bisect ] -[ --bisect-vars ] -[ --bisect-all ] -[ --merge ] -[ --reverse ] -[ --walk-reflogs ] -[ --no-walk ] [ --do-walk ] -[ --count ] -[ --use-bitmap-index ] -... [ \-- ... ] +'git rev-list' [] ... [[--] ...] DESCRIPTION --- -- 2.23.0.687.g391267506c
[PATCH] apply: tell user location of corrupted patch file
When `git am` runs into a corrupt patch, it'll error out and give a
message such as,
error: corrupt patch at line 87
Casual users of am may assume that this line number refers to the
file that they provided on the command-line. This assumption, however,
is incorrect. The line count really refers to the
.git/rebase-apply/patch file genrated by am.
Teach am to print the location of corrupted patch files so that users of
the tool will know where to look when fixing their corrupted patch. Thus
the error message will look like this:
error: corrupt patch at .git/rebase-apply/patch:87
An alternate design was considered which involved printing the line
numbers relative to the output of `git am --show-current-patch` (in
other words, the actual mail file that's provided to am). This design
was not chosen because am does not store the whole mail and instead,
splits the mail into several files. As a result of this, this would
break existing users' workflow if they piped their mail directly to am
from their mail client, the whole mail would not exist in any file and
they would have to manually recreate the mail to see the line number.
Let's avoid breaking Junio's workflow since he's probably one of the
most frequent user of `git am` in the world. ;)
Signed-off-by: Denton Liu
---
apply.c| 2 +-
t/t4012-diff-binary.sh | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/apply.c b/apply.c
index 57a61f2881..b0ba2e7b1a 100644
--- a/apply.c
+++ b/apply.c
@@ -1785,7 +1785,7 @@ static int parse_single_patch(struct apply_state *state,
len = parse_fragment(state, line, size, patch, fragment);
if (len <= 0) {
free(fragment);
- return error(_("corrupt patch at line %d"),
state->linenr);
+ return error(_("corrupt patch at %s:%d"),
state->patch_input_file, state->linenr);
}
fragment->patch = line;
fragment->size = len;
diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
index 6579c81216..42cb2dd404 100755
--- a/t/t4012-diff-binary.sh
+++ b/t/t4012-diff-binary.sh
@@ -68,7 +68,7 @@ test_expect_success C_LOCALE_OUTPUT 'apply detecting corrupt
patch correctly' '
sed -e "s/-CIT/xCIT/" broken &&
test_must_fail git apply --stat --summary broken 2>detected &&
detected=$(cat detected) &&
- detected=$(expr "$detected" : "error.*at line \\([0-9]*\\)\$") &&
+ detected=$(expr "$detected" : "error.*at broken:\\([0-9]*\\)\$") &&
detected=$(sed -ne "${detected}p" broken) &&
test "$detected" = xCIT
'
@@ -77,7 +77,7 @@ test_expect_success C_LOCALE_OUTPUT 'apply detecting corrupt
patch correctly' '
git diff --binary | sed -e "s/-CIT/xCIT/" >broken &&
test_must_fail git apply --stat --summary broken 2>detected &&
detected=$(cat detected) &&
- detected=$(expr "$detected" : "error.*at line \\([0-9]*\\)\$") &&
+ detected=$(expr "$detected" : "error.*at broken:\\([0-9]*\\)\$") &&
detected=$(sed -ne "${detected}p" broken) &&
test "$detected" = xCIT
'
--
2.23.0.687.g391267506c
[PATCH v2 3/5] t4214: generate expect in their own test cases
Before, the expect files of the test case were being generated in the setup method. However, it would make more sense to generate these files within the test cases that actually use them so that it's obvious to future readers where the expected values are coming from. Move the generation of the expect files in their own respective test cases. While we're at it, we want to establish a pattern in this test suite that, firstly, a non-colored test case is given then, immediately after, the colored version is given. Switch test cases "log --graph with tricky octopus merge, no color" and "log --graph with tricky octopus merge with colors" so that the "no color" version appears first. This patch is best viewed with `--color-moved`. Signed-off-by: Denton Liu --- t/t4214-log-graph-octopus.sh | 42 ++-- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh index f6e22ec825..16776e347c 100755 --- a/t/t4214-log-graph-octopus.sh +++ b/t/t4214-log-graph-octopus.sh @@ -5,6 +5,20 @@ test_description='git log --graph of skewed left octopus merge.' . ./test-lib.sh test_expect_success 'set up merge history' ' + test_commit initial && + for i in 1 2 3 4 ; do + git checkout master -b $i || return $? + # Make tag name different from branch name, to avoid + # ambiguity error when calling checkout. + test_commit $i $i $i tag$i || return $? + done && + git checkout 1 -b merge && + test_merge octopus-merge 1 2 3 4 && + git checkout 1 -b L && + test_commit left +' + +test_expect_success 'log --graph with tricky octopus merge, no color' ' cat >expect.uncolored <<-\EOF && * left | *---. octopus-merge @@ -19,6 +33,13 @@ test_expect_success 'set up merge history' ' |/ * initial EOF + git log --color=never --graph --date-order --pretty=tformat:%s --all >actual.raw && + sed "s/ *\$//" actual.raw >actual && + test_cmp expect.uncolored actual +' + +test_expect_success 'log --graph with tricky octopus merge with colors' ' + test_config log.graphColors red,green,yellow,blue,magenta,cyan && cat >expect.colors <<-\EOF && * left | *---. octopus-merge @@ -33,32 +54,11 @@ test_expect_success 'set up merge history' ' |/ * initial EOF - test_commit initial && - for i in 1 2 3 4 ; do - git checkout master -b $i || return $? - # Make tag name different from branch name, to avoid - # ambiguity error when calling checkout. - test_commit $i $i $i tag$i || return $? - done && - git checkout 1 -b merge && - test_merge octopus-merge 1 2 3 4 && - git checkout 1 -b L && - test_commit left -' - -test_expect_success 'log --graph with tricky octopus merge with colors' ' - test_config log.graphColors red,green,yellow,blue,magenta,cyan && git log --color=always --graph --date-order --pretty=tformat:%s --all >actual.colors.raw && test_decode_color actual.colors && test_cmp expect.colors actual.colors ' -test_expect_success 'log --graph with tricky octopus merge, no color' ' - git log --color=never --graph --date-order --pretty=tformat:%s --all >actual.raw && - sed "s/ *\$//" actual.raw >actual && - test_cmp expect.uncolored actual -' - # Repeat the previous two tests with "normal" octopus merge (i.e., # without the first parent skewing to the "left" branch column). -- 2.23.0.565.g1cc52d20df
[PATCH v2 2/5] t4214: use test_merge
In the previous commit, we extended test_merge() so that it could perform octopus merges. Now that the restriction is lifted, use test_merge() to perform the octopus merge instead of manually duplicating test_merge() functionality. Signed-off-by: Denton Liu --- t/t4214-log-graph-octopus.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh index dab96c89aa..f6e22ec825 100755 --- a/t/t4214-log-graph-octopus.sh +++ b/t/t4214-log-graph-octopus.sh @@ -41,8 +41,7 @@ test_expect_success 'set up merge history' ' test_commit $i $i $i tag$i || return $? done && git checkout 1 -b merge && - test_tick && - git merge -m octopus-merge 1 2 3 4 && + test_merge octopus-merge 1 2 3 4 && git checkout 1 -b L && test_commit left ' -- 2.23.0.565.g1cc52d20df
[PATCH v2 0/5] t4214: cleanup and demonstrate graph bug
Junio, the test cases from earlier didn't exactly cover the cases Peff talked about so I added a few more test cases. These should cover those situations and a few more so we can be extra sure when the bug is fixed. This patchset cleans up t4214 and then, in the last patch, demonstrates a bug in the way some octopus merges are colored. While I was playing around with Pratyush's octopus merge in git-gui, I noticed that there was a bug in `git log --graph`. The horizontal lines in the octopus merge seemed to have an off-by-one error in their coloring. More detail in the last patch. I tried my hand at fixing the bug but in the hour I spent going at it, I couldn't fix the logic up. The buggy logic is in graph.c: graph_draw_octopus_merge() in case anyone is interested. Changes since v1: * Add a few more test cases to demonstrate more failure (and passing) conditions Denton Liu (5): test-lib: let test_merge() perform octopus merges t4214: use test_merge t4214: generate expect in their own test cases t4214: explicitly list tags in log t4214: demonstrate octopus graph coloring failure t/t4214-log-graph-octopus.sh | 329 --- t/test-lib-functions.sh | 6 +- 2 files changed, 308 insertions(+), 27 deletions(-) Range-diff against v1: 1: e77af8cde5 = 1: e77af8cde5 test-lib: let test_merge() perform octopus merges 2: 4a93deb3f6 = 2: 4a93deb3f6 t4214: use test_merge 3: c09f761185 = 3: c09f761185 t4214: generate expect in their own test cases 4: ad6d89440b = 4: ad6d89440b t4214: explicitly list tags in log 5: 0b84bf5417 ! 5: e58c1929bc t4214: demonstrate octopus graph coloring failure @@ Commit message dash should be the color of the line to their bottom-right. Instead, they are currently the color of the line to their bottom. -Demonstrate this breakage with two sets of test cases. The first pair of -test cases demonstrates the breakage with a similar case as the above. -The second pair of test cases demonstrates a similar breakage but with -the last parent crossing over. +Demonstrate this breakage with a few sets of test cases. These test +cases should show not only simple cases of the bug occuring but trickier +situations that may not be handled properly in any attempt to fix the +bug. -The second pair of test cases are included as a result of my (poor) -attempts at fixing the bug. This case seems particularly tricky to -handle. Good luck! +While we're at it, include a passing test case as a canary in case an +attempt to fix the bug breaks existing operation. ## t/t4214-log-graph-octopus.sh ## @@ t/t4214-log-graph-octopus.sh: test_expect_success 'set up merge history' ' @@ t/t4214-log-graph-octopus.sh: test_expect_success 'set up merge history' ' - test_commit left + test_commit left && + git checkout 4 -b crossover && -+ test_commit after-4 ++ test_commit after-4 && ++ git checkout initial -b more-L && ++ test_commit after-initial ' test_expect_success 'log --graph with tricky octopus merge, no color' ' @@ t/t4214-log-graph-octopus.sh: test_expect_success 'log --graph with normal octop test_cmp expect.colors actual.colors ' + -+test_expect_success 'log --graph with tricky octopus merge and its parent, no color' ' ++test_expect_success 'log --graph with normal octopus merge and child, no color' ' ++ cat >expect.uncolored <<-\EOF && ++ * after-merge ++ *---. octopus-merge ++ |\ \ \ ++ | | | * 4 ++ | | * | 3 ++ | | |/ ++ | * | 2 ++ | |/ ++ * | 1 ++ |/ ++ * initial ++ EOF ++ git log --color=never --graph --date-order --pretty=tformat:%s after-merge >actual.raw && ++ sed "s/ *\$//" actual.raw >actual && ++ test_cmp expect.uncolored actual ++' ++ ++test_expect_failure 'log --graph with normal octopus and child merge with colors' ' ++ cat >expect.colors <<-\EOF && ++ * after-merge ++ *---. octopus-merge ++ |\ \ \ ++ | | | * 4 ++ | | * | 3 ++ | | |/ ++ | * | 2 ++ | |/ ++ * | 1 ++ |/ ++ * initial ++ EOF ++ test_config log.graphColors red,green,yellow,blue,magenta,cyan && ++ git log --color=always --graph --date-order --pretty=tformat:%s after-merge >actual.colors.raw && ++ test_decode_color actual.colors && ++ test_cmp expect.colors actual.colors ++' ++ ++test_expect_success 'log --graph with tricky octopus merge and its child, no color' '
[PATCH v2 4/5] t4214: explicitly list tags in log
In a future test case, we will be extending the commit graph. As a result, explicitly list the tags that will generate the graph so that when future additions are made, the current graph illustrations won't be affected. Signed-off-by: Denton Liu --- t/t4214-log-graph-octopus.sh | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh index 16776e347c..097151da39 100755 --- a/t/t4214-log-graph-octopus.sh +++ b/t/t4214-log-graph-octopus.sh @@ -33,7 +33,7 @@ test_expect_success 'log --graph with tricky octopus merge, no color' ' |/ * initial EOF - git log --color=never --graph --date-order --pretty=tformat:%s --all >actual.raw && + git log --color=never --graph --date-order --pretty=tformat:%s left octopus-merge >actual.raw && sed "s/ *\$//" actual.raw >actual && test_cmp expect.uncolored actual ' @@ -54,7 +54,7 @@ test_expect_success 'log --graph with tricky octopus merge with colors' ' |/ * initial EOF - git log --color=always --graph --date-order --pretty=tformat:%s --all >actual.colors.raw && + git log --color=always --graph --date-order --pretty=tformat:%s left octopus-merge >actual.colors.raw && test_decode_color actual.colors && test_cmp expect.colors actual.colors ' @@ -75,7 +75,7 @@ test_expect_success 'log --graph with normal octopus merge, no color' ' |/ * initial EOF - git log --color=never --graph --date-order --pretty=tformat:%s merge >actual.raw && + git log --color=never --graph --date-order --pretty=tformat:%s octopus-merge >actual.raw && sed "s/ *\$//" actual.raw >actual && test_cmp expect.uncolored actual ' @@ -94,7 +94,7 @@ test_expect_success 'log --graph with normal octopus merge with colors' ' * initial EOF test_config log.graphColors red,green,yellow,blue,magenta,cyan && - git log --color=always --graph --date-order --pretty=tformat:%s merge >actual.colors.raw && + git log --color=always --graph --date-order --pretty=tformat:%s octopus-merge >actual.colors.raw && test_decode_color actual.colors && test_cmp expect.colors actual.colors ' -- 2.23.0.565.g1cc52d20df
[PATCH v2 5/5] t4214: demonstrate octopus graph coloring failure
The graph coloring logic for octopus merges currently has a bug. This can be seen git.git with 74c7cfa875 (Merge of http://members.cox.net/junkio/git-jc.git, 2005-05-05), whose second child is 211232bae6 (Octopus merge of the following five patches., 2005-05-05). If one runs git log --graph 74c7cfa875 one can see that the octopus merge is colored incorrectly. In particular, the horizontal dashes are off by one color. Each horizontal dash should be the color of the line to their bottom-right. Instead, they are currently the color of the line to their bottom. Demonstrate this breakage with a few sets of test cases. These test cases should show not only simple cases of the bug occuring but trickier situations that may not be handled properly in any attempt to fix the bug. While we're at it, include a passing test case as a canary in case an attempt to fix the bug breaks existing operation. Signed-off-by: Denton Liu --- t/t4214-log-graph-octopus.sh | 282 ++- 1 file changed, 281 insertions(+), 1 deletion(-) diff --git a/t/t4214-log-graph-octopus.sh b/t/t4214-log-graph-octopus.sh index 097151da39..3ae8e51e50 100755 --- a/t/t4214-log-graph-octopus.sh +++ b/t/t4214-log-graph-octopus.sh @@ -14,8 +14,13 @@ test_expect_success 'set up merge history' ' done && git checkout 1 -b merge && test_merge octopus-merge 1 2 3 4 && + test_commit after-merge && git checkout 1 -b L && - test_commit left + test_commit left && + git checkout 4 -b crossover && + test_commit after-4 && + git checkout initial -b more-L && + test_commit after-initial ' test_expect_success 'log --graph with tricky octopus merge, no color' ' @@ -98,4 +103,279 @@ test_expect_success 'log --graph with normal octopus merge with colors' ' test_decode_color actual.colors && test_cmp expect.colors actual.colors ' + +test_expect_success 'log --graph with normal octopus merge and child, no color' ' + cat >expect.uncolored <<-\EOF && + * after-merge + *---. octopus-merge + |\ \ \ + | | | * 4 + | | * | 3 + | | |/ + | * | 2 + | |/ + * | 1 + |/ + * initial + EOF + git log --color=never --graph --date-order --pretty=tformat:%s after-merge >actual.raw && + sed "s/ *\$//" actual.raw >actual && + test_cmp expect.uncolored actual +' + +test_expect_failure 'log --graph with normal octopus and child merge with colors' ' + cat >expect.colors <<-\EOF && + * after-merge + *---. octopus-merge + |\ \ \ + | | | * 4 + | | * | 3 + | | |/ + | * | 2 + | |/ + * | 1 + |/ + * initial + EOF + test_config log.graphColors red,green,yellow,blue,magenta,cyan && + git log --color=always --graph --date-order --pretty=tformat:%s after-merge >actual.colors.raw && + test_decode_color actual.colors && + test_cmp expect.colors actual.colors +' + +test_expect_success 'log --graph with tricky octopus merge and its child, no color' ' + cat >expect.uncolored <<-\EOF && + * left + | * after-merge + | *---. octopus-merge + | |\ \ \ + |/ / / / + | | | * 4 + | | * | 3 + | | |/ + | * | 2 + | |/ + * | 1 + |/ + * initial + EOF + git log --color=never --graph --date-order --pretty=tformat:%s left after-merge >actual.raw && + sed "s/ *\$//" actual.raw >actual && + test_cmp expect.uncolored actual +' + +test_expect_failure 'log --graph with tricky octopus merge and its child with colors' ' + test_config log.graphColors red,green,yellow,blue,magenta,cyan && + cat >expect.colors <<-\EOF && + * left + | * after-merge + | *---. octopus-merge + | |\ \ \ + |/ / / / + | | | * 4 + | | * | 3 + | | |/ + | * | 2 + | |/ + * | 1 + |/ + * initial + EOF + git log --color=always --graph --date-order --pretty=tformat:%s left after-merge >actual.colors.raw && + test_decode_color actual.colors && + test_cmp expect.colors actual.colors +' + +test_expect_success 'log --graph with crossover in octopus merge, no color' ' + cat >expect.uncolored <<-\EOF && + * after-4 + | *---. octopus-merge + | |\ \ \ + | |_|_|/ + |/| | | + * | | | 4 + | | | * 3 + | |_|/ + |/| | +
[PATCH v2 1/5] test-lib: let test_merge() perform octopus merges
Currently test_merge() only allows developers to merge in one branch.
However, this restriction is artificial and there is no reason why it
needs to be this way.
Extend test_merge() to allow the specification of multiple branches so
that octopus merges can be performed.
Signed-off-by: Denton Liu
---
t/test-lib-functions.sh | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 87bf3a2287..b299ecc326 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -228,9 +228,11 @@ test_commit () {
# can be a tag pointing to the commit-to-merge.
test_merge () {
+ label="$1" &&
+ shift &&
test_tick &&
- git merge -m "$1" "$2" &&
- git tag "$1"
+ git merge -m "$label" "$@" &&
+ git tag "$label"
}
# Efficiently create commits, each with a unique number (from 1 to
--
2.23.0.565.g1cc52d20df
Re: [PATCH 1/3] format-patch: document and exercise that -o does only create the trailing directory
Hi Bert, > Subject: format-patch: document and exercise that -o does only create the > trailing directory s/does only create/only creates/ ? Anyway, as a prepatory patch, I don't think that it's necessary. Maybe it's just me but I assume that most tools create at most one directory deep. Even mkdir won't created nested dirs unless you pass `-p`. I dunno. On Wed, Oct 02, 2019 at 11:26:11PM +0200, Bert Wesarg wrote: > Signed-off-by: Bert Wesarg > --- > Documentation/config/format.txt| 3 ++- > Documentation/git-format-patch.txt | 4 +++- > t/t4014-format-patch.sh| 16 > 3 files changed, 21 insertions(+), 2 deletions(-) > > diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt > index 414a5a8a9d..e17c5d6b0f 100644 > --- a/Documentation/config/format.txt > +++ b/Documentation/config/format.txt > @@ -80,7 +80,8 @@ format.coverLetter:: > > format.outputDirectory:: > Set a custom directory to store the resulting files instead of the > - current working directory. > + current working directory. Only the trailing directory will be created > + though. > > format.useAutoBase:: > A boolean value which lets you enable the `--base=auto` option of > diff --git a/Documentation/git-format-patch.txt > b/Documentation/git-format-patch.txt > index b9b97e63ae..fe7492353e 100644 > --- a/Documentation/git-format-patch.txt > +++ b/Documentation/git-format-patch.txt > @@ -66,7 +66,9 @@ they are created in the current working directory. The > default path > can be set with the `format.outputDirectory` configuration option. > The `-o` option takes precedence over `format.outputDirectory`. > To store patches in the current working directory even when > -`format.outputDirectory` points elsewhere, use `-o .`. > +`format.outputDirectory` points elsewhere, use `-o .`. Note that only > +the trailing directory will be created by Git, leading directories must > +already exists. > > By default, the subject of a single patch is "[PATCH] " followed by > the concatenation of lines from the commit message up to the first blank > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > index ca7debf1d4..bf2715a503 100755 > --- a/t/t4014-format-patch.sh > +++ b/t/t4014-format-patch.sh > @@ -1632,6 +1632,22 @@ test_expect_success 'From line has expected format' ' > test_cmp from filtered > ' > > +test_expect_success 'format-patch -o with no leading directories' ' > + rm -fr patches && > + git format-patch -o patches master..side && > + test $(git rev-list master..side | wc -l) -eq $(ls patches | wc -l) For test case you write, please use the following pattern: git rev-list master..side >list && test_line_count = $(ls patches | wc -l) list The first benefit is that we get to take advantage of the test_line_count function that's already written for us. The second is that when we write tests, we shouldn't put Git commands in the upstream of a pipe because if they fail, their return codes will be lost and we won't be able to fail the test properly. > +' > + > +test_expect_success 'format-patch -o with leading existing directories' ' > + git format-patch -o patches/side master..side && > + test $(git rev-list master..side | wc -l) -eq $(ls patches/side | wc -l) > +' > + > +test_expect_failure 'format-patch -o with leading non-existing directories' ' > + rm -fr patches && > + git format-patch -o patches/side master..side > +' As above, I wouldn't really call this a bug in Git. I think we should leave this test case off until the next patch. > + > test_expect_success 'format-patch format.outputDirectory option' ' > test_config format.outputDirectory patches && > rm -fr patches && > -- > 2.23.0.11.g242cf7f110 >
