diff mbox series

[18/18] t1415: set REFFILES for test specific to storage format

Message ID 0665edb1308b8cd4536d6922fd36315e1abdd9d1.1618829584.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series Prepare tests for reftable backend | expand

Commit Message

Han-Wen Nienhuys April 19, 2021, 10:53 a.m. UTC 
From: Han-Wen Nienhuys <hanwen@google.com>

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t1415-worktree-refs.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ævar Arnfjörð Bjarmason April 21, 2021, 7:15 a.m. UTC | #1 
On Mon, Apr 19 2021, Han-Wen Nienhuys via GitGitGadget wrote:

> From: Han-Wen Nienhuys <hanwen@google.com>
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
>  t/t1415-worktree-refs.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
> index 7ab91241ab7c..a8083a0af3af 100755
> --- a/t/t1415-worktree-refs.sh
> +++ b/t/t1415-worktree-refs.sh
> @@ -16,7 +16,7 @@ test_expect_success 'setup' '
>  	git -C wt2 update-ref refs/worktree/foo HEAD
>  '
>  
> -test_expect_success 'refs/worktree must not be packed' '
> +test_expect_success REFFILES 'refs/worktree must not be packed' '
>  	git pack-refs --all &&
>  	test_path_is_missing .git/refs/tags/wt1 &&
>  	test_path_is_file .git/refs/worktree/foo &&

So a naïve:
    
    diff --git a/refs/files-backend.c b/refs/files-backend.c
    index 3f29f8c143..01e2dc8bc3 100644
    --- a/refs/files-backend.c
    +++ b/refs/files-backend.c
    @@ -212,7 +212,7 @@ static void files_ref_path(struct files_ref_store *refs,
      */
     static void add_per_worktree_entries_to_dir(struct ref_dir *dir, const char *dirname)
     {
    -       const char *prefixes[] = { "refs/bisect/", "refs/worktree/", "refs/rewritten/" };
    +       const char *prefixes[] = { "refs/bisect/", "refs/rewritten/" };
            int ip;
    
            if (strcmp(dirname, "refs/"))

Will fail the test under non-reftable, i.e. we somehow conflate "is
packed" with correctly discovering these refs? A discussion of how
8aff1a9ca5 (Add a place for (not) sharing stuff between worktrees,
2018-09-29) relates to reftable would be valuable here.

But on the tip of "seen" currently this test fails entirely with
GIT_TEST_REFTABLE=true, so I'm not sure if it got rid of the need for
this abstraction in the files backend or what...
Han-Wen Nienhuys April 26, 2021, 5:41 p.m. UTC | #2 
On Wed, Apr 21, 2021 at 9:15 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
>
> On Mon, Apr 19 2021, Han-Wen Nienhuys via GitGitGadget wrote:
>
> > From: Han-Wen Nienhuys <hanwen@google.com>
> >
> > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> > ---
> >  t/t1415-worktree-refs.sh | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
> > index 7ab91241ab7c..a8083a0af3af 100755
> > --- a/t/t1415-worktree-refs.sh
> > +++ b/t/t1415-worktree-refs.sh
> > @@ -16,7 +16,7 @@ test_expect_success 'setup' '
> >       git -C wt2 update-ref refs/worktree/foo HEAD
> >  '
> >
> > -test_expect_success 'refs/worktree must not be packed' '
> > +test_expect_success REFFILES 'refs/worktree must not be packed' '
> >       git pack-refs --all &&
> >       test_path_is_missing .git/refs/tags/wt1 &&
> >       test_path_is_file .git/refs/worktree/foo &&
>
> So a naïve:
>
>     diff --git a/refs/files-backend.c b/refs/files-backend.c
>     index 3f29f8c143..01e2dc8bc3 100644
>     --- a/refs/files-backend.c
>     +++ b/refs/files-backend.c
>     @@ -212,7 +212,7 @@ static void files_ref_path(struct files_ref_store *refs,
>       */
>      static void add_per_worktree_entries_to_dir(struct ref_dir *dir, const char *dirname)
>      {
>     -       const char *prefixes[] = { "refs/bisect/", "refs/worktree/", "refs/rewritten/" };
>     +       const char *prefixes[] = { "refs/bisect/", "refs/rewritten/" };
>             int ip;
>
>             if (strcmp(dirname, "refs/"))
>
> Will fail the test under non-reftable, i.e. we somehow conflate "is
> packed" with correctly discovering these refs?

AFAICT, what happens is that the packed-refs file is shared across all
worktrees, so refs that are per worktree should not be stored there. I
think that is what the test is trying to assert.

> A discussion of how
> 8aff1a9ca5 (Add a place for (not) sharing stuff between worktrees,
> 2018-09-29) relates to reftable would be valuable here.

reftable handles this slightly differently. I added a comment to
reftable-backend.c about this.

> But on the tip of "seen" currently this test fails entirely with
> GIT_TEST_REFTABLE=true, so I'm not sure if it got rid of the need for
> this abstraction in the files backend or what...

Before splitting the series into two (git support and test fixups),
all of these changes reduced the number of test breakages.
diff mbox series

Patch

diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
index 7ab91241ab7c..a8083a0af3af 100755
--- a/t/t1415-worktree-refs.sh
+++ b/t/t1415-worktree-refs.sh
@@ -16,7 +16,7 @@  test_expect_success 'setup' '
 	git -C wt2 update-ref refs/worktree/foo HEAD
 '
 
-test_expect_success 'refs/worktree must not be packed' '
+test_expect_success REFFILES 'refs/worktree must not be packed' '
 	git pack-refs --all &&
 	test_path_is_missing .git/refs/tags/wt1 &&
 	test_path_is_file .git/refs/worktree/foo &&