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

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Mar 24, 2019, 20:56 UTC
Message-ID: <20190324205624.GJ22459@szeder.dev>
In-Reply-To: <20190322232237.13293-2-matheus.bernardino@usp.br>

On Fri, Mar 22, 2019 at 08:22:31PM -0300, Matheus Tavares wrote:

Show 7 quoted lines

> From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> 
> 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.

Show 17 quoted lines

> 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/20190226002625.13022-5-avarab@gmail.com/
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
> Helped-by: Matheus Tavares <matheus.bernardino@usp.br>

Show 22 quoted lines

> +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.

Show 17 quoted lines

> +	) &&
> +	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 '../../../'.

Show 29 quoted lines

> +	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

Show 7 quoted lines

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