git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 5/6] t5526: break test submodule differently

From
Patrick Steinhardt <ps@pks.im>
Date
Jan 10, 2024, 07:41 UTC
Message-ID
<ZZ5KJX4tC0UIX00j@tanuki>
In-Reply-To
<CAPig+cTB5OH1hCD-EagxNAcaw1=RR7yCeeZ_AzeqHtFTGxT-0g@mail.gmail.com>
On Tue, Jan 09, 2024 at 02:23:55PM -0500, Eric Sunshine wrote:
Show 22 quoted lines
> On Tue, Jan 9, 2024 at 7:17 AM Patrick Steinhardt <ps@pks.im> wrote:
> > In 10f5c52656 (submodule: avoid auto-discovery in
> > prepare_submodule_repo_env(), 2016-09-01) we fixed a bug when doing a
> > recursive fetch with submodule in the case where the submodule is broken
> > due to whatever reason. The test to exercise that the fix works breaks
> > the submodule by deleting its `HEAD` reference, which will cause us to
> > not detect the directory as a Git repository.
> >
> > While this is perfectly fine in theory, this way of breaking the repo
> > becomes problematic with the current efforts to introduce another refdb
> > backend into Git. The new reftable backend has a stub HEAD file that
> > always contains "ref: refs/heads/.invalid" so that tools continue to be
> > able to detect such a repository. But as the reftable backend will never
> > delete this file even when asked to delete `HEAD` the current way to
> > delete the `HEAD` reference will stop working.
> 
> This patch is not the appropriate place to bikeshed (but since this is
> the first time I've read or paid attention to it), if I saw "ref:
> refs/heads/.invalid", the word ".invalid" would make me think
> something was broken in my repository. Would it make sense to use some
> less alarming word, such as perhaps ".placeholder", ".stand-in",
> ".synthesized" or even the name of the non-file-based backend in use?

Well, something _is_ broken in your repository in case Git ever tries to read the "HEAD" placeholder in a reftable-enabled repository. But I guess you rather come from the angle of using `cat HEAD` as a user. I do agree that using a better hint could help users, but this detail has already been recorded as such in "Documentation/technical/reftable.txt".

We can of course change this to be "ref: refs/heads/.reftable", but as you already say this is something that should be discussed in a separate thread.

Show 20 quoted lines
> > Adapt the code to instead delete the objects database. Going back with
> > this new way to cuase breakage confirms that it triggers the infinite
> > recursion just the same, and there are no equivalent ongoing efforts to
> > replace the object database with an alternate backend.
> 
> s/cuase/cause/
> 
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> > diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
> > @@ -771,7 +771,7 @@ test_expect_success 'fetching submodule into a broken repository' '
> >         # Break the receiving submodule
> > -       test-tool -C dst/sub ref-store main delete-refs REF_NO_DEREF msg HEAD &&
> > +       rm -r dst/sub/.git/objects &&
> 
> If there is no guarantee that .git/objects will exist when any
> particular backend is in use, would it be more robust to use -f here,
> as well?
> 
>     rm -rf dst/sub/.git/objects &&

`.git/objects` always exists in a healthy repository. If it doesn't, then `is_git_directory()` would return a false-ish value and we wouldn't recognize the repository as such. Or are you saying that this could potentially change in the future if there was ever to be an alternate ODB format? If so that is a valid remark, but the test would break regardless of whether we use `-f` or not: if a missing ".git/objects" directory does not lead to a corrupted repository then the whole premise of the test is broken.

Patrick
Previous: Eric SunshineNext: Patrick Steinhardt
Message 17 of 54 in “t: mark "files"-backend specific tests”
  1. 0/6 t: mark "files"-backend specific testsPatrick Steinhardt, Jan 9, 2024
  2. 1/6 t1300: mark tests to require default repo formatPatrick Steinhardt, Jan 9, 2024
  3. Taylor BlauJan 9, 2024
  4. Patrick SteinhardtJan 10, 2024
  5. Eric SunshineJan 9, 2024
  6. Patrick SteinhardtJan 10, 2024
  7. 2/6 t1301: mark test for `core.sharedRepository` as reffiles specificPatrick Steinhardt, Jan 9, 2024
  8. 3/6 t1302: make tests more robust with new extensionsPatrick Steinhardt, Jan 9, 2024
  9. Taylor BlauJan 9, 2024
  10. 4/6 t1419: mark test suite as files-backend specificPatrick Steinhardt, Jan 9, 2024
  11. Eric SunshineJan 9, 2024
  12. Patrick SteinhardtJan 10, 2024
  13. Junio C HamanoJan 10, 2024
  14. Patrick SteinhardtJan 11, 2024
  15. 5/6 t5526: break test submodule differentlyPatrick Steinhardt, Jan 9, 2024
  16. Eric SunshineJan 9, 2024
  17. Patrick SteinhardtJan 10, 2024
  18. 6/6 t: mark tests regarding git-pack-refs(1) to be backend specificPatrick Steinhardt, Jan 9, 2024
  19. 0/6 t: mark "files"-backend specific testsPatrick Steinhardt, Jan 10, 2024
  20. 1/6 t1300: make tests more robust with non-default ref backendsPatrick Steinhardt, Jan 10, 2024
  21. Toon ClaesJan 23, 2024
  22. Patrick SteinhardtJan 23, 2024
  23. Justin ToblerJan 23, 2024
  24. Christian CouderJan 23, 2024
  25. Patrick SteinhardtJan 24, 2024
  26. Christian CouderJan 29, 2024
  27. Patrick SteinhardtJan 29, 2024
  28. 2/6 t1301: mark test for `core.sharedRepository` as reffiles specificPatrick Steinhardt, Jan 10, 2024
  29. 3/6 t1302: make tests more robust with new extensionsPatrick Steinhardt, Jan 10, 2024
  30. Toon ClaesJan 23, 2024
  31. Patrick SteinhardtJan 23, 2024
  32. Christian CouderJan 23, 2024
  33. Patrick SteinhardtJan 24, 2024
  34. 4/6 t1419: mark test suite as files-backend specificPatrick Steinhardt, Jan 10, 2024
  35. 5/6 t5526: break test submodule differentlyPatrick Steinhardt, Jan 10, 2024
  36. 6/6 t: mark tests regarding git-pack-refs(1) to be backend specificPatrick Steinhardt, Jan 10, 2024
  37. Christian CouderJan 23, 2024
  38. 0/6 t: mark "files"-backend specific testsPatrick Steinhardt, Jan 24, 2024
  39. 1/6 t1300: make tests more robust with non-default ref backendsPatrick Steinhardt, Jan 24, 2024
  40. 2/6 t1301: mark test for `core.sharedRepository` as reffiles specificPatrick Steinhardt, Jan 24, 2024
  41. 3/6 t1302: make tests more robust with new extensionsPatrick Steinhardt, Jan 24, 2024
  42. 4/6 t1419: mark test suite as files-backend specificPatrick Steinhardt, Jan 24, 2024
  43. 5/6 t5526: break test submodule differentlyPatrick Steinhardt, Jan 24, 2024
  44. 6/6 t: mark tests regarding git-pack-refs(1) to be backend specificPatrick Steinhardt, Jan 24, 2024
  45. 0/6 t: mark "files"-backend specific testsPatrick Steinhardt, Jan 29, 2024
  46. 1/6 t1300: make tests more robust with non-default ref backendsPatrick Steinhardt, Jan 29, 2024
  47. Christian CouderJan 29, 2024
  48. 2/6 t1301: mark test for `core.sharedRepository` as reffiles specificPatrick Steinhardt, Jan 29, 2024
  49. 3/6 t1302: make tests more robust with new extensionsPatrick Steinhardt, Jan 29, 2024
  50. 4/6 t1419: mark test suite as files-backend specificPatrick Steinhardt, Jan 29, 2024
  51. 5/6 t5526: break test submodule differentlyPatrick Steinhardt, Jan 29, 2024
  52. 6/6 t: mark tests regarding git-pack-refs(1) to be backend specificPatrick Steinhardt, Jan 29, 2024
  53. Christian CouderJan 29, 2024
  54. Junio C HamanoJan 29, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.