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

[PATCH v4 0/4] Preliminary patches before git-std-lib

From
JTJonathan Tan <jonathantanmy@google.com>
Date
Sep 29, 2023, 21:20 UTC
Message-ID
<cover.1696021277.git.jonathantanmy@google.com>
In-Reply-To
<20230627195251.1973421-1-calvinwan@google.com>

Calvin will be away for a few weeks and I'll be handling the git-std-lib effort in the meantime. My goals will be:

- Get the preliminary patches in Calvin's patch set (patches 1-4) merged
first.
- Updating patches 5-6 based on reviewer feedback (including my
feedback). I have several aims including reducing or eliminating the
need for the GIT_STD_LIB preprocessor variable, and making stubs a test-
only concern (I think Phillip has some similar ideas [1] but I haven't
looked at their repo on GitHub yet).
[1] https://lore.kernel.org/git/98f3edcf-7f37-45ff-abd2-c0038d4e0589@gmail.com/

This patch set is in service of the first goal. Because the libification patches are no longer included in this patch set, I have rewritten the commit messages to justify the patches in terms of code organization. There are no changes in the code itself. Also, I have retained Calvin's name as the author.

Putting on my reviewer hat, if I was reviewing hex.h and config.h from scratch, I would have not thought twice about requesting the changes in these patches. But since we are not creating them from scratch but modifying existing files, a question does arise about whether it's worth the additional noise that someone looking through history needs to handle. In this case, I think it's worth it - I think the future code delver would appreciate being able to see the evolution of hex, hash algo, config, and parse functions as their own files rather than, when looking at one of them, having to filter out unrelated changes.

Besides that, as Calvin has described in his other emails, these patches are prerequisites to being able to independently compile and use a certain subset of the .c files. With patches that solely refactor, there is sometimes a worry that the benefits are nebulous and that we would be moving code around for nothing, but I don't think that that applies here: there is still more work to be done on patches 5 and 6, but what we have in patches 5 and 6 now shows that the benefits are concrete and within reach.

Calvin Wan (4):
  hex-ll: separate out non-hash-algo functions
  wrapper: reduce scope of remove_or_warn()
  config: correct bad boolean env value error message
  parse: separate out parsing functions from config.h
 Makefile                   |   2 +
 attr.c                     |   2 +-
 color.c                    |   2 +-
 config.c                   | 173 +----------------------------------
 config.h                   |  14 +--
 entry.c                    |   5 +
 entry.h                    |   6 ++
 hex-ll.c                   |  49 ++++++++++
 hex-ll.h                   |  27 ++++++
 hex.c                      |  47 ----------
 hex.h                      |  24 +----
 mailinfo.c                 |   2 +-
 pack-objects.c             |   2 +-
 pack-revindex.c            |   2 +-
 parse-options.c            |   3 +-
 parse.c                    | 182 +++++++++++++++++++++++++++++++++++++
 parse.h                    |  20 ++++
 pathspec.c                 |   2 +-
 preload-index.c            |   2 +-
 progress.c                 |   2 +-
 prompt.c                   |   2 +-
 rebase.c                   |   2 +-
 strbuf.c                   |   2 +-
 t/helper/test-env-helper.c |   2 +-
 unpack-trees.c             |   2 +-
 url.c                      |   2 +-
 urlmatch.c                 |   2 +-
 wrapper.c                  |   8 +-
 wrapper.h                  |   5 -
 write-or-die.c             |   2 +-
 30 files changed, 313 insertions(+), 284 deletions(-)
 create mode 100644 hex-ll.c
 create mode 100644 hex-ll.h
 create mode 100644 parse.c
 create mode 100644 parse.h
Range-diff against v3:
1:  fcce01bc19 ! 1:  02ecc00e9c hex-ll: split out functionality from hex
    @@ Metadata
     Author: Calvin Wan <calvinwan@google.com>
     
      ## Commit message ##
    -    hex-ll: split out functionality from hex
    +    hex-ll: separate out non-hash-algo functions
     
    -    Separate out hex functionality that doesn't require a hash algo into
    -    hex-ll.[ch]. Since the hash algo is currently a global that sits in
    -    repository, this separation removes that dependency for files that only
    -    need basic hex manipulation functions.
    +    In order to further reduce all-in-one headers, separate out functions in
    +    hex.h that do not operate on object hashes into its own file, hex-ll.h,
    +    and update the include directives in the .c files that need only such
    +    functions accordingly.
     
         Signed-off-by: Calvin Wan <calvinwan@google.com>
    -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
    +    Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
     
      ## Makefile ##
     @@ Makefile: LIB_OBJS += hash-lookup.o
2:  95a369d02b ! 2:  c9e7cd7857 wrapper: remove dependency to Git-specific internal file
    @@ Metadata
     Author: Calvin Wan <calvinwan@google.com>
     
      ## Commit message ##
    -    wrapper: remove dependency to Git-specific internal file
    +    wrapper: reduce scope of remove_or_warn()
     
    -    In order for wrapper.c to be built independently as part of a smaller
    -    library, it cannot have dependencies to other Git specific
    -    internals. remove_or_warn() creates an unnecessary dependency to
    -    object.h in wrapper.c. Therefore move the function to entry.[ch] which
    -    performs changes on the worktree based on the Git-specific file modes in
    -    the index.
    +    remove_or_warn() is only used by entry.c and apply.c, but it is
    +    currently declared and defined in wrapper.{h,c}, so it has a scope much
    +    greater than it needs. This needlessly large scope also causes wrapper.c
    +    to need to include object.h, when this file is largely unconcerned with
    +    Git objects.
    +
    +    Move remove_or_warn() to entry.{h,c}. The file apply.c still has access
    +    to it, since it already includes entry.h for another reason.
     
         Signed-off-by: Calvin Wan <calvinwan@google.com>
    -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
    +    Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
     
      ## entry.c ##
     @@ entry.c: void unlink_entry(const struct cache_entry *ce, const char *super_prefix)
3:  5348528865 = 3:  e4c20a81f9 config: correct bad boolean env value error message
4:  b5a8945c5c ! 4:  5d9f0b3de0 parse: create new library for parsing strings and env values
    @@ Metadata
     Author: Calvin Wan <calvinwan@google.com>
     
      ## Commit message ##
    -    parse: create new library for parsing strings and env values
    +    parse: separate out parsing functions from config.h
     
    -    While string and environment value parsing is mainly consumed by
    -    config.c, there are other files that only need parsing functionality and
    -    not config functionality. By separating out string and environment value
    -    parsing from config, those files can instead be dependent on parse,
    -    which has a much smaller dependency chain than config. This ultimately
    -    allows us to inclue parse.[ch] in an independent library since it
    -    doesn't have dependencies to Git-specific internals unlike in
    -    config.[ch].
    +    The files config.{h,c} contain functions that have to do with parsing,
    +    but not config.
     
    -    Move general string and env parsing functions from config.[ch] to
    -    parse.[ch].
    +    In order to further reduce all-in-one headers, separate out functions in
    +    config.c that do not operate on config into its own file, parse.h,
    +    and update the include directives in the .c files that need only such
    +    functions accordingly.
     
         Signed-off-by: Calvin Wan <calvinwan@google.com>
    -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
    +    Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
     
      ## Makefile ##
     @@ Makefile: LIB_OBJS += pack-write.o
-- 
2.42.0.582.g8ccd20d70d-goog
Previous: Junio C HamanoNext: Jonathan Tan
Message 73 of 111 in “Introduce Git Standard Library”
  1. 0/8 Introduce Git Standard LibraryCalvin Wan, Jun 27, 2023
  2. 1/8 trace2: log fsync stats in trace2 rather than wrapperCalvin Wan, Jun 27, 2023
  3. Victoria DyeJun 28, 2023
  4. Calvin WanJul 5, 2023
  5. Victoria DyeJul 5, 2023
  6. Jeff HostetlerJul 11, 2023
  7. 2/8 hex-ll: split out functionality from hexCalvin Wan, Jun 27, 2023
  8. Phillip WoodJun 28, 2023
  9. Calvin WanJun 28, 2023
  10. 3/8 object: move function to object.cCalvin Wan, Jun 27, 2023
  11. 4/8 config: correct bad boolean env value error messageCalvin Wan, Jun 27, 2023
  12. 5/8 parse: create new library for parsing strings and env valuesCalvin Wan, Jun 27, 2023
  13. Junio C HamanoJun 27, 2023
  14. 6/8 pager: remove pager_in_use()Calvin Wan, Jun 27, 2023
  15. Junio C HamanoJun 27, 2023
  16. Calvin WanJun 27, 2023
  17. Glen ChooJun 28, 2023
  18. Glen ChooJun 28, 2023
  19. Calvin WanJun 28, 2023
  20. Junio C HamanoJun 28, 2023
  21. Junio C HamanoJun 28, 2023
  22. 8/8 git-std-lib: add test file to call git-std-lib.a functionsCalvin Wan, Jun 27, 2023
  23. 7/8 git-std-lib: introduce git standard libraryCalvin Wan, Jun 27, 2023
  24. Phillip WoodJun 28, 2023
  25. Calvin WanJun 28, 2023
  26. Phillip WoodJun 30, 2023
  27. Glen ChooJun 28, 2023
  28. Calvin WanJun 28, 2023
  29. Linus ArverJun 30, 2023
  30. 0/7 Introduce Git Standard LibraryCalvin Wan, Aug 10, 2023
  31. 2/7 object: move function to object.cCalvin Wan, Aug 10, 2023
  32. Junio C HamanoAug 10, 2023
  33. Glen ChooAug 10, 2023
  34. Junio C HamanoAug 10, 2023
  35. 1/7 hex-ll: split out functionality from hexCalvin Wan, Aug 10, 2023
  36. 3/7 config: correct bad boolean env value error messageCalvin Wan, Aug 10, 2023
  37. Junio C HamanoAug 10, 2023
  38. 5/7 date: push pager.h dependency upCalvin Wan, Aug 10, 2023
  39. Glen ChooAug 10, 2023
  40. Jonathan TanAug 14, 2023
  41. 4/7 parse: create new library for parsing strings and env valuesCalvin Wan, Aug 10, 2023
  42. Glen ChooAug 10, 2023
  43. Junio C HamanoAug 10, 2023
  44. Jonathan TanAug 14, 2023
  45. Jonathan TanAug 14, 2023
  46. Junio C HamanoAug 14, 2023
  47. 7/7 git-std-lib: add test file to call git-std-lib.a functionsCalvin Wan, Aug 10, 2023
  48. Jonathan TanAug 14, 2023
  49. 6/7 git-std-lib: introduce git standard libraryCalvin Wan, Aug 10, 2023
  50. Jonathan TanAug 14, 2023
  51. Glen ChooAug 10, 2023
  52. Phillip WoodAug 15, 2023
  53. Calvin WanAug 16, 2023
  54. Junio C HamanoAug 16, 2023
  55. Phillip WoodAug 15, 2023
  56. 0/6 Introduce Git Standard LibraryCalvin Wan, Sep 8, 2023
  57. 2/6 wrapper: remove dependency to Git-specific internal fileCalvin Wan, Sep 8, 2023
  58. Jonathan TanSep 15, 2023
  59. 1/6 hex-ll: split out functionality from hexCalvin Wan, Sep 8, 2023
  60. 3/6 config: correct bad boolean env value error messageCalvin Wan, Sep 8, 2023
  61. 4/6 parse: create new library for parsing strings and env valuesCalvin Wan, Sep 8, 2023
  62. 5/6 git-std-lib: introduce git standard libraryCalvin Wan, Sep 8, 2023
  63. Phillip WoodSep 11, 2023
  64. Phillip WoodSep 27, 2023
  65. Jonathan TanSep 15, 2023
  66. phillip.wood123@gmail.comSep 26, 2023
  67. 6/6 git-std-lib: add test file to call git-std-lib.a functionsCalvin Wan, Sep 8, 2023
  68. Junio C HamanoSep 9, 2023
  69. Jonathan TanSep 15, 2023
  70. Junio C HamanoSep 15, 2023
  71. Junio C HamanoSep 8, 2023
  72. Junio C HamanoSep 8, 2023
  73. 0/4 Preliminary patches before git-std-libJonathan Tan, Sep 29, 2023
  74. 1/4 hex-ll: separate out non-hash-algo functionsJonathan Tan, Sep 29, 2023
  75. Linus ArverOct 21, 2023
  76. 2/4 wrapper: reduce scope of remove_or_warn()Jonathan Tan, Sep 29, 2023
  77. phillip.wood123@gmail.comOct 10, 2023
  78. Junio C HamanoOct 10, 2023
  79. Jonathan TanOct 10, 2023
  80. 3/4 config: correct bad boolean env value error messageJonathan Tan, Sep 29, 2023
  81. Junio C HamanoSep 29, 2023
  82. 4/4 parse: separate out parsing functions from config.hJonathan Tan, Sep 29, 2023
  83. phillip.wood123@gmail.comOct 10, 2023
  84. Jonathan TanOct 10, 2023
  85. Phillip WoodOct 10, 2023
  86. Junio C HamanoOct 10, 2023
  87. phillip.wood123@gmail.comOct 10, 2023
  88. Jonathan TanOct 10, 2023
  89. 0/3 Introduce Git Standard LibraryCalvin Wan, Feb 22, 2024
  90. 1/3 pager: include stdint.h because uintmax_t is usedCalvin Wan, Feb 22, 2024
  91. Junio C HamanoFeb 22, 2024
  92. Kyle LippincottFeb 26, 2024
  93. Junio C HamanoFeb 27, 2024
  94. Kyle LippincottFeb 27, 2024
  95. Junio C HamanoFeb 27, 2024
  96. Kyle LippincottFeb 27, 2024
  97. Junio C HamanoFeb 27, 2024
  98. Jeff KingFeb 27, 2024
  99. Jeff KingFeb 27, 2024
  100. Kyle LippincottFeb 27, 2024
  101. Kyle LippincottFeb 24, 2024
  102. Junio C HamanoFeb 24, 2024
  103. 2/3 git-std-lib: introduce Git Standard LibraryCalvin Wan, Feb 22, 2024
  104. Phillip WoodFeb 29, 2024
  105. Junio C HamanoFeb 29, 2024
  106. Linus ArverFeb 29, 2024
  107. Junio C HamanoFeb 29, 2024
  108. Linus ArverFeb 29, 2024
  109. 3/3 test-stdlib: show that git-std-lib is independentCalvin Wan, Feb 22, 2024
  110. Junio C HamanoFeb 22, 2024
  111. Junio C HamanoMar 7, 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.