Re: [GSoC][PATCH v4 1/7] clone: test for our behavior on odd objects/* content
- From
Matheus Tavares Bernardino <matheus.bernardino@usp.br>
- Date
- Mar 26, 2019, 19:43 UTC
- Message-ID
- <CAHd-oW4qZWhHJVnnOpdUnN1J9r9V=LZ+dEdceWD=o==OwTwFRg@mail.gmail.com>
- In-Reply-To
- <20190324205624.GJ22459@szeder.dev>
On Sun, Mar 24, 2019 at 5:56 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:
Show 12 quoted lines
> > On Fri, Mar 22, 2019 at 08:22:31PM -0300, Matheus Tavares wrote: > > 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/ >
Thanks, nice catch.
Show 5 quoted lines
> > 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. >
Yes, I think it should be just "SHA-1s". Thanks.
Show 45 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> > > > > +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.
Ok!
Show 26 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 '../../../'.
Sounds good, but in the next patch of this series, another 'find' statement will be added inside this subshell, so I think that change is not really possible, unfortunately.
Show 33 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-filesIndeed, much simpler. Thanks.
Show 7 quoted lines
> > + test_cmp expected-files T--shared.objects-files.raw > > +' > > + > > test_done > > -- > > 2.20.1 > >