diff mbox series

[v3,07/22] t1413: use tar to save and restore entire .git directory

Message ID f6ab40c4e6599540da38ae5af8e574dc65909e79.1622480197.git.gitgitgadget@gmail.com (mailing list archive)
State Accepted
Commit 1142746cbbf016fe2568076d6f30b2b73b29e731
Headers show
Series Prepare tests for reftable backend | expand

Commit Message

Han-Wen Nienhuys May 31, 2021, 4:56 p.m. UTC 
From: Han-Wen Nienhuys <hanwen@google.com>

This makes the test independent of the particulars of the storage formats.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t1413-reflog-detach.sh | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Bagas Sanjaya June 1, 2021, 4:55 a.m. UTC | #1 
Hi,

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?
Han-Wen Nienhuys June 1, 2021, 9:23 a.m. UTC | #2 
On Tue, Jun 1, 2021 at 6:55 AM Bagas Sanjaya <bagasdotme@gmail.com> 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.
Junio C Hamano June 1, 2021, 8:44 p.m. UTC | #3 
Han-Wen Nienhuys <hanwen@google.com> writes:

> On Tue, Jun 1, 2021 at 6:55 AM Bagas Sanjaya <bagasdotme@gmail.com> 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.
Han-Wen Nienhuys June 2, 2021, 7:49 a.m. UTC | #4 
On Tue, Jun 1, 2021 at 10:44 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Han-Wen Nienhuys <hanwen@google.com> writes:
>
> > On Tue, Jun 1, 2021 at 6:55 AM Bagas Sanjaya <bagasdotme@gmail.com> 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?
diff mbox series

Patch

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
 }
 
 test_expect_success setup '
@@ -17,7 +16,7 @@  test_expect_success setup '
 	git branch side &&
 	test_tick &&
 	git commit --allow-empty -m second &&
-	cat .git/logs/HEAD >saved_reflog
+	"$TAR" cf .git-saved.tar .git
 '
 
 test_expect_success baseline '