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

[PATCH v5 0/3] align the behavior when opening "packed-refs"

From
shejialuo <shejialuo@gmail.com>
Date
May 14, 2025, 15:48 UTC
Message-ID
<aCS7O8tNekg_u9Wp@ArchLinux>
In-Reply-To
<aCMnrwkoJ2WyqGZT@ArchLinux>
Hi All:

As discussed in [1], we need to use mmap mechanism to open large "packed_refs" file to save the memory usage. This patch mainly does the following things:

1: Fix an issue that we would report an error when the "packed-refs" file is empty, which does not align with the runtime behavior. 2-4: Extract some logic from the existing code and then use these created helper functions to let fsck code to use mmap necessarily

[1] https://lore.kernel.org/git/20250503133158.GA4450@coredump.intra.peff.net
Really thank Peff and Patrick to suggest me to do above change.
---
Change in v2:
1. Update the commit message of [PATCH 1/4]. And use redirection to
create an empty file instead of using `touch`.
2. Don't use if for the refactored function in [PATCH 3/4] and then
update the commit message to align with the new function name.
3. Enhance the commit message of [PATCH 4/4].
---
Change in v3:
1. Drop the patch which creates a new function
"munmap_temporary_snapshot". As discussed, there is no need to munmap
the file during fsck.
2. Allocate snapshot variable in the stack instead of heap.
3. Fix rebase issue, remove unneeded code to check the file size
explicitly.
---
Change in v4:
1. Report the "emptyPackedRefsFile(INFO)" to the user when the
"packed-refs" is empty instead of ONLY skipping checking the content and
update the shell script and commit message.
2. Apply Peff's advice to make [PATCH v3 2/3] more clear.
---
Change in v5:
1. Improve the commit message in the first patch to be more clear:
    1. Talk about the current behavior, what error we would report if
       "packed-refs" is empty.
    2. To align with the runtime behavior, we should skip checking the
       content of "packed-refs".
    3. Why do we need to report to the user when the "packed-refs" is
       empty
2. Fix grammar issue in the last patch.

Thanks, Jialuo

shejialuo (3):
  packed-backend: fsck should warn when "packed-refs" file is empty
  packed-backend: extract snapshot allocation in `load_contents`
  packed-backend: mmap large "packed-refs" file during fsck
 Documentation/fsck-msgids.adoc |  6 +++
 fsck.h                         |  1 +
 refs/packed-backend.c          | 73 ++++++++++++++++++++--------------
 t/t0602-reffiles-fsck.sh       | 17 ++++++++
 4 files changed, 67 insertions(+), 30 deletions(-)
Range-diff against v4:
1:  75636c9c85 ! 1:  3487692a03 packed-backend: fsck should warn when "packed-refs" file is empty
    @@ Metadata
      ## Commit message ##
         packed-backend: fsck should warn when "packed-refs" file is empty
     
    -    During fsck, an empty "packed-refs" gives an error; this is unwarranted.
    -    The runtime code paths would accept an empty "packed-refs" file, such as
    -    "create_snapshot" would simply return the "snapshot" without checking
    -    the content of "packed-refs".
    +    We assume the "packed-refs" won't be empty and instead has at least one
    +    line in it (even when there are no refs packed, there is the file header
    +    line). Because there is no terminating LF in the empty file, we will
    +    report "packedRefEntryNotTerminated(ERROR)" to the user.
     
    -    And we should also skip checking the content of "packed-refs" when it is
    -    empty during fsck. However, we should think about whether we need to
    -    report something to the users in this case.
    +    However, the runtime code paths would accept an empty "packed-refs"
    +    file, for example, "create_snapshot" would simply return the "snapshot"
    +    without checking the content of "packed-refs". So, we should skip
    +    checking the content of "packed-refs" when it is empty during fsck.
     
         After 694b7a1999 (repack_without_ref(): write peeled refs in the
         rewritten file, 2013-04-22), we would always write a header into the
    -    "packed-refs" file where we would never create empty file since then.
    -    Because we only create empty "packed-refs" in the very early versions,
    -    we may tighten this rule in the future. In order to notify the users
    -    about this, we should at least report an warning to the users.
    +    "packed-refs" file. So, versions of Git that are not too ancient never
    +    write such an empty "packed-refs" file.
     
    -    But we need to consider the fsck message type carefully, it is not
    -    appropriate that we use "FSCK_ERROR". This is because we would
    -    definitely break the compatibility. Let's create a "FSCK_INFO" message
    -    id EMPTY_PACKED_REFS_FILE" to indicate that "packed-refs" is empty.
    +    As an empty file often indicates a sign of a filesystem-level issue, the
    +    way we want to resolve this inconsistency is not make everybody totally
    +    silent but notice and report the anomaly.
    +
    +    Let's create a "FSCK_INFO" message id "EMPTY_PACKED_REFS_FILE" to report
    +    to the users that "packed-refs" is empty.
     
         Signed-off-by: shejialuo <shejialuo@gmail.com>
     
2:  1a5893379d = 2:  0d050849bc packed-backend: extract snapshot allocation in `load_contents`
3:  31e272db7e ! 3:  fe5ffec8fb packed-backend: mmap large "packed-refs" file during fsck
    @@ Commit message
         current codebase.
     
         As we have introduced the helper function "allocate_snapshot_buffer", we
    -    could simple use this function to use mmap mechanism.
    +    can simply use this function to use mmap mechanism.
     
         Suggested-by: Jeff King <peff@peff.net>
         Suggested-by: Patrick Steinhardt <ps@pks.im>
-- 
2.49.0
Previous: shejialuoNext: shejialuo
Message 59 of 67 in “align the behavior when opening "packed-refs"”
  1. 0/4 align the behavior when opening "packed-refs"shejialuo, May 6, 2025
  2. 1/4 packed-backend: skip checking consistency of empty packed-refs fileshejialuo, May 6, 2025
  3. Junio C HamanoMay 6, 2025
  4. shejialuoMay 7, 2025
  5. Junio C HamanoMay 6, 2025
  6. shejialuoMay 7, 2025
  7. 2/4 packed-backend: extract snapshot allocation in `load_contents`shejialuo, May 6, 2025
  8. Junio C HamanoMay 6, 2025
  9. 3/4 packed-backend: extract munmap operation for `MMAP_TEMPORARY`shejialuo, May 6, 2025
  10. Junio C HamanoMay 6, 2025
  11. Junio C HamanoMay 6, 2025
  12. shejialuoMay 7, 2025
  13. 4/4 packed-backend: use mmap when opening large "packed-refs" fileshejialuo, May 6, 2025
  14. Junio C HamanoMay 6, 2025
  15. Junio C HamanoMay 6, 2025
  16. shejialuoMay 7, 2025
  17. 0/4 align the behavior when opening "packed-refs"shejialuo, May 7, 2025
  18. 1/4 packed-backend: fsck should allow an empty "packed-refs" fileshejialuo, May 7, 2025
  19. 2/4 packed-backend: extract snapshot allocation in `load_contents`shejialuo, May 7, 2025
  20. 3/4 packed-backend: extract munmap operation for `MMAP_TEMPORARY`shejialuo, May 7, 2025
  21. Jeff KingMay 8, 2025
  22. Junio C HamanoMay 8, 2025
  23. shejialuoMay 9, 2025
  24. 4/4 packed-backend: mmap large "packed-refs" file during fsckshejialuo, May 7, 2025
  25. Jeff KingMay 8, 2025
  26. shejialuoMay 9, 2025
  27. Jeff KingMay 9, 2025
  28. shejialuoMay 9, 2025
  29. Junio C HamanoMay 7, 2025
  30. Jeff KingMay 8, 2025
  31. Junio C HamanoMay 8, 2025
  32. Jeff KingMay 8, 2025
  33. shejialuoMay 9, 2025
  34. 0/3 align the behavior when opening "packed-refs"shejialuo, May 11, 2025
  35. 1/3 packed-backend: fsck should allow an empty "packed-refs" fileshejialuo, May 11, 2025
  36. Patrick SteinhardtMay 12, 2025
  37. shejialuoMay 12, 2025
  38. Patrick SteinhardtMay 12, 2025
  39. Jeff KingMay 12, 2025
  40. Junio C HamanoMay 12, 2025
  41. Patrick SteinhardtMay 13, 2025
  42. shejialuoMay 13, 2025
  43. 2/3 packed-backend: extract snapshot allocation in `load_contents`shejialuo, May 11, 2025
  44. Patrick SteinhardtMay 12, 2025
  45. shejialuoMay 12, 2025
  46. Patrick SteinhardtMay 12, 2025
  47. Jeff KingMay 12, 2025
  48. shejialuoMay 13, 2025
  49. 3/3 packed-backend: mmap large "packed-refs" file during fsckshejialuo, May 11, 2025
  50. Jeff KingMay 12, 2025
  51. 0/3 align the behavior when opening "packed-refs"shejialuo, May 13, 2025
  52. 1/3 packed-backend: fsck should warn when "packed-refs" file is emptyshejialuo, May 13, 2025
  53. Junio C HamanoMay 13, 2025
  54. shejialuoMay 14, 2025
  55. 3/3 packed-backend: mmap large "packed-refs" file during fsckshejialuo, May 13, 2025
  56. Junio C HamanoMay 13, 2025
  57. shejialuoMay 14, 2025
  58. 2/3 packed-backend: extract snapshot allocation in `load_contents`shejialuo, May 13, 2025
  59. 0/3 align the behavior when opening "packed-refs"shejialuo, May 14, 2025
  60. 2/3 packed-backend: extract snapshot allocation in `load_contents`shejialuo, May 14, 2025
  61. 1/3 packed-backend: fsck should warn when "packed-refs" file is emptyshejialuo, May 14, 2025
  62. 3/3 packed-backend: mmap large "packed-refs" file during fsckshejialuo, May 14, 2025
  63. Junio C HamanoMay 15, 2025
  64. Junio C HamanoMay 21, 2025
  65. Jeff KingMay 22, 2025
  66. Patrick SteinhardtMay 23, 2025
  67. Junio C HamanoMay 23, 2025

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.