Re: git diff-tree commit detail bug in 2.0.2 and 2.0.3
On Tue, Jul 29, 2014 at 11:06:25AM +1000, Bryan Turner wrote: On Tue, Jul 29, 2014 at 10:11 AM, Junio C Hamano gits...@pobox.com wrote: OK, I pushed out updated 'maint' and 'master'. The former merges a rebased version of jk/alloc-commit-id in to make the reorganize the way we manage the in-core commit data topic, and the latter reverts the Use SSE to micro-optimize a leaf function to check the format of a ref string. Please give them some quick sanity check. They both look OK to me. Thanks both of you; I appreciate your efforts! I've run my suite of tests against the tips of master and maint and all 681 pass for each. Looks good to me. So what suite of tests is this? Is it worth getting you to fold it into our regular test suite? :) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git diff-tree commit detail bug in 2.0.2 and 2.0.3
Using git diff-tree --stdin on 2.0.2 and 2.0.3 produces incorrect commit messages. Here's an example to reproduce the issue: bturner@ubuntu:/tmp$ git init --bare test.git Initialized empty Git repository in /tmp/test.git/ bturner@ubuntu:/tmp$ git clone test.git Cloning into 'test'... warning: You appear to have cloned an empty repository. done. bturner@ubuntu:/tmp$ cd test bturner@ubuntu:/tmp/test$ echo Hello file.txt bturner@ubuntu:/tmp/test$ git add file.txt bturner@ubuntu:/tmp/test$ git commit -m Initial commit [master (root-commit) c5e16f3] Initial commit 1 file changed, 1 insertion(+) create mode 100644 file.txt bturner@ubuntu:/tmp/test$ echo World file.txt bturner@ubuntu:/tmp/test$ git commit -am Second commit [master 9214ac7] Second commit 1 file changed, 1 insertion(+) bturner@ubuntu:/tmp/test$ git push origin HEAD Counting objects: 6, done. Delta compression using up to 4 threads. Compressing objects: 100% (2/2), done. Writing objects: 100% (6/6), 446 bytes | 0 bytes/s, done. Total 6 (delta 0), reused 0 (delta 0) To /tmp/test.git * [new branch] HEAD - master bturner@ubuntu:/tmp/test$ cd ../test.git/ bturner@ubuntu:/tmp/test.git$ git rev-list -1 --format=%H|%h|%P|%p|%aN|%aE|%at%n%B%n 9214ac7 commit 9214ac79728424a971244c34432c6d948754198d 9214ac79728424a971244c34432c6d948754198d|9214ac7|c5e16f37164f1b7411685def64d7390775437f07|c5e16f3|Bryan Turner|btur...@atlassian.com|1406539558 Second commit bturner@ubuntu:/tmp/test.git$ /opt/git/2.0.3/bin/git diff-tree --no-renames --always --format=commit %H%n%H|%h|%P|%p|%aN|%aE|%at|%B%n --root 9214ac79728424a971244c34432c6d948754198d commit 9214ac79728424a971244c34432c6d948754198d 9214ac79728424a971244c34432c6d948754198d|9214ac79728424a971244c34432c6d948754198d|c5e16f37164f1b7411685def64d7390775437f07|c5e16f37164f1b7411685def64d7390775437f07|Bryan Turner|btur...@atlassian.com|1406539558|Second commit :100644 100644 e965047ad7c57865823c7d992b1d046ea66edf78 f9264f7fbd31ae7a18b7931ed8946fb0aebb0af3 Mfile.txt bturner@ubuntu:/tmp/test.git$ /opt/git/2.0.3/bin/git diff-tree --no-renames --always --format=commit %H%n%H|%h|%P|%p|%aN|%aE|%at|%B%n --root --stdin --9214ac79728424a971244c34432c6d948754198d commit 9214ac79728424a971244c34432c6d948754198d 9214ac79728424a971244c34432c6d948754198d|9214ac79728424a971244c34432c6d948754198d|c5e16f37164f1b7411685def64d7390775437f07|c5e16f37164f1b7411685def64d7390775437f07|Bryan Turner|btur...@atlassian.com|1406539543|Initial commit :100644 100644 e965047ad7c57865823c7d992b1d046ea66edf78 f9264f7fbd31ae7a18b7931ed8946fb0aebb0af3 Mfile.txt bturner@ubuntu:/tmp/test.git$ Running a git bisect between v2.0.1, which does not manifest this issue, and v2.0.2 fingers the following commit: bturner@ubuntu:~/Development/oss/git/git$ git bisect bad c1b3c71f4b4571abb2b2a457122fd100dc9f7eb0 is the first bad commit commit c1b3c71f4b4571abb2b2a457122fd100dc9f7eb0 Author: Jeff King p...@peff.net Date: Tue Jun 10 17:43:02 2014 -0400 commit: convert commit-buffer to a slab Bryan Turner -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git diff-tree commit detail bug in 2.0.2 and 2.0.3
On 28/07/14 10:42, Bryan Turner wrote: Using git diff-tree --stdin on 2.0.2 and 2.0.3 produces incorrect commit messages. Here's an example to reproduce the issue: bturner@ubuntu:/tmp$ git init --bare test.git Initialized empty Git repository in /tmp/test.git/ bturner@ubuntu:/tmp$ git clone test.git Cloning into 'test'... warning: You appear to have cloned an empty repository. done. bturner@ubuntu:/tmp$ cd test bturner@ubuntu:/tmp/test$ echo Hello file.txt bturner@ubuntu:/tmp/test$ git add file.txt bturner@ubuntu:/tmp/test$ git commit -m Initial commit [master (root-commit) c5e16f3] Initial commit 1 file changed, 1 insertion(+) create mode 100644 file.txt bturner@ubuntu:/tmp/test$ echo World file.txt bturner@ubuntu:/tmp/test$ git commit -am Second commit [master 9214ac7] Second commit 1 file changed, 1 insertion(+) bturner@ubuntu:/tmp/test$ git push origin HEAD Counting objects: 6, done. Delta compression using up to 4 threads. Compressing objects: 100% (2/2), done. Writing objects: 100% (6/6), 446 bytes | 0 bytes/s, done. Total 6 (delta 0), reused 0 (delta 0) To /tmp/test.git * [new branch] HEAD - master bturner@ubuntu:/tmp/test$ cd ../test.git/ bturner@ubuntu:/tmp/test.git$ git rev-list -1 --format=%H|%h|%P|%p|%aN|%aE|%at%n%B%n 9214ac7 commit 9214ac79728424a971244c34432c6d948754198d 9214ac79728424a971244c34432c6d948754198d|9214ac7|c5e16f37164f1b7411685def64d7390775437f07|c5e16f3|Bryan Turner|btur...@atlassian.com|1406539558 Second commit bturner@ubuntu:/tmp/test.git$ /opt/git/2.0.3/bin/git diff-tree ^^^ You appear to have used v2.0.3 on both invocations of diff-tree (see also below); cut-n-paste error? --no-renames --always --format=commit %H%n%H|%h|%P|%p|%aN|%aE|%at|%B%n --root 9214ac79728424a971244c34432c6d948754198d commit 9214ac79728424a971244c34432c6d948754198d 9214ac79728424a971244c34432c6d948754198d|9214ac79728424a971244c34432c6d948754198d|c5e16f37164f1b7411685def64d7390775437f07|c5e16f37164f1b7411685def64d7390775437f07|Bryan Turner|btur...@atlassian.com|1406539558|Second commit :100644 100644 e965047ad7c57865823c7d992b1d046ea66edf78 f9264f7fbd31ae7a18b7931ed8946fb0aebb0af3 Mfile.txt bturner@ubuntu:/tmp/test.git$ /opt/git/2.0.3/bin/git diff-tree --no-renames --always --format=commit %H%n%H|%h|%P|%p|%aN|%aE|%at|%B%n --root --stdin --9214ac79728424a971244c34432c6d948754198d commit 9214ac79728424a971244c34432c6d948754198d 9214ac79728424a971244c34432c6d948754198d|9214ac79728424a971244c34432c6d948754198d|c5e16f37164f1b7411685def64d7390775437f07|c5e16f37164f1b7411685def64d7390775437f07|Bryan Turner|btur...@atlassian.com|1406539543|Initial commit ---^^ The timestamp is also different than the above. :100644 100644 e965047ad7c57865823c7d992b1d046ea66edf78 f9264f7fbd31ae7a18b7931ed8946fb0aebb0af3 Mfile.txt bturner@ubuntu:/tmp/test.git$ Running a git bisect between v2.0.1, which does not manifest this issue, and v2.0.2 fingers the following commit: bturner@ubuntu:~/Development/oss/git/git$ git bisect bad c1b3c71f4b4571abb2b2a457122fd100dc9f7eb0 is the first bad commit commit c1b3c71f4b4571abb2b2a457122fd100dc9f7eb0 Author: Jeff King p...@peff.net Date: Tue Jun 10 17:43:02 2014 -0400 commit: convert commit-buffer to a slab ATB, Ramsay Jones -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git diff-tree commit detail bug in 2.0.2 and 2.0.3
On Mon, Jul 28, 2014 at 07:42:16PM +1000, Bryan Turner wrote: Running a git bisect between v2.0.1, which does not manifest this issue, and v2.0.2 fingers the following commit: bturner@ubuntu:~/Development/oss/git/git$ git bisect bad c1b3c71f4b4571abb2b2a457122fd100dc9f7eb0 is the first bad commit commit c1b3c71f4b4571abb2b2a457122fd100dc9f7eb0 Author: Jeff King p...@peff.net Date: Tue Jun 10 17:43:02 2014 -0400 commit: convert commit-buffer to a slab I haven't reproduced here yet, but this is almost certainly the bug where lookup_unknown_object causes a bogus commit-index field (and prior to the commit you found, diff-tree did not use commit-index). The series that Junio has in jk/alloc-commit-id should fix the problem (it's in master already, and slated for v2.1.0). Junio, we should consider a v2.0.4 with that series, I think. This is a pretty serious regression in diff-tree (I didn't even realize that the buffer-slab work went into the maint series; that may have been a little ambitious). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git diff-tree commit detail bug in 2.0.2 and 2.0.3
On Mon, Jul 28, 2014 at 06:35:04AM -0400, Jeff King wrote: I haven't reproduced here yet, but this is almost certainly the bug where lookup_unknown_object causes a bogus commit-index field (and prior to the commit you found, diff-tree did not use commit-index). The series that Junio has in jk/alloc-commit-id should fix the problem (it's in master already, and slated for v2.1.0). Yep, that's definitely it. Here's the minimum reproduction: git init git commit --allow-empty -m one git commit --allow-empty -m two git rev-list HEAD | git diff-tree --stdin --always --format=%s That yields: one one on v2.0.3, but merging in jk/alloc-commit-id yields: two one -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git diff-tree commit detail bug in 2.0.2 and 2.0.3
On Mon, Jul 28, 2014 at 8:44 PM, Jeff King p...@peff.net wrote: On Mon, Jul 28, 2014 at 06:35:04AM -0400, Jeff King wrote: I haven't reproduced here yet, but this is almost certainly the bug where lookup_unknown_object causes a bogus commit-index field (and prior to the commit you found, diff-tree did not use commit-index). The series that Junio has in jk/alloc-commit-id should fix the problem (it's in master already, and slated for v2.1.0). Yep, that's definitely it. Here's the minimum reproduction: git init git commit --allow-empty -m one git commit --allow-empty -m two git rev-list HEAD | git diff-tree --stdin --always --format=%s That yields: one one on v2.0.3, but merging in jk/alloc-commit-id yields: two one -Peff Thanks for digging into it, Jeff. I should have tried it against 2.1.0 myself. I've run my entire matrix of tests now against 2.1.0-rc0 and the diff-tree bug appears fixed on that tag. I noticed a different change, though: bturner@ubuntu:~/tmp/test$ /opt/git/2.1.0-rc0/bin/git check-ref-format ref/with/trailing/dot. bturner@ubuntu:~/tmp/test$ echo $? 0 bturner@ubuntu:~/tmp/test$ /opt/git/2.0.3/bin/git check-ref-format ref/with/trailing/dot. bturner@ubuntu:~/tmp/test$ echo $? 1 It looks like refs ending in a dot are now legal in 2.1.0? Is that intentional? A quick git bisect is fingering: bturner@ubuntu:~/Development/oss/git/git$ git bisect bad 745224e04a03e4544c58d5d38d3c54f67100f8eb is the first bad commit commit 745224e04a03e4544c58d5d38d3c54f67100f8eb Author: David Turner dtur...@twopensource.com Date: Wed Jun 18 01:54:42 2014 -0400 Best regards, Bryan Turner -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git diff-tree commit detail bug in 2.0.2 and 2.0.3
Bryan Turner btur...@atlassian.com writes: It looks like refs ending in a dot are now legal in 2.1.0? Is that intentional? A quick git bisect is fingering: bturner@ubuntu:~/Development/oss/git/git$ git bisect bad 745224e04a03e4544c58d5d38d3c54f67100f8eb is the first bad commit commit 745224e04a03e4544c58d5d38d3c54f67100f8eb Author: David Turner dtur...@twopensource.com Date: Wed Jun 18 01:54:42 2014 -0400 Thanks for a report. I am tempted to revert that series; it already caused oops, this needs a further fix before it hit 'master' at least once, and we do not want any more headaches at this point in the release cycle. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git diff-tree commit detail bug in 2.0.2 and 2.0.3
On Mon, Jul 28, 2014 at 08:35:52AM -0700, Junio C Hamano wrote: I am tempted to revert that series; it already caused oops, this needs a further fix before it hit 'master' at least once, and we do not want any more headaches at this point in the release cycle. Yeah, that sounds reasonable to me. I'm a little doubtful of its value (and maintainability) at all, but certainly I do not think it would be a big deal to push it off for one more cycle if David wants to rework it. If you do revert it, we may want a test like below. That will make sure the case is covered for future attempts. -- 8 -- Subject: [PATCH] t1402: check for refs ending with a dot This has been illegal since cbdffe4 (check_ref_format(): tighten refname rules, 2009-03-21), but we never tested it. Signed-off-by: Jeff King p...@peff.net --- t/t1402-check-ref-format.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh index 9aeb352..8d2f75f 100755 --- a/t/t1402-check-ref-format.sh +++ b/t/t1402-check-ref-format.sh @@ -48,6 +48,7 @@ invalid_ref './foo/bar' invalid_ref 'foo/./bar' invalid_ref 'foo/bar/.' invalid_ref '.refs/foo' +invalid_ref 'refs/heads/foo.' invalid_ref 'heads/foo..bar' invalid_ref 'heads/foo?bar' valid_ref 'foo./bar' -- 2.0.0.566.gfe3e6b2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git diff-tree commit detail bug in 2.0.2 and 2.0.3
Jeff King p...@peff.net writes: On Mon, Jul 28, 2014 at 07:42:16PM +1000, Bryan Turner wrote: Running a git bisect between v2.0.1, which does not manifest this issue, and v2.0.2 fingers the following commit: bturner@ubuntu:~/Development/oss/git/git$ git bisect bad c1b3c71f4b4571abb2b2a457122fd100dc9f7eb0 is the first bad commit commit c1b3c71f4b4571abb2b2a457122fd100dc9f7eb0 Author: Jeff King p...@peff.net Date: Tue Jun 10 17:43:02 2014 -0400 commit: convert commit-buffer to a slab I haven't reproduced here yet, but this is almost certainly the bug where lookup_unknown_object causes a bogus commit-index field (and prior to the commit you found, diff-tree did not use commit-index). The series that Junio has in jk/alloc-commit-id should fix the problem (it's in master already, and slated for v2.1.0). Junio, we should consider a v2.0.4 with that series, I think. This is a pretty serious regression in diff-tree (I didn't even realize that the buffer-slab work went into the maint series; that may have been a little ambitious). Or v2.0.4 without that series, which is how we usually do things, but let me see if jk/alloc-commit-id is easily applicable there first. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git diff-tree commit detail bug in 2.0.2 and 2.0.3
On Mon, Jul 28, 2014 at 10:32:45AM -0700, Junio C Hamano wrote: Junio, we should consider a v2.0.4 with that series, I think. This is a pretty serious regression in diff-tree (I didn't even realize that the buffer-slab work went into the maint series; that may have been a little ambitious). Or v2.0.4 without that series, which is how we usually do things, but let me see if jk/alloc-commit-id is easily applicable there first. Yeah, I'm fine with a straight revert, too (I think it is fine to keep in master, though). I think jk/alloc-commit-id is built right on top of the original commit-slab topic, so it should be easy to do either way. Thanks for dealing with it. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git diff-tree commit detail bug in 2.0.2 and 2.0.3
On Mon, Jul 28, 2014 at 01:37:34PM -0400, Jeff King wrote: On Mon, Jul 28, 2014 at 10:32:45AM -0700, Junio C Hamano wrote: Junio, we should consider a v2.0.4 with that series, I think. This is a pretty serious regression in diff-tree (I didn't even realize that the buffer-slab work went into the maint series; that may have been a little ambitious). Or v2.0.4 without that series, which is how we usually do things, but let me see if jk/alloc-commit-id is easily applicable there first. Yeah, I'm fine with a straight revert, too (I think it is fine to keep in master, though). I think jk/alloc-commit-id is built right on top of the original commit-slab topic, so it should be easy to do either way. Thanks for dealing with it. Whatever we do, perhaps it is worth applying the test below on top? -- 8 -- Subject: t4013: test diff-tree's --stdin commit formatting Once upon a time, git-log was just rev-list | diff-tree, and we did not bother to test it separately. These days git-log is implemented internally, but we want to make sure that the rev-list to diff-tree pipeline continues to function. Let's add a basic sanity test. Signed-off-by: Jeff King p...@peff.net --- t/t4013-diff-various.sh | 10 ++ 1 file changed, 10 insertions(+) diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index 805b055..6ec6072 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -324,4 +324,14 @@ test_expect_success 'diff --cached -- file on unborn branch' ' test_cmp $TEST_DIRECTORY/t4013/diff.diff_--cached_--_file0 result ' +test_expect_success 'diff-tree --stdin with log formatting' ' + cat expect -\EOF + Side + Third + Second + EOF + git rev-list master | git diff-tree --stdin --format=%s -s actual + test_cmp expect actual +' + test_done -- 2.0.0.566.gfe3e6b2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git diff-tree commit detail bug in 2.0.2 and 2.0.3
Jeff King p...@peff.net writes: On Mon, Jul 28, 2014 at 01:37:34PM -0400, Jeff King wrote: On Mon, Jul 28, 2014 at 10:32:45AM -0700, Junio C Hamano wrote: Junio, we should consider a v2.0.4 with that series, I think. This is a pretty serious regression in diff-tree (I didn't even realize that the buffer-slab work went into the maint series; that may have been a little ambitious). Or v2.0.4 without that series, which is how we usually do things, but let me see if jk/alloc-commit-id is easily applicable there first. Yeah, I'm fine with a straight revert, too (I think it is fine to keep in master, though). I think jk/alloc-commit-id is built right on top of the original commit-slab topic, so it should be easy to do either way. Thanks for dealing with it. Whatever we do, perhaps it is worth applying the test below on top? Yeah, thanks. I think that is a good idea. I was preparing a patch to tuck your minimum reproduction at the end of 4202, but your version and placement makes good sense. -- 8 -- Subject: t4013: test diff-tree's --stdin commit formatting Once upon a time, git-log was just rev-list | diff-tree, and we did not bother to test it separately. These days git-log is implemented internally, but we want to make sure that the rev-list to diff-tree pipeline continues to function. Let's add a basic sanity test. Signed-off-by: Jeff King p...@peff.net --- t/t4013-diff-various.sh | 10 ++ 1 file changed, 10 insertions(+) diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index 805b055..6ec6072 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -324,4 +324,14 @@ test_expect_success 'diff --cached -- file on unborn branch' ' test_cmp $TEST_DIRECTORY/t4013/diff.diff_--cached_--_file0 result ' +test_expect_success 'diff-tree --stdin with log formatting' ' + cat expect -\EOF + Side + Third + Second + EOF + git rev-list master | git diff-tree --stdin --format=%s -s actual + test_cmp expect actual +' + test_done -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git diff-tree commit detail bug in 2.0.2 and 2.0.3
Junio C Hamano gits...@pobox.com writes: Yeah, I'm fine with a straight revert, too (I think it is fine to keep in master, though). I think jk/alloc-commit-id is built right on top of the original commit-slab topic, so it should be easy to do either way. Thanks for dealing with it. Whatever we do, perhaps it is worth applying the test below on top? Yeah, thanks. I think that is a good idea. I was preparing a patch to tuck your minimum reproduction at the end of 4202, but your version and placement makes good sense. OK, I pushed out updated 'maint' and 'master'. The former merges a rebased version of jk/alloc-commit-id in to make the reorganize the way we manage the in-core commit data topic, and the latter reverts the Use SSE to micro-optimize a leaf function to check the format of a ref string. Please give them some quick sanity check. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git diff-tree commit detail bug in 2.0.2 and 2.0.3
On Tue, Jul 29, 2014 at 10:11 AM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: Yeah, I'm fine with a straight revert, too (I think it is fine to keep in master, though). I think jk/alloc-commit-id is built right on top of the original commit-slab topic, so it should be easy to do either way. Thanks for dealing with it. Whatever we do, perhaps it is worth applying the test below on top? Yeah, thanks. I think that is a good idea. I was preparing a patch to tuck your minimum reproduction at the end of 4202, but your version and placement makes good sense. OK, I pushed out updated 'maint' and 'master'. The former merges a rebased version of jk/alloc-commit-id in to make the reorganize the way we manage the in-core commit data topic, and the latter reverts the Use SSE to micro-optimize a leaf function to check the format of a ref string. Please give them some quick sanity check. Thanks. Thanks both of you; I appreciate your efforts! I've run my suite of tests against the tips of master and maint and all 681 pass for each. Looks good to me. Best regards, Bryan Turner -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
