* [PATCH 0/3] Add regression tests for recent rebase -i fixes @ 2017-05-31 10:42 Phillip Wood 2017-05-31 10:42 ` [PATCH 1/3] rebase -i: Add test for reflog message Phillip Wood ` (4 more replies) 0 siblings, 5 replies; 59+ messages in thread From: Phillip Wood @ 2017-05-31 10:42 UTC (permalink / raw) To: git; +Cc: Johannes.Schindelin, avarab, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> These patches add some tests for the fixes I sent the week before last. The output tests check the messages for stashes that apply and fail for all three variants of rebase. They are quite strict but I couldn't think of a better way to check that the output is correct and doesn't contain anything it isn't meant to (such as the output of git status). Phillip Wood (3): rebase -i: Add test for reflog message rebase: Add tests for console output rebase: Add tests for console output with conflicting stash t/t3404-rebase-interactive.sh | 7 +++++++ t/t3420-rebase-autostash.sh | 20 ++++++++++++++++++-- t/t3420/expected-fail-am | 8 ++++++++ t/t3420/expected-fail-interactive | 6 ++++++ t/t3420/expected-fail-merge | 32 ++++++++++++++++++++++++++++++++ t/t3420/expected-success-am | 6 ++++++ t/t3420/expected-success-interactive | 4 ++++ t/t3420/expected-success-merge | 30 ++++++++++++++++++++++++++++++ t/t3420/remove-ids.sed | 6 ++++++ 9 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 t/t3420/expected-fail-am create mode 100644 t/t3420/expected-fail-interactive create mode 100644 t/t3420/expected-fail-merge create mode 100644 t/t3420/expected-success-am create mode 100644 t/t3420/expected-success-interactive create mode 100644 t/t3420/expected-success-merge create mode 100644 t/t3420/remove-ids.sed -- 2.13.0 ^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 1/3] rebase -i: Add test for reflog message 2017-05-31 10:42 [PATCH 0/3] Add regression tests for recent rebase -i fixes Phillip Wood @ 2017-05-31 10:42 ` Phillip Wood 2017-06-01 2:00 ` Junio C Hamano 2017-05-31 10:42 ` [PATCH 2/3] rebase: Add tests for console output Phillip Wood ` (3 subsequent siblings) 4 siblings, 1 reply; 59+ messages in thread From: Phillip Wood @ 2017-05-31 10:42 UTC (permalink / raw) To: git; +Cc: Johannes.Schindelin, avarab, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> Check that the reflog message written to the branch reflog when the rebase is completed is correct. This checks for regressions for the fix in commit 4ab867b8fc rebase -i: fix reflog message Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- t/t3404-rebase-interactive.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 5bd0275930b715c25507fc9744c8946e7f73900b..37821d245433f757fa13f0a3e27da0312bebb7db 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -169,6 +169,13 @@ test_expect_success 'reflog for the branch shows state before rebase' ' test $(git rev-parse branch1@{1}) = $(git rev-parse original-branch1) ' +test_expect_success 'reflog for the branch shows correct finish message' ' + printf "rebase -i (finish): refs/heads/branch1 onto %s\n" \ + "$(git rev-parse branch2)" >expected && + git log -g --pretty=%gs -1 refs/heads/branch1 >actual && + test_cmp expected actual +' + test_expect_success 'exchange two commits' ' set_fake_editor && FAKE_LINES="2 1" git rebase -i HEAD~2 && -- 2.13.0 ^ permalink raw reply related [flat|nested] 59+ messages in thread
* Re: [PATCH 1/3] rebase -i: Add test for reflog message 2017-05-31 10:42 ` [PATCH 1/3] rebase -i: Add test for reflog message Phillip Wood @ 2017-06-01 2:00 ` Junio C Hamano 0 siblings, 0 replies; 59+ messages in thread From: Junio C Hamano @ 2017-06-01 2:00 UTC (permalink / raw) To: Phillip Wood; +Cc: git, Johannes.Schindelin, avarab, Phillip Wood Phillip Wood <phillip.wood@talktalk.net> writes: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > Check that the reflog message written to the branch reflog when the > rebase is completed is correct. This checks for regressions for the > fix in commit > 4ab867b8fc rebase -i: fix reflog message > > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> > --- Good idea. Thanks. > t/t3404-rebase-interactive.sh | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh > index 5bd0275930b715c25507fc9744c8946e7f73900b..37821d245433f757fa13f0a3e27da0312bebb7db 100755 > --- a/t/t3404-rebase-interactive.sh > +++ b/t/t3404-rebase-interactive.sh > @@ -169,6 +169,13 @@ test_expect_success 'reflog for the branch shows state before rebase' ' > test $(git rev-parse branch1@{1}) = $(git rev-parse original-branch1) > ' > > +test_expect_success 'reflog for the branch shows correct finish message' ' > + printf "rebase -i (finish): refs/heads/branch1 onto %s\n" \ > + "$(git rev-parse branch2)" >expected && > + git log -g --pretty=%gs -1 refs/heads/branch1 >actual && > + test_cmp expected actual > +' > + > test_expect_success 'exchange two commits' ' > set_fake_editor && > FAKE_LINES="2 1" git rebase -i HEAD~2 && ^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 2/3] rebase: Add tests for console output 2017-05-31 10:42 [PATCH 0/3] Add regression tests for recent rebase -i fixes Phillip Wood 2017-05-31 10:42 ` [PATCH 1/3] rebase -i: Add test for reflog message Phillip Wood @ 2017-05-31 10:42 ` Phillip Wood 2017-05-31 19:02 ` Phillip Wood 2017-06-01 12:56 ` Johannes Schindelin 2017-05-31 10:42 ` [PATCH 3/3] rebase: Add tests for console output with conflicting stash Phillip Wood ` (2 subsequent siblings) 4 siblings, 2 replies; 59+ messages in thread From: Phillip Wood @ 2017-05-31 10:42 UTC (permalink / raw) To: git; +Cc: Johannes.Schindelin, avarab, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> Check the console output when using --autostash and the stash applies cleanly is what we expect. To avoid this test depending on commit and stash hashes it uses sed to replace them with XXX. The sed script also replaces carriage returns in the output with '\r' to avoid embedded '^M's in the expected output files. Unfortunately this means we still end up with an embedded '^M' in the sed script which may not be preserved when sending this. The last line of the sed script should be +s/^M/\\r/g Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- t/t3420-rebase-autostash.sh | 10 +++++++++- t/t3420/expected-success-am | 6 ++++++ t/t3420/expected-success-interactive | 4 ++++ t/t3420/expected-success-merge | 30 ++++++++++++++++++++++++++++++ t/t3420/remove-ids.sed | 6 ++++++ 5 files changed, 55 insertions(+), 1 deletion(-) diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh index ab8a63e8d6dc643b28eb0c74ba3f032b7532226f..886be63c6d13e1ac4197a1b185659fb3d7d7eb26 100755 --- a/t/t3420-rebase-autostash.sh +++ b/t/t3420-rebase-autostash.sh @@ -53,12 +53,20 @@ testrebase() { git checkout -b rebased-feature-branch feature-branch && test_when_finished git branch -D rebased-feature-branch && echo dirty >>file3 && - git rebase$type unrelated-onto-branch && + git rebase$type unrelated-onto-branch >tmp 2>&1 && grep unrelated file4 && grep dirty file3 && git checkout feature-branch ' + test_expect_success "rebase$type --autostash: check output" ' + suffix=${type#\ -} && suffix=${suffix:--am} && + sed -f $TEST_DIRECTORY/t3420/remove-ids.sed tmp \ + >actual-success$suffix && + test_cmp $TEST_DIRECTORY/t3420/expected-success$suffix \ + actual-success$suffix + ' + test_expect_success "rebase$type: dirty index, non-conflicting rebase" ' test_config rebase.autostash true && git reset --hard && diff --git a/t/t3420/expected-success-am b/t/t3420/expected-success-am new file mode 100644 index 0000000000000000000000000000000000000000..c18ded04f703ed2aa83d5e62589a908d0a44cf7e --- /dev/null +++ b/t/t3420/expected-success-am @@ -0,0 +1,6 @@ +Created autostash: XXX +HEAD is now at XXX third commit +First, rewinding head to replay your work on top of it... +Applying: second commit +Applying: third commit +Applied autostash. diff --git a/t/t3420/expected-success-interactive b/t/t3420/expected-success-interactive new file mode 100644 index 0000000000000000000000000000000000000000..b31f71c95ddc9c18ce9956c1aadf53cedd966801 --- /dev/null +++ b/t/t3420/expected-success-interactive @@ -0,0 +1,4 @@ +Created autostash: XXX +HEAD is now at XXX third commit +Rebasing (1/2)\rRebasing (2/2)\rSuccessfully rebased and updated refs/heads/rebased-feature-branch. +Applied autostash. diff --git a/t/t3420/expected-success-merge b/t/t3420/expected-success-merge new file mode 100644 index 0000000000000000000000000000000000000000..66386f7cb5242a255d9cc64aad741e651ec7ec1e --- /dev/null +++ b/t/t3420/expected-success-merge @@ -0,0 +1,30 @@ +Created autostash: XXX +HEAD is now at XXX third commit +First, rewinding head to replay your work on top of it... +Merging unrelated-onto-branch with HEAD~1 +Merging: +XXX unrelated commit +XXX second commit +found 1 common ancestor: +XXX initial commit +[detached HEAD XXX] second commit + Author: A U Thor <author@example.com> + Date: Thu Apr 7 15:14:13 2005 -0700 + 2 files changed, 2 insertions(+) + create mode 100644 file1 + create mode 100644 file2 +Committed: 0001 second commit +Merging unrelated-onto-branch with HEAD~0 +Merging: +XXX second commit +XXX third commit +found 1 common ancestor: +XXX second commit +[detached HEAD XXX] third commit + Author: A U Thor <author@example.com> + Date: Thu Apr 7 15:15:13 2005 -0700 + 1 file changed, 1 insertion(+) + create mode 100644 file3 +Committed: 0002 third commit +All done. +Applied autostash. diff --git a/t/t3420/remove-ids.sed b/t/t3420/remove-ids.sed new file mode 100644 index 0000000000000000000000000000000000000000..9e9048b02bd04d287461543d85db0bb715b89f8c --- /dev/null +++ b/t/t3420/remove-ids.sed @@ -0,0 +1,6 @@ +s/^\(Created autostash: \)[0-9a-f]\{6,\}$/\1XXX/ +s/^\(HEAD is now at \)[0-9a-f]\{6,\}\( .* commit\)$/\1XXX\2/ +s/^[0-9a-f]\{6,\}\( .* commit\)$/XXX\1/ +s/\(detached HEAD \)[0-9a-f]\{6,\}/\1XXX/ +s/\(could not apply \)[0-9a-f]\{6,\}/\1XXX/g +s/\r/\\r/g -- 2.13.0 ^ permalink raw reply related [flat|nested] 59+ messages in thread
* Re: [PATCH 2/3] rebase: Add tests for console output 2017-05-31 10:42 ` [PATCH 2/3] rebase: Add tests for console output Phillip Wood @ 2017-05-31 19:02 ` Phillip Wood 2017-06-01 1:59 ` Junio C Hamano 2017-06-01 12:56 ` Johannes Schindelin 1 sibling, 1 reply; 59+ messages in thread From: Phillip Wood @ 2017-05-31 19:02 UTC (permalink / raw) To: git; +Cc: Johannes.Schindelin, avarab, Phillip Wood On 31/05/17 11:42, Phillip Wood wrote: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > Check the console output when using --autostash and the stash applies > cleanly is what we expect. To avoid this test depending on commit and > stash hashes it uses sed to replace them with XXX. The sed script also > replaces carriage returns in the output with '\r' to avoid embedded > '^M's in the expected output files. Unfortunately this means we still > end up with an embedded '^M' in the sed script which may not be > preserved when sending this. The last line of the sed script should be > +s/^M/\\r/g Thinking about this it might be better to create the sed script with printf when running the test > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> > --- > t/t3420-rebase-autostash.sh | 10 +++++++++- > t/t3420/expected-success-am | 6 ++++++ > t/t3420/expected-success-interactive | 4 ++++ > t/t3420/expected-success-merge | 30 ++++++++++++++++++++++++++++++ > t/t3420/remove-ids.sed | 6 ++++++ > 5 files changed, 55 insertions(+), 1 deletion(-) > > diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh > index ab8a63e8d6dc643b28eb0c74ba3f032b7532226f..886be63c6d13e1ac4197a1b185659fb3d7d7eb26 100755 > --- a/t/t3420-rebase-autostash.sh > +++ b/t/t3420-rebase-autostash.sh > @@ -53,12 +53,20 @@ testrebase() { > git checkout -b rebased-feature-branch feature-branch && > test_when_finished git branch -D rebased-feature-branch && > echo dirty >>file3 && > - git rebase$type unrelated-onto-branch && > + git rebase$type unrelated-onto-branch >tmp 2>&1 && > grep unrelated file4 && > grep dirty file3 && > git checkout feature-branch > ' > > + test_expect_success "rebase$type --autostash: check output" ' > + suffix=${type#\ -} && suffix=${suffix:--am} && > + sed -f $TEST_DIRECTORY/t3420/remove-ids.sed tmp \ > + >actual-success$suffix && > + test_cmp $TEST_DIRECTORY/t3420/expected-success$suffix \ > + actual-success$suffix > + ' > + > test_expect_success "rebase$type: dirty index, non-conflicting rebase" ' > test_config rebase.autostash true && > git reset --hard && > diff --git a/t/t3420/expected-success-am b/t/t3420/expected-success-am > new file mode 100644 > index 0000000000000000000000000000000000000000..c18ded04f703ed2aa83d5e62589a908d0a44cf7e > --- /dev/null > +++ b/t/t3420/expected-success-am > @@ -0,0 +1,6 @@ > +Created autostash: XXX > +HEAD is now at XXX third commit > +First, rewinding head to replay your work on top of it... > +Applying: second commit > +Applying: third commit > +Applied autostash. > diff --git a/t/t3420/expected-success-interactive b/t/t3420/expected-success-interactive > new file mode 100644 > index 0000000000000000000000000000000000000000..b31f71c95ddc9c18ce9956c1aadf53cedd966801 > --- /dev/null > +++ b/t/t3420/expected-success-interactive > @@ -0,0 +1,4 @@ > +Created autostash: XXX > +HEAD is now at XXX third commit > +Rebasing (1/2)\rRebasing (2/2)\rSuccessfully rebased and updated refs/heads/rebased-feature-branch. > +Applied autostash. > diff --git a/t/t3420/expected-success-merge b/t/t3420/expected-success-merge > new file mode 100644 > index 0000000000000000000000000000000000000000..66386f7cb5242a255d9cc64aad741e651ec7ec1e > --- /dev/null > +++ b/t/t3420/expected-success-merge > @@ -0,0 +1,30 @@ > +Created autostash: XXX > +HEAD is now at XXX third commit > +First, rewinding head to replay your work on top of it... > +Merging unrelated-onto-branch with HEAD~1 > +Merging: > +XXX unrelated commit > +XXX second commit > +found 1 common ancestor: > +XXX initial commit > +[detached HEAD XXX] second commit > + Author: A U Thor <author@example.com> > + Date: Thu Apr 7 15:14:13 2005 -0700 > + 2 files changed, 2 insertions(+) > + create mode 100644 file1 > + create mode 100644 file2 > +Committed: 0001 second commit > +Merging unrelated-onto-branch with HEAD~0 > +Merging: > +XXX second commit > +XXX third commit > +found 1 common ancestor: > +XXX second commit > +[detached HEAD XXX] third commit > + Author: A U Thor <author@example.com> > + Date: Thu Apr 7 15:15:13 2005 -0700 > + 1 file changed, 1 insertion(+) > + create mode 100644 file3 > +Committed: 0002 third commit > +All done. > +Applied autostash. > diff --git a/t/t3420/remove-ids.sed b/t/t3420/remove-ids.sed > new file mode 100644 > index 0000000000000000000000000000000000000000..9e9048b02bd04d287461543d85db0bb715b89f8c > --- /dev/null > +++ b/t/t3420/remove-ids.sed > @@ -0,0 +1,6 @@ > +s/^\(Created autostash: \)[0-9a-f]\{6,\}$/\1XXX/ > +s/^\(HEAD is now at \)[0-9a-f]\{6,\}\( .* commit\)$/\1XXX\2/ > +s/^[0-9a-f]\{6,\}\( .* commit\)$/XXX\1/ > +s/\(detached HEAD \)[0-9a-f]\{6,\}/\1XXX/ > +s/\(could not apply \)[0-9a-f]\{6,\}/\1XXX/g > +s//\\r/g > ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH 2/3] rebase: Add tests for console output 2017-05-31 19:02 ` Phillip Wood @ 2017-06-01 1:59 ` Junio C Hamano 0 siblings, 0 replies; 59+ messages in thread From: Junio C Hamano @ 2017-06-01 1:59 UTC (permalink / raw) To: Phillip Wood; +Cc: git, Johannes.Schindelin, avarab, Phillip Wood Phillip Wood <phillip.wood@talktalk.net> writes: > On 31/05/17 11:42, Phillip Wood wrote: >> From: Phillip Wood <phillip.wood@dunelm.org.uk> >> >> Check the console output when using --autostash and the stash applies >> cleanly is what we expect. To avoid this test depending on commit and >> stash hashes it uses sed to replace them with XXX. The sed script also >> replaces carriage returns in the output with '\r' to avoid embedded >> '^M's in the expected output files. Unfortunately this means we still >> end up with an embedded '^M' in the sed script which may not be >> preserved when sending this. Or such a sed script may simply be not portable. ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH 2/3] rebase: Add tests for console output 2017-05-31 10:42 ` [PATCH 2/3] rebase: Add tests for console output Phillip Wood 2017-05-31 19:02 ` Phillip Wood @ 2017-06-01 12:56 ` Johannes Schindelin 2017-06-01 23:40 ` Junio C Hamano 2017-06-07 10:47 ` Phillip Wood 1 sibling, 2 replies; 59+ messages in thread From: Johannes Schindelin @ 2017-06-01 12:56 UTC (permalink / raw) To: Phillip Wood; +Cc: git, avarab Hi Phillip, On Wed, 31 May 2017, Phillip Wood wrote: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > Check the console output when using --autostash and the stash applies > cleanly is what we expect. To avoid this test depending on commit and > stash hashes it uses sed to replace them with XXX. The sed script also > replaces carriage returns in the output with '\r' to avoid embedded > '^M's in the expected output files. Unfortunately this means we still > end up with an embedded '^M' in the sed script which may not be > preserved when sending this. The last line of the sed script should be > +s/^M/\\r/g Like Junio pointed out, this sed script would not be portable. To elaborate: there are two major variants of sed out there, GNU sed and BSD sed. Typically GNU sed allows a little more powerful instructions, e.g. \t and \r. But we should simply take a step back and ask why test_cmp is not enough to ignore the CRs in the output? Also, about the commit IDs. As long as the tests are consistent (i.e. they use test_commit rather than plain `git commit`, or at least call `test_tick` beforehand), the commit IDs should actually be identical between runs and not depend on the time of day or the date. The only exception to that rule is when some previous test cases call `test_commit` but are guarded behind some prereq and may not be executed at all. In that case, the precise commit IDs depend on the particular set of active prereqs. But as far as I can tell, t3420 does not have any test cases guarded by prereqs. Taking an additional step back, I wonder whether we have to hard-code the commit IDs (or XXX) to begin with. Why not generate the `expect` files with the information at hand? We can simply ask `git rev-parse --short`. For the stash's commit ID, there is no record in the ref space, of course (because it was created with `git stash create`). But I think in this case, it is legitimate to simply grep the output. That way, the test would be precise and resilient. So for example instead of adding the t/t3420/expected-success-am verbatim, you could generate the output via cat >expect <<-EOF $(grep "^Created autostash: [0-9a-f][0-9a-f]*\\$" output) HEAD is now at $(git rev-parse --short HEAD~2) third commit First, rewinding head to replay your work on top of it... Applying: second commit Applying: third commit Applied autostash. EOF Ciao, Johannes ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH 2/3] rebase: Add tests for console output 2017-06-01 12:56 ` Johannes Schindelin @ 2017-06-01 23:40 ` Junio C Hamano 2017-06-01 23:47 ` Stefan Beller 2017-06-07 10:47 ` Phillip Wood 1 sibling, 1 reply; 59+ messages in thread From: Junio C Hamano @ 2017-06-01 23:40 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Phillip Wood, git, avarab Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > Also, about the commit IDs. As long as the tests are consistent (i.e. they > use test_commit rather than plain `git commit`, or at least call > `test_tick` beforehand), the commit IDs should actually be identical > between runs and not depend on the time of day or the date. > > The only exception to that rule is when some previous test cases call > `test_commit` but are guarded behind some prereq and may not be executed > at all. In that case, the precise commit IDs depend on the particular set > of active prereqs. Good observation. The tests written such a way may make later introduction of new hash function troublesome, though (we already have tons of them, and it is already a hassle just imagining that we will have to migrate them X-<). And what you gave below is an excellent suggestion to even solve that future headaches. > But as far as I can tell, t3420 does not have any test cases guarded by > prereqs. > > Taking an additional step back, I wonder whether we have to hard-code the > commit IDs (or XXX) to begin with. Why not generate the `expect` files > with the information at hand? We can simply ask `git rev-parse --short`. > > For the stash's commit ID, there is no record in the ref space, of course > (because it was created with `git stash create`). But I think in this > case, it is legitimate to simply grep the output. > > That way, the test would be precise and resilient. > > So for example instead of adding the t/t3420/expected-success-am verbatim, > you could generate the output via > > cat >expect <<-EOF > $(grep "^Created autostash: [0-9a-f][0-9a-f]*\\$" output) > HEAD is now at $(git rev-parse --short HEAD~2) third commit > First, rewinding head to replay your work on top of it... > Applying: second commit > Applying: third commit > Applied autostash. > EOF > > Ciao, > Johannes ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH 2/3] rebase: Add tests for console output 2017-06-01 23:40 ` Junio C Hamano @ 2017-06-01 23:47 ` Stefan Beller 2017-06-02 12:47 ` pushing for a new hash, was " Johannes Schindelin 0 siblings, 1 reply; 59+ messages in thread From: Stefan Beller @ 2017-06-01 23:47 UTC (permalink / raw) To: Junio C Hamano Cc: Johannes Schindelin, Phillip Wood, git@vger.kernel.org, Ævar Arnfjörð Bjarmason On Thu, Jun 1, 2017 at 4:40 PM, Junio C Hamano <gitster@pobox.com> wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > >> Also, about the commit IDs. As long as the tests are consistent (i.e. they >> use test_commit rather than plain `git commit`, or at least call >> `test_tick` beforehand), the commit IDs should actually be identical >> between runs and not depend on the time of day or the date. >> >> The only exception to that rule is when some previous test cases call >> `test_commit` but are guarded behind some prereq and may not be executed >> at all. In that case, the precise commit IDs depend on the particular set >> of active prereqs. > > Good observation. The tests written such a way may make later > introduction of new hash function troublesome, though (we already > have tons of them, and it is already a hassle just imagining that we > will have to migrate them X-<). > > And what you gave below is an excellent suggestion to even solve > that future headaches. > We had a discussion off list how much of the test suite is in bad shape, and "$ git grep ^index" points out a lot of places as well. ^ permalink raw reply [flat|nested] 59+ messages in thread
* pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console output 2017-06-01 23:47 ` Stefan Beller @ 2017-06-02 12:47 ` Johannes Schindelin 2017-06-02 17:54 ` Jonathan Nieder 0 siblings, 1 reply; 59+ messages in thread From: Johannes Schindelin @ 2017-06-02 12:47 UTC (permalink / raw) To: Stefan Beller Cc: Junio C Hamano, Phillip Wood, git@vger.kernel.org, Ævar Arnfjörð Bjarmason Hi, On Thu, 1 Jun 2017, Stefan Beller wrote: > On Thu, Jun 1, 2017 at 4:40 PM, Junio C Hamano <gitster@pobox.com> wrote: > > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > >> Also, about the commit IDs. As long as the tests are consistent (i.e. they > >> use test_commit rather than plain `git commit`, or at least call > >> `test_tick` beforehand), the commit IDs should actually be identical > >> between runs and not depend on the time of day or the date. > >> > >> The only exception to that rule is when some previous test cases call > >> `test_commit` but are guarded behind some prereq and may not be executed > >> at all. In that case, the precise commit IDs depend on the particular set > >> of active prereqs. > > > > Good observation. The tests written such a way may make later > > introduction of new hash function troublesome, though (we already > > have tons of them, and it is already a hassle just imagining that we > > will have to migrate them X-<). > > > > And what you gave below is an excellent suggestion to even solve > > that future headaches. > > We had a discussion off list how much of the test suite is in bad shape, > and "$ git grep ^index" points out a lot of places as well. Maybe we should call out a specific month (or even a longer period) during which we try to push toward that new hash function, and focus more on those tasks (and on critical bug fixes, if any) than anything else. I also wonder how we can attract (back) cryptographic talent to help us avoid repeating mistakes when picking a new hash algorithm. So far, the only undisputable expert opinion I read was from the Keccak team, and I did not have the impression that their opinion had any impact on the discussion. Needless to say: I think it should. Cryptography is hard. We proved it ;-) Ciao, Dscho ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console output 2017-06-02 12:47 ` pushing for a new hash, was " Johannes Schindelin @ 2017-06-02 17:54 ` Jonathan Nieder 2017-06-02 18:05 ` Jonathan Nieder ` (3 more replies) 0 siblings, 4 replies; 59+ messages in thread From: Jonathan Nieder @ 2017-06-02 17:54 UTC (permalink / raw) To: Johannes Schindelin Cc: Stefan Beller, Junio C Hamano, Phillip Wood, git@vger.kernel.org, Ævar Arnfjörð Bjarmason Hi Dscho, Johannes Schindelin wrote: > On Thu, 1 Jun 2017, Stefan Beller wrote: >> We had a discussion off list how much of the test suite is in bad shape, >> and "$ git grep ^index" points out a lot of places as well. > > Maybe we should call out a specific month (or even a longer period) during > which we try to push toward that new hash function, and focus more on > those tasks (and on critical bug fixes, if any) than anything else. Thanks for offering. ;-) Here's a rough list of some useful tasks, in no particular order: 1. bc/object-id: This patch series continues, eliminating assumptions about the size of object ids by encapsulating them in a struct. One straightforward way to find code that still needs to be converted is to grep for "sha" --- often the conversion patches change function and variable names to refer to oid_ where they used to use sha1_, making the stragglers easier to spot. 2. Hard-coded object ids in tests: As Stefan hinted, many tests beyond t00* make assumptions about the exact values of object ids. That's bad for maintainability for other reasons beyond the hash function transition, too. It should be possible to suss them out by patching git's sha1 routine to use the ones-complement of sha1 (~sha1) instead and seeing which tests fail. 3. Repository format extension to use a different hash function: we want git to be able to work with two hash functions: sha1 and something else. For interoperability and simplity, it is useful for a single git binary to support both hash functions. That means a repository needs to be able to specify what hash function is used for the objects in that repository. This can be configured by setting '[core] repositoryformatversion=1' (to avoid confusing old versions of git) and '[extensions] experimentalNewHashFunction = true'. Documentation/technical/repository-version.txt has more details. We can start experimenting with this using e.g. the ~sha1 function described at (2), or the 160-bit hash of the patch author's choice (e.g. truncated blake2bp-256). 4. When choosing a hash function, people may argue about performance. It would be useful for run some benchmarks for git (running the test suite, t/perf tests, etc) using a variety of hash functions as input to such a discussion. 5. Longer hash: Even once all object id references in git use struct object_id (see (1)), we need to tackle other assumptions about object id size in git and its tests. It should be possible to suss them out by replacing git's sha1 routine with a 40-byte hash: sha1 with each byte repeated (sha1+sha1) and seeing what fails. 6. Repository format extension for longer hash: As in (3), we could add a repository format extension to experiment with using the sha1+sha1 function. 7. Avoiding wasted memory from unused hash functions: struct object_id has definition 'unsigned char hash[GIT_MAX_RAWSZ]', where GIT_MAX_RAWSZ is the size of the largest supported hash function. When operating on a repository that only uses sha1, this wastes memory. Avoid that by making object identifiers variable-sized. That is, something like struct object_id { union { unsigned char hash20[20]; unsigned char hash32[32]; } *hash; } or struct object_id { unsigned char *hash; } The hard part is that allocation and destruction have to be explicit instead of happening automatically when an object_id is an automatic variable. 8. Implement http://public-inbox.org/git/20170307001709.GC26789@aiede.mtv.corp.google.com/ :) I'd like to send a breakdown of that too, but that probably should happen in a separate message. 9. We can use help from security experts in all of this. Fuzzing, analysis of how we use cryptography, security review of other parts of the design, and information to help choose a hash function are all appreciated. > I also wonder how we can attract (back) cryptographic talent to help us > avoid repeating mistakes when picking a new hash algorithm. > > So far, the only undisputable expert opinion I read was from the Keccak > team, and I did not have the impression that their opinion had any impact > on the discussion. Needless to say: I think it should. Cryptography is > hard. We proved it ;-) Do you have some ideas in mind here? How did you get the impression that their opinion had no impact? We have been getting feedback about the choice of hash function both on and off list from a variety of people, some indisputably security experts. Sometimes the best one can do is to just listen. For what it's worth my personal opinion is currently leaning toward blake2bp-256 as choice of hash function, not SHA2 or SHA3. But we still have time to learn more and make a better decision. Thanks and hope that helps, Jonathan ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console output 2017-06-02 17:54 ` Jonathan Nieder @ 2017-06-02 18:05 ` Jonathan Nieder 2017-06-02 20:29 ` Ævar Arnfjörð Bjarmason ` (2 subsequent siblings) 3 siblings, 0 replies; 59+ messages in thread From: Jonathan Nieder @ 2017-06-02 18:05 UTC (permalink / raw) To: Johannes Schindelin Cc: Stefan Beller, Junio C Hamano, Phillip Wood, git@vger.kernel.org, Ævar Arnfjörð Bjarmason, Brian M. Carlson Jonathan Nieder wrote: > Here's a rough list of some useful tasks, in no particular order: > > 1. bc/object-id: This patch series continues, eliminating assumptions > about the size of object ids by encapsulating them in a struct. > One straightforward way to find code that still needs to be > converted is to grep for "sha" --- often the conversion patches > change function and variable names to refer to oid_ where they used > to use sha1_, making the stragglers easier to spot. I forgot to say: please coordinate with Brian M. Carlson if working on this one. That makes it possible to divide up the work in a way that doesn't lead to people patching the same files and stepping on each other's toes. This is one of the larger tasks and contributions to it are very useful. Just do coordinate. For comparison, the other suggestions in the list should be possible for a rogue agent to experiment with alone, with advice from the list where they need it. :) Jonathan ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console output 2017-06-02 17:54 ` Jonathan Nieder 2017-06-02 18:05 ` Jonathan Nieder @ 2017-06-02 20:29 ` Ævar Arnfjörð Bjarmason 2017-06-15 10:38 ` Johannes Schindelin 2017-06-03 0:36 ` Junio C Hamano 2017-06-06 22:22 ` Johannes Schindelin 3 siblings, 1 reply; 59+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2017-06-02 20:29 UTC (permalink / raw) To: Jonathan Nieder Cc: Johannes Schindelin, Stefan Beller, Junio C Hamano, Phillip Wood, git@vger.kernel.org On Fri, Jun 2, 2017 at 7:54 PM, Jonathan Nieder <jrnieder@gmail.com> wrote: > Hi Dscho, > > Johannes Schindelin wrote: >> On Thu, 1 Jun 2017, Stefan Beller wrote: > >>> We had a discussion off list how much of the test suite is in bad shape, >>> and "$ git grep ^index" points out a lot of places as well. >> >> Maybe we should call out a specific month (or even a longer period) during >> which we try to push toward that new hash function, and focus more on >> those tasks (and on critical bug fixes, if any) than anything else. > > Thanks for offering. ;-) > > Here's a rough list of some useful tasks, in no particular order: > > 1. bc/object-id: This patch series continues, eliminating assumptions > about the size of object ids by encapsulating them in a struct. > One straightforward way to find code that still needs to be > converted is to grep for "sha" --- often the conversion patches > change function and variable names to refer to oid_ where they used > to use sha1_, making the stragglers easier to spot. > > 2. Hard-coded object ids in tests: As Stefan hinted, many tests beyond > t00* make assumptions about the exact values of object ids. That's > bad for maintainability for other reasons beyond the hash function > transition, too. > > It should be possible to suss them out by patching git's sha1 > routine to use the ones-complement of sha1 (~sha1) instead and > seeing which tests fail. I just hacked this up locally. While a lot of stuff fails, it's not exactly an out of control garbage fire and could be churned through by someone interested. A lot of it's just lazy sha1 hardcoding for no good reason where we could use a tag, or e.g. test_cmp on ls-tree output without the test really caring about the hash. Things that really need to test the sha1 could be guarded by some new prereq. Both of my attempts to fuzz SHA1DCInit though resulted in some segfaults, I tried both changing "0x" to "~0x" in the ihv assignment, and just calling SHA1DCUpdate(ctx, "x", 1) at the end of the function, not sure why that's happening. If someone knows offhand what I'm doing wrong here I might be interested in going through this if I don't have to sift through the segfaults. I.e. what's an easy hack to make all of git pretend that "foo" hashes to "xfoo", "" to "x" etc. > 4. When choosing a hash function, people may argue about performance. > It would be useful for run some benchmarks for git (running > the test suite, t/perf tests, etc) using a variety of hash > functions as input to such a discussion. To the extent that such benchmarks matter, it seems prudent to heavily weigh them in favor of whatever seems to be likely to be the more common hash function going forward, since those are likely to get faster through future hardware acceleration. E.g. Intel announced Goldmont last year which according to one SHA-1 implementation improved from 9.5 cycles per byte to 2.7 cpb[1]. They only have acceleration for SHA-1 and SHA-256[2] 1. https://github.com/weidai11/cryptopp/issues/139#issuecomment-264283385 2. https://en.wikipedia.org/wiki/Goldmont ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console output 2017-06-02 20:29 ` Ævar Arnfjörð Bjarmason @ 2017-06-15 10:38 ` Johannes Schindelin 0 siblings, 0 replies; 59+ messages in thread From: Johannes Schindelin @ 2017-06-15 10:38 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Jonathan Nieder, Stefan Beller, Junio C Hamano, Phillip Wood, git@vger.kernel.org [-- Attachment #1: Type: text/plain, Size: 2941 bytes --] Hi Ævar, On Fri, 2 Jun 2017, Ævar Arnfjörð Bjarmason wrote: > On Fri, Jun 2, 2017 at 7:54 PM, Jonathan Nieder <jrnieder@gmail.com> wrote: > > > > Johannes Schindelin wrote: > >> On Thu, 1 Jun 2017, Stefan Beller wrote: > > > >>> We had a discussion off list how much of the test suite is in bad > >>> shape, and "$ git grep ^index" points out a lot of places as well. > >> > >> Maybe we should call out a specific month (or even a longer period) > >> during which we try to push toward that new hash function, and focus > >> more on those tasks (and on critical bug fixes, if any) than anything > >> else. > > > > Thanks for offering. ;-) > > > > Here's a rough list of some useful tasks, in no particular order: > > > > 1. bc/object-id: This patch series continues, eliminating assumptions > > about the size of object ids by encapsulating them in a struct. > > One straightforward way to find code that still needs to be > > converted is to grep for "sha" --- often the conversion patches > > change function and variable names to refer to oid_ where they used > > to use sha1_, making the stragglers easier to spot. > > > > 2. Hard-coded object ids in tests: As Stefan hinted, many tests beyond > > t00* make assumptions about the exact values of object ids. That's > > bad for maintainability for other reasons beyond the hash function > > transition, too. > > > > It should be possible to suss them out by patching git's sha1 > > routine to use the ones-complement of sha1 (~sha1) instead and > > seeing which tests fail. > > I just hacked this up locally. Would you mind turning that into a patch? I figure this could be a compile-time switch and applied to Git's `master` so that it is easier to find those assumptions, as well as verify fixes on multiple platforms. > > 4. When choosing a hash function, people may argue about performance. > > It would be useful for run some benchmarks for git (running > > the test suite, t/perf tests, etc) using a variety of hash > > functions as input to such a discussion. > > To the extent that such benchmarks matter, it seems prudent to heavily > weigh them in favor of whatever seems to be likely to be the more > common hash function going forward, since those are likely to get > faster through future hardware acceleration. Exactly. As I just mentioned elsewhere, the cryptographers I talked to expect SHA-256 and SHA3-256 to see the most focus of all hash functions, by far. > E.g. Intel announced Goldmont last year which according to one SHA-1 > implementation improved from 9.5 cycles per byte to 2.7 cpb[1]. They > only have acceleration for SHA-1 and SHA-256[2] > > 1. https://github.com/weidai11/cryptopp/issues/139#issuecomment-264283385 > > 2. https://en.wikipedia.org/wiki/Goldmont Thanks for digging that up! Very valuable information. Ciao, Dscho ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console output 2017-06-02 17:54 ` Jonathan Nieder 2017-06-02 18:05 ` Jonathan Nieder 2017-06-02 20:29 ` Ævar Arnfjörð Bjarmason @ 2017-06-03 0:36 ` Junio C Hamano 2017-06-06 22:22 ` Johannes Schindelin 3 siblings, 0 replies; 59+ messages in thread From: Junio C Hamano @ 2017-06-03 0:36 UTC (permalink / raw) To: Jonathan Nieder Cc: Johannes Schindelin, Stefan Beller, Phillip Wood, git@vger.kernel.org, Ævar Arnfjörð Bjarmason Jonathan Nieder <jrnieder@gmail.com> writes: > 2. Hard-coded object ids in tests: As Stefan hinted, many tests beyond > t00* make assumptions about the exact values of object ids. That's > bad for maintainability for other reasons beyond the hash function > transition, too. > > It should be possible to suss them out by patching git's sha1 > routine to use the ones-complement of sha1 (~sha1) instead and > seeing which tests fail. One particularly nasty one is t1512-rev-parse-disambiguation that ensures that the abbreviation and disambiguation works correctly. It uses a set of objects (tags, commits, trees and blobs) whose object names all begin with number of "0"; which will of course become useless once we change the hash function. No matter what new hash function is chosen, we'd need a similar test to ensure that disambiguation works correctly, so one of the tasks for hash migration is to port (not drop) this test. ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console output 2017-06-02 17:54 ` Jonathan Nieder ` (2 preceding siblings ...) 2017-06-03 0:36 ` Junio C Hamano @ 2017-06-06 22:22 ` Johannes Schindelin 2017-06-06 22:45 ` Jonathan Nieder 2017-06-06 22:45 ` pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console output Stefan Beller 3 siblings, 2 replies; 59+ messages in thread From: Johannes Schindelin @ 2017-06-06 22:22 UTC (permalink / raw) To: Jonathan Nieder Cc: Stefan Beller, Junio C Hamano, Phillip Wood, git@vger.kernel.org, Ævar Arnfjörð Bjarmason Hi Jonathan, On Fri, 2 Jun 2017, Jonathan Nieder wrote: > Johannes Schindelin wrote: > > On Thu, 1 Jun 2017, Stefan Beller wrote: > > >> We had a discussion off list how much of the test suite is in bad shape, > >> and "$ git grep ^index" points out a lot of places as well. > > > > Maybe we should call out a specific month (or even a longer period) during > > which we try to push toward that new hash function, and focus more on > > those tasks (and on critical bug fixes, if any) than anything else. > > Thanks for offering. ;-) Undoubtedly my lack of command of the English language is to blame for this misunderstanding. By no means did I try to indicate that I am ready to accept the responsibility of working toward a new hash dumped on me. What I wanted to suggest instead was that the current direction looks very unfocused to me, and that I do not see anything going forward in a coherent manner. Hence my suggestion to make it public known that a certain time period would be dedicated (and contributions would be highly encouraged) to work on replacing SHA-1 by something else. But: 1) this cannot be a one-person effort, it is too large 2) it cannot even be as uncoordinated an effort as it is now, because that leads only to bikeshedding instead of progress 3) the only person who could make that call is Junio 4) we still have the problem that there is no cryptography expert among those who in the Git project are listened to > How did you get the impression that their opinion had no impact? We have > been getting feedback about the choice of hash function both on and off > list from a variety of people, some indisputably security experts. > Sometimes the best one can do is to just listen. I did get the impression by talking at length to a cryptography expert who successfully resisted any suggestions to get involved in the Git mailing list. There were also accounts floating around on Twitter that a certain cryptography expert who dared to mention already back in 2005 how dangerous it would be to hardcode SHA-1 into Git was essentially shown the finger, and I cannot fault him for essentially saying "I told you so" publicly. In my mind, it would have made sense to ask well-respected cryptographers about their opinions and then try to figure out a consensus among them (as opposed to what I saw so far, a lot of enthusastic talk by developers with little standing in the cryptography community, mostly revolving around hash size and speed as opposed to security). And then try to implement that consensus in Git. Given my recent success rate with SHA-1 related concerns, I am unfortunately not the person who can bring that about. But maybe you are. Ciao, Dscho ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console output 2017-06-06 22:22 ` Johannes Schindelin @ 2017-06-06 22:45 ` Jonathan Nieder 2017-06-07 1:09 ` Junio C Hamano 2017-06-06 22:45 ` pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console output Stefan Beller 1 sibling, 1 reply; 59+ messages in thread From: Jonathan Nieder @ 2017-06-06 22:45 UTC (permalink / raw) To: Johannes Schindelin Cc: Stefan Beller, Junio C Hamano, Phillip Wood, git@vger.kernel.org, Ævar Arnfjörð Bjarmason Hi, Johannes Schindelin wrote: > On Fri, 2 Jun 2017, Jonathan Nieder wrote: >> Johannes Schindelin wrote: >>> Maybe we should call out a specific month (or even a longer period) during >>> which we try to push toward that new hash function, and focus more on >>> those tasks (and on critical bug fixes, if any) than anything else. >> >> Thanks for offering. ;-) > > Undoubtedly my lack of command of the English language is to blame for > this misunderstanding. > > By no means did I try to indicate that I am ready to accept the > responsibility of working toward a new hash dumped on me. It was a joke. More seriously, I do appreciate your questions to get this discussion going. [...] > 3) the only person who could make that call is Junio I strongly disagree with this. > 4) we still have the problem that there is no cryptography expert among > those who in the Git project are listened to *shrug* I still don't know what you are suggesting here. Are you saying we should find a cryptography expert to pay? Or do you have other specific suggestions of how to attract them? >> How did you get the impression that their opinion had no impact? We have >> been getting feedback about the choice of hash function both on and off >> list from a variety of people, some indisputably security experts. >> Sometimes the best one can do is to just listen. > > I did get the impression by talking at length to a cryptography expert who > successfully resisted any suggestions to get involved in the Git mailing > list. I know of other potential Git contributors that have resisted getting involved in the Git mailing list, too. I still don't know what you are suggesting here. Forgive me for being dense. > There were also accounts floating around on Twitter that a certain > cryptography expert who dared to mention already back in 2005 how > dangerous it would be to hardcode SHA-1 into Git was essentially shown the > finger, and I cannot fault him for essentially saying "I told you so" > publicly. I think there is a concrete suggestion embedded here: when discussions go in an unproductive direction, my usual practice has been to keep away from them. This means that to a casual observer there can appear to be a consensus that doesn't really exist. We need to do better than that: when a prominent contributor like Linus and people newer to the project are emphatically dismissing the security impact of using a broken hash function, others in the project need to speak up to make it clear that those are not the actual opinions of the project. To put it another way: "The standard you walk past is the standard you accept". I have failed at this. It is a very hard problem to solve, but it is worth solving. > In my mind, it would have made sense to ask well-respected cryptographers > about their opinions and then try to figure out a consensus among them (as > opposed to what I saw so far, a lot of enthusastic talk by developers with > little standing in the cryptography community, mostly revolving around > hash size and speed as opposed to security). And then try to implement > that consensus in Git. Given my recent success rate with SHA-1 related > concerns, I am unfortunately not the person who can bring that about. > > But maybe you are. I think you are being a bit dismissive of both the work done so far and the value of your own work. I am happy to solicit more input from security researchers, though, and your suggestion to do so is good advice. Thanks and hope that helps, Jonathan ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console output 2017-06-06 22:45 ` Jonathan Nieder @ 2017-06-07 1:09 ` Junio C Hamano 2017-06-07 2:18 ` [PATCH] t4005: modernize style and drop hard coded sha1 Stefan Beller 0 siblings, 1 reply; 59+ messages in thread From: Junio C Hamano @ 2017-06-07 1:09 UTC (permalink / raw) To: Jonathan Nieder Cc: Johannes Schindelin, Stefan Beller, Phillip Wood, git@vger.kernel.org, Ævar Arnfjörð Bjarmason Jonathan Nieder <jrnieder@gmail.com> writes: >> 3) the only person who could make that call is Junio > > I strongly disagree with this. If it helps, I _can_ make any set of declarations to make it sound more official, e.g. (the remainder of) June is the "make sure our tests are ready" month where we try to eradiate uchar[20] by advancing the object-id effort, replace EMPTY_TREE_SHA1_HEX and friends with EMPTY_TREE_OID_HEX, add a build-time configuration that allows us to build a Git binary that uses a phony hash e.g. ~sha1 truncated to 16 bytes, and build Git with such a configuration and then run tests, and we concentrate on this effort without adding new things until we make the tests pass. And make similar declarations for the remainder of the year. But the actual input for formulating what each step does and the timeline we aim for needs to come from the list wisdom; it won't have much impact if the project's official declaration is not followed-thru, and a unilateral declaration that is pulled out of thin-air likely will not be. > I am happy to solicit more input from security researchers, though, > and your suggestion to do so is good advice. One good thing is that we can prepare the framework to adopt a new hash before knowing what the exact hash function is; crypto-minded folks can work on the hash selection in parallel without waiting for the framework to become ready. And I do agree with Dscho that having crypto experts would be very helpful for the latter. ^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH] t4005: modernize style and drop hard coded sha1 2017-06-07 1:09 ` Junio C Hamano @ 2017-06-07 2:18 ` Stefan Beller 2017-06-07 17:39 ` Brandon Williams 0 siblings, 1 reply; 59+ messages in thread From: Stefan Beller @ 2017-06-07 2:18 UTC (permalink / raw) To: gitster; +Cc: Johannes.Schindelin, avarab, git, jrnieder, phillip.wood, sbeller Use modern style in the test t4005. Remove hard coded sha1 values. Combine test prep work and the actual test. Rename the first test to contain the word "setup". Signed-off-by: Stefan Beller <sbeller@google.com> --- Junio wrote: > If it helps, I _can_ make any set of declarations to make it sound > more official, e.g. (the remainder of) June is the "make sure our > tests are ready" If it helps, I can write code for that. :) Do get a good grasp on which tests need to be fixed, I changed the seed value for the sha1 computation and then run the test suite. There are a lot of tests passing for this, but also quite a few failing. Then I picked t4005 randomly to start with. This patch works even with a crippled hash function as we use hash-object to get the object id. Thanks, Stefan t/t4005-diff-rename-2.sh | 95 ++++++++++++++++++++++-------------------------- 1 file changed, 43 insertions(+), 52 deletions(-) diff --git a/t/t4005-diff-rename-2.sh b/t/t4005-diff-rename-2.sh index 135addbfbd..f542d2929d 100755 --- a/t/t4005-diff-rename-2.sh +++ b/t/t4005-diff-rename-2.sh @@ -3,84 +3,75 @@ # Copyright (c) 2005 Junio C Hamano # -test_description='Same rename detection as t4003 but testing diff-raw. +test_description='Same rename detection as t4003 but testing diff-raw.' -' . ./test-lib.sh . "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash -test_expect_success \ - 'prepare reference tree' \ - 'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING && - echo frotz >rezrov && - git update-index --add COPYING rezrov && - tree=$(git write-tree) && - echo $tree' - -test_expect_success \ - 'prepare work tree' \ - 'sed -e 's/HOWEVER/However/' <COPYING >COPYING.1 && - sed -e 's/GPL/G.P.L/g' <COPYING >COPYING.2 && - rm -f COPYING && - git update-index --add --remove COPYING COPYING.?' +test_expect_success 'setup reference tree' ' + cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING && + echo frotz >rezrov && + git update-index --add COPYING rezrov && + tree=$(git write-tree) && + echo $tree && + sed -e 's/HOWEVER/However/' <COPYING >COPYING.1 && + sed -e 's/GPL/G.P.L/g' <COPYING >COPYING.2 && + origoid=$(git hash-object COPYING) && + oid1=$(git hash-object COPYING.1) && + oid2=$(git hash-object COPYING.2) +' +################################################################ # tree has COPYING and rezrov. work tree has COPYING.1 and COPYING.2, # both are slightly edited, and unchanged rezrov. We say COPYING.1 # and COPYING.2 are based on COPYING, and do not say anything about # rezrov. -git diff-index -C $tree >current - -cat >expected <<\EOF -:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234 COPYING COPYING.1 -:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 06c67961bbaed34a127f76d261f4c0bf73eda471 R1234 COPYING COPYING.2 -EOF +test_expect_success 'validate output from rename/copy detection (#1)' ' + rm -f COPYING && + git update-index --add --remove COPYING COPYING.? && -test_expect_success \ - 'validate output from rename/copy detection (#1)' \ - 'compare_diff_raw current expected' + cat <<-EOF >expected && + :100644 100644 $origoid $oid1 C1234 COPYING COPYING.1 + :100644 100644 $origoid $oid2 R1234 COPYING COPYING.2 + EOF + git diff-index -C $tree >current && + compare_diff_raw expected current +' ################################################################ - -test_expect_success \ - 'prepare work tree again' \ - 'mv COPYING.2 COPYING && - git update-index --add --remove COPYING COPYING.1 COPYING.2' - # tree has COPYING and rezrov. work tree has COPYING and COPYING.1, # both are slightly edited, and unchanged rezrov. We say COPYING.1 # is based on COPYING and COPYING is still there, and do not say anything # about rezrov. -git diff-index -C $tree >current -cat >expected <<\EOF -:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 06c67961bbaed34a127f76d261f4c0bf73eda471 M COPYING -:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234 COPYING COPYING.1 -EOF +test_expect_success 'validate output from rename/copy detection (#2)' ' + mv COPYING.2 COPYING && + git update-index --add --remove COPYING COPYING.1 COPYING.2 && -test_expect_success \ - 'validate output from rename/copy detection (#2)' \ - 'compare_diff_raw current expected' + cat <<-EOF >expected && + :100644 100644 $origoid $oid2 M COPYING + :100644 100644 $origoid $oid1 C1234 COPYING COPYING.1 + EOF + git diff-index -C $tree >current && + compare_diff_raw current expected +' ################################################################ - # tree has COPYING and rezrov. work tree has the same COPYING and # copy-edited COPYING.1, and unchanged rezrov. We should not say # anything about rezrov or COPYING, since the revised again diff-raw # nows how to say Copy. -test_expect_success \ - 'prepare work tree once again' \ - 'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING && - git update-index --add --remove COPYING COPYING.1' - -git diff-index -C --find-copies-harder $tree >current -cat >expected <<\EOF -:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234 COPYING COPYING.1 -EOF +test_expect_success 'validate output from rename/copy detection (#3)' ' + cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING && + git update-index --add --remove COPYING COPYING.1 && -test_expect_success \ - 'validate output from rename/copy detection (#3)' \ - 'compare_diff_raw current expected' + cat <<-EOF >expected && + :100644 100644 $origoid $oid1 C1234 COPYING COPYING.1 + EOF + git diff-index -C --find-copies-harder $tree >current && + compare_diff_raw current expected +' test_done -- 2.13.0.17.gf3d7728391 ^ permalink raw reply related [flat|nested] 59+ messages in thread
* Re: [PATCH] t4005: modernize style and drop hard coded sha1 2017-06-07 2:18 ` [PATCH] t4005: modernize style and drop hard coded sha1 Stefan Beller @ 2017-06-07 17:39 ` Brandon Williams 0 siblings, 0 replies; 59+ messages in thread From: Brandon Williams @ 2017-06-07 17:39 UTC (permalink / raw) To: Stefan Beller Cc: gitster, Johannes.Schindelin, avarab, git, jrnieder, phillip.wood On 06/06, Stefan Beller wrote: > Use modern style in the test t4005. Remove hard coded sha1 values. > Combine test prep work and the actual test. Rename the first > test to contain the word "setup". > > Signed-off-by: Stefan Beller <sbeller@google.com> > --- > > Junio wrote: > > If it helps, I _can_ make any set of declarations to make it sound > > more official, e.g. (the remainder of) June is the "make sure our > > tests are ready" > > If it helps, I can write code for that. :) > > Do get a good grasp on which tests need to be fixed, I changed the seed > value for the sha1 computation and then run the test suite. There are a lot > of tests passing for this, but also quite a few failing. Then I picked t4005 > randomly to start with. This patch works even with a crippled hash function > as we use hash-object to get the object id. > > Thanks, > Stefan > > t/t4005-diff-rename-2.sh | 95 ++++++++++++++++++++++-------------------------- > 1 file changed, 43 insertions(+), 52 deletions(-) > > diff --git a/t/t4005-diff-rename-2.sh b/t/t4005-diff-rename-2.sh > index 135addbfbd..f542d2929d 100755 > --- a/t/t4005-diff-rename-2.sh > +++ b/t/t4005-diff-rename-2.sh > @@ -3,84 +3,75 @@ > # Copyright (c) 2005 Junio C Hamano > # > > -test_description='Same rename detection as t4003 but testing diff-raw. > +test_description='Same rename detection as t4003 but testing diff-raw.' > > -' > . ./test-lib.sh > . "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash > > -test_expect_success \ > - 'prepare reference tree' \ > - 'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING && > - echo frotz >rezrov && > - git update-index --add COPYING rezrov && > - tree=$(git write-tree) && > - echo $tree' > - > -test_expect_success \ > - 'prepare work tree' \ > - 'sed -e 's/HOWEVER/However/' <COPYING >COPYING.1 && > - sed -e 's/GPL/G.P.L/g' <COPYING >COPYING.2 && > - rm -f COPYING && > - git update-index --add --remove COPYING COPYING.?' > +test_expect_success 'setup reference tree' ' > + cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING && > + echo frotz >rezrov && > + git update-index --add COPYING rezrov && > + tree=$(git write-tree) && > + echo $tree && > + sed -e 's/HOWEVER/However/' <COPYING >COPYING.1 && > + sed -e 's/GPL/G.P.L/g' <COPYING >COPYING.2 && > + origoid=$(git hash-object COPYING) && > + oid1=$(git hash-object COPYING.1) && > + oid2=$(git hash-object COPYING.2) > +' This conversation looks good to me. The only thing that made me scratch my head a bit were the shell variables origoid, oid1, and oid2. It was just slightly confusing figuring out where they came from in the tests below before I noticed they were initialized up here. > > +################################################################ > # tree has COPYING and rezrov. work tree has COPYING.1 and COPYING.2, > # both are slightly edited, and unchanged rezrov. We say COPYING.1 > # and COPYING.2 are based on COPYING, and do not say anything about > # rezrov. > > -git diff-index -C $tree >current > - > -cat >expected <<\EOF > -:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234 COPYING COPYING.1 > -:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 06c67961bbaed34a127f76d261f4c0bf73eda471 R1234 COPYING COPYING.2 > -EOF > +test_expect_success 'validate output from rename/copy detection (#1)' ' > + rm -f COPYING && > + git update-index --add --remove COPYING COPYING.? && > > -test_expect_success \ > - 'validate output from rename/copy detection (#1)' \ > - 'compare_diff_raw current expected' > + cat <<-EOF >expected && > + :100644 100644 $origoid $oid1 C1234 COPYING COPYING.1 > + :100644 100644 $origoid $oid2 R1234 COPYING COPYING.2 > + EOF > + git diff-index -C $tree >current && > + compare_diff_raw expected current > +' > > ################################################################ > - > -test_expect_success \ > - 'prepare work tree again' \ > - 'mv COPYING.2 COPYING && > - git update-index --add --remove COPYING COPYING.1 COPYING.2' > - > # tree has COPYING and rezrov. work tree has COPYING and COPYING.1, > # both are slightly edited, and unchanged rezrov. We say COPYING.1 > # is based on COPYING and COPYING is still there, and do not say anything > # about rezrov. > > -git diff-index -C $tree >current > -cat >expected <<\EOF > -:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 06c67961bbaed34a127f76d261f4c0bf73eda471 M COPYING > -:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234 COPYING COPYING.1 > -EOF > +test_expect_success 'validate output from rename/copy detection (#2)' ' > + mv COPYING.2 COPYING && > + git update-index --add --remove COPYING COPYING.1 COPYING.2 && > > -test_expect_success \ > - 'validate output from rename/copy detection (#2)' \ > - 'compare_diff_raw current expected' > + cat <<-EOF >expected && > + :100644 100644 $origoid $oid2 M COPYING > + :100644 100644 $origoid $oid1 C1234 COPYING COPYING.1 > + EOF > + git diff-index -C $tree >current && > + compare_diff_raw current expected > +' > > ################################################################ > - > # tree has COPYING and rezrov. work tree has the same COPYING and > # copy-edited COPYING.1, and unchanged rezrov. We should not say > # anything about rezrov or COPYING, since the revised again diff-raw > # nows how to say Copy. > > -test_expect_success \ > - 'prepare work tree once again' \ > - 'cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING && > - git update-index --add --remove COPYING COPYING.1' > - > -git diff-index -C --find-copies-harder $tree >current > -cat >expected <<\EOF > -:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234 COPYING COPYING.1 > -EOF > +test_expect_success 'validate output from rename/copy detection (#3)' ' > + cat "$TEST_DIRECTORY"/diff-lib/COPYING >COPYING && > + git update-index --add --remove COPYING COPYING.1 && > > -test_expect_success \ > - 'validate output from rename/copy detection (#3)' \ > - 'compare_diff_raw current expected' > + cat <<-EOF >expected && > + :100644 100644 $origoid $oid1 C1234 COPYING COPYING.1 > + EOF > + git diff-index -C --find-copies-harder $tree >current && > + compare_diff_raw current expected > +' > > test_done > -- > 2.13.0.17.gf3d7728391 > -- Brandon Williams ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console output 2017-06-06 22:22 ` Johannes Schindelin 2017-06-06 22:45 ` Jonathan Nieder @ 2017-06-06 22:45 ` Stefan Beller 2017-06-06 22:52 ` Jonathan Nieder ` (2 more replies) 1 sibling, 3 replies; 59+ messages in thread From: Stefan Beller @ 2017-06-06 22:45 UTC (permalink / raw) To: Johannes Schindelin Cc: Jonathan Nieder, Junio C Hamano, Phillip Wood, git@vger.kernel.org, Ævar Arnfjörð Bjarmason On Tue, Jun 6, 2017 at 3:22 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote: >> Thanks for offering. ;-) > > Undoubtedly my lack of command of the English language is to blame for > this misunderstanding. Sometimes it is best to not be a native speaker, just fluent enough to get by. :) > What I wanted to suggest instead was that the current direction looks very > unfocused to me That is unfortunate but reality of being a *real* community project. Neither you nor me (nor Junio) can command people to do things. The best we can do is reject an idea going off. >, and that I do not see anything going forward in a > coherent manner. But is this bad? > 1) this cannot be a one-person effort, it is too large I agree. But there are efforts by multiple people. See Brians series (lots of different reviewers), also Brandon picked up parts of it (origin/bw/object-id). Or the design that was discussed on list, which was lots of people participation. > > 2) it cannot even be as uncoordinated an effort as it is now, because that > leads only to bikeshedding instead of progress Jonathan presented a list of things, that can be done in parallel in an uncoordinated effort, because that is how the project works. (C.f. he mentioned "rogue agents") > 3) the only person who could make that call is Junio Occasionally I think the same, but in fact it is not true. As said above, Junio has strong veto power for things going off rails, but in his role as a maintainer he does not coordinate people. (He occasionally asks them to coordinate between themselves, though) > > 4) we still have the problem that there is no cryptography expert among > those who in the Git project are listened to I can assure you that Jonathan listened to crypto experts. It just did not happen on the mailing list, which is sad regarding openness and transparency. 5. The timeline you seem to favor would be really great for people working on Git at $BIG_CORP, as big corps usually plan things by the quarter. So maybe by having a timeline (known in advance of the quarter) can convince managers easier. > >> How did you get the impression that their opinion had no impact? We have >> been getting feedback about the choice of hash function both on and off >> list from a variety of people, some indisputably security experts. >> Sometimes the best one can do is to just listen. > > I did get the impression by talking at length to a cryptography expert who > successfully resisted any suggestions to get involved in the Git mailing > list. > > There were also accounts floating around on Twitter that a certain > cryptography expert who dared to mention already back in 2005 how > dangerous it would be to hardcode SHA-1 into Git was essentially shown the > finger, and I cannot fault him for essentially saying "I told you so" > publicly. Heh. The community between 2005 and now has changed. (I was not there for example. ;-) ) So let's hope the community changes for the better. > In my mind, it would have made sense to ask well-respected cryptographers > about their opinions and then try to figure out a consensus among them (as > opposed to what I saw so far, a lot of enthusastic talk by developers with > little standing in the cryptography community, mostly revolving around > hash size and speed as opposed to security). And then try to implement > that consensus in Git. Sounds good to me. That is why I personally think point (4) from Jonathans list above over-emphasizes performance/size over security. On the other hand if we find a smart way now, then this hash function transition will open the road to switching the hash function down the road once again with less or even no penalty if we make mistakes in choosing yet another bad hash function now. > Given my recent success rate with SHA-1 related > concerns, I am unfortunately not the person who can bring that about. > > But maybe you are. > > Ciao, > Dscho Thanks for bringing the discussion back to life, Stefan ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console output 2017-06-06 22:45 ` pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console output Stefan Beller @ 2017-06-06 22:52 ` Jonathan Nieder 2017-06-07 0:34 ` Samuel Lijin 2017-06-07 14:47 ` Johannes Schindelin 2 siblings, 0 replies; 59+ messages in thread From: Jonathan Nieder @ 2017-06-06 22:52 UTC (permalink / raw) To: Stefan Beller Cc: Johannes Schindelin, Junio C Hamano, Phillip Wood, git@vger.kernel.org, Ævar Arnfjörð Bjarmason Stefan Beller wrote: > On Tue, Jun 6, 2017 at 3:22 PM, Johannes Schindelin > <Johannes.Schindelin@gmx.de> wrote: >> In my mind, it would have made sense to ask well-respected cryptographers >> about their opinions and then try to figure out a consensus among them (as >> opposed to what I saw so far, a lot of enthusastic talk by developers with >> little standing in the cryptography community, mostly revolving around >> hash size and speed as opposed to security). And then try to implement >> that consensus in Git. > > Sounds good to me. That is why I personally think point (4) from > Jonathans list above over-emphasizes performance/size over security. The very least the only kind of replies my example task (4) led to were of this kind, so you can get a clear sense of whether the community values performance over security. :) I happen to think that performance and security both matter and are related (since if performance regresses enough, then people end up using the faster but insecure thing). This has shown up in the history of SSL, for example. But I am very happy to see people focusing more on the security properties than the performance properties --- that is a correct prioritization. Jonathan ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console output 2017-06-06 22:45 ` pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console output Stefan Beller 2017-06-06 22:52 ` Jonathan Nieder @ 2017-06-07 0:34 ` Samuel Lijin 2017-06-07 14:47 ` Johannes Schindelin 2 siblings, 0 replies; 59+ messages in thread From: Samuel Lijin @ 2017-06-07 0:34 UTC (permalink / raw) To: Stefan Beller Cc: Johannes Schindelin, Jonathan Nieder, Junio C Hamano, Phillip Wood, git@vger.kernel.org, Ævar Arnfjörð Bjarmason On Tue, Jun 6, 2017 at 6:45 PM, Stefan Beller <sbeller@google.com> wrote: > On Tue, Jun 6, 2017 at 3:22 PM, Johannes Schindelin > <Johannes.Schindelin@gmx.de> wrote: >> >> 4) we still have the problem that there is no cryptography expert among >> those who in the Git project are listened to > > I can assure you that Jonathan listened to crypto experts. It just did not > happen on the mailing list, which is sad regarding openness and transparency. In the interest of openness and transparency, perhaps a blue doc should be put together to outline and document the hash function that succeeds SHA1, and the rationales for doing so? It would, ideally, cite (preferably by including, and not just linking to) any discussions with crypto experts that have chimed in off-list (given said experts' consent for any such communication to be publicized, naturally). If I'm not mistaken, the only such doc behind the transition right now is the Git hash function transition document, which covers the technical barriers to replacing SHA1, but not why we might choose X to replace SHA1. ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console output 2017-06-06 22:45 ` pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console output Stefan Beller 2017-06-06 22:52 ` Jonathan Nieder 2017-06-07 0:34 ` Samuel Lijin @ 2017-06-07 14:47 ` Johannes Schindelin 2017-06-07 16:53 ` Stefan Beller 2 siblings, 1 reply; 59+ messages in thread From: Johannes Schindelin @ 2017-06-07 14:47 UTC (permalink / raw) To: Stefan Beller Cc: Jonathan Nieder, Junio C Hamano, Phillip Wood, git@vger.kernel.org, Ævar Arnfjörð Bjarmason Hi Stefan, On Tue, 6 Jun 2017, Stefan Beller wrote: > On Tue, Jun 6, 2017 at 3:22 PM, Johannes Schindelin > <Johannes.Schindelin@gmx.de> wrote: > > > 3) the only person who could make that call is Junio > > Occasionally I think the same, but in fact it is not true. Again my poor English skillz make sure I get misunderstood. So bear with me, please, and let me try again. The current What's cooking mails are full of stuff other than the transition from SHA-1 to a new function. In fact, every once in a while I see brian carlson's patch series with the remark "Needs review" while other patch series get reviewed even by Junio. In my mind, this sends a message. If, hypothetically, a couple of What's cooking mails would have in their header some language to the extent that we need to focus on transitioning away from SHA-1, and maybe even have the promise that Junio would not review other patch series as long as there are patches to review that prepare the tests for the transition, that convert more 20 and 40 constants, that convert more users to object_ids (and maybe strongly encourage to coordinate with brian so as not to trip over each others' toes), to implement a command to convert a SHA-1 based repository to a repository based on a different hash, to implement caching of legacy SHA-1 <=> new hash mapping, then that would send a wholly different message. And in my mind, if anybody else than Junio sent this message, it would sound ludicrous. For example, if I sent a mail to that extent, I would find it ridiculous myself, in particular since I am a very unprolific reviewer, and the promise to focus on favoring reviews of SHA-1 transition related patches would sound very unsincere from somebody like me. > As said above, Junio has strong veto power for things going off rails, > but in his role as a maintainer he does not coordinate people. (He > occasionally asks them to coordinate between themselves, though) I never had in mind that Junio would coordinate people or distribute tasks. Instead, I had in mind that a certain time period could be called out as focusing on that pretty important direction. That would be mostly symbolic, of course. And encouraging. In a positive way. With a direction. > > 4) we still have the problem that there is no cryptography expert among > > those who in the Git project are listened to > > I can assure you that Jonathan listened to crypto experts. It just did > not happen on the mailing list, which is sad regarding openness and > transparency. True. Same goes for me, of course. I just felt pretty uncomfortable sharing the contents of my private conversation publicly, when I tried very hard to convince my conversation partner to join the discussion on this mailing list, and they refused. The gist of it was: SHA-256 should be preferred to SHA3-256 because we will soon have good hardware support (and performance is really, really important when you need to work on the largest Git repository on this planet). And if there is no consensus about that, BLAKE should be considered over other algorithms because it has been studied pretty well. Ciao, Dscho > > > 5. The timeline you seem to favor would be really great for people working > on Git at $BIG_CORP, as big corps usually plan things by the quarter. So maybe > by having a timeline (known in advance of the quarter) can convince managers > easier. > > > > >> How did you get the impression that their opinion had no impact? We have > >> been getting feedback about the choice of hash function both on and off > >> list from a variety of people, some indisputably security experts. > >> Sometimes the best one can do is to just listen. > > > > I did get the impression by talking at length to a cryptography expert who > > successfully resisted any suggestions to get involved in the Git mailing > > list. > > > > There were also accounts floating around on Twitter that a certain > > cryptography expert who dared to mention already back in 2005 how > > dangerous it would be to hardcode SHA-1 into Git was essentially shown the > > finger, and I cannot fault him for essentially saying "I told you so" > > publicly. > > Heh. The community between 2005 and now has changed. (I was not there > for example. ;-) ) So let's hope the community changes for the better. > > > In my mind, it would have made sense to ask well-respected cryptographers > > about their opinions and then try to figure out a consensus among them (as > > opposed to what I saw so far, a lot of enthusastic talk by developers with > > little standing in the cryptography community, mostly revolving around > > hash size and speed as opposed to security). And then try to implement > > that consensus in Git. > > Sounds good to me. That is why I personally think point (4) from > Jonathans list above over-emphasizes performance/size over security. > > On the other hand if we find a smart way now, then this hash function > transition will open the road to switching the hash function down the road > once again with less or even no penalty if we make mistakes in choosing > yet another bad hash function now. > > > Given my recent success rate with SHA-1 related > > concerns, I am unfortunately not the person who can bring that about. > > > > But maybe you are. > > > > Ciao, > > Dscho > > Thanks for bringing the discussion back to life, > Stefan > ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console output 2017-06-07 14:47 ` Johannes Schindelin @ 2017-06-07 16:53 ` Stefan Beller 0 siblings, 0 replies; 59+ messages in thread From: Stefan Beller @ 2017-06-07 16:53 UTC (permalink / raw) To: Johannes Schindelin Cc: Jonathan Nieder, Junio C Hamano, Phillip Wood, git@vger.kernel.org, Ævar Arnfjörð Bjarmason On Wed, Jun 7, 2017 at 7:47 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote: > Hi Stefan, > > On Tue, 6 Jun 2017, Stefan Beller wrote: > >> On Tue, Jun 6, 2017 at 3:22 PM, Johannes Schindelin >> <Johannes.Schindelin@gmx.de> wrote: >> >> > 3) the only person who could make that call is Junio >> >> Occasionally I think the same, but in fact it is not true. > > Again my poor English skillz make sure I get misunderstood. So bear with > me, please, and let me try again. I don't think it is a language thing, but a matter of perspective. > > The current What's cooking mails are full of stuff other than the > transition from SHA-1 to a new function. True, but there is also * bw/object-id (2017-06-05) 33 commits ... Conversion from uchar[20] to struct object_id continues. Will merge to 'next'. > In fact, every once in a while I > see brian carlson's patch series with the remark "Needs review" while > other patch series get reviewed even by Junio. So are you trying to impose priorities on what Junio has to review? (It sounds like so, but maybe you are just stating an observation, and a conclusion with an actionable item comes next) Sometimes I disagree with what Junio does and in which order, too. But he has more experience in how to guide a successful community, so I respect what he does even if I would have done it differently (such as a different order). > > In my mind, this sends a message. In the simplest form the message could be understood as a call for help to review the patches Brian sent. And the fact that YOU are not reviewing the patches, tells me that you have more important things to do, such as running a fork of Git. In my perception the conversion is picking up speed. It used to be Brian who decided to start this years ago as a one-man show, but now we have multiple people working on it * Brian sending out more patches, as more review happens: https://public-inbox.org/git/5973919a-e282-a02e-9b04-d313c77e250d@google.com/ https://public-inbox.org/git/20170509221322.GA106700@google.com/ * Brandon picking up one part of the conversion series (mentioned before, see current cooking email) * A potential migration plan has been made "Git hash function transition" (https://goo.gl/gh2Mzc). See the note atop the document "Note: this draft is somewhat out of date and is being reworked (in particular to use a different hash function). See public-inbox.org/git for more current discussion." * There are not a lot of patches, which do not have written "SHA1 CONVERISON" all over their face. (This one has, I made it last night as a one off response to this thread: https://public-inbox.org/git/20170607021805.11849-1-sbeller@google.com/) > > If, hypothetically, a couple of What's cooking mails would have in their > header some language to the extent that we need to focus on transitioning > away from SHA-1, and maybe even have the promise that Junio would not > review other patch series as long as there are patches to review that > prepare the tests for the transition, that convert more 20 and 40 > constants, that convert more users to object_ids (and maybe strongly > encourage to coordinate with brian so as not to trip over each others' > toes), to implement a command to convert a SHA-1 based repository to a > repository based on a different hash, to implement caching of legacy SHA-1 > <=> new hash mapping, then that would send a wholly different message. That message sent there would be "Junio thinks the SHA1 conversion is the most important thing now, so he does the work (the work a maintainer does to guide the project)". You could send the same message "Johannes thinks the SHA1 conversion is the most important thing and do the work (Johannes being a well respected contributor would send patches, and that would attract a lot of reviews for sure. -- I don't mean this snarky, please don't read any snark in this.) > And in my mind, if anybody else than Junio sent this message, it would > sound ludicrous. Yes I have seen a couple of these messages (unrelated topics). "Git should do X. Git should not do Y. k, thx bye!" and I ignored them, because these one-offs do not convince me to invest my own time in it to produce a reasonable ROI. If there are patches attached, it is not ludicrous any more as the "proof of work done" shows that the voice raised is more than just hot talk, but actual a genuine interest in moving things towards the right direction. Another big one: "Move Git away from global state all over the place". If you think about all subsystems, it may even reach the order of magnitude to the sha1 conversion, but the way the message was sent it did not seem ludicrous to me: https://public-inbox.org/git/20170530171217.GB2798@google.com/ Or the other big project "Protocol v2, that scales and serves large repos well (number of refs, large binary files omitted, refs in wants and so on)" took a different approach, and mostly discussed design (I recall emails both from Microsoft as well as Google discussing the design, most of them having RFC patches attached, such that it very much looked like "proof of work done, so it is not ludicrous") > For example, if I sent a mail to that extent, I would > find it ridiculous myself, in particular since I am a very unprolific > reviewer, and the promise to focus on favoring reviews of SHA-1 transition > related patches would sound very unsincere from somebody like me. If you were to actually follow through after such an announcement, and in fact review the sha1 conversion patches thoroughly and in a timely manner, I would think such a call up front would be well received. I thought about doing that myself, but I dread my future commitment. Specifically as my $DAY_JOBs wants me to work on submodules instead of the sha1 conversion. ("Submodules are more important than the SHA1 conversion [for me] ", if I were to trust the infinite wisdom of our management process. ) > >> As said above, Junio has strong veto power for things going off rails, >> but in his role as a maintainer he does not coordinate people. (He >> occasionally asks them to coordinate between themselves, though) > > I never had in mind that Junio would coordinate people or distribute > tasks. Coordinate is a strong word here. Most recent observed examples: https://public-inbox.org/git/xmqq60gpfvqj.fsf@gitster.mtv.corp.google.com/ (My patch series conflicted with Ævars series, so we had to figure out how to fix it) https://public-inbox.org/git/20170602182215.GA57260@google.com/ (Aforementioned object id conversion series having merge conflicts) Note that this is only about coordination "You should talk to person Y so we can figure out how to make these 2 patches work well together, not about distributing tasks, as in "You must do X". > Instead, I had in mind that a certain time period could be called out as > focusing on that pretty important direction. As remarked in an earlier email, if such a thing is called out, I would very much prefer it in the next quarter, as then I can convince my manager to have more time following such a goal. Otherwise *I* would ignore it. The community at large may be different and jump on it like crazy. > That would be mostly symbolic, of course. And encouraging. In a positive > way. With a direction. So you want a project roadmap? As hinted at before the best would be to lead a good example here and show *your* roadmap such that others see how valuable of a tool it is to have a roadmap in the open. >> > 4) we still have the problem that there is no cryptography expert among >> > those who in the Git project are listened to >> >> I can assure you that Jonathan listened to crypto experts. It just did >> not happen on the mailing list, which is sad regarding openness and >> transparency. > > True. Same goes for me, of course. I just felt pretty uncomfortable > sharing the contents of my private conversation publicly, when I tried > very hard to convince my conversation partner to join the discussion on > this mailing list, and they refused. > > The gist of it was: SHA-256 should be preferred to SHA3-256 because we > will soon have good hardware support (and performance is really, really > important when you need to work on the largest Git repository on this > planet). And if there is no consensus about that, BLAKE should be > considered over other algorithms because it has been studied pretty well. BLAKE is what we're currently leaning on. (we=authors of "Git hash function transition"; Leaning in the sense: If nobody ever speaks up until all work is done, we'd just go with that. As soon as someone comes up with any reasonable argument either publicly or privately on why other hashes are better, we're easily persuaded to go with that) > Again my poor English skillz make sure I get misunderstood. So bear with > me, please, and let me try again. Same for me, if I misunderstood you please point out. tl;dr: Discussions are nice, but someone has to do the actual work, too. Thanks, Stefan ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH 2/3] rebase: Add tests for console output 2017-06-01 12:56 ` Johannes Schindelin 2017-06-01 23:40 ` Junio C Hamano @ 2017-06-07 10:47 ` Phillip Wood 2017-06-09 16:39 ` Junio C Hamano 2017-06-14 12:51 ` Johannes Schindelin 1 sibling, 2 replies; 59+ messages in thread From: Phillip Wood @ 2017-06-07 10:47 UTC (permalink / raw) To: Johannes Schindelin Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason, git Hi Johannes Thanks for your feedback On 01/06/17 13:56, Johannes Schindelin wrote: > Hi Phillip, > > On Wed, 31 May 2017, Phillip Wood wrote: > >> From: Phillip Wood <phillip.wood@dunelm.org.uk> >> >> Check the console output when using --autostash and the stash applies >> cleanly is what we expect. To avoid this test depending on commit and >> stash hashes it uses sed to replace them with XXX. The sed script also >> replaces carriage returns in the output with '\r' to avoid embedded >> '^M's in the expected output files. Unfortunately this means we still >> end up with an embedded '^M' in the sed script which may not be >> preserved when sending this. The last line of the sed script should be >> +s/^M/\\r/g > > Like Junio pointed out, this sed script would not be portable. To > elaborate: there are two major variants of sed out there, GNU sed and BSD > sed. Typically GNU sed allows a little more powerful instructions, e.g. \t > and \r. I'm confused by this as my script does not use the escape sequence "\r" out of portability concerns. It has a literal carriage return as you get from typing Ctrl-Q Ctrl-M in emacs or Ctrl-V Ctrl-M in bash which I think should be portable and replaces the carriage returns in git's output with the literal string '\r'. I did this so that the expected files don't have control characters in them that mess up the display when you cat them or read them in an email client > But we should simply take a step back and ask why test_cmp is not enough > to ignore the CRs in the output? > > Also, about the commit IDs. As long as the tests are consistent (i.e. they > use test_commit rather than plain `git commit`, or at least call > `test_tick` beforehand), the commit IDs should actually be identical > between runs and not depend on the time of day or the date. > > The only exception to that rule is when some previous test cases call > `test_commit` but are guarded behind some prereq and may not be executed > at all. In that case, the precise commit IDs depend on the particular set > of active prereqs. The other exceptions are when the commit hash algorithm changes or the contents of the commits changes because of some update to the test script. That's why I didn't want to rely on matching fixed SHA1s > > But as far as I can tell, t3420 does not have any test cases guarded by > prereqs. > > Taking an additional step back, I wonder whether we have to hard-code the > commit IDs (or XXX) to begin with. Why not generate the `expect` files > with the information at hand? We can simply ask `git rev-parse --short`. > > For the stash's commit ID, there is no record in the ref space, of course > (because it was created with `git stash create`). But I think in this > case, it is legitimate to simply grep the output. That's a good approach to handling the stash hash if we want to generate the expected files from the test script > That way, the test would be precise and resilient. > > So for example instead of adding the t/t3420/expected-success-am verbatim, > you could generate the output via > > cat >expect <<-EOF > $(grep "^Created autostash: [0-9a-f][0-9a-f]*\\$" output) > HEAD is now at $(git rev-parse --short HEAD~2) third commit > First, rewinding head to replay your work on top of it... > Applying: second commit > Applying: third commit > Applied autostash. > EOF This approach works well for the cases where there aren't control characters embedded in git's output, but for the interactive tests there are so we'd end up with control characters in the test script which I wanted to avoid or doing $(printf '\r'). I steered clear of generating the expected file in the test as i) it was more work (both for me (rebase --merge has a few commit hashes in it's output) and when the script is running) and ii) it's a bit messy to implement given the way the tests are structured in a helper function that's called with a parameter indicating the type of rebase to test. I can go ahead with generating the expected files from the script if you really want but I wonder if changing the test to generate the sed script with printf as below might be a simpler way to mitigate the carriage return problem, though it would be less strict than generating the real hashes with rev-parse. --- a/t/t3420-rebase-autostash.sh +++ b/t/t3420-rebase-autostash.sh @@ -189,6 +189,16 @@ test_expect_success "rebase: noop rebase" ' git checkout feature-branch ' +test_expect_sucess 'rebase: create sed script to sanitize output' ' + printf "\ +s/^\(Created autostash: \)[0-9a-f]\{6,\}$/\1XXX/ +s/^\(HEAD is now at \)[0-9a-f]\{6,\}\( .* commit\)$/\1XXX\2/ +s/^[0-9a-f]\{6,\}\( .* commit\)$/XXX\1/ +s/\(detached HEAD \)[0-9a-f]\{6,\}/\1XXX/ +s/\(could not apply \)[0-9a-f]\{6,\}/\1XXX/g +s/\\r/\\r/g" >$TEST_DIRECTORY/t4320/remove-ids.sed +' + Let me know what you think, Best Wishes Phillip ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH 2/3] rebase: Add tests for console output 2017-06-07 10:47 ` Phillip Wood @ 2017-06-09 16:39 ` Junio C Hamano 2017-06-14 10:18 ` Phillip Wood 2017-06-14 12:51 ` Johannes Schindelin 1 sibling, 1 reply; 59+ messages in thread From: Junio C Hamano @ 2017-06-09 16:39 UTC (permalink / raw) To: Phillip Wood Cc: Johannes Schindelin, Ævar Arnfjörð Bjarmason, git Phillip Wood <phillip.wood@talktalk.net> writes: > I'm confused by this as my script does not use the escape sequence "\r" > out of portability concerns. It has a literal carriage return as you get > from typing Ctrl-Q Ctrl-M in emacs or Ctrl-V Ctrl-M in bash ... I think the worry is that some implementations of sed may be unhappy to see these raw control characters in the sed script. ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH 2/3] rebase: Add tests for console output 2017-06-09 16:39 ` Junio C Hamano @ 2017-06-14 10:18 ` Phillip Wood 0 siblings, 0 replies; 59+ messages in thread From: Phillip Wood @ 2017-06-14 10:18 UTC (permalink / raw) To: Junio C Hamano Cc: Johannes Schindelin, Ævar Arnfjörð Bjarmason, git On 09/06/17 17:39, Junio C Hamano wrote: > Phillip Wood <phillip.wood@talktalk.net> writes: > >> I'm confused by this as my script does not use the escape sequence "\r" >> out of portability concerns. It has a literal carriage return as you get >> from typing Ctrl-Q Ctrl-M in emacs or Ctrl-V Ctrl-M in bash ... > > I think the worry is that some implementations of sed may be unhappy > to see these raw control characters in the sed script. > Ah, I'd misunderstood the problem, thanks for taking the time to point that out. I'll redo the tests as Johannes suggested Thanks Phillip ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH 2/3] rebase: Add tests for console output 2017-06-07 10:47 ` Phillip Wood 2017-06-09 16:39 ` Junio C Hamano @ 2017-06-14 12:51 ` Johannes Schindelin 1 sibling, 0 replies; 59+ messages in thread From: Johannes Schindelin @ 2017-06-14 12:51 UTC (permalink / raw) To: phillip.wood; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason, git Hi Phillip, On Wed, 7 Jun 2017, Phillip Wood wrote: > On 01/06/17 13:56, Johannes Schindelin wrote: > > > > On Wed, 31 May 2017, Phillip Wood wrote: > > > >> From: Phillip Wood <phillip.wood@dunelm.org.uk> > >> > >> Check the console output when using --autostash and the stash applies > >> cleanly is what we expect. To avoid this test depending on commit and > >> stash hashes it uses sed to replace them with XXX. The sed script also > >> replaces carriage returns in the output with '\r' to avoid embedded > >> '^M's in the expected output files. Unfortunately this means we still > >> end up with an embedded '^M' in the sed script which may not be > >> preserved when sending this. The last line of the sed script should be > >> +s/^M/\\r/g > > > > Like Junio pointed out, this sed script would not be portable. To > > elaborate: there are two major variants of sed out there, GNU sed and BSD > > sed. Typically GNU sed allows a little more powerful instructions, e.g. \t > > and \r. > > I'm confused by this as my script does not use the escape sequence "\r" > out of portability concerns. It has a literal carriage return as you get > from typing Ctrl-Q Ctrl-M in emacs or Ctrl-V Ctrl-M in bash which I > think should be portable and replaces the carriage returns in git's > output with the literal string '\r'. I did this so that the expected > files don't have control characters in them that mess up the display > when you cat them or read them in an email client Junio elaborated elsewhere what my main concern is: while a literal backslash followed by a lower-case r may be unportable, a plain CR byte is almost certainly unportable. > > Taking an additional step back, I wonder whether we have to hard-code > > the commit IDs (or XXX) to begin with. Why not generate the `expect` > > files with the information at hand? We can simply ask `git rev-parse > > --short`. > > > > For the stash's commit ID, there is no record in the ref space, of > > course (because it was created with `git stash create`). But I think > > in this case, it is legitimate to simply grep the output. > > That's a good approach to handling the stash hash if we want to generate > the expected files from the test script I would strongly favor that, especially since it would make the transition away from SHA-1 easier rather than more difficult. > > That way, the test would be precise and resilient. > > > > So for example instead of adding the t/t3420/expected-success-am verbatim, > > you could generate the output via > > > > cat >expect <<-EOF > > $(grep "^Created autostash: [0-9a-f][0-9a-f]*\\$" output) > > HEAD is now at $(git rev-parse --short HEAD~2) third commit > > First, rewinding head to replay your work on top of it... > > Applying: second commit > > Applying: third commit > > Applied autostash. > > EOF > > This approach works well for the cases where there aren't control > characters embedded in git's output, but for the interactive tests there > are so we'd end up with control characters in the test script which I > wanted to avoid or doing $(printf '\r'). I steered clear of generating > the expected file in the test as i) it was more work (both for me > (rebase --merge has a few commit hashes in it's output) and when the > script is running) and ii) it's a bit messy to implement given the way > the tests are structured in a helper function that's called with a > parameter indicating the type of rebase to test. > > I can go ahead with generating the expected files from the script if you > really want but I wonder if changing the test to generate the sed script > with printf as below might be a simpler way to mitigate the carriage > return problem, though it would be less strict than generating the real > hashes with rev-parse. > > --- a/t/t3420-rebase-autostash.sh > +++ b/t/t3420-rebase-autostash.sh > @@ -189,6 +189,16 @@ test_expect_success "rebase: noop rebase" ' > git checkout feature-branch > ' > > +test_expect_sucess 'rebase: create sed script to sanitize output' ' > + printf "\ > +s/^\(Created autostash: \)[0-9a-f]\{6,\}$/\1XXX/ > +s/^\(HEAD is now at \)[0-9a-f]\{6,\}\( .* commit\)$/\1XXX\2/ > +s/^[0-9a-f]\{6,\}\( .* commit\)$/XXX\1/ > +s/\(detached HEAD \)[0-9a-f]\{6,\}/\1XXX/ > +s/\(could not apply \)[0-9a-f]\{6,\}/\1XXX/g > +s/\\r/\\r/g" >$TEST_DIRECTORY/t4320/remove-ids.sed > +' > + > > Let me know what you think, I'd rather generate the expected output. If there are control characters to be embedded anywhere, we have good prior art to do that: see the q_to_nul(), q_to_cr() and q_to_tab() functions in test-lib-functions. If you still think that it is too daunting, please point me to a branch (you're on GitHub, right?) and I can try my hand at a PR. Ciao, Dscho ^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH 3/3] rebase: Add tests for console output with conflicting stash 2017-05-31 10:42 [PATCH 0/3] Add regression tests for recent rebase -i fixes Phillip Wood 2017-05-31 10:42 ` [PATCH 1/3] rebase -i: Add test for reflog message Phillip Wood 2017-05-31 10:42 ` [PATCH 2/3] rebase: Add tests for console output Phillip Wood @ 2017-05-31 10:42 ` Phillip Wood 2017-06-14 10:24 ` [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes Phillip Wood 2017-06-19 17:56 ` [PATCH v3 0/4] Add regression tests for recent " Phillip Wood 4 siblings, 0 replies; 59+ messages in thread From: Phillip Wood @ 2017-05-31 10:42 UTC (permalink / raw) To: git; +Cc: Johannes.Schindelin, avarab, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> Check that the console output is what we expect when restoring autostashed changes fails. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- t/t3420-rebase-autostash.sh | 10 +++++++++- t/t3420/expected-fail-am | 8 ++++++++ t/t3420/expected-fail-interactive | 6 ++++++ t/t3420/expected-fail-merge | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 1 deletion(-) diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh index 886be63c6d13e1ac4197a1b185659fb3d7d7eb26..4ff0d7aebd66b0ce091ceb11986b3928503c8c5f 100755 --- a/t/t3420-rebase-autostash.sh +++ b/t/t3420-rebase-autostash.sh @@ -148,7 +148,7 @@ testrebase() { test_when_finished git branch -D rebased-feature-branch && echo dirty >file4 && git add file4 && - git rebase$type unrelated-onto-branch && + git rebase$type unrelated-onto-branch >tmp 2>&1 && test_path_is_missing $dotest && git reset --hard && grep unrelated file4 && @@ -157,6 +157,14 @@ testrebase() { git stash pop && grep dirty file4 ' + + test_expect_success "rebase$type: check output with conflicting stash" ' + suffix=${type#\ -} && suffix=${suffix:--am} && + sed -f $TEST_DIRECTORY/t3420/remove-ids.sed tmp \ + >actual-fail$suffix && + test_cmp $TEST_DIRECTORY/t3420/expected-fail$suffix \ + actual-fail$suffix + ' } test_expect_success "rebase: fast-forward rebase" ' diff --git a/t/t3420/expected-fail-am b/t/t3420/expected-fail-am new file mode 100644 index 0000000000000000000000000000000000000000..1d643fa2ef7e67f1ad1a49a02e81d2d6c66e6920 --- /dev/null +++ b/t/t3420/expected-fail-am @@ -0,0 +1,8 @@ +Created autostash: XXX +HEAD is now at XXX third commit +First, rewinding head to replay your work on top of it... +Applying: second commit +Applying: third commit +Applying autostash resulted in conflicts. +Your changes are safe in the stash. +You can run "git stash pop" or "git stash drop" at any time. diff --git a/t/t3420/expected-fail-interactive b/t/t3420/expected-fail-interactive new file mode 100644 index 0000000000000000000000000000000000000000..9d1dd5264237e8f58e1960c6c0fb5b736df6f86d --- /dev/null +++ b/t/t3420/expected-fail-interactive @@ -0,0 +1,6 @@ +Created autostash: XXX +HEAD is now at XXX third commit +Rebasing (1/2)\rRebasing (2/2)\rSuccessfully rebased and updated refs/heads/rebased-feature-branch. +Applying autostash resulted in conflicts. +Your changes are safe in the stash. +You can run "git stash pop" or "git stash drop" at any time. diff --git a/t/t3420/expected-fail-merge b/t/t3420/expected-fail-merge new file mode 100644 index 0000000000000000000000000000000000000000..5dcb2ff3acdb93f55131d67ae227d231e097a2a2 --- /dev/null +++ b/t/t3420/expected-fail-merge @@ -0,0 +1,32 @@ +Created autostash: XXX +HEAD is now at XXX third commit +First, rewinding head to replay your work on top of it... +Merging unrelated-onto-branch with HEAD~1 +Merging: +XXX unrelated commit +XXX second commit +found 1 common ancestor: +XXX initial commit +[detached HEAD XXX] second commit + Author: A U Thor <author@example.com> + Date: Thu Apr 7 15:14:13 2005 -0700 + 2 files changed, 2 insertions(+) + create mode 100644 file1 + create mode 100644 file2 +Committed: 0001 second commit +Merging unrelated-onto-branch with HEAD~0 +Merging: +XXX second commit +XXX third commit +found 1 common ancestor: +XXX second commit +[detached HEAD XXX] third commit + Author: A U Thor <author@example.com> + Date: Thu Apr 7 15:15:13 2005 -0700 + 1 file changed, 1 insertion(+) + create mode 100644 file3 +Committed: 0002 third commit +All done. +Applying autostash resulted in conflicts. +Your changes are safe in the stash. +You can run "git stash pop" or "git stash drop" at any time. -- 2.13.0 ^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes 2017-05-31 10:42 [PATCH 0/3] Add regression tests for recent rebase -i fixes Phillip Wood ` (2 preceding siblings ...) 2017-05-31 10:42 ` [PATCH 3/3] rebase: Add tests for console output with conflicting stash Phillip Wood @ 2017-06-14 10:24 ` Phillip Wood 2017-06-14 10:24 ` [PATCH v2 1/3] rebase -i: Add test for reflog message Phillip Wood ` (4 more replies) 2017-06-19 17:56 ` [PATCH v3 0/4] Add regression tests for recent " Phillip Wood 4 siblings, 5 replies; 59+ messages in thread From: Phillip Wood @ 2017-06-14 10:24 UTC (permalink / raw) To: Git Mailing List Cc: Junio C Hamano, Johannes Schindelin, Ævar Arnfjörð Bjarmason, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> I've revised the second two tests as Johannes suggested to drop the sed script. The first one is unchanged. Phillip Wood (3): rebase -i: Add test for reflog message rebase: Add regression tests for console output rebase: Add more regression tests for console output t/t3404-rebase-interactive.sh | 7 +++ t/t3420-rebase-autostash.sh | 138 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 141 insertions(+), 4 deletions(-) -- 2.13.0 ^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v2 1/3] rebase -i: Add test for reflog message 2017-06-14 10:24 ` [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes Phillip Wood @ 2017-06-14 10:24 ` Phillip Wood 2017-06-14 10:24 ` [PATCH v2 2/3] rebase: Add regression tests for console output Phillip Wood ` (3 subsequent siblings) 4 siblings, 0 replies; 59+ messages in thread From: Phillip Wood @ 2017-06-14 10:24 UTC (permalink / raw) To: Git Mailing List Cc: Junio C Hamano, Johannes Schindelin, Ævar Arnfjörð Bjarmason, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> Check that the reflog message written to the branch reflog when the rebase is completed is correct Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- t/t3404-rebase-interactive.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 5bd0275930b715c25507fc9744c8946e7f73900b..37821d245433f757fa13f0a3e27da0312bebb7db 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -169,6 +169,13 @@ test_expect_success 'reflog for the branch shows state before rebase' ' test $(git rev-parse branch1@{1}) = $(git rev-parse original-branch1) ' +test_expect_success 'reflog for the branch shows correct finish message' ' + printf "rebase -i (finish): refs/heads/branch1 onto %s\n" \ + "$(git rev-parse branch2)" >expected && + git log -g --pretty=%gs -1 refs/heads/branch1 >actual && + test_cmp expected actual +' + test_expect_success 'exchange two commits' ' set_fake_editor && FAKE_LINES="2 1" git rebase -i HEAD~2 && -- 2.13.0 ^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v2 2/3] rebase: Add regression tests for console output 2017-06-14 10:24 ` [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes Phillip Wood 2017-06-14 10:24 ` [PATCH v2 1/3] rebase -i: Add test for reflog message Phillip Wood @ 2017-06-14 10:24 ` Phillip Wood 2017-06-14 10:24 ` [PATCH v2 3/3] rebase: Add more " Phillip Wood ` (2 subsequent siblings) 4 siblings, 0 replies; 59+ messages in thread From: Phillip Wood @ 2017-06-14 10:24 UTC (permalink / raw) To: Git Mailing List Cc: Junio C Hamano, Johannes Schindelin, Ævar Arnfjörð Bjarmason, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> Check the console output when using --autostash and the stash applies cleanly is what we expect. The test is quite strict but should catch any changes to the console output from the various rebase flavors. Thanks-to: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- t/t3420-rebase-autostash.sh | 66 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh index ab8a63e8d6dc643b28eb0c74ba3f032b7532226f..2cbc2e89cd026c370a86da35e181d77f27081c7a 100755 --- a/t/t3420-rebase-autostash.sh +++ b/t/t3420-rebase-autostash.sh @@ -33,6 +33,62 @@ test_expect_success setup ' git commit -m "related commit" ' +create_expected_success_am() { + cat >expected <<-EOF + $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) + HEAD is now at $(git rev-parse --short feature-branch) third commit + First, rewinding head to replay your work on top of it... + Applying: second commit + Applying: third commit + Applied autostash. + EOF +} + +create_expected_success_interactive() { + cr=$'\r' && + cat >expected <<-EOF + $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) + HEAD is now at $(git rev-parse --short feature-branch) third commit + Rebasing (1/2)${cr}Rebasing (2/2)${cr}Successfully rebased and updated refs/heads/rebased-feature-branch. + Applied autostash. + EOF +} + +create_expected_success_merge() { + cat >expected <<-EOF + $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) + HEAD is now at $(git rev-parse --short feature-branch) third commit + First, rewinding head to replay your work on top of it... + Merging unrelated-onto-branch with HEAD~1 + Merging: + $(git rev-parse --short unrelated-onto-branch) unrelated commit + $(git rev-parse --short feature-branch^) second commit + found 1 common ancestor: + $(git rev-parse --short feature-branch~2) initial commit + [detached HEAD $(git rev-parse --short rebased-feature-branch~1)] second commit + Author: A U Thor <author@example.com> + Date: Thu Apr 7 15:14:13 2005 -0700 + 2 files changed, 2 insertions(+) + create mode 100644 file1 + create mode 100644 file2 + Committed: 0001 second commit + Merging unrelated-onto-branch with HEAD~0 + Merging: + $(git rev-parse --short rebased-feature-branch~1) second commit + $(git rev-parse --short feature-branch) third commit + found 1 common ancestor: + $(git rev-parse --short feature-branch~1) second commit + [detached HEAD $(git rev-parse --short rebased-feature-branch)] third commit + Author: A U Thor <author@example.com> + Date: Thu Apr 7 15:15:13 2005 -0700 + 1 file changed, 1 insertion(+) + create mode 100644 file3 + Committed: 0002 third commit + All done. + Applied autostash. + EOF +} + testrebase() { type=$1 dotest=$2 @@ -51,14 +107,20 @@ testrebase() { test_config rebase.autostash true && git reset --hard && git checkout -b rebased-feature-branch feature-branch && - test_when_finished git branch -D rebased-feature-branch && echo dirty >>file3 && - git rebase$type unrelated-onto-branch && + git rebase$type unrelated-onto-branch >actual 2>&1 && grep unrelated file4 && grep dirty file3 && git checkout feature-branch ' + test_expect_success "rebase$type --autostash: check output" ' + test_when_finished git branch -D rebased-feature-branch && + suffix=${type#\ --} && suffix=${suffix:-am} && + create_expected_success_$suffix && + test_cmp expected actual + ' + test_expect_success "rebase$type: dirty index, non-conflicting rebase" ' test_config rebase.autostash true && git reset --hard && -- 2.13.0 ^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v2 3/3] rebase: Add more regression tests for console output 2017-06-14 10:24 ` [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes Phillip Wood 2017-06-14 10:24 ` [PATCH v2 1/3] rebase -i: Add test for reflog message Phillip Wood 2017-06-14 10:24 ` [PATCH v2 2/3] rebase: Add regression tests for console output Phillip Wood @ 2017-06-14 10:24 ` Phillip Wood 2017-06-14 20:35 ` [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes Johannes Schindelin 2017-06-15 23:05 ` Junio C Hamano 4 siblings, 0 replies; 59+ messages in thread From: Phillip Wood @ 2017-06-14 10:24 UTC (permalink / raw) To: Git Mailing List Cc: Junio C Hamano, Johannes Schindelin, Ævar Arnfjörð Bjarmason, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> Check the console output when using --autostash and the stash does not apply is what we expect. The test is quite strict but should catch any changes to the console output from the various rebase flavors. Thanks-to: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- t/t3420-rebase-autostash.sh | 72 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 70 insertions(+), 2 deletions(-) diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh index 2cbc2e89cd026c370a86da35e181d77f27081c7a..325ec75353b200bb88ab223e4a3b87d885cf2cf2 100755 --- a/t/t3420-rebase-autostash.sh +++ b/t/t3420-rebase-autostash.sh @@ -89,6 +89,68 @@ create_expected_success_merge() { EOF } +create_expected_failure_am() { + cat >expected <<-EOF + $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) + HEAD is now at $(git rev-parse --short feature-branch) third commit + First, rewinding head to replay your work on top of it... + Applying: second commit + Applying: third commit + Applying autostash resulted in conflicts. + Your changes are safe in the stash. + You can run "git stash pop" or "git stash drop" at any time. + EOF +} + +create_expected_failure_interactive() { + cr=$'\r' && + cat >expected <<-EOF + $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) + HEAD is now at $(git rev-parse --short feature-branch) third commit + Rebasing (1/2)${cr}Rebasing (2/2)${cr}Successfully rebased and updated refs/heads/rebased-feature-branch. + Applying autostash resulted in conflicts. + Your changes are safe in the stash. + You can run "git stash pop" or "git stash drop" at any time. + EOF +} + +create_expected_failure_merge() { + cat >expected <<-EOF + $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) + HEAD is now at $(git rev-parse --short feature-branch) third commit + First, rewinding head to replay your work on top of it... + Merging unrelated-onto-branch with HEAD~1 + Merging: + $(git rev-parse --short unrelated-onto-branch) unrelated commit + $(git rev-parse --short feature-branch^) second commit + found 1 common ancestor: + $(git rev-parse --short feature-branch~2) initial commit + [detached HEAD $(git rev-parse --short rebased-feature-branch~1)] second commit + Author: A U Thor <author@example.com> + Date: Thu Apr 7 15:14:13 2005 -0700 + 2 files changed, 2 insertions(+) + create mode 100644 file1 + create mode 100644 file2 + Committed: 0001 second commit + Merging unrelated-onto-branch with HEAD~0 + Merging: + $(git rev-parse --short rebased-feature-branch~1) second commit + $(git rev-parse --short feature-branch) third commit + found 1 common ancestor: + $(git rev-parse --short feature-branch~1) second commit + [detached HEAD $(git rev-parse --short rebased-feature-branch)] third commit + Author: A U Thor <author@example.com> + Date: Thu Apr 7 15:15:13 2005 -0700 + 1 file changed, 1 insertion(+) + create mode 100644 file3 + Committed: 0002 third commit + All done. + Applying autostash resulted in conflicts. + Your changes are safe in the stash. + You can run "git stash pop" or "git stash drop" at any time. + EOF +} + testrebase() { type=$1 dotest=$2 @@ -199,10 +261,9 @@ testrebase() { test_config rebase.autostash true && git reset --hard && git checkout -b rebased-feature-branch feature-branch && - test_when_finished git branch -D rebased-feature-branch && echo dirty >file4 && git add file4 && - git rebase$type unrelated-onto-branch && + git rebase$type unrelated-onto-branch >actual 2>&1 && test_path_is_missing $dotest && git reset --hard && grep unrelated file4 && @@ -211,6 +272,13 @@ testrebase() { git stash pop && grep dirty file4 ' + + test_expect_success "rebase$type: check output with conflicting stash" ' + test_when_finished git branch -D rebased-feature-branch && + suffix=${type#\ --} && suffix=${suffix:-am} && + create_expected_failure_$suffix && + test_cmp expected actual + ' } test_expect_success "rebase: fast-forward rebase" ' -- 2.13.0 ^ permalink raw reply related [flat|nested] 59+ messages in thread
* Re: [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes 2017-06-14 10:24 ` [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes Phillip Wood ` (2 preceding siblings ...) 2017-06-14 10:24 ` [PATCH v2 3/3] rebase: Add more " Phillip Wood @ 2017-06-14 20:35 ` Johannes Schindelin 2017-06-15 23:05 ` Junio C Hamano 4 siblings, 0 replies; 59+ messages in thread From: Johannes Schindelin @ 2017-06-14 20:35 UTC (permalink / raw) To: Phillip Wood Cc: Git Mailing List, Junio C Hamano, Ævar Arnfjörð Bjarmason Hi Phillip, On Wed, 14 Jun 2017, Phillip Wood wrote: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > I've revised the second two tests as Johannes suggested to drop the > sed script. The first one is unchanged. This iteration looks pretty good to me! Ciao, Johannes ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes 2017-06-14 10:24 ` [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes Phillip Wood ` (3 preceding siblings ...) 2017-06-14 20:35 ` [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes Johannes Schindelin @ 2017-06-15 23:05 ` Junio C Hamano 2017-06-15 23:23 ` Junio C Hamano 4 siblings, 1 reply; 59+ messages in thread From: Junio C Hamano @ 2017-06-15 23:05 UTC (permalink / raw) To: Phillip Wood Cc: Git Mailing List, Johannes Schindelin, Ævar Arnfjörð Bjarmason, Phillip Wood Phillip Wood <phillip.wood@talktalk.net> writes: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > I've revised the second two tests as Johannes suggested to drop the > sed script. The first one is unchanged. > > Phillip Wood (3): > rebase -i: Add test for reflog message > rebase: Add regression tests for console output > rebase: Add more regression tests for console output > > t/t3404-rebase-interactive.sh | 7 +++ > t/t3420-rebase-autostash.sh | 138 ++++++++++++++++++++++++++++++++++++++++-- > 2 files changed, 141 insertions(+), 4 deletions(-) Thanks (and thanks for Dscho for reading it over). Unfortunately this breaks unless your shell is bash (I didn't have time to look further into it), i.e. "make SHELL_PATH=/bin/dash test" ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes 2017-06-15 23:05 ` Junio C Hamano @ 2017-06-15 23:23 ` Junio C Hamano 2017-06-15 23:29 ` Junio C Hamano 0 siblings, 1 reply; 59+ messages in thread From: Junio C Hamano @ 2017-06-15 23:23 UTC (permalink / raw) To: Phillip Wood Cc: Git Mailing List, Johannes Schindelin, Ævar Arnfjörð Bjarmason, Phillip Wood Junio C Hamano <gitster@pobox.com> writes: > Phillip Wood <phillip.wood@talktalk.net> writes: > >> From: Phillip Wood <phillip.wood@dunelm.org.uk> >> >> I've revised the second two tests as Johannes suggested to drop the >> sed script. The first one is unchanged. >> >> Phillip Wood (3): >> rebase -i: Add test for reflog message >> rebase: Add regression tests for console output >> rebase: Add more regression tests for console output >> >> t/t3404-rebase-interactive.sh | 7 +++ >> t/t3420-rebase-autostash.sh | 138 ++++++++++++++++++++++++++++++++++++++++-- >> 2 files changed, 141 insertions(+), 4 deletions(-) > > Thanks (and thanks for Dscho for reading it over). > > Unfortunately this breaks unless your shell is bash (I didn't have > time to look further into it), i.e. "make SHELL_PATH=/bin/dash test" This is the bash-ism that broke it, I think. create_expected_success_interactive() { cr=$'\r' && cat >expected <<-EOF ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes 2017-06-15 23:23 ` Junio C Hamano @ 2017-06-15 23:29 ` Junio C Hamano 2017-06-16 13:49 ` Johannes Schindelin 2017-06-19 9:52 ` Phillip Wood 0 siblings, 2 replies; 59+ messages in thread From: Junio C Hamano @ 2017-06-15 23:29 UTC (permalink / raw) To: Phillip Wood Cc: Git Mailing List, Johannes Schindelin, Ævar Arnfjörð Bjarmason, Phillip Wood Junio C Hamano <gitster@pobox.com> writes: > Junio C Hamano <gitster@pobox.com> writes: > >> Phillip Wood <phillip.wood@talktalk.net> writes: >> >>> From: Phillip Wood <phillip.wood@dunelm.org.uk> >>> >>> I've revised the second two tests as Johannes suggested to drop the >>> sed script. The first one is unchanged. >>> >>> Phillip Wood (3): >>> rebase -i: Add test for reflog message >>> rebase: Add regression tests for console output >>> rebase: Add more regression tests for console output >>> >>> t/t3404-rebase-interactive.sh | 7 +++ >>> t/t3420-rebase-autostash.sh | 138 ++++++++++++++++++++++++++++++++++++++++-- >>> 2 files changed, 141 insertions(+), 4 deletions(-) >> >> Thanks (and thanks for Dscho for reading it over). >> >> Unfortunately this breaks unless your shell is bash (I didn't have >> time to look further into it), i.e. "make SHELL_PATH=/bin/dash test" > > This is the bash-ism that broke it, I think. > > create_expected_success_interactive() { > cr=$'\r' && > cat >expected <<-EOF FYI, I've queued the following as a fix-up and pushed the result out. t/t3420-rebase-autostash.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh index 325ec75353..801bce25da 100755 --- a/t/t3420-rebase-autostash.sh +++ b/t/t3420-rebase-autostash.sh @@ -45,7 +45,7 @@ create_expected_success_am() { } create_expected_success_interactive() { - cr=$'\r' && + cr=$(echo . | tr '.' '\015') && cat >expected <<-EOF $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) HEAD is now at $(git rev-parse --short feature-branch) third commit @@ -103,7 +103,7 @@ create_expected_failure_am() { } create_expected_failure_interactive() { - cr=$'\r' && + cr=$(echo . | tr '.' '\015') && cat >expected <<-EOF $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) HEAD is now at $(git rev-parse --short feature-branch) third commit -- 2.13.1-591-gf514f8f1c0 ^ permalink raw reply related [flat|nested] 59+ messages in thread
* Re: [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes 2017-06-15 23:29 ` Junio C Hamano @ 2017-06-16 13:49 ` Johannes Schindelin 2017-06-16 18:43 ` Johannes Sixt 2017-06-19 9:49 ` Phillip Wood 2017-06-19 9:52 ` Phillip Wood 1 sibling, 2 replies; 59+ messages in thread From: Johannes Schindelin @ 2017-06-16 13:49 UTC (permalink / raw) To: Junio C Hamano Cc: Phillip Wood, Git Mailing List, Ævar Arnfjörð Bjarmason, Phillip Wood [-- Attachment #1: Type: text/plain, Size: 1950 bytes --] Hi Junio, On Thu, 15 Jun 2017, Junio C Hamano wrote: > diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh > index 325ec75353..801bce25da 100755 > --- a/t/t3420-rebase-autostash.sh > +++ b/t/t3420-rebase-autostash.sh > @@ -45,7 +45,7 @@ create_expected_success_am() { > } > > create_expected_success_interactive() { > - cr=$'\r' && > + cr=$(echo . | tr '.' '\015') && > cat >expected <<-EOF > $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) > HEAD is now at $(git rev-parse --short feature-branch) third commit This is still incorrect, as the \r\n (which $(echo . | tr \.\ '\015') would emit) is interpreted correctly as a line break on Windows, meaning that cr is now *empty*. Not what we want. What I did is to replace the `cat` by `q_to_cr` (we have that lovely function, might just as well use it), replace `${cr}` by `Q` and skip the cr variable altogether. Additionally, there is another huge problem: the "Applied autostash." is printed to stdout, not stderr, in line with how things were done in the shell version of rebase -i. While this was just a minor bug previously, now we exercise that bug, by redirecting stderr to stdout and redirecting stdout to the file `actual`. Nothing says that stderr should be printed to that file before stdout, but that is exactly what the test case tries to verify. There is only one slight problem: in my Git for Windows SDK, stdout is printed first. The quick fix would be to redirect stderr and stdout independently. However, I think it is time for that bug to be fixed: autostash messages should really go to stderr, just like the rest of them rebase messages. I attached the patch, together with the two fixups to Phillip's patches, and a fixup for the autostash-messages-to-stderr patch that I think should be squashed in but I really ran out of time testing this. Phillip, would you mind picking those changes up as you deem appropriate? Ciao, Dscho [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: Type: text/x-diff; name=0001-sequencer-print-autostash-messages-to-stderr.patch, Size: 1874 bytes --] From c5a8319f0378d93be5aea05b833bb5e23c9f0b3d Mon Sep 17 00:00:00 2001 From: Johannes Schindelin <johannes.schindelin@gmx.de> Date: Fri, 16 Jun 2017 15:34:50 +0200 Subject: [PATCH 1/4] sequencer: print autostash messages to stderr The rebase messages are printed to stderr traditionally. It was a bug introduced in 587947750bd (rebase: implement --[no-]autostash and rebase.autostash, 2013-05-12) and that bug has been faithfully copied when reimplementing parts of the interactive rebase in the sequencer. It is time to fix that: let's print the autostash messages to stderr instead of stdout. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- sequencer.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/sequencer.c b/sequencer.c index a59408a23a2..8713cc8d1d5 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1923,7 +1923,7 @@ static int apply_autostash(struct replay_opts *opts) argv_array_push(&child.args, "apply"); argv_array_push(&child.args, stash_sha1.buf); if (!run_command(&child)) - printf(_("Applied autostash.\n")); + fprintf(stderr, _("Applied autostash.\n")); else { struct child_process store = CHILD_PROCESS_INIT; @@ -1937,10 +1937,11 @@ static int apply_autostash(struct replay_opts *opts) if (run_command(&store)) ret = error(_("cannot store %s"), stash_sha1.buf); else - printf(_("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")); + 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(&stash_sha1); -- 2.12.2.windows.1 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #3: Type: text/x-diff; name=0002-fixup-rebase-add-regression-tests-for-console-output.patch, Size: 1203 bytes --] From 3425f7c9bfb62c3d2d4adaff41a664dd4ae2efa9 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin <johannes.schindelin@gmx.de> Date: Fri, 16 Jun 2017 15:39:20 +0200 Subject: [PATCH 2/4] fixup! rebase: add regression tests for console output Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t3420-rebase-autostash.sh | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh index 801bce25da4..64904bef069 100755 --- a/t/t3420-rebase-autostash.sh +++ b/t/t3420-rebase-autostash.sh @@ -45,12 +45,11 @@ create_expected_success_am() { } create_expected_success_interactive() { - cr=$(echo . | tr '.' '\015') && - cat >expected <<-EOF + q_to_cr >expected <<-EOF $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) HEAD is now at $(git rev-parse --short feature-branch) third commit - Rebasing (1/2)${cr}Rebasing (2/2)${cr}Successfully rebased and updated refs/heads/rebased-feature-branch. - Applied autostash. + Rebasing (1/2)QRebasing (2/2)QApplied autostash. + Successfully rebased and updated refs/heads/rebased-feature-branch. EOF } -- 2.12.2.windows.1 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #4: Type: text/x-diff; name=0003-fixup-rebase-add-more-regression-tests-for-console-o.patch, Size: 1361 bytes --] From e16ad989c85c55bdfcf45fe561911a699e962b44 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin <johannes.schindelin@gmx.de> Date: Fri, 16 Jun 2017 15:39:35 +0200 Subject: [PATCH 3/4] fixup! rebase: add more regression tests for console output Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t3420-rebase-autostash.sh | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh index 64904bef069..2c01ae6ad2a 100755 --- a/t/t3420-rebase-autostash.sh +++ b/t/t3420-rebase-autostash.sh @@ -102,14 +102,13 @@ create_expected_failure_am() { } create_expected_failure_interactive() { - cr=$(echo . | tr '.' '\015') && - cat >expected <<-EOF + q_to_cr >expected <<-EOF $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) HEAD is now at $(git rev-parse --short feature-branch) third commit - Rebasing (1/2)${cr}Rebasing (2/2)${cr}Successfully rebased and updated refs/heads/rebased-feature-branch. - Applying autostash resulted in conflicts. + Rebasing (1/2)QRebasing (2/2)QApplying autostash resulted in conflicts. Your changes are safe in the stash. You can run "git stash pop" or "git stash drop" at any time. + Successfully rebased and updated refs/heads/rebased-feature-branch. EOF } -- 2.12.2.windows.1 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #5: Type: text/x-diff; name=0004-fixup-sequencer-print-autostash-messages-to-stderr.patch, Size: 1246 bytes --] From 128c4a66ab3413b2135264f17024e1f5e9250f03 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin <johannes.schindelin@gmx.de> Date: Fri, 16 Jun 2017 15:40:46 +0200 Subject: [PATCH 4/4] fixup! sequencer: print autostash messages to stderr This is the companion to the proposed patch, but I had no time to verify that the test suite still passes (it may actually test for the buggy behavior...) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- git-rebase.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git-rebase.sh b/git-rebase.sh index db1deed8464..2cf73b88e8e 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -166,14 +166,14 @@ apply_autostash () { stash_sha1=$(cat "$state_dir/autostash") if git stash apply $stash_sha1 2>&1 >/dev/null then - echo "$(gettext 'Applied autostash.')" + echo "$(gettext 'Applied autostash.')" >&2 else git stash store -m "autostash" -q $stash_sha1 || die "$(eval_gettext "Cannot store \$stash_sha1")" gettext 'Applying autostash resulted in conflicts. Your changes are safe in the stash. You can run "git stash pop" or "git stash drop" at any time. -' +' >&2 fi fi } -- 2.12.2.windows.1 ^ permalink raw reply related [flat|nested] 59+ messages in thread
* Re: [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes 2017-06-16 13:49 ` Johannes Schindelin @ 2017-06-16 18:43 ` Johannes Sixt 2017-06-16 21:05 ` Junio C Hamano 2017-06-19 19:45 ` Johannes Sixt 2017-06-19 9:49 ` Phillip Wood 1 sibling, 2 replies; 59+ messages in thread From: Johannes Sixt @ 2017-06-16 18:43 UTC (permalink / raw) To: Johannes Schindelin Cc: Junio C Hamano, Phillip Wood, Git Mailing List, Ævar Arnfjörð Bjarmason, Phillip Wood Am 16.06.2017 um 15:49 schrieb Johannes Schindelin: > On Thu, 15 Jun 2017, Junio C Hamano wrote: >> diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh >> index 325ec75353..801bce25da 100755 >> --- a/t/t3420-rebase-autostash.sh >> +++ b/t/t3420-rebase-autostash.sh >> @@ -45,7 +45,7 @@ create_expected_success_am() { >> } >> >> create_expected_success_interactive() { >> - cr=$'\r' && >> + cr=$(echo . | tr '.' '\015') && >> cat >expected <<-EOF >> $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) >> HEAD is now at $(git rev-parse --short feature-branch) third commit > > This is still incorrect, as the \r\n (which $(echo . | tr \.\ '\015') > would emit) is interpreted correctly as a line break on Windows, meaning > that cr is now *empty*. Not what we want. > > What I did is to replace the `cat` by `q_to_cr` (we have that lovely > function, might just as well use it), replace `${cr}` by `Q` and skip the > cr variable altogether. You beat me to it. I came up with the identical q_to_cr changes, but haven't dug the remaining failure regarding the swapped output lines. You seem to have nailed it. Will test your proposed changes tomorrow. -- Hannes ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes 2017-06-16 18:43 ` Johannes Sixt @ 2017-06-16 21:05 ` Junio C Hamano 2017-06-19 19:45 ` Johannes Sixt 1 sibling, 0 replies; 59+ messages in thread From: Junio C Hamano @ 2017-06-16 21:05 UTC (permalink / raw) To: Johannes Sixt Cc: Johannes Schindelin, Phillip Wood, Git Mailing List, Ævar Arnfjörð Bjarmason, Phillip Wood Johannes Sixt <j6t@kdbg.org> writes: > Am 16.06.2017 um 15:49 schrieb Johannes Schindelin: >> On Thu, 15 Jun 2017, Junio C Hamano wrote: >>> diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh >>> index 325ec75353..801bce25da 100755 >>> --- a/t/t3420-rebase-autostash.sh >>> +++ b/t/t3420-rebase-autostash.sh >>> @@ -45,7 +45,7 @@ create_expected_success_am() { >>> } >>> create_expected_success_interactive() { >>> - cr=$'\r' && >>> + cr=$(echo . | tr '.' '\015') && >>> cat >expected <<-EOF >>> $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) >>> HEAD is now at $(git rev-parse --short feature-branch) third commit >> >> This is still incorrect, as the \r\n (which $(echo . | tr \.\ '\015') >> would emit) is interpreted correctly as a line break on Windows, meaning >> that cr is now *empty*. Not what we want. >> >> What I did is to replace the `cat` by `q_to_cr` (we have that lovely >> function, might just as well use it), replace `${cr}` by `Q` and skip the >> cr variable altogether. > > You beat me to it. I came up with the identical q_to_cr changes, but > haven't dug the remaining failure regarding the swapped output > lines. You seem to have nailed it. Will test your proposed changes > tomorrow. Ouch. Thanks, both of you. ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes 2017-06-16 18:43 ` Johannes Sixt 2017-06-16 21:05 ` Junio C Hamano @ 2017-06-19 19:45 ` Johannes Sixt 2017-06-19 20:02 ` Junio C Hamano 1 sibling, 1 reply; 59+ messages in thread From: Johannes Sixt @ 2017-06-19 19:45 UTC (permalink / raw) To: Johannes Schindelin Cc: Junio C Hamano, Phillip Wood, Git Mailing List, Ævar Arnfjörð Bjarmason, Phillip Wood Am 16.06.2017 um 20:43 schrieb Johannes Sixt: > Am 16.06.2017 um 15:49 schrieb Johannes Schindelin: >> On Thu, 15 Jun 2017, Junio C Hamano wrote: >>> diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh >>> index 325ec75353..801bce25da 100755 >>> --- a/t/t3420-rebase-autostash.sh >>> +++ b/t/t3420-rebase-autostash.sh >>> @@ -45,7 +45,7 @@ create_expected_success_am() { >>> } >>> create_expected_success_interactive() { >>> - cr=$'\r' && >>> + cr=$(echo . | tr '.' '\015') && >>> cat >expected <<-EOF >>> $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) >>> HEAD is now at $(git rev-parse --short feature-branch) third >>> commit >> >> This is still incorrect, as the \r\n (which $(echo . | tr \.\ '\015') >> would emit) is interpreted correctly as a line break on Windows, meaning >> that cr is now *empty*. Not what we want. >> >> What I did is to replace the `cat` by `q_to_cr` (we have that lovely >> function, might just as well use it), replace `${cr}` by `Q` and skip the >> cr variable altogether. > > You beat me to it. I came up with the identical q_to_cr changes, but > haven't dug the remaining failure regarding the swapped output lines. > You seem to have nailed it. Will test your proposed changes tomorrow. As expected, the patches fix the observed test failures for me, too, if that's still relevant. -- Hannes ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes 2017-06-19 19:45 ` Johannes Sixt @ 2017-06-19 20:02 ` Junio C Hamano 0 siblings, 0 replies; 59+ messages in thread From: Junio C Hamano @ 2017-06-19 20:02 UTC (permalink / raw) To: Johannes Sixt Cc: Johannes Schindelin, Phillip Wood, Git Mailing List, Ævar Arnfjörð Bjarmason, Phillip Wood Johannes Sixt <j6t@kdbg.org> writes: > Am 16.06.2017 um 20:43 schrieb Johannes Sixt: >> Am 16.06.2017 um 15:49 schrieb Johannes Schindelin: >>> On Thu, 15 Jun 2017, Junio C Hamano wrote: >>>> diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh >>>> index 325ec75353..801bce25da 100755 >>>> --- a/t/t3420-rebase-autostash.sh >>>> +++ b/t/t3420-rebase-autostash.sh >>>> @@ -45,7 +45,7 @@ create_expected_success_am() { >>>> } >>>> create_expected_success_interactive() { >>>> - cr=$'\r' && >>>> + cr=$(echo . | tr '.' '\015') && >>>> cat >expected <<-EOF >>>> $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) >>>> HEAD is now at $(git rev-parse --short feature-branch) third >>>> commit >>> >>> This is still incorrect, as the \r\n (which $(echo . | tr \.\ '\015') >>> would emit) is interpreted correctly as a line break on Windows, meaning >>> that cr is now *empty*. Not what we want. >>> >>> What I did is to replace the `cat` by `q_to_cr` (we have that lovely >>> function, might just as well use it), replace `${cr}` by `Q` and skip the >>> cr variable altogether. >> >> You beat me to it. I came up with the identical q_to_cr changes, but >> haven't dug the remaining failure regarding the swapped output >> lines. You seem to have nailed it. Will test your proposed changes >> tomorrow. > > As expected, the patches fix the observed test failures for me, too, > if that's still relevant. Thanks for double-checking. ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes 2017-06-16 13:49 ` Johannes Schindelin 2017-06-16 18:43 ` Johannes Sixt @ 2017-06-19 9:49 ` Phillip Wood 2017-06-19 15:45 ` Junio C Hamano 1 sibling, 1 reply; 59+ messages in thread From: Phillip Wood @ 2017-06-19 9:49 UTC (permalink / raw) To: Johannes Schindelin, Junio C Hamano Cc: Git Mailing List, Ævar Arnfjörð Bjarmason, Phillip Wood On 16/06/17 14:49, Johannes Schindelin wrote: > Hi Junio, > > On Thu, 15 Jun 2017, Junio C Hamano wrote: > >> diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh >> index 325ec75353..801bce25da 100755 >> --- a/t/t3420-rebase-autostash.sh >> +++ b/t/t3420-rebase-autostash.sh >> @@ -45,7 +45,7 @@ create_expected_success_am() { >> } >> >> create_expected_success_interactive() { >> - cr=$'\r' && >> + cr=$(echo . | tr '.' '\015') && >> cat >expected <<-EOF >> $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) >> HEAD is now at $(git rev-parse --short feature-branch) third commit > > This is still incorrect, as the \r\n (which $(echo . | tr \.\ '\015') > would emit) is interpreted correctly as a line break on Windows, meaning > that cr is now *empty*. Not what we want. > > What I did is to replace the `cat` by `q_to_cr` (we have that lovely > function, might just as well use it), replace `${cr}` by `Q` and skip the > cr variable altogether. > > Additionally, there is another huge problem: the "Applied autostash." is > printed to stdout, not stderr, in line with how things were done in the > shell version of rebase -i. > > While this was just a minor bug previously, now we exercise that bug, by > redirecting stderr to stdout and redirecting stdout to the file `actual`. > Nothing says that stderr should be printed to that file before stdout, but > that is exactly what the test case tries to verify. > > There is only one slight problem: in my Git for Windows SDK, stdout is > printed first. > > The quick fix would be to redirect stderr and stdout independently. > > However, I think it is time for that bug to be fixed: autostash messages > should really go to stderr, just like the rest of them rebase messages. > > I attached the patch, together with the two fixups to Phillip's patches, > and a fixup for the autostash-messages-to-stderr patch that I think should > be squashed in but I really ran out of time testing this. > > Phillip, would you mind picking those changes up as you deem appropriate? Will do, thanks for the patches ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes 2017-06-19 9:49 ` Phillip Wood @ 2017-06-19 15:45 ` Junio C Hamano 0 siblings, 0 replies; 59+ messages in thread From: Junio C Hamano @ 2017-06-19 15:45 UTC (permalink / raw) To: Phillip Wood Cc: Johannes Schindelin, Git Mailing List, Ævar Arnfjörð Bjarmason, Phillip Wood Phillip Wood <phillip.wood@talktalk.net> writes: >> Phillip, would you mind picking those changes up as you deem appropriate? > > Will do, thanks for the patches Thanks, both. ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes 2017-06-15 23:29 ` Junio C Hamano 2017-06-16 13:49 ` Johannes Schindelin @ 2017-06-19 9:52 ` Phillip Wood 1 sibling, 0 replies; 59+ messages in thread From: Phillip Wood @ 2017-06-19 9:52 UTC (permalink / raw) To: Junio C Hamano Cc: Git Mailing List, Johannes Schindelin, Ævar Arnfjörð Bjarmason, Phillip Wood On 16/06/17 00:29, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > >> Junio C Hamano <gitster@pobox.com> writes: >> >>> Phillip Wood <phillip.wood@talktalk.net> writes: >>> >>>> From: Phillip Wood <phillip.wood@dunelm.org.uk> >>>> >>>> I've revised the second two tests as Johannes suggested to drop the >>>> sed script. The first one is unchanged. >>>> >>>> Phillip Wood (3): >>>> rebase -i: Add test for reflog message >>>> rebase: Add regression tests for console output >>>> rebase: Add more regression tests for console output >>>> >>>> t/t3404-rebase-interactive.sh | 7 +++ >>>> t/t3420-rebase-autostash.sh | 138 ++++++++++++++++++++++++++++++++++++++++-- >>>> 2 files changed, 141 insertions(+), 4 deletions(-) >>> >>> Thanks (and thanks for Dscho for reading it over). >>> >>> Unfortunately this breaks unless your shell is bash (I didn't have >>> time to look further into it), i.e. "make SHELL_PATH=/bin/dash test" >> >> This is the bash-ism that broke it, I think. >> >> create_expected_success_interactive() { >> cr=$'\r' && >> cat >expected <<-EOF Sorry about that, for some reason I thought $' was in the posix shell standard but it's not. I'll redo the patches with the changes Johannes suggested. ^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v3 0/4] Add regression tests for recent rebase -i fixes 2017-05-31 10:42 [PATCH 0/3] Add regression tests for recent rebase -i fixes Phillip Wood ` (3 preceding siblings ...) 2017-06-14 10:24 ` [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes Phillip Wood @ 2017-06-19 17:56 ` Phillip Wood 2017-06-19 17:56 ` [PATCH v3 1/4] sequencer: print autostash messages to stderr Phillip Wood ` (4 more replies) 4 siblings, 5 replies; 59+ messages in thread From: Phillip Wood @ 2017-06-19 17:56 UTC (permalink / raw) To: Git Mailing List Cc: Junio C Hamano, Johannes Schindelin, Ævar Arnfjörð Bjarmason, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> I've updated the second two tests to be portable using q_to_cr() as Johannes suggested and added his patch to fix the autostash messages going to stdout rather than stderr. The reflog message test is unchanged. Thanks to Johannes for his help and to Junio for picking up the bashism in the last iteration. Johannes Schindelin (1): sequencer: print autostash messages to stderr Phillip Wood (3): rebase -i: Add test for reflog message rebase: Add regression tests for console output rebase: Add more regression tests for console output git-rebase.sh | 4 +- sequencer.c | 11 ++-- t/t3404-rebase-interactive.sh | 7 +++ t/t3420-rebase-autostash.sh | 136 ++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 147 insertions(+), 11 deletions(-) -- 2.13.0 ^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v3 1/4] sequencer: print autostash messages to stderr 2017-06-19 17:56 ` [PATCH v3 0/4] Add regression tests for recent " Phillip Wood @ 2017-06-19 17:56 ` Phillip Wood 2017-06-19 17:56 ` [PATCH v3 2/4] rebase -i: Add test for reflog message Phillip Wood ` (3 subsequent siblings) 4 siblings, 0 replies; 59+ messages in thread From: Phillip Wood @ 2017-06-19 17:56 UTC (permalink / raw) To: Git Mailing List Cc: Junio C Hamano, Johannes Schindelin, Ævar Arnfjörð Bjarmason, Johannes Schindelin, Phillip Wood From: Johannes Schindelin <johannes.schindelin@gmx.de> The rebase messages are printed to stderr traditionally. However due to a bug introduced in 587947750bd (rebase: implement --[no-]autostash and rebase.autostash, 2013-05-12) which was faithfully copied when reimplementing parts of the interactive rebase in the sequencer the autostash messages are printed to stdout instead. It is time to fix that: let's print the autostash messages to stderr instead of stdout. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- git-rebase.sh | 4 ++-- sequencer.c | 11 ++++++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/git-rebase.sh b/git-rebase.sh index db1deed8464f0643763ed6e3c5e54221cad8c985..2cf73b88e8e83ca34b9eb319dbc2b0a220139b0f 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -166,14 +166,14 @@ apply_autostash () { stash_sha1=$(cat "$state_dir/autostash") if git stash apply $stash_sha1 2>&1 >/dev/null then - echo "$(gettext 'Applied autostash.')" + echo "$(gettext 'Applied autostash.')" >&2 else git stash store -m "autostash" -q $stash_sha1 || die "$(eval_gettext "Cannot store \$stash_sha1")" gettext 'Applying autostash resulted in conflicts. Your changes are safe in the stash. You can run "git stash pop" or "git stash drop" at any time. -' +' >&2 fi fi } diff --git a/sequencer.c b/sequencer.c index a23b948ac148304dbebfe38955ec8b40cab3e1e5..606750b1e0c900a9fb43cea224d27ab36ca29ab9 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1921,7 +1921,7 @@ static int apply_autostash(struct replay_opts *opts) argv_array_push(&child.args, "apply"); argv_array_push(&child.args, stash_sha1.buf); if (!run_command(&child)) - printf(_("Applied autostash.\n")); + fprintf(stderr, _("Applied autostash.\n")); else { struct child_process store = CHILD_PROCESS_INIT; @@ -1935,10 +1935,11 @@ static int apply_autostash(struct replay_opts *opts) if (run_command(&store)) ret = error(_("cannot store %s"), stash_sha1.buf); else - printf(_("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")); + 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(&stash_sha1); -- 2.13.0 ^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v3 2/4] rebase -i: Add test for reflog message 2017-06-19 17:56 ` [PATCH v3 0/4] Add regression tests for recent " Phillip Wood 2017-06-19 17:56 ` [PATCH v3 1/4] sequencer: print autostash messages to stderr Phillip Wood @ 2017-06-19 17:56 ` Phillip Wood 2017-06-19 17:56 ` [PATCH v3 3/4] rebase: Add regression tests for console output Phillip Wood ` (2 subsequent siblings) 4 siblings, 0 replies; 59+ messages in thread From: Phillip Wood @ 2017-06-19 17:56 UTC (permalink / raw) To: Git Mailing List Cc: Junio C Hamano, Johannes Schindelin, Ævar Arnfjörð Bjarmason, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> Check that the reflog message written to the branch reflog when the rebase is completed is correct Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- t/t3404-rebase-interactive.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 5bd0275930b715c25507fc9744c8946e7f73900b..37821d245433f757fa13f0a3e27da0312bebb7db 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -169,6 +169,13 @@ test_expect_success 'reflog for the branch shows state before rebase' ' test $(git rev-parse branch1@{1}) = $(git rev-parse original-branch1) ' +test_expect_success 'reflog for the branch shows correct finish message' ' + printf "rebase -i (finish): refs/heads/branch1 onto %s\n" \ + "$(git rev-parse branch2)" >expected && + git log -g --pretty=%gs -1 refs/heads/branch1 >actual && + test_cmp expected actual +' + test_expect_success 'exchange two commits' ' set_fake_editor && FAKE_LINES="2 1" git rebase -i HEAD~2 && -- 2.13.0 ^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v3 3/4] rebase: Add regression tests for console output 2017-06-19 17:56 ` [PATCH v3 0/4] Add regression tests for recent " Phillip Wood 2017-06-19 17:56 ` [PATCH v3 1/4] sequencer: print autostash messages to stderr Phillip Wood 2017-06-19 17:56 ` [PATCH v3 2/4] rebase -i: Add test for reflog message Phillip Wood @ 2017-06-19 17:56 ` Phillip Wood 2017-06-19 17:56 ` [PATCH v3 4/4] rebase: Add more " Phillip Wood 2017-06-23 4:17 ` [PATCH v3 0/4] Add regression tests for recent rebase -i fixes Junio C Hamano 4 siblings, 0 replies; 59+ messages in thread From: Phillip Wood @ 2017-06-19 17:56 UTC (permalink / raw) To: Git Mailing List Cc: Junio C Hamano, Johannes Schindelin, Ævar Arnfjörð Bjarmason, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> Check the console output when using --autostash and the stash applies cleanly is what we expect. The test is quite strict but should catch any changes to the console output from the various rebase flavors. Thanks-to: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- t/t3420-rebase-autostash.sh | 65 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh index ab8a63e8d6dc643b28eb0c74ba3f032b7532226f..cd1012798cb300f4f1ddeba6fdcad544ca9ea1d9 100755 --- a/t/t3420-rebase-autostash.sh +++ b/t/t3420-rebase-autostash.sh @@ -33,6 +33,61 @@ test_expect_success setup ' git commit -m "related commit" ' +create_expected_success_am() { + cat >expected <<-EOF + $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) + HEAD is now at $(git rev-parse --short feature-branch) third commit + First, rewinding head to replay your work on top of it... + Applying: second commit + Applying: third commit + Applied autostash. + EOF +} + +create_expected_success_interactive() { + q_to_cr >expected <<-EOF + $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) + HEAD is now at $(git rev-parse --short feature-branch) third commit + Rebasing (1/2)QRebasing (2/2)QApplied autostash. + Successfully rebased and updated refs/heads/rebased-feature-branch. + EOF +} + +create_expected_success_merge() { + cat >expected <<-EOF + $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) + HEAD is now at $(git rev-parse --short feature-branch) third commit + First, rewinding head to replay your work on top of it... + Merging unrelated-onto-branch with HEAD~1 + Merging: + $(git rev-parse --short unrelated-onto-branch) unrelated commit + $(git rev-parse --short feature-branch^) second commit + found 1 common ancestor: + $(git rev-parse --short feature-branch~2) initial commit + [detached HEAD $(git rev-parse --short rebased-feature-branch~1)] second commit + Author: A U Thor <author@example.com> + Date: Thu Apr 7 15:14:13 2005 -0700 + 2 files changed, 2 insertions(+) + create mode 100644 file1 + create mode 100644 file2 + Committed: 0001 second commit + Merging unrelated-onto-branch with HEAD~0 + Merging: + $(git rev-parse --short rebased-feature-branch~1) second commit + $(git rev-parse --short feature-branch) third commit + found 1 common ancestor: + $(git rev-parse --short feature-branch~1) second commit + [detached HEAD $(git rev-parse --short rebased-feature-branch)] third commit + Author: A U Thor <author@example.com> + Date: Thu Apr 7 15:15:13 2005 -0700 + 1 file changed, 1 insertion(+) + create mode 100644 file3 + Committed: 0002 third commit + All done. + Applied autostash. + EOF +} + testrebase() { type=$1 dotest=$2 @@ -51,14 +106,20 @@ testrebase() { test_config rebase.autostash true && git reset --hard && git checkout -b rebased-feature-branch feature-branch && - test_when_finished git branch -D rebased-feature-branch && echo dirty >>file3 && - git rebase$type unrelated-onto-branch && + git rebase$type unrelated-onto-branch >actual 2>&1 && grep unrelated file4 && grep dirty file3 && git checkout feature-branch ' + test_expect_success "rebase$type --autostash: check output" ' + test_when_finished git branch -D rebased-feature-branch && + suffix=${type#\ --} && suffix=${suffix:-am} && + create_expected_success_$suffix && + test_cmp expected actual + ' + test_expect_success "rebase$type: dirty index, non-conflicting rebase" ' test_config rebase.autostash true && git reset --hard && -- 2.13.0 ^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v3 4/4] rebase: Add more regression tests for console output 2017-06-19 17:56 ` [PATCH v3 0/4] Add regression tests for recent " Phillip Wood ` (2 preceding siblings ...) 2017-06-19 17:56 ` [PATCH v3 3/4] rebase: Add regression tests for console output Phillip Wood @ 2017-06-19 17:56 ` Phillip Wood 2017-06-23 4:17 ` [PATCH v3 0/4] Add regression tests for recent rebase -i fixes Junio C Hamano 4 siblings, 0 replies; 59+ messages in thread From: Phillip Wood @ 2017-06-19 17:56 UTC (permalink / raw) To: Git Mailing List Cc: Junio C Hamano, Johannes Schindelin, Ævar Arnfjörð Bjarmason, Phillip Wood From: Phillip Wood <phillip.wood@dunelm.org.uk> Check the console output when using --autostash and the stash does not apply is what we expect. The test is quite strict but should catch any changes to the console output from the various rebase flavors. Thanks-to: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- t/t3420-rebase-autostash.sh | 71 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 69 insertions(+), 2 deletions(-) diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh index cd1012798cb300f4f1ddeba6fdcad544ca9ea1d9..2c01ae6ad2a104940e388013984e7fa2a0d5aed5 100755 --- a/t/t3420-rebase-autostash.sh +++ b/t/t3420-rebase-autostash.sh @@ -88,6 +88,67 @@ create_expected_success_merge() { EOF } +create_expected_failure_am() { + cat >expected <<-EOF + $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) + HEAD is now at $(git rev-parse --short feature-branch) third commit + First, rewinding head to replay your work on top of it... + Applying: second commit + Applying: third commit + Applying autostash resulted in conflicts. + Your changes are safe in the stash. + You can run "git stash pop" or "git stash drop" at any time. + EOF +} + +create_expected_failure_interactive() { + q_to_cr >expected <<-EOF + $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) + HEAD is now at $(git rev-parse --short feature-branch) third commit + Rebasing (1/2)QRebasing (2/2)QApplying autostash resulted in conflicts. + Your changes are safe in the stash. + You can run "git stash pop" or "git stash drop" at any time. + Successfully rebased and updated refs/heads/rebased-feature-branch. + EOF +} + +create_expected_failure_merge() { + cat >expected <<-EOF + $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) + HEAD is now at $(git rev-parse --short feature-branch) third commit + First, rewinding head to replay your work on top of it... + Merging unrelated-onto-branch with HEAD~1 + Merging: + $(git rev-parse --short unrelated-onto-branch) unrelated commit + $(git rev-parse --short feature-branch^) second commit + found 1 common ancestor: + $(git rev-parse --short feature-branch~2) initial commit + [detached HEAD $(git rev-parse --short rebased-feature-branch~1)] second commit + Author: A U Thor <author@example.com> + Date: Thu Apr 7 15:14:13 2005 -0700 + 2 files changed, 2 insertions(+) + create mode 100644 file1 + create mode 100644 file2 + Committed: 0001 second commit + Merging unrelated-onto-branch with HEAD~0 + Merging: + $(git rev-parse --short rebased-feature-branch~1) second commit + $(git rev-parse --short feature-branch) third commit + found 1 common ancestor: + $(git rev-parse --short feature-branch~1) second commit + [detached HEAD $(git rev-parse --short rebased-feature-branch)] third commit + Author: A U Thor <author@example.com> + Date: Thu Apr 7 15:15:13 2005 -0700 + 1 file changed, 1 insertion(+) + create mode 100644 file3 + Committed: 0002 third commit + All done. + Applying autostash resulted in conflicts. + Your changes are safe in the stash. + You can run "git stash pop" or "git stash drop" at any time. + EOF +} + testrebase() { type=$1 dotest=$2 @@ -198,10 +259,9 @@ testrebase() { test_config rebase.autostash true && git reset --hard && git checkout -b rebased-feature-branch feature-branch && - test_when_finished git branch -D rebased-feature-branch && echo dirty >file4 && git add file4 && - git rebase$type unrelated-onto-branch && + git rebase$type unrelated-onto-branch >actual 2>&1 && test_path_is_missing $dotest && git reset --hard && grep unrelated file4 && @@ -210,6 +270,13 @@ testrebase() { git stash pop && grep dirty file4 ' + + test_expect_success "rebase$type: check output with conflicting stash" ' + test_when_finished git branch -D rebased-feature-branch && + suffix=${type#\ --} && suffix=${suffix:-am} && + create_expected_failure_$suffix && + test_cmp expected actual + ' } test_expect_success "rebase: fast-forward rebase" ' -- 2.13.0 ^ permalink raw reply related [flat|nested] 59+ messages in thread
* Re: [PATCH v3 0/4] Add regression tests for recent rebase -i fixes 2017-06-19 17:56 ` [PATCH v3 0/4] Add regression tests for recent " Phillip Wood ` (3 preceding siblings ...) 2017-06-19 17:56 ` [PATCH v3 4/4] rebase: Add more " Phillip Wood @ 2017-06-23 4:17 ` Junio C Hamano 2017-06-23 5:07 ` Junio C Hamano 4 siblings, 1 reply; 59+ messages in thread From: Junio C Hamano @ 2017-06-23 4:17 UTC (permalink / raw) To: Phillip Wood Cc: Git Mailing List, Johannes Schindelin, Ævar Arnfjörð Bjarmason, Phillip Wood Phillip Wood <phillip.wood@talktalk.net> writes: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > I've updated the second two tests to be portable using q_to_cr() as > Johannes suggested and added his patch to fix the autostash messages > going to stdout rather than stderr. The reflog message test is > unchanged. Thanks to Johannes for his help and to Junio for picking up > the bashism in the last iteration. > > Johannes Schindelin (1): > sequencer: print autostash messages to stderr > > Phillip Wood (3): > rebase -i: Add test for reflog message > rebase: Add regression tests for console output > rebase: Add more regression tests for console output > > git-rebase.sh | 4 +- > sequencer.c | 11 ++-- > t/t3404-rebase-interactive.sh | 7 +++ > t/t3420-rebase-autostash.sh | 136 ++++++++++++++++++++++++++++++++++++++++-- > 4 files changed, 147 insertions(+), 11 deletions(-) I've merged this to 'next' but I probably shouldn't have before making sure that Travis tests passes 'pu' while this was still in there. At least t3420 seems to fail under GETTEXT_POISON build. https://travis-ci.org/git/git/jobs/245990993 ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v3 0/4] Add regression tests for recent rebase -i fixes 2017-06-23 4:17 ` [PATCH v3 0/4] Add regression tests for recent rebase -i fixes Junio C Hamano @ 2017-06-23 5:07 ` Junio C Hamano 2017-06-23 9:53 ` Phillip Wood 0 siblings, 1 reply; 59+ messages in thread From: Junio C Hamano @ 2017-06-23 5:07 UTC (permalink / raw) To: Phillip Wood Cc: Git Mailing List, Johannes Schindelin, Ævar Arnfjörð Bjarmason, Phillip Wood Junio C Hamano <gitster@pobox.com> writes: >> git-rebase.sh | 4 +- >> sequencer.c | 11 ++-- >> t/t3404-rebase-interactive.sh | 7 +++ >> t/t3420-rebase-autostash.sh | 136 ++++++++++++++++++++++++++++++++++++++++-- >> 4 files changed, 147 insertions(+), 11 deletions(-) > > I've merged this to 'next' but I probably shouldn't have before > making sure that Travis tests passes 'pu' while this was still in > there. > > At least t3420 seems to fail under GETTEXT_POISON build. > > https://travis-ci.org/git/git/jobs/245990993 This should be sufficient to make t3420 pass. It seems that t3404 is also broken under GETTEXT_POISON build, but I won't have time to look at it, at least tonight. $ make GETTEXT_POISON=YesPlease $ cd t && sh ./t3404-*.sh -i -v to see how it breaks. Thanks. t/t3420-rebase-autostash.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh index 6826c38cbd..e243700660 100755 --- a/t/t3420-rebase-autostash.sh +++ b/t/t3420-rebase-autostash.sh @@ -178,7 +178,7 @@ testrebase () { test_when_finished git branch -D rebased-feature-branch && suffix=${type#\ --} && suffix=${suffix:-am} && create_expected_success_$suffix && - test_cmp expected actual + test_i18ncmp expected actual ' test_expect_success "rebase$type: dirty index, non-conflicting rebase" ' @@ -275,7 +275,7 @@ testrebase () { test_when_finished git branch -D rebased-feature-branch && suffix=${type#\ --} && suffix=${suffix:-am} && create_expected_failure_$suffix && - test_cmp expected actual + test_i18ncmp expected actual ' } ^ permalink raw reply related [flat|nested] 59+ messages in thread
* Re: [PATCH v3 0/4] Add regression tests for recent rebase -i fixes 2017-06-23 5:07 ` Junio C Hamano @ 2017-06-23 9:53 ` Phillip Wood 2017-06-23 17:03 ` Junio C Hamano 0 siblings, 1 reply; 59+ messages in thread From: Phillip Wood @ 2017-06-23 9:53 UTC (permalink / raw) To: Junio C Hamano Cc: Git Mailing List, Johannes Schindelin, Ævar Arnfjörð Bjarmason, Phillip Wood On 23/06/17 06:07, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > >>> git-rebase.sh | 4 +- >>> sequencer.c | 11 ++-- >>> t/t3404-rebase-interactive.sh | 7 +++ >>> t/t3420-rebase-autostash.sh | 136 ++++++++++++++++++++++++++++++++++++++++-- >>> 4 files changed, 147 insertions(+), 11 deletions(-) >> >> I've merged this to 'next' but I probably shouldn't have before >> making sure that Travis tests passes 'pu' while this was still in >> there. >> >> At least t3420 seems to fail under GETTEXT_POISON build. >> >> https://travis-ci.org/git/git/jobs/245990993 > > This should be sufficient to make t3420 pass. It seems that t3404 > is also broken under GETTEXT_POISON build, but I won't have time to > look at it, at least tonight. > > $ make GETTEXT_POISON=YesPlease > $ cd t && sh ./t3404-*.sh -i -v > > to see how it breaks. t3404 passes for me, $ make GETTEXT_POISON=YesPlease $ cd t &&sh t3404-rebase-interactive.sh -i -v ... # still have 1 known breakage(s) # passed all remaining 95 test(s) 1..96 Also as far as I can see it passes on travis - https://travis-ci.org/git/git/jobs/245990993#L910 have I missed something? Do you want me to submit a fixup patch for t3420 or have you got one already? Thanks Phillip > Thanks. > > t/t3420-rebase-autostash.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh > index 6826c38cbd..e243700660 100755 > --- a/t/t3420-rebase-autostash.sh > +++ b/t/t3420-rebase-autostash.sh > @@ -178,7 +178,7 @@ testrebase () { > test_when_finished git branch -D rebased-feature-branch && > suffix=${type#\ --} && suffix=${suffix:-am} && > create_expected_success_$suffix && > - test_cmp expected actual > + test_i18ncmp expected actual > ' > > test_expect_success "rebase$type: dirty index, non-conflicting rebase" ' > @@ -275,7 +275,7 @@ testrebase () { > test_when_finished git branch -D rebased-feature-branch && > suffix=${type#\ --} && suffix=${suffix:-am} && > create_expected_failure_$suffix && > - test_cmp expected actual > + test_i18ncmp expected actual > ' > } > > ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v3 0/4] Add regression tests for recent rebase -i fixes 2017-06-23 9:53 ` Phillip Wood @ 2017-06-23 17:03 ` Junio C Hamano 2017-06-23 18:53 ` Junio C Hamano 2017-06-23 19:01 ` Junio C Hamano 0 siblings, 2 replies; 59+ messages in thread From: Junio C Hamano @ 2017-06-23 17:03 UTC (permalink / raw) To: Phillip Wood Cc: Git Mailing List, Johannes Schindelin, Ævar Arnfjörð Bjarmason, Phillip Wood Phillip Wood <phillip.wood@talktalk.net> writes: > t3404 passes for me, > $ make GETTEXT_POISON=YesPlease > $ cd t &&sh t3404-rebase-interactive.sh -i -v > ... > # still have 1 known breakage(s) > # passed all remaining 95 test(s) > 1..96 Interesting. The tip of 'pu', which includes this series, does pass for me, too, but the tip of this topic 7d70e6b9 ("rebase: add more regression tests for console output", 2017-06-19) tested in isolation fails, and gives the output at the end of this message. > Do you want me to submit a fixup patch for t3420 or have you > got one already? For 3420, I can wrap the two-liner patch I showed here earlier into a commit on top of the series. 3404 needs a similar fix-up for the series to be able to stand on its own. Alternatively, at least we need to understand what in 'pu' makes the result of the merge pass---the symptom indicates that this topic cannot be merged to a released version without that unknown other topic in 'pu' merged if we want to keep POISON build passing the tests. Thanks. -- output from 3404 follows -- Initialized empty Git repository in /home/gitster/git/t/trash directory.t3404-rebase-interactive/.git/ expecting success: test_commit A file1 && test_commit B file1 && test_commit C file2 && test_commit D file1 && test_commit E file3 && git checkout -b branch1 A && test_commit F file4 && test_commit G file1 && test_commit H file5 && git checkout -b branch2 F && test_commit I file6 && git checkout -b conflict-branch A && test_commit one conflict && test_commit two conflict && test_commit three conflict && test_commit four conflict && git checkout -b no-conflict-branch A && test_commit J fileJ && test_commit K fileK && test_commit L fileL && test_commit M fileM && git checkout -b no-ff-branch A && test_commit N fileN && test_commit O fileO && test_commit P fileP [master# GETTEXT POISON # 6e62bf8] A Author: A U Thor <author@example.com> 1 file changed, 1 insertion(+) create mode 100644 file1 [master 313fe96] B Author: A U Thor <author@example.com> 1 file changed, 1 insertion(+), 1 deletion(-) [master d0f65f2] C Author: A U Thor <author@example.com> 1 file changed, 1 insertion(+) create mode 100644 file2 [master 0547e3f] D Author: A U Thor <author@example.com> 1 file changed, 1 insertion(+), 1 deletion(-) [master 8f99a4f] E Author: A U Thor <author@example.com> 1 file changed, 1 insertion(+) create mode 100644 file3 # GETTEXT POISON #[branch1 cfefd94] F Author: A U Thor <author@example.com> 1 file changed, 1 insertion(+) create mode 100644 file4 [branch1 83751a6] G Author: A U Thor <author@example.com> 1 file changed, 1 insertion(+), 1 deletion(-) [branch1 4373208] H Author: A U Thor <author@example.com> 1 file changed, 1 insertion(+) create mode 100644 file5 # GETTEXT POISON #[branch2 615be62] I Author: A U Thor <author@example.com> 1 file changed, 1 insertion(+) create mode 100644 file6 # GETTEXT POISON #[conflict-branch b895952] one Author: A U Thor <author@example.com> 1 file changed, 1 insertion(+) create mode 100644 conflict [conflict-branch 766a798] two Author: A U Thor <author@example.com> 1 file changed, 1 insertion(+), 1 deletion(-) [conflict-branch 1eadf03] three Author: A U Thor <author@example.com> 1 file changed, 1 insertion(+), 1 deletion(-) [conflict-branch f91a2b3] four Author: A U Thor <author@example.com> 1 file changed, 1 insertion(+), 1 deletion(-) # GETTEXT POISON #[no-conflict-branch 808874f] J Author: A U Thor <author@example.com> 1 file changed, 1 insertion(+) create mode 100644 fileJ [no-conflict-branch 265b89e] K Author: A U Thor <author@example.com> 1 file changed, 1 insertion(+) create mode 100644 fileK [no-conflict-branch 6b0f5e6] L Author: A U Thor <author@example.com> 1 file changed, 1 insertion(+) create mode 100644 fileL [no-conflict-branch 3389558] M Author: A U Thor <author@example.com> 1 file changed, 1 insertion(+) create mode 100644 fileM # GETTEXT POISON #[no-ff-branch 53b4423] N Author: A U Thor <author@example.com> 1 file changed, 1 insertion(+) create mode 100644 fileN [no-ff-branch cc47714] O Author: A U Thor <author@example.com> 1 file changed, 1 insertion(+) create mode 100644 fileO [no-ff-branch faef1a5] P Author: A U Thor <author@example.com> 1 file changed, 1 insertion(+) create mode 100644 fileP ok 1 - setup expecting success: git checkout -b emptybranch master && git commit --allow-empty -m "empty" && git rebase --keep-empty -i HEAD~2 && git log --oneline >actual && test_line_count = 6 actual # GETTEXT POISON #[emptybranch da33401] empty Author: A U Thor <author@example.com> Successfully rebased and updated refs/heads/emptybranch. ok 2 - rebase --keep-empty expecting success: git checkout master && ( set_fake_editor && FAKE_LINES="1 exec_>touch-one 2 exec_>touch-two exec_false exec_>touch-three 3 4 exec_>\"touch-file__name_with_spaces\";_>touch-after-semicolon 5" && export FAKE_LINES && test_must_fail git rebase -i A ) && test_path_is_file touch-one && test_path_is_file touch-two && test_path_is_missing touch-three " (should have stopped before)" && test_cmp_rev C HEAD && git rebase --continue && test_path_is_file touch-three && test_path_is_file "touch-file name with spaces" && test_path_is_file touch-after-semicolon && test_cmp_rev master HEAD && rm -f touch-* # GETTEXT POISON #rebase -i script before editing: pick 313fe96 B pick d0f65f2 C pick 0547e3f D pick 8f99a4f E rebase -i script after editing: pick 313fe96 B exec >touch-one pick d0f65f2 C exec >touch-two exec false exec >touch-three pick 0547e3f D pick 8f99a4f E exec >"touch-file name with spaces"; >touch-after-semicolon Rebasing (2/9)Executing: >touch-one Rebasing (3/9)Rebasing (4/9)Executing: >touch-two Rebasing (5/9)Executing: false warning: # GETTEXT POISON # Rebasing (6/9)Executing: >touch-three Rebasing (7/9)Rebasing (8/9)Rebasing (9/9)Executing: >"touch-file name with spaces"; >touch-after-semicolon Successfully rebased and updated refs/heads/master. ok 3 - rebase -i with the exec command expecting success: git checkout master && mkdir subdir && (cd subdir && set_fake_editor && FAKE_LINES="1 exec_>touch-subdir" \ git rebase -i HEAD^ ) && test_path_is_file touch-subdir && rm -fr subdir # GETTEXT POISON #rebase -i script before editing: pick 8f99a4f E rebase -i script after editing: pick 8f99a4f E exec >touch-subdir Rebasing (2/2)Executing: >touch-subdir Successfully rebased and updated refs/heads/master. ok 4 - rebase -i with the exec command runs from tree root expecting success: git checkout master && set_fake_editor && test_must_fail env FAKE_LINES="exec_echo_foo_>file1 1" git rebase -i HEAD^ && test_cmp_rev master^ HEAD && git reset --hard && git rebase --continue # GETTEXT POISON #rebase -i script before editing: pick 8f99a4f E rebase -i script after editing: exec echo foo >file1 pick 8f99a4f E Rebasing (1/2)Executing: echo foo >file1 error: # GETTEXT POISON # warning: # GETTEXT POISON # # GETTEXT POISON # D Rebasing (2/2)Successfully rebased and updated refs/heads/master. ok 5 - rebase -i with the exec command checks tree cleanness expecting success: git checkout master && test_when_finished "git rebase --abort" && set_fake_editor && test_must_fail env FAKE_LINES="exec_this-command-does-not-exist 1" \ git rebase -i HEAD^ >actual 2>&1 && ! grep "Maybe git-rebase is broken" actual # GETTEXT POISON #ok 6 - rebase -i with exec of inexistent command expecting success: git checkout branch2 && set_fake_editor && git rebase -i F && test "$(git symbolic-ref -q HEAD)" = "refs/heads/branch2" && test $(git rev-parse I) = $(git rev-parse HEAD) # GETTEXT POISON #Successfully rebased and updated refs/heads/branch2. ok 7 - no changes are a nop expecting success: git checkout -b dead-end && git rm file6 && git commit -m "stop here" && set_fake_editor && git rebase -i F branch2 && test "$(git symbolic-ref -q HEAD)" = "refs/heads/branch2" && test $(git rev-parse I) = $(git rev-parse branch2) && test $(git rev-parse I) = $(git rev-parse HEAD) # GETTEXT POISON #rm 'file6' [dead-end f814f58] stop here Author: A U Thor <author@example.com> 1 file changed, 1 deletion(-) delete mode 100644 file6 Successfully rebased and updated refs/heads/branch2. ok 8 - test the [branch] option expecting success: git checkout -b test-onto branch2 && set_fake_editor && git rebase -i --onto branch1 F && test "$(git symbolic-ref -q HEAD)" = "refs/heads/test-onto" && test $(git rev-parse HEAD^) = $(git rev-parse branch1) && test $(git rev-parse I) = $(git rev-parse branch2) # GETTEXT POISON #Rebasing (1/1)Successfully rebased and updated refs/heads/test-onto. ok 9 - test --onto <branch> expecting success: git checkout branch1 && git tag original-branch1 && set_fake_editor && git rebase -i branch2 && test file6 = $(git diff --name-only original-branch1) && test "$(git symbolic-ref -q HEAD)" = "refs/heads/branch1" && test $(git rev-parse I) = $(git rev-parse branch2) && test $(git rev-parse I) = $(git rev-parse HEAD~2) # GETTEXT POISON #Rebasing (1/2)Rebasing (2/2)Successfully rebased and updated refs/heads/branch1. ok 10 - rebase on top of a non-conflicting commit expecting success: test $(git rev-parse branch1@{1}) = $(git rev-parse original-branch1) ok 11 - reflog for the branch shows state before rebase expecting success: printf "rebase -i (finish): refs/heads/branch1 onto %s\n" \ "$(git rev-parse branch2)" >expected && git log -g --pretty=%gs -1 refs/heads/branch1 >actual && test_cmp expected actual ok 12 - reflog for the branch shows correct finish message expecting success: set_fake_editor && FAKE_LINES="2 1" git rebase -i HEAD~2 && test H = $(git cat-file commit HEAD^ | sed -ne \$p) && test G = $(git cat-file commit HEAD | sed -ne \$p) rebase -i script before editing: pick ae8f65e G pick f5f5249 H rebase -i script after editing: pick f5f5249 H pick ae8f65e G Rebasing (1/2)Rebasing (2/2)Successfully rebased and updated refs/heads/branch1. ok 13 - exchange two commits expecting success: git tag new-branch1 && set_fake_editor && test_must_fail git rebase -i master && test "$(git rev-parse HEAD~3)" = "$(git rev-parse master)" && test_cmp expect .git/rebase-merge/patch && test_cmp expect2 file1 && test "$(git diff --name-status | sed -n -e "/^U/s/^U[^a-z]*//p")" = file1 && test 4 = $(grep -v "^#" < .git/rebase-merge/done | wc -l) && test 0 = $(grep -c "^[^#]" < .git/rebase-merge/git-rebase-todo) Rebasing (1/4)Rebasing (2/4)Rebasing (3/4)Rebasing (4/4)error: # GETTEXT POISON # # GETTEXT POISON # Could not apply 5d18e54... G # GETTEXT POISON # # GETTEXT POISON # ok 14 - stop on conflicting pick expecting success: git rebase --abort && test $(git rev-parse new-branch1) = $(git rev-parse HEAD) && test "$(git symbolic-ref -q HEAD)" = "refs/heads/branch1" && test_path_is_missing .git/rebase-merge ok 15 - abort expecting success: git rm --cached file1 && git commit -m "remove file in base" && set_fake_editor && test_must_fail git rebase -i master > output 2>&1 && test_i18ngrep "The following untracked working tree files would be overwritten by checkout:" \ output && test_i18ngrep "file1" output && test_path_is_missing .git/rebase-merge && git reset --hard HEAD^ rm 'file1' [branch1 2dd5570] remove file in base Author: A U Thor <author@example.com> 1 file changed, 1 deletion(-) delete mode 100644 file1 # GETTEXT POISON # G ok 16 - abort with error when new base cannot be checked out expecting success: echo A > file7 && git add file7 && test_tick && GIT_AUTHOR_NAME="Twerp Snog" git commit -m "different author" && git tag twerp && set_fake_editor && git rebase -i --onto master HEAD^ && git show HEAD | grep "^Author: Twerp Snog" [branch1 2596307] different author Author: Twerp Snog <author@example.com> 1 file changed, 1 insertion(+) create mode 100644 file7 Rebasing (1/1)Successfully rebased and updated refs/heads/branch1. Author: Twerp Snog <author@example.com> ok 17 - retain authorship expecting success: git reset --hard twerp && test_commit a conflict a conflict-a && git reset --hard twerp && GIT_AUTHOR_NAME=AttributeMe \ test_commit b conflict b conflict-b && set_fake_editor && test_must_fail git rebase -i conflict-a && echo resolved >conflict && git add conflict && git rebase --continue && test $(git rev-parse conflict-a^0) = $(git rev-parse HEAD^) && git show >out && grep AttributeMe out # GETTEXT POISON # different author [branch1 748a43d] a Author: A U Thor <author@example.com> 1 file changed, 1 insertion(+) create mode 100644 conflict # GETTEXT POISON # different author [branch1 34d1a6c] b Author: AttributeMe <author@example.com> 1 file changed, 1 insertion(+) create mode 100644 conflict Rebasing (1/1)error: # GETTEXT POISON # # GETTEXT POISON # Could not apply 34d1a6c... b # GETTEXT POISON # # GETTEXT POISON # [# GETTEXT POISON # 7866b12] b Author: AttributeMe <author@example.com> 1 file changed, 1 insertion(+), 1 deletion(-) Successfully rebased and updated refs/heads/branch1. Author: AttributeMe <author@example.com> ok 18 - retain authorship w/ conflicts expecting success: git reset --hard twerp && echo B > file7 && test_tick && GIT_AUTHOR_NAME="Nitfol" git commit -m "nitfol" file7 && echo "******************************" && set_fake_editor && FAKE_LINES="1 squash 2" EXPECT_HEADER_COUNT=2 \ git rebase -i --onto master HEAD~2 && test B = $(cat file7) && test $(git rev-parse HEAD^) = $(git rev-parse master) # GETTEXT POISON # different author [branch1 cdf577a] nitfol Author: Nitfol <author@example.com> 1 file changed, 1 insertion(+), 1 deletion(-) ****************************** rebase -i script before editing: pick 2596307 different author pick cdf577a nitfol rebase -i script after editing: pick 2596307 different author squash cdf577a nitfol Rebasing (1/2)Rebasing (2/2)[# GETTEXT POISON # d2d5ba7] different author Author: Twerp Snog <author@example.com> Date: Thu Apr 7 15:33:13 2005 -0700 1 file changed, 1 insertion(+) create mode 100644 file7 Successfully rebased and updated refs/heads/branch1. ok 19 - squash expecting success: git show HEAD | grep "^Author: Twerp Snog" Author: Twerp Snog <author@example.com> ok 20 - retain authorship when squashing expecting success: HEAD=$(git rev-parse HEAD) && set_fake_editor && git rebase -i -p HEAD^ && git update-index --refresh && git diff-files --quiet && git diff-index --quiet --cached HEAD -- && test $HEAD = $(git rev-parse HEAD) # GETTEXT POISON ## GETTEXT POISON # ok 21 - -p handles "no changes" gracefully checking known breakage: git checkout H && set_fake_editor && FAKE_LINES="2 1" git rebase -i -p HEAD~2 && test H = $(git cat-file commit HEAD^ | sed -ne \$p) && test G = $(git cat-file commit HEAD | sed -ne \$p) # GETTEXT POISON ## GETTEXT POISON # 4373208... H rebase -i script before editing: pick 83751a6 G pick 4373208 H rebase -i script after editing: pick 4373208 H pick 83751a6 G # GETTEXT POISON ## GETTEXT POISON ## GETTEXT POISON # not ok 22 - exchange two commits with -p # TODO known breakage expecting success: git checkout -b to-be-preserved master^ && : > unrelated-file && git add unrelated-file && test_tick && git commit -m "unrelated" && git checkout -b another-branch master && echo B > file1 && test_tick && git commit -m J file1 && test_tick && git merge to-be-preserved && echo C > file1 && test_tick && git commit -m K file1 && echo D > file1 && test_tick && git commit -m L1 file1 && git checkout HEAD^ && echo 1 > unrelated-file && test_tick && git commit -m L2 unrelated-file && test_tick && git merge another-branch && echo E > file1 && test_tick && git commit -m M file1 && git checkout -b to-be-rebased && test_tick && set_fake_editor && git rebase -i -p --onto branch1 master && git update-index --refresh && git diff-files --quiet && git diff-index --quiet --cached HEAD -- && test $(git rev-parse HEAD~6) = $(git rev-parse branch1) && test $(git rev-parse HEAD~4^2) = $(git rev-parse to-be-preserved) && test $(git rev-parse HEAD^^2^) = $(git rev-parse HEAD^^^) && test $(git show HEAD~5:file1) = B && test $(git show HEAD~3:file1) = C && test $(git show HEAD:file1) = E && test $(git show HEAD:unrelated-file) = 1 # GETTEXT POISON # 83751a6... G # GETTEXT POISON #[to-be-preserved 4b3f608] unrelated Author: AttributeMe <author@example.com> 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 unrelated-file # GETTEXT POISON #[another-branch 8e3f1bd] J Author: AttributeMe <author@example.com> 1 file changed, 1 insertion(+), 1 deletion(-) # GETTEXT POISON # 8e3f1bd J virtual to-be-preserved # GETTEXT POISON # 0547e3f D Merge made by the 'recursive' strategy. unrelated-file | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 unrelated-file [another-branch 0f839ac] K Author: AttributeMe <author@example.com> 1 file changed, 1 insertion(+), 1 deletion(-) [another-branch 916bd1f] L1 Author: AttributeMe <author@example.com> 1 file changed, 1 insertion(+), 1 deletion(-) # GETTEXT POISON ## GETTEXT POISON # 0f839ac... K [# GETTEXT POISON # 8fa3ce5] L2 Author: AttributeMe <author@example.com> 1 file changed, 1 insertion(+) # GETTEXT POISON # 8fa3ce5 L2 virtual another-branch # GETTEXT POISON # 0f839ac K Merge made by the 'recursive' strategy. file1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) [# GETTEXT POISON # e12d8fb] M Author: AttributeMe <author@example.com> 1 file changed, 1 insertion(+), 1 deletion(-) # GETTEXT POISON ## GETTEXT POISON ## GETTEXT POISON ## GETTEXT POISON ## GETTEXT POISON ## GETTEXT POISON ## GETTEXT POISON ## GETTEXT POISON ## GETTEXT POISON # ok 23 - preserve merges with -p expecting success: set_fake_editor && FAKE_LINES="1 2 edit 3 4" git rebase -i -p HEAD~3 && echo 2 > unrelated-file && test_tick && git commit -m L2-modified --amend unrelated-file && git rebase --continue && git update-index --refresh && git diff-files --quiet && git diff-index --quiet --cached HEAD -- && test $(git show HEAD:unrelated-file) = 2 rebase -i script before editing: pick 60cb1bd L2 pick f2d0f15 L1 pick 8b9d7f7 Merge branch 'another-branch' into HEAD pick ecf20bb M rebase -i script after editing: pick 60cb1bd L2 pick f2d0f15 L1 edit 8b9d7f7 Merge branch 'another-branch' into HEAD pick ecf20bb M # GETTEXT POISON ## GETTEXT POISON ## GETTEXT POISON ## GETTEXT POISON # # GETTEXT POISON # [# GETTEXT POISON # 82f9bab] L2-modified Author: AttributeMe <author@example.com> Date: Thu Apr 7 15:43:13 2005 -0700 # GETTEXT POISON ## GETTEXT POISON # ok 24 - edit ancestor with -p expecting success: test_tick && set_fake_editor && test_must_fail git rebase -i --onto new-branch1 HEAD^ && echo resolved > file1 && git add file1 && FAKE_COMMIT_MESSAGE="chouette!" git rebase --continue && test $(git rev-parse HEAD^) = $(git rev-parse new-branch1) && git show HEAD | grep chouette Rebasing (1/1)error: # GETTEXT POISON # # GETTEXT POISON # Could not apply 23e2f4c... M # GETTEXT POISON # # GETTEXT POISON # [# GETTEXT POISON # be87a39] chouette! Author: AttributeMe <author@example.com> 1 file changed, 1 insertion(+), 1 deletion(-) Successfully rebased and updated refs/heads/to-be-rebased. chouette! ok 25 - --continue tries to commit expecting success: git reset --hard master@{1} && test_tick && set_fake_editor && test_must_fail git rebase -v -i --onto new-branch1 HEAD^ && echo resolved > file1 && git add file1 && git rebase --continue > output && grep "^ file1 | 2 +-$" output # GETTEXT POISON # D # GETTEXT POISON # file1 | 2 +- file4 | 1 + file5 | 1 + file6 | 1 + 4 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 file4 create mode 100644 file5 create mode 100644 file6 # GETTEXT POISON ## GETTEXT POISON # 5d18e54... G Rebasing (1/1) error: # GETTEXT POISON # # GETTEXT POISON # Could not apply 0547e3f... D # GETTEXT POISON # # GETTEXT POISON # Successfully rebased and updated refs/heads/to-be-rebased. file1 | 2 +- ok 26 - verbose flag is heeded, even after --continue expecting success: base=$(git rev-parse HEAD~4) && set_fake_editor && FAKE_COMMIT_AMEND="ONCE" FAKE_LINES="1 squash 2 squash 3 squash 4" \ EXPECT_HEADER_COUNT=4 \ git rebase -i $base && test $base = $(git rev-parse HEAD^) && test 1 = $(git show | grep ONCE | wc -l) rebase -i script before editing: pick 615be62 I pick 0626e8d H pick 5d18e54 G pick 2e6badc D rebase -i script after editing: pick 615be62 I squash 0626e8d H squash 5d18e54 G squash 2e6badc D Rebasing (2/4)Rebasing (3/4)error: # GETTEXT POISON # Could not apply 5d18e54... G not ok 27 - multi-squash only fires up editor once # # base=$(git rev-parse HEAD~4) && # set_fake_editor && # FAKE_COMMIT_AMEND="ONCE" FAKE_LINES="1 squash 2 squash 3 squash 4" \ # EXPECT_HEADER_COUNT=4 \ # git rebase -i $base && # test $base = $(git rev-parse HEAD^) && # test 1 = $(git show | grep ONCE | wc -l) # ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v3 0/4] Add regression tests for recent rebase -i fixes 2017-06-23 17:03 ` Junio C Hamano @ 2017-06-23 18:53 ` Junio C Hamano 2017-06-26 9:17 ` Phillip Wood 2017-06-23 19:01 ` Junio C Hamano 1 sibling, 1 reply; 59+ messages in thread From: Junio C Hamano @ 2017-06-23 18:53 UTC (permalink / raw) To: Phillip Wood Cc: Git Mailing List, Johannes Schindelin, Ævar Arnfjörð Bjarmason, Phillip Wood Junio C Hamano <gitster@pobox.com> writes: > 3404 needs a similar fix-up for the series to be able to stand on > its own. Alternatively, at least we need to understand what in 'pu' > makes the result of the merge pass---the symptom indicates that this > topic cannot be merged to a released version without that unknown > other topic in 'pu' merged if we want to keep POISON build passing > the tests. Ah, no worries. I think I figured it out. The topic "rebase -i regression fix", which this "regression fix tests" builds on, is queued on an older codebase than 0d75bfe6 ("tests: fix tests broken under GETTEXT_POISON=YesPlease", 2017-05-05); it is natural these old test breakages can be seen when the topic is tested alone. So we can safely merge this topic down. Thanks for prodding me to take a deeper look. ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v3 0/4] Add regression tests for recent rebase -i fixes 2017-06-23 18:53 ` Junio C Hamano @ 2017-06-26 9:17 ` Phillip Wood 0 siblings, 0 replies; 59+ messages in thread From: Phillip Wood @ 2017-06-26 9:17 UTC (permalink / raw) To: Junio C Hamano Cc: Git Mailing List, Johannes Schindelin, Ævar Arnfjörð Bjarmason, Phillip Wood On 23/06/17 19:53, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > >> 3404 needs a similar fix-up for the series to be able to stand on >> its own. Alternatively, at least we need to understand what in 'pu' >> makes the result of the merge pass---the symptom indicates that this >> topic cannot be merged to a released version without that unknown >> other topic in 'pu' merged if we want to keep POISON build passing >> the tests. > > Ah, no worries. I think I figured it out. > > The topic "rebase -i regression fix", which this "regression fix > tests" builds on, is queued on an older codebase than 0d75bfe6 > ("tests: fix tests broken under GETTEXT_POISON=YesPlease", > 2017-05-05); it is natural these old test breakages can be seen when > the topic is tested alone. Oh, that explains it, I was pretty sure the reflog messages were not translated so couldn't understand why it would fail under GETTEXT_POISON=YesPlease > So we can safely merge this topic down. That's great, thanks for taking the time to track down the reason for the test failure Best Wishes Phillip ^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v3 0/4] Add regression tests for recent rebase -i fixes 2017-06-23 17:03 ` Junio C Hamano 2017-06-23 18:53 ` Junio C Hamano @ 2017-06-23 19:01 ` Junio C Hamano 2017-06-26 9:23 ` Phillip Wood 1 sibling, 1 reply; 59+ messages in thread From: Junio C Hamano @ 2017-06-23 19:01 UTC (permalink / raw) To: Phillip Wood Cc: Git Mailing List, Johannes Schindelin, Ævar Arnfjörð Bjarmason, Phillip Wood Junio C Hamano <gitster@pobox.com> writes: > For 3420, I can wrap the two-liner patch I showed here earlier into > a commit on top of the series. So, here is what I'll queue on top before merging the topic down to 'master'. -- >8 -- Subject: [PATCH] t3420: fix under GETTEXT_POISON build Newly added tests to t3420 in this series prepare expected human-readable output from "git rebase -i" and then compare the actual output with it. As the output from the command is designed to go through i18n/l10n, we need to use test_i18ncmp to tell GETTEXT_POISON build that it is OK the output does not match. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- t/t3420-rebase-autostash.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh index 6826c38cbd..e243700660 100755 --- a/t/t3420-rebase-autostash.sh +++ b/t/t3420-rebase-autostash.sh @@ -178,7 +178,7 @@ testrebase () { test_when_finished git branch -D rebased-feature-branch && suffix=${type#\ --} && suffix=${suffix:-am} && create_expected_success_$suffix && - test_cmp expected actual + test_i18ncmp expected actual ' test_expect_success "rebase$type: dirty index, non-conflicting rebase" ' @@ -275,7 +275,7 @@ testrebase () { test_when_finished git branch -D rebased-feature-branch && suffix=${type#\ --} && suffix=${suffix:-am} && create_expected_failure_$suffix && - test_cmp expected actual + test_i18ncmp expected actual ' } -- 2.13.1-678-g93553a431c ^ permalink raw reply related [flat|nested] 59+ messages in thread
* Re: [PATCH v3 0/4] Add regression tests for recent rebase -i fixes 2017-06-23 19:01 ` Junio C Hamano @ 2017-06-26 9:23 ` Phillip Wood 0 siblings, 0 replies; 59+ messages in thread From: Phillip Wood @ 2017-06-26 9:23 UTC (permalink / raw) To: Junio C Hamano Cc: Git Mailing List, Johannes Schindelin, Ævar Arnfjörð Bjarmason, Phillip Wood On 23/06/17 20:01, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > >> For 3420, I can wrap the two-liner patch I showed here earlier into >> a commit on top of the series. > > So, here is what I'll queue on top before merging the topic down to > 'master'. Thanks for creating this fixup, I'll remember to think about GETTEXT_POISON when I'm writing tests in the future. > -- >8 -- > Subject: [PATCH] t3420: fix under GETTEXT_POISON build > > Newly added tests to t3420 in this series prepare expected > human-readable output from "git rebase -i" and then compare the > actual output with it. As the output from the command is designed > to go through i18n/l10n, we need to use test_i18ncmp to tell > GETTEXT_POISON build that it is OK the output does not match. > > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > t/t3420-rebase-autostash.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh > index 6826c38cbd..e243700660 100755 > --- a/t/t3420-rebase-autostash.sh > +++ b/t/t3420-rebase-autostash.sh > @@ -178,7 +178,7 @@ testrebase () { > test_when_finished git branch -D rebased-feature-branch && > suffix=${type#\ --} && suffix=${suffix:-am} && > create_expected_success_$suffix && > - test_cmp expected actual > + test_i18ncmp expected actual > ' > > test_expect_success "rebase$type: dirty index, non-conflicting rebase" ' > @@ -275,7 +275,7 @@ testrebase () { > test_when_finished git branch -D rebased-feature-branch && > suffix=${type#\ --} && suffix=${suffix:-am} && > create_expected_failure_$suffix && > - test_cmp expected actual > + test_i18ncmp expected actual > ' > } > > ^ permalink raw reply [flat|nested] 59+ messages in thread
end of thread, other threads:[~2017-06-26 9:23 UTC | newest] Thread overview: 59+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-05-31 10:42 [PATCH 0/3] Add regression tests for recent rebase -i fixes Phillip Wood 2017-05-31 10:42 ` [PATCH 1/3] rebase -i: Add test for reflog message Phillip Wood 2017-06-01 2:00 ` Junio C Hamano 2017-05-31 10:42 ` [PATCH 2/3] rebase: Add tests for console output Phillip Wood 2017-05-31 19:02 ` Phillip Wood 2017-06-01 1:59 ` Junio C Hamano 2017-06-01 12:56 ` Johannes Schindelin 2017-06-01 23:40 ` Junio C Hamano 2017-06-01 23:47 ` Stefan Beller 2017-06-02 12:47 ` pushing for a new hash, was " Johannes Schindelin 2017-06-02 17:54 ` Jonathan Nieder 2017-06-02 18:05 ` Jonathan Nieder 2017-06-02 20:29 ` Ævar Arnfjörð Bjarmason 2017-06-15 10:38 ` Johannes Schindelin 2017-06-03 0:36 ` Junio C Hamano 2017-06-06 22:22 ` Johannes Schindelin 2017-06-06 22:45 ` Jonathan Nieder 2017-06-07 1:09 ` Junio C Hamano 2017-06-07 2:18 ` [PATCH] t4005: modernize style and drop hard coded sha1 Stefan Beller 2017-06-07 17:39 ` Brandon Williams 2017-06-06 22:45 ` pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console output Stefan Beller 2017-06-06 22:52 ` Jonathan Nieder 2017-06-07 0:34 ` Samuel Lijin 2017-06-07 14:47 ` Johannes Schindelin 2017-06-07 16:53 ` Stefan Beller 2017-06-07 10:47 ` Phillip Wood 2017-06-09 16:39 ` Junio C Hamano 2017-06-14 10:18 ` Phillip Wood 2017-06-14 12:51 ` Johannes Schindelin 2017-05-31 10:42 ` [PATCH 3/3] rebase: Add tests for console output with conflicting stash Phillip Wood 2017-06-14 10:24 ` [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes Phillip Wood 2017-06-14 10:24 ` [PATCH v2 1/3] rebase -i: Add test for reflog message Phillip Wood 2017-06-14 10:24 ` [PATCH v2 2/3] rebase: Add regression tests for console output Phillip Wood 2017-06-14 10:24 ` [PATCH v2 3/3] rebase: Add more " Phillip Wood 2017-06-14 20:35 ` [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes Johannes Schindelin 2017-06-15 23:05 ` Junio C Hamano 2017-06-15 23:23 ` Junio C Hamano 2017-06-15 23:29 ` Junio C Hamano 2017-06-16 13:49 ` Johannes Schindelin 2017-06-16 18:43 ` Johannes Sixt 2017-06-16 21:05 ` Junio C Hamano 2017-06-19 19:45 ` Johannes Sixt 2017-06-19 20:02 ` Junio C Hamano 2017-06-19 9:49 ` Phillip Wood 2017-06-19 15:45 ` Junio C Hamano 2017-06-19 9:52 ` Phillip Wood 2017-06-19 17:56 ` [PATCH v3 0/4] Add regression tests for recent " Phillip Wood 2017-06-19 17:56 ` [PATCH v3 1/4] sequencer: print autostash messages to stderr Phillip Wood 2017-06-19 17:56 ` [PATCH v3 2/4] rebase -i: Add test for reflog message Phillip Wood 2017-06-19 17:56 ` [PATCH v3 3/4] rebase: Add regression tests for console output Phillip Wood 2017-06-19 17:56 ` [PATCH v3 4/4] rebase: Add more " Phillip Wood 2017-06-23 4:17 ` [PATCH v3 0/4] Add regression tests for recent rebase -i fixes Junio C Hamano 2017-06-23 5:07 ` Junio C Hamano 2017-06-23 9:53 ` Phillip Wood 2017-06-23 17:03 ` Junio C Hamano 2017-06-23 18:53 ` Junio C Hamano 2017-06-26 9:17 ` Phillip Wood 2017-06-23 19:01 ` Junio C Hamano 2017-06-26 9:23 ` Phillip Wood
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).