| Message ID | YEj8x5fQl1fyLGNg@coredump.intra.peff.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 6d875d19fd7346f4a6dc47d2a1363db91e9643d5 |
| Headers | show |
| Series | sha256 fixes for filter-branch | expand |
On Wed, Mar 10 2021, Jeff King wrote: I've read it all over, looks good to me. > Note these all pass currently, but the latter two will fail when run > with GIT_TEST_DEFAULT_HASH=sha256. I don't know if anyone relies on that. Probably fine to have this as-is. But wouldn't fixing that just be a matter of adding these tests as test_expect_failure, and then flipping them to test_expect_success in 3/3?
On 2021-03-11 at 00:10:29, Ævar Arnfjörð Bjarmason wrote: > > On Wed, Mar 10 2021, Jeff King wrote: > > > I've read it all over, looks good to me. > > > Note these all pass currently, but the latter two will fail when run > > with GIT_TEST_DEFAULT_HASH=sha256. > > I don't know if anyone relies on that. Probably fine to have this as-is. Our CI runs that way, and I always use that mode. Are you saying that these will always fail in that mode, or that they will pass at the end of the series?
On Thu, Mar 11, 2021 at 01:55:04AM +0000, brian m. carlson wrote: > On 2021-03-11 at 00:10:29, Ævar Arnfjörð Bjarmason wrote: > > > > On Wed, Mar 10 2021, Jeff King wrote: > > > > > > I've read it all over, looks good to me. > > > > > Note these all pass currently, but the latter two will fail when run > > > with GIT_TEST_DEFAULT_HASH=sha256. > > > > I don't know if anyone relies on that. Probably fine to have this as-is. > > Our CI runs that way, and I always use that mode. Are you saying that > these will always fail in that mode, or that they will pass at the end > of the series? The latter; everything passes at the end. The only thing that is hurt is if we care about bisecting the tests with GIT_TEST_DEFAULT_HASH set -Peff
On 2021-03-11 at 02:24:29, Jeff King wrote: > On Thu, Mar 11, 2021 at 01:55:04AM +0000, brian m. carlson wrote: > > > On 2021-03-11 at 00:10:29, Ævar Arnfjörð Bjarmason wrote: > > > > > > On Wed, Mar 10 2021, Jeff King wrote: > > > > > > > Note these all pass currently, but the latter two will fail when run > > > > with GIT_TEST_DEFAULT_HASH=sha256. > > > > > > I don't know if anyone relies on that. Probably fine to have this as-is. > > > > Our CI runs that way, and I always use that mode. Are you saying that > > these will always fail in that mode, or that they will pass at the end > > of the series? > > The latter; everything passes at the end. The only thing that is hurt is > if we care about bisecting the tests with GIT_TEST_DEFAULT_HASH set I don't have a strong opinion on this. I'm unlikely to bisect with the full testsuite in that way; I'd use a git bisect run with a smaller testcase.
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh index 1c55695034..1349e5b232 100755 --- a/t/t7003-filter-branch.sh +++ b/t/t7003-filter-branch.sh @@ -506,4 +506,35 @@ test_expect_success 'rewrite repository including refs that point at non-commit ! fgrep fatal filter-output ' +test_expect_success 'filter-branch handles ref deletion' ' + git switch --orphan empty-commit && + git commit --allow-empty -m "empty commit" && + git tag empty && + git branch to-delete && + git filter-branch -f --prune-empty to-delete >out 2>&1 && + grep "to-delete.*was deleted" out && + test_must_fail git rev-parse --verify to-delete +' + +test_expect_success 'filter-branch handles ref rewrite' ' + git checkout empty && + test_commit to-drop && + git branch rewrite && + git filter-branch -f \ + --index-filter "git rm --ignore-unmatch --cached to-drop.t" \ + rewrite >out 2>&1 && + grep "rewrite.*was rewritten" out && + ! grep -i warning out && + git diff-tree empty rewrite +' + +test_expect_success 'filter-branch handles ancestor rewrite' ' + test_commit to-exclude && + git branch ancestor && + git filter-branch -f ancestor -- :^to-exclude.t >out 2>&1 && + grep "ancestor.*was rewritten" out && + ! grep -i warning out && + git diff-tree HEAD^ ancestor +' + test_done
After it has rewritten all of the commits, filter-branch will then rewrite each of the input refs based on the resulting map of old/new commits. But we don't have any explicit test coverage of this code. Let's make sure we are covering each of those cases: - deleting a ref when all of its commits were pruned - rewriting a ref based on the mapping (this happens throughout the script, but let's make sure we generate the correct messages) - rewriting a ref whose tip was excluded, in which case we rewrite to the nearest ancestor. Note in this case that we still insist that no "warning" line is present (even though it looks like we'd trigger the "... was rewritten into multiple commits" one). See the next commit for more details. Note these all pass currently, but the latter two will fail when run with GIT_TEST_DEFAULT_HASH=sha256. Signed-off-by: Jeff King <peff@peff.net> --- t/t7003-filter-branch.sh | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)