On Tue, Jun 1, 2021 at 10:44 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Han-Wen Nienhuys <hanwen@xxxxxxxxxx> writes:
>
> > On Tue, Jun 1, 2021 at 6:55 AM Bagas Sanjaya <bagasdotme@xxxxxxxxx> wrote:
> >>
> >> On 31/05/21 23.56, Han-Wen Nienhuys via GitGitGadget wrote:
> >> > diff --git a/t/t1413-reflog-detach.sh b/t/t1413-reflog-detach.sh
> >> > index bde05208ae6a..934688a1ee82 100755
> >> > --- a/t/t1413-reflog-detach.sh
> >> > +++ b/t/t1413-reflog-detach.sh
> >> > @@ -7,8 +7,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> >> > . ./test-lib.sh
> >> >
> >> > reset_state () {
> >> > - git checkout main &&
> >> > - cp saved_reflog .git/logs/HEAD
> >> > + rm -rf .git && "$TAR" xf .git-saved.tar
> >> > }
> >> >
> >>
> >> Why do you do rm -rf git directory then extract tar archive to reset?
> >
> > I'm not sure I understand your question. Are you asking why we have to
> > do a reset, or why we'd use rm + tar? The rm + tar restores the former
> > state reliably, so we can be sure it is correct. It's also independent
> > of the storage format details.
>
> I think a short answer is "without rm -rf .git, a stale file in that
> directory will stay there when .git-saved.tar gets extracted", but
> the whole arrangement makes me worried what would happen if somebody
> manages to interrupt "rm -rf" without killing the whole test
> framework (or letting the when-finished handlers run). The test
> framework thinks it is working in a throw-away repository but the
> $TRASH_DIRECTORY that was supposed to be removed and extracted but
> failed to do so due to interruption in the middle may not look like
> a git repository, in which case it may try to do the usual repository
> discovery and trash the git project repository instead.
Similar problems could occur if a developer is trying out a change to
the Git source code and makes an error. It's also easy to mess up the
check-out by doing a "cd .." too many in a shell test under
development.
I don't understand why the temp directories for tests are inside the
source tree. Every other project that I've worked on uses mktemp -d
for temporary directories instead.
For the problem you describe here, using atomic rename won't work,
because we want to swap out a complete directory. So we'd need a
command that replicates a source into a destination tree, using atomic
rename. Maybe rsync --delete might do the trick, but I don't think
we'd want that as a dependency?
--
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--
Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
