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

[PATCH v5 0/3] Use default values from settings instead of config

From
Glen Choo <chooglen@google.com>
Date
Oct 15, 2021, 20:16 UTC
Message-ID
<20211015201631.91524-1-chooglen@google.com>
In-Reply-To
<20211012174208.95161-1-chooglen@google.com>
  Hi everyone! This patch was created in response to something we observed in
  Google, where fsck failed to detect that the commit graph was invalid. We
  initially assumed that fsck never checked the commit graph, but it turns out
  that it does so only when core.commitgraph is set, even though we set defaults
  for "whether to use the commit graph" in the repository settings.
  Instead of using the config, let's use repository settings where
  available. Replace core.commitGraph and core.multiPackIndex with their
  equivalent repository settings in fsck and gc.

v5 is based off Ævar's fix (commit-graph: make "verify" work with replace refs, [1]). As Ævar noted in v4, the mktag test failures indicated buggy behavior "git commit-graph verify" when replace refs are enabled, so we should fix the bug instead of just disabling commit-graph.

Ævar, Derrick, thanks for the productive discussion, I feel like I understand this area a little better now.

Barring any unseen bugs, this should be ready to merge.
[1] https://lore.kernel.org/git/cover-0.3-00000000000-20211014T233343Z-avarab@gmail.com/
Changes since v4
* Rebase onto [1], which fixes a bug with commit-graph verify with replace refs
  enabled.
* Remove GIT_TEST_COMMIT_GRAPH=0 in t3800-mktag.sh because the bug that it was
  covering up is now fixed.
Changes since v3
* Disable GIT_TEST_COMMIT_GRAPH in tests that intentionally corrupt things in a
  way that is incompatible with commit-graphs.
* Make patch 1 and 2's commit messages more concise (thanks Ævar!).
Changes since v2:
* Various small test fixes (thanks Eric!). Most notably, I've used -c instead of
  test_config because test_config can affect the values in subsequent tests.
* Rewording fix in patch 3 commit message
* Refactor tests in patch 3 so that we use a single helper function instead of
  copy-pasted code
Changes since v1:
* clean up typo in patch 1 commit message 
* document the commits that patches 1 and 2 address
* use test helpers in patch 1
* rewrite patch 2's tests so that it's easier to tell that each test
  does something different
* reword patch 3 commit message to explain the bug
* add tests to patch 3
Glen Choo (3):
  fsck: verify commit graph when implicitly enabled
  fsck: verify multi-pack-index when implictly enabled
  gc: perform incremental repack when implictly enabled
 builtin/fsck.c              |  5 +++--
 builtin/gc.c                |  5 ++---
 t/t0410-partial-clone.sh    |  6 +++++-
 t/t5318-commit-graph.sh     | 23 ++++++++++++++++++++++-
 t/t5319-multi-pack-index.sh |  5 ++++-
 t/t7900-maintenance.sh      | 28 ++++++++++++++++++++++++----
 6 files changed, 60 insertions(+), 12 deletions(-)
Range-diff against v4:
1:  aac1253e7b ! 1:  567d40849a fsck: verify commit graph when implicitly enabled
    @@ Commit message
     
         Add tests to "t5318-commit-graph.sh" to verify that fsck checks the
         commit-graph as expected for the 3 values of core.commitGraph. Also,
    -    disable GIT_TEST_COMMIT_GRAPH for tests that use fsck in ways that
    -    assume that commit-graph checking is disabled (t/t3800-mktag.sh,
    -    t/t0410-partial-clone.sh).
    +    disable GIT_TEST_COMMIT_GRAPH in t/t0410-partial-clone.sh because some
    +    test cases use fsck in ways that assume that commit-graph checking is
    +    disabled.
     
         Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
         Signed-off-by: Glen Choo <chooglen@google.com>
    @@ t/t0410-partial-clone.sh: test_expect_success 'rev-list stops traversal at missi
      	! grep $FOO out
      '
     
    - ## t/t3800-mktag.sh ##
    -@@ t/t3800-mktag.sh: test_description='git mktag: tag object verify test'
    - 
    - . ./test-lib.sh
    - 
    -+# When enabled, some commands will automatically write commit-graphs.
    -+# This will cause the mktag tests to fail because fsck will attempt to
    -+# verify the out-of-sync commit graph.
    -+GIT_TEST_COMMIT_GRAPH=0
    -+
    - ###########################################################
    - # check the tag.sig file, expecting verify_tag() to fail,
    - # and checking that the error message matches the pattern
    -
      ## t/t5318-commit-graph.sh ##
     @@ t/t5318-commit-graph.sh: test_expect_success 'detect incorrect chunk count' '
      		$GRAPH_CHUNK_LOOKUP_OFFSET
2:  ed64983430 = 2:  7cb44a7aeb fsck: verify multi-pack-index when implictly enabled
3:  821b492d8b = 3:  a436c45af7 gc: perform incremental repack when implictly enabled
-- 
2.33.0.1079.g6e70778dc9-goog
Previous: Junio C HamanoNext: Glen Choo
Message 61 of 65 in “Use default values from settings instead of config”
  1. 0/3 Use default values from settings instead of configGlen Choo, Sep 13, 2021
  2. 1/3 fsck: verify commit graph when implicitly enabledGlen Choo, Sep 13, 2021
  3. Taylor BlauSep 13, 2021
  4. Eric SunshineSep 13, 2021
  5. Taylor BlauSep 13, 2021
  6. Glen ChooSep 13, 2021
  7. Eric SunshineSep 13, 2021
  8. Taylor BlauSep 14, 2021
  9. Eric SunshineSep 14, 2021
  10. Taylor BlauSep 14, 2021
  11. 2/3 fsck: verify multi-pack-index when implictly enabledGlen Choo, Sep 13, 2021
  12. Taylor BlauSep 13, 2021
  13. 3/3 gc: perform incremental repack when implictly enabledGlen Choo, Sep 13, 2021
  14. Taylor BlauSep 13, 2021
  15. Glen ChooSep 14, 2021
  16. Bagas SanjayaSep 14, 2021
  17. Glen ChooSep 16, 2021
  18. Taylor BlauSep 13, 2021
  19. 0/3 Use default values from settings instead of configGlen Choo, Sep 17, 2021
  20. 1/3 fsck: verify commit graph when implicitly enabledGlen Choo, Sep 17, 2021
  21. Eric SunshineSep 29, 2021
  22. Glen ChooSep 30, 2021
  23. Glen ChooSep 30, 2021
  24. Eric SunshineSep 30, 2021
  25. Glen ChooOct 1, 2021
  26. 2/3 fsck: verify multi-pack-index when implictly enabledGlen Choo, Sep 17, 2021
  27. Eric SunshineSep 29, 2021
  28. Glen ChooSep 29, 2021
  29. 3/3 gc: perform incremental repack when implictly enabledGlen Choo, Sep 17, 2021
  30. Eric SunshineSep 29, 2021
  31. Glen ChooSep 27, 2021
  32. Eric SunshineSep 29, 2021
  33. Glen ChooSep 29, 2021
  34. 0/3 Use default values from settings instead of configGlen Choo, Oct 5, 2021
  35. 1/3 fsck: verify commit graph when implicitly enabledGlen Choo, Oct 5, 2021
  36. 2/3 fsck: verify multi-pack-index when implictly enabledGlen Choo, Oct 5, 2021
  37. 3/3 gc: perform incremental repack when implictly enabledGlen Choo, Oct 5, 2021
  38. Ævar Arnfjörð BjarmasonOct 5, 2021
  39. Derrick StoleeOct 5, 2021
  40. Ævar Arnfjörð BjarmasonOct 5, 2021
  41. Glen ChooOct 5, 2021
  42. Junio C HamanoOct 9, 2021
  43. Glen ChooOct 11, 2021
  44. Junio C HamanoOct 11, 2021
  45. Glen ChooOct 11, 2021
  46. 0/3 Use default values from settings instead of configGlen Choo, Oct 12, 2021
  47. 1/3 fsck: verify commit graph when implicitly enabledGlen Choo, Oct 12, 2021
  48. 2/3 fsck: verify multi-pack-index when implictly enabledGlen Choo, Oct 12, 2021
  49. 3/3 gc: perform incremental repack when implictly enabledGlen Choo, Oct 12, 2021
  50. Junio C HamanoOct 12, 2021
  51. Ævar Arnfjörð BjarmasonOct 12, 2021
  52. Glen ChooOct 12, 2021
  53. Ævar Arnfjörð BjarmasonOct 14, 2021
  54. Derrick StoleeOct 13, 2021
  55. Ævar Arnfjörð BjarmasonOct 13, 2021
  56. Derrick StoleeOct 14, 2021
  57. Glen ChooOct 14, 2021
  58. Ævar Arnfjörð BjarmasonOct 14, 2021
  59. Junio C HamanoOct 14, 2021
  60. Junio C HamanoOct 15, 2021
  61. 0/3 Use default values from settings instead of configGlen Choo, Oct 15, 2021
  62. 1/3 fsck: verify commit graph when implicitly enabledGlen Choo, Oct 15, 2021
  63. 2/3 fsck: verify multi-pack-index when implictly enabledGlen Choo, Oct 15, 2021
  64. 3/3 gc: perform incremental repack when implictly enabledGlen Choo, Oct 15, 2021
  65. Junio C HamanoOct 15, 2021

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.