Subject: Re: [GSoC][PATCH v4 1/7] clone: test for our behavior on odd objects/* content

From: SZEDER Gábor <[email-masked]>
Date: Sun, 24 Mar 2019 21:56:24 +0100

On Fri, Mar 22, 2019 at 08:22:31PM -0300, Matheus Tavares wrote:
> From: Ævar Arnfjörð Bjarmason <[email-masked]>
> 
> Add tests for what happens when we perform a local clone on a repo
> containing odd files at .git/object directory, such as symlinks to other
> dirs, or unknown files.
> 
> I'm bending over backwards here to avoid a SHA1 dependency. See [1]

s/SHA1/SHA-1/

> for an earlier and simpler version that hardcoded a SHA-1s.

s/SHA-1s/SHA-1/ or s/a SHA-1s/SHA-1s/, depending on what you consider
multiple occurrances of the same SHA-1.

> This behavior has been the same for a *long* time, but hasn't been
> tested for.
> 
> There's a good post-hoc argument to be made for copying over unknown
> things, e.g. I'd like a git version that doesn't know about the
> commit-graph to copy it under "clone --local" so a newer git version
> can make use of it.
> 
> In follow-up commits we'll look at changing some of this behavior, but
> for now let's just assert it as-is so we'll notice what we'll change
> later.
> 
> 1. https://public-inbox.org/git/[email-masked]/
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <[email-masked]>
> Signed-off-by: Matheus Tavares <[email-masked]>
> Helped-by: Matheus Tavares <[email-masked]>

> +test_expect_success SYMLINKS 'setup repo with manually symlinked dirs and unknown files at objects/' '
> +	git init T &&
> +	(
> +		cd T &&
> +		test_commit A &&
> +		git gc &&
> +		(
> +			cd .git/objects &&
> +			mv pack packs &&
> +			ln -s packs pack
> +		) &&
> +		test_commit B &&
> +		(
> +			cd .git/objects &&
> +			find ?? -type d >loose-dirs &&
> +			last_loose=$(tail -n 1 loose-dirs) &&
> +			rm -f loose-dirs &&
> +			mv $last_loose a-loose-dir &&
> +			ln -s a-loose-dir $last_loose &&
> +			find . -type f | sort >../../../T.objects-files.raw &&
> +			echo unknown_content> unknown_file
> +		)

Please drop these inner subshells.  They are unnecessary, because the
outer subshell alone is sufficient to ensure that the test script
returns to the original directory if one of the commands were to fail.

> +	) &&
> +	git -C T fsck &&
> +	git -C T rev-list --all --objects >T.objects
> +'
> +
> +
> +test_expect_success SYMLINKS 'clone repo with symlinked dirs and unknown files at objects/' '
> +	for option in --local --no-hardlinks --shared --dissociate
> +	do
> +		git clone $option T T$option || return 1 &&
> +		git -C T$option fsck || return 1 &&
> +		git -C T$option rev-list --all --objects >T$option.objects &&
> +		test_cmp T.objects T$option.objects &&
> +		(
> +			cd T$option/.git/objects &&
> +			find . -type f | sort >../../../T$option.objects-files.raw
> +		)

Nit: this might be a bit easier on the eyes when written as

  ( 
        cd T$option/.git/objects &&
        find . -type f
  ) | sort >T$option.objects-files.raw

because it would avoid that '../../../'.

> +	done &&
> +
> +	for raw in $(ls T*.raw)
> +	do
> +		sed -e "s!/..\$!/X!; s!/../!/Y/!; s![0-9a-f]\{38,\}!Z!" \
> +		    -e "/multi-pack-index/d" -e "/commit-graph/d" <$raw >$raw.de-sha || return 1
> +	done &&
> +
> +	cat >expected-files <<-EOF &&
> +	./Y/Z
> +	./Y/Z
> +	./a-loose-dir/Z
> +	./Y/Z
> +	./info/packs
> +	./pack/pack-Z.idx
> +	./pack/pack-Z.pack
> +	./packs/pack-Z.idx
> +	./packs/pack-Z.pack
> +	./unknown_file
> +	EOF
> +
> +	for option in --local --dissociate --no-hardlinks
> +	do
> +		test_cmp expected-files T$option.objects-files.raw.de-sha || return 1
> +	done &&
> +
> +	cat >expected-files <<-EOF &&
> +	./info/alternates
> +	EOF

Perhaps

  echo ./info/alternates >expected-files

> +	test_cmp expected-files T--shared.objects-files.raw
> +'
> +
>  test_done
> -- 
> 2.20.1
>