Re: [PATCH v2 3/4] Add 'promisor-remote' capability to protocol v2
- From
Christian Couder <christian.couder@gmail.com>
- Date
- Nov 28, 2024, 15:31 UTC
- Message-ID
- <CAP8UFD2pAtX6UPD6FATuepwVk+0wGkmU5rt=nWpWHejeCe_6rg@mail.gmail.com>
- In-Reply-To
- <xmqqserboqot.fsf@gitster.g>
On Thu, Nov 28, 2024 at 6:47 AM Junio C Hamano <gitster@pobox.com> wrote:
Show 31 quoted lines
>
> I was looking at test breakages caused by this topic (in 'seen',
> t5710 fails leak checking).
>
> Then I noticed something strange. Next to the "$(TRASH_DIRECTORY)",
> running this script leaves a few garbage files under the "t/"
> directory.
>
> I think the culprit is this helper function.
>
> > +initialize_server () {
> > + # Repack everything first
> > + git -C server -c repack.writebitmaps=false repack -a -d &&
> > +
> > + # Remove promisor file in case they exist, useful when reinitializing
> > + rm -rf server/objects/pack/*.promisor &&
> > +
> > + # Repack without the largest object and create a promisor pack on server
> > + git -C server -c repack.writebitmaps=false repack -a -d \
> > + --filter=blob:limit=5k --filter-to="$(pwd)" &&
>
> This --filter-to="$(pwd)" expands to $(TRASH_DIRECTORY), which is
> "..../t/trash-directory.t5710-promisor-remote-capability". I think
> that is the cause for two extra trash files that are _OUTSIDE_ the
> trash directory, which is an absolute no-no for tests to be safely
> runnable. Next to the trash directory, this ends up creating three
> files
>
> trash directory.t5710-...-980d3ff591aae1651cdd52f7dfad4fb6319ee3c2.idx
> trash directory.t5710-...-980d3ff591aae1651cdd52f7dfad4fb6319ee3c2.pack
> trash directory.t5710-...-980d3ff591aae1651cdd52f7dfad4fb6319ee3c2.revYeah, right. It should be --filter-to="$(pwd)/pack"
> > + promisor_file=$(ls server/objects/pack/*.pack | sed "s/\.pack/.promisor/") && > > + touch "$promisor_file" && > > Style: don't "touch" a single file to create it. Instead >redirect_into_it.
I have fixed this in the current version.
Show 22 quoted lines
> The first failure in leak check seems to be > > not ok 5 - fetch with promisor.advertise set to 'false' > # > # git -C server config promisor.advertise false && > # > # # Clone from server to create a client > # GIT_NO_LAZY_FETCH=0 git clone -c remote.server2.promisor=true \ > # -c remote.server2.fetch="+refs/heads/*:refs/remotes/server2/*" \ > # -c remote.server2.url="file://$(pwd)/server2" \ > # -c promisor.acceptfromserver=All \ > # --no-local --filter="blob:limit=5k" server client && > # test_when_finished "rm -rf client" && > # > # # Check that the largest object is not missing on the server > # check_missing_objects server 0 "" && > # > # # Reinitialize server so that the largest object is missing again > # initialize_server > > but I didn't dig further. Can you take a look? I'll eject the > topic from 'seen' in the meantime to unblock the CI.
No problem with ejecting the topic from 'seen'. I hope to send a new version with a design doc hopefully next week.
Thanks.