git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jonathan Tan <jonathantanmy@google.com>
To: git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>,
	Calvin Wan <calvinwan@google.com>,
	phillip.wood123@gmail.com, Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v4 0/4] Preliminary patches before git-std-lib
Date: Fri, 29 Sep 2023 14:20:47 -0700	[thread overview]
Message-ID: <cover.1696021277.git.jonathantanmy@google.com> (raw)
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


  parent reply	other threads:[~2023-09-29 21:21 UTC|newest]

Thread overview: 111+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-27 19:52 [RFC PATCH 0/8] Introduce Git Standard Library Calvin Wan
2023-06-27 19:52 ` [RFC PATCH 1/8] trace2: log fsync stats in trace2 rather than wrapper Calvin Wan
2023-06-28  2:05   ` Victoria Dye
2023-07-05 17:57     ` Calvin Wan
2023-07-05 18:22       ` Victoria Dye
2023-07-11 20:07   ` Jeff Hostetler
2023-06-27 19:52 ` [RFC PATCH 2/8] hex-ll: split out functionality from hex Calvin Wan
2023-06-28 13:15   ` Phillip Wood
2023-06-28 16:55     ` Calvin Wan
2023-06-27 19:52 ` [RFC PATCH 3/8] object: move function to object.c Calvin Wan
2023-06-27 19:52 ` [RFC PATCH 4/8] config: correct bad boolean env value error message Calvin Wan
2023-06-27 19:52 ` [RFC PATCH 5/8] parse: create new library for parsing strings and env values Calvin Wan
2023-06-27 22:58   ` Junio C Hamano
2023-06-27 19:52 ` [RFC PATCH 6/8] pager: remove pager_in_use() Calvin Wan
2023-06-27 23:00   ` Junio C Hamano
2023-06-27 23:18     ` Calvin Wan
2023-06-28  0:30     ` Glen Choo
2023-06-28 16:37       ` Glen Choo
2023-06-28 16:44         ` Calvin Wan
2023-06-28 17:30           ` Junio C Hamano
2023-06-28 20:58       ` Junio C Hamano
2023-06-27 19:52 ` [RFC PATCH 7/8] git-std-lib: introduce git standard library Calvin Wan
2023-06-28 13:27   ` Phillip Wood
2023-06-28 21:15     ` Calvin Wan
2023-06-30 10:00       ` Phillip Wood
2023-06-27 19:52 ` [RFC PATCH 8/8] git-std-lib: add test file to call git-std-lib.a functions Calvin Wan
2023-06-28  0:14 ` [RFC PATCH 0/8] Introduce Git Standard Library Glen Choo
2023-06-28 16:30   ` Calvin Wan
2023-06-30  7:01 ` Linus Arver
2023-08-10 16:33 ` [RFC PATCH v2 0/7] " Calvin Wan
2023-08-10 16:36   ` [RFC PATCH v2 1/7] hex-ll: split out functionality from hex Calvin Wan
2023-08-10 16:36   ` [RFC PATCH v2 2/7] object: move function to object.c Calvin Wan
2023-08-10 20:32     ` Junio C Hamano
2023-08-10 22:36     ` Glen Choo
2023-08-10 22:43       ` Junio C Hamano
2023-08-10 16:36   ` [RFC PATCH v2 3/7] config: correct bad boolean env value error message Calvin Wan
2023-08-10 20:36     ` Junio C Hamano
2023-08-10 16:36   ` [RFC PATCH v2 4/7] parse: create new library for parsing strings and env values Calvin Wan
2023-08-10 23:21     ` Glen Choo
2023-08-10 23:43       ` Junio C Hamano
2023-08-14 22:15       ` Jonathan Tan
2023-08-14 22:09     ` Jonathan Tan
2023-08-14 22:19       ` Junio C Hamano
2023-08-10 16:36   ` [RFC PATCH v2 5/7] date: push pager.h dependency up Calvin Wan
2023-08-10 23:41     ` Glen Choo
2023-08-14 22:17     ` Jonathan Tan
2023-08-10 16:36   ` [RFC PATCH v2 6/7] git-std-lib: introduce git standard library Calvin Wan
2023-08-14 22:26     ` Jonathan Tan
2023-08-10 16:36   ` [RFC PATCH v2 7/7] git-std-lib: add test file to call git-std-lib.a functions Calvin Wan
2023-08-14 22:28     ` Jonathan Tan
2023-08-10 22:05   ` [RFC PATCH v2 0/7] Introduce Git Standard Library Glen Choo
2023-08-15  9:20     ` Phillip Wood
2023-08-16 17:17       ` Calvin Wan
2023-08-16 21:19         ` Junio C Hamano
2023-08-15  9:41   ` Phillip Wood
2023-09-08 17:41     ` [PATCH v3 0/6] " Calvin Wan
2023-09-08 17:44       ` [PATCH v3 1/6] hex-ll: split out functionality from hex Calvin Wan
2023-09-08 17:44       ` [PATCH v3 2/6] wrapper: remove dependency to Git-specific internal file Calvin Wan
2023-09-15 17:54         ` Jonathan Tan
2023-09-08 17:44       ` [PATCH v3 3/6] config: correct bad boolean env value error message Calvin Wan
2023-09-08 17:44       ` [PATCH v3 4/6] parse: create new library for parsing strings and env values Calvin Wan
2023-09-08 17:44       ` [PATCH v3 5/6] git-std-lib: introduce git standard library Calvin Wan
2023-09-11 13:22         ` Phillip Wood
2023-09-27 14:14           ` Phillip Wood
2023-09-15 18:39         ` Jonathan Tan
2023-09-26 14:23         ` phillip.wood123
2023-09-08 17:44       ` [PATCH v3 6/6] git-std-lib: add test file to call git-std-lib.a functions Calvin Wan
2023-09-09  5:26         ` Junio C Hamano
2023-09-15 18:43         ` Jonathan Tan
2023-09-15 20:22           ` Junio C Hamano
2023-09-08 20:36       ` [PATCH v3 0/6] Introduce Git Standard Library Junio C Hamano
2023-09-08 21:30         ` Junio C Hamano
2023-09-29 21:20 ` Jonathan Tan [this message]
2023-09-29 21:20   ` [PATCH v4 1/4] hex-ll: separate out non-hash-algo functions Jonathan Tan
2023-10-21  4:14     ` Linus Arver
2023-09-29 21:20   ` [PATCH v4 2/4] wrapper: reduce scope of remove_or_warn() Jonathan Tan
2023-10-10  9:59     ` phillip.wood123
2023-10-10 16:13       ` Junio C Hamano
2023-10-10 17:38         ` Jonathan Tan
2023-09-29 21:20   ` [PATCH v4 3/4] config: correct bad boolean env value error message Jonathan Tan
2023-09-29 23:03     ` Junio C Hamano
2023-09-29 21:20   ` [PATCH v4 4/4] parse: separate out parsing functions from config.h Jonathan Tan
2023-10-10 10:00     ` phillip.wood123
2023-10-10 17:43       ` Jonathan Tan
2023-10-10 17:58         ` Phillip Wood
2023-10-10 20:57           ` Junio C Hamano
2023-10-10 10:05   ` [PATCH v4 0/4] Preliminary patches before git-std-lib phillip.wood123
2023-10-10 16:21     ` Jonathan Tan
2024-02-22 17:50   ` [PATCH v5 0/3] Introduce Git Standard Library Calvin Wan
2024-02-22 17:50   ` [PATCH v5 1/3] pager: include stdint.h because uintmax_t is used Calvin Wan
2024-02-22 21:43     ` Junio C Hamano
2024-02-26 18:59       ` Kyle Lippincott
2024-02-27  0:20         ` Junio C Hamano
2024-02-27  0:56           ` Kyle Lippincott
2024-02-27  2:45             ` Junio C Hamano
2024-02-27 22:29               ` Kyle Lippincott
2024-02-27 23:25                 ` Junio C Hamano
2024-02-27  8:45             ` Jeff King
2024-02-27  9:05               ` Jeff King
2024-02-27 20:10               ` Kyle Lippincott
2024-02-24  1:33     ` Kyle Lippincott
2024-02-24  7:58       ` Junio C Hamano
2024-02-22 17:50   ` [PATCH v5 2/3] git-std-lib: introduce Git Standard Library Calvin Wan
2024-02-29 11:16     ` Phillip Wood
2024-02-29 17:23       ` Junio C Hamano
2024-02-29 18:27         ` Linus Arver
2024-02-29 18:54           ` Junio C Hamano
2024-02-29 20:03             ` Linus Arver
2024-02-22 17:50   ` [PATCH v5 3/3] test-stdlib: show that git-std-lib is independent Calvin Wan
2024-02-22 22:24     ` Junio C Hamano
2024-03-07 21:13     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1696021277.git.jonathantanmy@google.com \
    --to=jonathantanmy@google.com \
    --cc=calvinwan@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=phillip.wood123@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).