From 32d8b4226a22ef2b4c22cddda7e134980bbabaf6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 3 Oct 2016 16:33:51 -0400 Subject: t5613: drop reachable_via function This function was never used since its inception in dd05ea1 (test case for transitive info/alternates, 2006-05-07). Which is just as well, since it mutates the repo state in a way that would invalidate further tests, without cleaning up after itself. Let's get rid of it so that nobody is tempted to use it. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t5613-info-alternate.sh | 10 ---------- 1 file changed, 10 deletions(-) (limited to 't/t5613-info-alternate.sh') diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh index 9cd2626dba..e13f57d262 100755 --- a/t/t5613-info-alternate.sh +++ b/t/t5613-info-alternate.sh @@ -6,16 +6,6 @@ test_description='test transitive info/alternate entries' . ./test-lib.sh -# test that a file is not reachable in the current repository -# but that it is after creating a info/alternate entry -reachable_via() { - alternate="$1" - file="$2" - if git cat-file -e "HEAD:$file"; then return 1; fi - echo "$alternate" >> .git/objects/info/alternate - git cat-file -e "HEAD:$file" -} - test_valid_repo() { git fsck --full > fsck.log && test_line_count = 0 fsck.log -- cgit v1.2.3 From b28a88f26a21e6134a099e2d57aa088bc6d93818 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 3 Oct 2016 16:33:58 -0400 Subject: t5613: drop test_valid_repo function This function makes sure that "git fsck" does not report any errors. But "--full" has been the default since f29cd39 (fsck: default to "git fsck --full", 2009-10-20), and we can use the exit code (instead of counting the lines) since e2b4f63 (fsck: exit with non-zero status upon errors, 2007-03-05). So we can just use "git fsck", which is shorter and more flexible (e.g., we can use "git -C"). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t5613-info-alternate.sh | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) (limited to 't/t5613-info-alternate.sh') diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh index e13f57d262..4548fb0ab9 100755 --- a/t/t5613-info-alternate.sh +++ b/t/t5613-info-alternate.sh @@ -6,11 +6,6 @@ test_description='test transitive info/alternate entries' . ./test-lib.sh -test_valid_repo() { - git fsck --full > fsck.log && - test_line_count = 0 fsck.log -} - base_dir=$(pwd) test_expect_success 'preparing first repository' \ @@ -52,7 +47,7 @@ git clone --bare -l -s G H' test_expect_success 'invalidity of deepest repository' \ 'cd H && { - test_valid_repo + git fsck test $? -ne 0 }' @@ -60,41 +55,41 @@ cd "$base_dir" test_expect_success 'validity of third repository' \ 'cd C && -test_valid_repo' +git fsck' cd "$base_dir" test_expect_success 'validity of fourth repository' \ 'cd D && -test_valid_repo' +git fsck' cd "$base_dir" test_expect_success 'breaking of loops' \ 'echo "$base_dir"/B/.git/objects >> "$base_dir"/A/.git/objects/info/alternates&& cd C && -test_valid_repo' +git fsck' cd "$base_dir" test_expect_success 'that info/alternates is necessary' \ 'cd C && rm -f .git/objects/info/alternates && -! (test_valid_repo)' +! (git fsck)' cd "$base_dir" test_expect_success 'that relative alternate is possible for current dir' \ 'cd C && echo "../../../B/.git/objects" > .git/objects/info/alternates && -test_valid_repo' +git fsck' cd "$base_dir" test_expect_success \ 'that relative alternate is only possible for current dir' ' cd D && - ! (test_valid_repo) + ! (git fsck) ' cd "$base_dir" -- cgit v1.2.3 From 195eb4654631bf6747649e22b0776c988669d829 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 3 Oct 2016 16:34:01 -0400 Subject: t5613: use test_must_fail Besides being our normal style, this correctly checks for an error exit() versus signal death. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t5613-info-alternate.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 't/t5613-info-alternate.sh') diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh index 4548fb0ab9..65074ddc13 100755 --- a/t/t5613-info-alternate.sh +++ b/t/t5613-info-alternate.sh @@ -46,10 +46,9 @@ git clone -l -s F G && git clone --bare -l -s G H' test_expect_success 'invalidity of deepest repository' \ -'cd H && { - git fsck - test $? -ne 0 -}' +'cd H && +test_must_fail git fsck +' cd "$base_dir" @@ -75,7 +74,8 @@ cd "$base_dir" test_expect_success 'that info/alternates is necessary' \ 'cd C && rm -f .git/objects/info/alternates && -! (git fsck)' +test_must_fail git fsck +' cd "$base_dir" @@ -89,7 +89,7 @@ cd "$base_dir" test_expect_success \ 'that relative alternate is only possible for current dir' ' cd D && - ! (git fsck) + test_must_fail git fsck ' cd "$base_dir" -- cgit v1.2.3 From 128a348d049b9c793b89bde4d2ad57feef4e0f26 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 3 Oct 2016 16:34:05 -0400 Subject: t5613: whitespace/style cleanups Our normal test style these days puts the opening quote of the body on the description line, and indents the body with a single tab. This ancient test did not follow this. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t5613-info-alternate.sh | 114 +++++++++++++++++++++++++--------------------- 1 file changed, 62 insertions(+), 52 deletions(-) (limited to 't/t5613-info-alternate.sh') diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh index 65074ddc13..1f283a5567 100755 --- a/t/t5613-info-alternate.sh +++ b/t/t5613-info-alternate.sh @@ -8,88 +8,98 @@ test_description='test transitive info/alternate entries' base_dir=$(pwd) -test_expect_success 'preparing first repository' \ -'test_create_repo A && cd A && -echo "Hello World" > file1 && -git add file1 && -git commit -m "Initial commit" file1 && -git repack -a -d && -git prune' +test_expect_success 'preparing first repository' ' + test_create_repo A && + cd A && + echo "Hello World" > file1 && + git add file1 && + git commit -m "Initial commit" file1 && + git repack -a -d && + git prune +' cd "$base_dir" -test_expect_success 'preparing second repository' \ -'git clone -l -s A B && cd B && -echo "foo bar" > file2 && -git add file2 && -git commit -m "next commit" file2 && -git repack -a -d -l && -git prune' +test_expect_success 'preparing second repository' ' + git clone -l -s A B && + cd B && + echo "foo bar" > file2 && + git add file2 && + git commit -m "next commit" file2 && + git repack -a -d -l && + git prune +' cd "$base_dir" -test_expect_success 'preparing third repository' \ -'git clone -l -s B C && cd C && -echo "Goodbye, cruel world" > file3 && -git add file3 && -git commit -m "one more" file3 && -git repack -a -d -l && -git prune' +test_expect_success 'preparing third repository' ' + git clone -l -s B C && + cd C && + echo "Goodbye, cruel world" > file3 && + git add file3 && + git commit -m "one more" file3 && + git repack -a -d -l && + git prune +' cd "$base_dir" -test_expect_success 'creating too deep nesting' \ -'git clone -l -s C D && -git clone -l -s D E && -git clone -l -s E F && -git clone -l -s F G && -git clone --bare -l -s G H' +test_expect_success 'creating too deep nesting' ' + git clone -l -s C D && + git clone -l -s D E && + git clone -l -s E F && + git clone -l -s F G && + git clone --bare -l -s G H +' -test_expect_success 'invalidity of deepest repository' \ -'cd H && -test_must_fail git fsck +test_expect_success 'invalidity of deepest repository' ' + cd H && + test_must_fail git fsck ' cd "$base_dir" -test_expect_success 'validity of third repository' \ -'cd C && -git fsck' +test_expect_success 'validity of third repository' ' + cd C && + git fsck +' cd "$base_dir" -test_expect_success 'validity of fourth repository' \ -'cd D && -git fsck' +test_expect_success 'validity of fourth repository' ' + cd D && + git fsck +' cd "$base_dir" -test_expect_success 'breaking of loops' \ -'echo "$base_dir"/B/.git/objects >> "$base_dir"/A/.git/objects/info/alternates&& -cd C && -git fsck' +test_expect_success 'breaking of loops' ' + echo "$base_dir"/B/.git/objects >>"$base_dir"/A/.git/objects/info/alternatesi && + cd C && + git fsck +' cd "$base_dir" -test_expect_success 'that info/alternates is necessary' \ -'cd C && -rm -f .git/objects/info/alternates && -test_must_fail git fsck +test_expect_success 'that info/alternates is necessary' ' + cd C && + rm -f .git/objects/info/alternates && + test_must_fail git fsck ' cd "$base_dir" -test_expect_success 'that relative alternate is possible for current dir' \ -'cd C && -echo "../../../B/.git/objects" > .git/objects/info/alternates && -git fsck' +test_expect_success 'that relative alternate is possible for current dir' ' + cd C && + echo "../../../B/.git/objects" > .git/objects/info/alternates && + git fsck +' cd "$base_dir" -test_expect_success \ - 'that relative alternate is only possible for current dir' ' - cd D && - test_must_fail git fsck +test_expect_success 'that relative alternate is only possible for current dir' ' + cd D && + test_must_fail git fsck ' cd "$base_dir" -- cgit v1.2.3 From 8f2ed2675358a6e6543f962f6e1b3ec0737e8079 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 3 Oct 2016 16:34:08 -0400 Subject: t5613: do not chdir in main process Our usual style when working with subdirectories is to chdir inside a subshell or to use "git -C", which means we do not have to constantly return to the main test directory. Let's convert this old test, which does not follow that style. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t5613-info-alternate.sh | 92 +++++++++++++++++------------------------------ 1 file changed, 33 insertions(+), 59 deletions(-) (limited to 't/t5613-info-alternate.sh') diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh index 1f283a5567..7bc1c3caed 100755 --- a/t/t5613-info-alternate.sh +++ b/t/t5613-info-alternate.sh @@ -6,44 +6,39 @@ test_description='test transitive info/alternate entries' . ./test-lib.sh -base_dir=$(pwd) - test_expect_success 'preparing first repository' ' - test_create_repo A && - cd A && - echo "Hello World" > file1 && - git add file1 && - git commit -m "Initial commit" file1 && - git repack -a -d && - git prune + test_create_repo A && ( + cd A && + echo "Hello World" > file1 && + git add file1 && + git commit -m "Initial commit" file1 && + git repack -a -d && + git prune + ) ' -cd "$base_dir" - test_expect_success 'preparing second repository' ' - git clone -l -s A B && - cd B && - echo "foo bar" > file2 && - git add file2 && - git commit -m "next commit" file2 && - git repack -a -d -l && - git prune + git clone -l -s A B && ( + cd B && + echo "foo bar" > file2 && + git add file2 && + git commit -m "next commit" file2 && + git repack -a -d -l && + git prune + ) ' -cd "$base_dir" - test_expect_success 'preparing third repository' ' - git clone -l -s B C && - cd C && - echo "Goodbye, cruel world" > file3 && - git add file3 && - git commit -m "one more" file3 && - git repack -a -d -l && - git prune + git clone -l -s B C && ( + cd C && + echo "Goodbye, cruel world" > file3 && + git add file3 && + git commit -m "one more" file3 && + git repack -a -d -l && + git prune + ) ' -cd "$base_dir" - test_expect_success 'creating too deep nesting' ' git clone -l -s C D && git clone -l -s D E && @@ -53,55 +48,34 @@ test_expect_success 'creating too deep nesting' ' ' test_expect_success 'invalidity of deepest repository' ' - cd H && - test_must_fail git fsck + test_must_fail git -C H fsck ' -cd "$base_dir" - test_expect_success 'validity of third repository' ' - cd C && - git fsck + git -C C fsck ' -cd "$base_dir" - test_expect_success 'validity of fourth repository' ' - cd D && - git fsck + git -C D fsck ' -cd "$base_dir" - test_expect_success 'breaking of loops' ' - echo "$base_dir"/B/.git/objects >>"$base_dir"/A/.git/objects/info/alternatesi && - cd C && - git fsck + echo "$(pwd)"/B/.git/objects >>A/.git/objects/info/alternates && + git -C C fsck ' -cd "$base_dir" - test_expect_success 'that info/alternates is necessary' ' - cd C && - rm -f .git/objects/info/alternates && - test_must_fail git fsck + rm -f C/.git/objects/info/alternates && + test_must_fail git -C C fsck ' -cd "$base_dir" - test_expect_success 'that relative alternate is possible for current dir' ' - cd C && - echo "../../../B/.git/objects" > .git/objects/info/alternates && + echo "../../../B/.git/objects" >C/.git/objects/info/alternates && git fsck ' -cd "$base_dir" - test_expect_success 'that relative alternate is only possible for current dir' ' - cd D && - test_must_fail git fsck + test_must_fail git -C D fsck ' -cd "$base_dir" - test_done -- cgit v1.2.3 From f0f86fa9f3e2c0199d492ce7779761f1b5470b65 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 3 Oct 2016 16:34:12 -0400 Subject: t5613: clarify "too deep" recursion tests These tests are just trying to show that we allow recursion up to a certain depth, but not past it. But the counting is a bit non-intuitive, and rather than test at the edge of the breakage, we test "OK" cases in the middle of the chain. Let's explain what's going on, and explicitly test the switch between "OK" and "too deep". Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t5613-info-alternate.sh | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) (limited to 't/t5613-info-alternate.sh') diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh index 7bc1c3caed..62170b7659 100755 --- a/t/t5613-info-alternate.sh +++ b/t/t5613-info-alternate.sh @@ -39,6 +39,21 @@ test_expect_success 'preparing third repository' ' ) ' +# Note: These tests depend on the hard-coded value of 5 as the maximum depth +# we will follow recursion. We start the depth at 0 and count links, not +# repositories. This means that in a chain like: +# +# A --> B --> C --> D --> E --> F --> G --> H +# 0 1 2 3 4 5 6 +# +# we are OK at "G", but break at "H", even though "H" is actually the 8th +# repository, not the 6th, which you might expect. Counting the links allows +# N+1 repositories, and counting from 0 to 5 inclusive allows 6 links. +# +# Note also that we must use "--bare -l" to make the link to H. The "-l" +# ensures we do not do a connectivity check, and the "--bare" makes sure +# we do not try to checkout the result (which needs objects), either of +# which would cause the clone to fail. test_expect_success 'creating too deep nesting' ' git clone -l -s C D && git clone -l -s D E && @@ -47,16 +62,12 @@ test_expect_success 'creating too deep nesting' ' git clone --bare -l -s G H ' -test_expect_success 'invalidity of deepest repository' ' - test_must_fail git -C H fsck -' - -test_expect_success 'validity of third repository' ' - git -C C fsck +test_expect_success 'validity of seventh repository' ' + git -C G fsck ' -test_expect_success 'validity of fourth repository' ' - git -C D fsck +test_expect_success 'invalidity of eighth repository' ' + test_must_fail git -C H fsck ' test_expect_success 'breaking of loops' ' -- cgit v1.2.3 From 5fe849d651e259af58f29f9cfb1b1405154ffacc Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 3 Oct 2016 16:36:18 -0400 Subject: count-objects: report alternates via verbose mode There's no way to get the list of alternates that git computes internally; our tests only infer it based on which objects are available. In addition to testing, knowing this list may be helpful for somebody debugging their alternates setup. Let's add it to the "count-objects -v" output. We could give it a separate flag, but there's not really any need. "count-objects -v" is already a debugging catch-all for the object database, its output is easily extensible to new data items, and printing the alternates is not expensive (we already had to find them to count the objects). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t5613-info-alternate.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 't/t5613-info-alternate.sh') diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh index 62170b7659..e146a8def6 100755 --- a/t/t5613-info-alternate.sh +++ b/t/t5613-info-alternate.sh @@ -39,6 +39,16 @@ test_expect_success 'preparing third repository' ' ) ' +test_expect_success 'count-objects shows the alternates' ' + cat >expect <<-EOF && + alternate: $(pwd)/B/.git/objects + alternate: $(pwd)/A/.git/objects + EOF + git -C C count-objects -v >actual && + grep ^alternate: actual >actual.alternates && + test_cmp expect actual.alternates +' + # Note: These tests depend on the hard-coded value of 5 as the maximum depth # we will follow recursion. We start the depth at 0 and count links, not # repositories. This means that in a chain like: -- cgit v1.2.3 From 087b6d584062f5b704356286d6445bcc84d686fb Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 3 Oct 2016 16:36:22 -0400 Subject: sha1_file: always allow relative paths to alternates We recursively expand alternates repositories, so that if A borrows from B which borrows from C, A can see all objects. For the root object database, we allow relative paths, so A can point to B as "../B/objects". However, we currently do not allow relative paths when recursing, so B must use an absolute path to reach C. That is an ancient protection from c2f493a (Transitively read alternatives, 2006-05-07) that tries to avoid adding the same alternate through two different paths. Since 5bdf0a8 (sha1_file: normalize alt_odb path before comparing and storing, 2011-09-07), we use a normalized absolute path for each alt_odb entry. This means that in most cases the protection is no longer necessary; we will detect the duplicate no matter how we got there (but see below). And it's a good idea to get rid of it, as it creates an unnecessary complication when setting up recursive alternates (B has to know that A is going to borrow from it and make sure to use an absolute path). Note that our normalization doesn't actually look at the filesystem, so it can still be fooled by crossing symbolic links. But that's also true of absolute paths, so it's not a good reason to disallow only relative paths (it's potentially a reason to switch to real_path(), but that's a separate and non-trivial change). We adjust the test script here to demonstrate that this now works, and add new tests to show that the normalization does indeed suppress duplicates. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t5613-info-alternate.sh | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) (limited to 't/t5613-info-alternate.sh') diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh index e146a8def6..76f1a20e2c 100755 --- a/t/t5613-info-alternate.sh +++ b/t/t5613-info-alternate.sh @@ -95,8 +95,28 @@ test_expect_success 'that relative alternate is possible for current dir' ' git fsck ' -test_expect_success 'that relative alternate is only possible for current dir' ' - test_must_fail git -C D fsck +test_expect_success 'that relative alternate is recursive' ' + git -C D fsck +' + +# we can reach "A" from our new repo both directly, and via "C". +# The deep/subdir is there to make sure we are not doing a stupid +# pure-text comparison of the alternate names. +test_expect_success 'relative duplicates are eliminated' ' + mkdir -p deep/subdir && + git init --bare deep/subdir/duplicate.git && + cat >deep/subdir/duplicate.git/objects/info/alternates <<-\EOF && + ../../../../C/.git/objects + ../../../../A/.git/objects + EOF + cat >expect <<-EOF && + alternate: $(pwd)/C/.git/objects + alternate: $(pwd)/B/.git/objects + alternate: $(pwd)/A/.git/objects + EOF + git -C deep/subdir/duplicate.git count-objects -v >actual && + grep ^alternate: actual >actual.alternates && + test_cmp expect actual.alternates ' test_done -- cgit v1.2.3 From ea0fc3b4176a424a2b20eb76a6a503dc4d59cebb Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 3 Oct 2016 16:36:26 -0400 Subject: alternates: use fspathcmp to detect duplicates On a case-insensitive filesystem, we should realize that "a/objects" and "A/objects" are the same path. We already use fspathcmp() to check against the main object directory, but until recently we couldn't use it for comparing against other alternates (because their paths were not NUL-terminated strings). But now we can, so let's do so. Note that we also need to adjust count-objects to load the config, so that it can see the setting of core.ignorecase (this is required by the test, but is also a general bugfix for users of count-objects). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t5613-info-alternate.sh | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 't/t5613-info-alternate.sh') diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh index 76f1a20e2c..895f46bb91 100755 --- a/t/t5613-info-alternate.sh +++ b/t/t5613-info-alternate.sh @@ -119,4 +119,21 @@ test_expect_success 'relative duplicates are eliminated' ' test_cmp expect actual.alternates ' +test_expect_success CASE_INSENSITIVE_FS 'dup finding can be case-insensitive' ' + git init --bare insensitive.git && + # the previous entry for "A" will have used uppercase + cat >insensitive.git/objects/info/alternates <<-\EOF && + ../../C/.git/objects + ../../a/.git/objects + EOF + cat >expect <<-EOF && + alternate: $(pwd)/C/.git/objects + alternate: $(pwd)/B/.git/objects + alternate: $(pwd)/A/.git/objects + EOF + git -C insensitive.git count-objects -v >actual && + grep ^alternate: actual >actual.alternates && + test_cmp expect actual.alternates +' + test_done -- cgit v1.2.3