Merge branch 'jk/alt-odb-cleanup'

Codepaths involved in interacting alternate object store have
been cleaned up.

* jk/alt-odb-cleanup:
  alternates: use fspathcmp to detect duplicates
  sha1_file: always allow relative paths to alternates
  count-objects: report alternates via verbose mode
  fill_sha1_file: write into a strbuf
  alternates: store scratch buffer as strbuf
  fill_sha1_file: write "boring" characters
  alternates: use a separate scratch space
  alternates: encapsulate alt->base munging
  alternates: provide helper for allocating alternate
  alternates: provide helper for adding to alternates list
  link_alt_odb_entry: refactor string handling
  link_alt_odb_entry: handle normalize_path errors
  t5613: clarify "too deep" recursion tests
  t5613: do not chdir in main process
  t5613: whitespace/style cleanups
  t5613: use test_must_fail
  t5613: drop test_valid_repo function
  t5613: drop reachable_via function

1 files changed, 116 insertions, 89 deletions

diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh
index 9cd2626dba..895f46bb91 100755
--- a/t/t5613-info-alternate.sh
+++ b/t/t5613-info-alternate.sh
@@ -6,107 +6,134 @@
 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
-}
-
-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'
-
-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'
-
-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'
-
-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 'invalidity of deepest repository' \
-'cd H && {
-	test_valid_repo
-	test $? -ne 0
-}'
-
-cd "$base_dir"
+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 'validity of third repository' \
-'cd C &&
-test_valid_repo'
+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 'validity of fourth repository' \
-'cd D &&
-test_valid_repo'
+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
+'
 
-cd "$base_dir"
+# 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 &&
+	git clone -l -s E F &&
+	git clone -l -s F G &&
+	git clone --bare -l -s G H
+'
 
-test_expect_success 'breaking of loops' \
-'echo "$base_dir"/B/.git/objects >> "$base_dir"/A/.git/objects/info/alternates&&
-cd C &&
-test_valid_repo'
+test_expect_success 'validity of seventh repository' '
+	git -C G fsck
+'
 
-cd "$base_dir"
+test_expect_success 'invalidity of eighth repository' '
+	test_must_fail git -C H fsck
+'
 
-test_expect_success 'that info/alternates is necessary' \
-'cd C &&
-rm -f .git/objects/info/alternates &&
-! (test_valid_repo)'
+test_expect_success 'breaking of loops' '
+	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' '
+	rm -f C/.git/objects/info/alternates &&
+	test_must_fail git -C C fsck
+'
 
-test_expect_success 'that relative alternate is possible for current dir' \
-'cd C &&
-echo "../../../B/.git/objects" > .git/objects/info/alternates &&
-test_valid_repo'
+test_expect_success 'that relative alternate is possible for current dir' '
+	echo "../../../B/.git/objects" >C/.git/objects/info/alternates &&
+	git fsck
+'
 
-cd "$base_dir"
+test_expect_success 'that relative alternate is recursive' '
+	git -C D fsck
+'
 
-test_expect_success \
-    'that relative alternate is only possible for current dir' '
-    cd D &&
-    ! (test_valid_repo)
+# 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
 '
 
-cd "$base_dir"
+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