Message ID | 20230810163346.274132-1-calvinwan@google.com (mailing list archive) |
---|---|
Headers | show |
Series | Introduce Git Standard Library | expand |
Calvin Wan <calvinwan@google.com> writes: > Calvin Wan (7): > hex-ll: split out functionality from hex > object: move function to object.c > config: correct bad boolean env value error message > parse: create new library for parsing strings and env values > date: push pager.h dependency up > git-std-lib: introduce git standard library > git-std-lib: add test file to call git-std-lib.a functions This doesn't seem to apply to 'master'. Do you have a base commit that reviewers could apply the patches to?
On 10/08/2023 23:05, Glen Choo wrote: > Calvin Wan <calvinwan@google.com> writes: > >> Calvin Wan (7): >> hex-ll: split out functionality from hex >> object: move function to object.c >> config: correct bad boolean env value error message >> parse: create new library for parsing strings and env values >> date: push pager.h dependency up >> git-std-lib: introduce git standard library >> git-std-lib: add test file to call git-std-lib.a functions > > This doesn't seem to apply to 'master'. Do you have a base commit that > reviewers could apply the patches to? I don't know what they are based on, but I did manage to apply them to master by using "am -3" and resolving the conflicts. The result is at https://github.com/phillipwood/git/tree/cw/git-std-lib/rfc-v2 if anyone is interested. Best Wishes Phillip
Hi Calvin On 10/08/2023 17:33, Calvin Wan wrote: > Original cover letter: > https://lore.kernel.org/git/20230627195251.1973421-1-calvinwan@google.com/ > > In the initial RFC, I had a patch that removed the trace2 dependency > from usage.c so that git-std-lib.a would not have dependencies outside > of git-std-lib.a files. Consequently this meant that tracing would not > be possible in git-std-lib.a files for other developers of Git, and it > is not a good idea for the libification effort to close the door on > tracing in certain files for future development (thanks Victoria for > pointing this out). That patch has been removed and instead I introduce > stubbed out versions of repository.[ch] and trace2.[ch] that are swapped > in during compilation time (I'm no Makefile expert so any advice on how > on I could do this better would be much appreciated). These stubbed out > files contain no implementations and therefore do not have any > additional dependencies, allowing git-std-lib.a to compile with only the > stubs as additional dependencies. I think stubbing out trace2 is a sensible approach. I don't think we need separate headers when using the stub though, or a stub for repository.c as we don't call any of the functions declared in that header. I've appended a patch that shows a simplified stub. It also removes the recursive make call as it no-longer needs to juggle the header files. > This also has the added benefit of > removing `#ifdef GIT_STD_LIB` macros in C files for specific library > compilation rules. Libification shouldn't pollute C files with these > macros. The boundaries for git-std-lib.a have also been updated to > contain these stubbed out files. Do you have any plans to support building with gettext support so we can use git-std-lib.a as a dependency of libgit.a? > I have also made some additional changes to the Makefile to piggy back > off of our existing build rules for .c/.o targets and their > dependencies. As I learn more about Makefiles, I am continuing to look > for ways to improve these rules. Eventually I would like to be able to > have a set of rules that future libraries can emulate and is scalable > in the sense of not creating additional toil for developers that are not > interested in libification. I'm not sure reusing LIB_OBJS for different targets is a good idea. Once libgit.a starts to depend on git-std-lib.a we'll want to build them both with a single make invocation without resorting to recursive make calls. I think we could perhaps make a template function to create the compilation rules for each library - see the end of https://wingolog.org/archives/2023/08/08/a-negative-result Best Wishes Phillip ---- >8 ----- From 194403e42f116cc3c6ed8eb8b03d6933b24067e4 Mon Sep 17 00:00:00 2001 From: Phillip Wood <phillip.wood@dunelm.org.uk> Date: Sat, 12 Aug 2023 17:27:23 +0100 Subject: [PATCH] git-std-lib: simplify sub implementation The code in std-lib does not depend directly on the functions declared in repository.h and so it does not need to provide stub implementations of the functions declared in repository.h. There is a transitive dependency on `struct repository` from the functions declared in trace2.h but the stub implementation of those functions can simply define its own stub for struct repository. There is also no need to use different headers when compiling against the stub implementation of trace2. This means we can simplify the stub implementation by removing stubs/{repository.[ch],trace2.h} and simplify the Makefile by removing the code that replaces header files when compiling against the trace2 stub. git-std-lib.a can now be built by running make git-std-lib.a GIT_STD_LIB=YesPlease STUB_TRACE2=YesPlease There is one other small fixup in this commit: - `wrapper.c` includes `repository.h` but does not use any of the declarations. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- Makefile | 29 +------------------- stubs/repository.c | 4 --- stubs/repository.h | 8 ------ stubs/trace2.c | 5 ++++ stubs/trace2.h | 68 ---------------------------------------------- wrapper.c | 1 - 6 files changed, 6 insertions(+), 109 deletions(-) delete mode 100644 stubs/repository.c delete mode 100644 stubs/repository.h delete mode 100644 stubs/trace2.h diff --git a/Makefile b/Makefile index a821d73c9d0..8eff4021025 100644 --- a/Makefile +++ b/Makefile @@ -1209,10 +1209,6 @@ LIB_OBJS += usage.o LIB_OBJS += utf8.o LIB_OBJS += wrapper.o -ifdef STUB_REPOSITORY -STUB_OBJS += stubs/repository.o -endif - ifdef STUB_TRACE2 STUB_OBJS += stubs/trace2.o endif @@ -3866,31 +3862,8 @@ fuzz-all: $(FUZZ_PROGRAMS) ### Libified Git rules # git-std-lib -# `make git-std-lib GIT_STD_LIB=YesPlease STUB_REPOSITORY=YesPlease STUB_TRACE2=YesPlease` +# `make git-std-lib.a GIT_STD_LIB=YesPlease STUB_TRACE2=YesPlease` STD_LIB = git-std-lib.a $(STD_LIB): $(LIB_OBJS) $(COMPAT_OBJS) $(STUB_OBJS) $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^ - -TEMP_HEADERS = temp_headers/ - -git-std-lib: -# Move headers to temporary folder and replace them with stubbed headers. -# After building, move headers and stubbed headers back. -ifneq ($(STUB_OBJS),) - mkdir -p $(TEMP_HEADERS); \ - for d in $(STUB_OBJS); do \ - BASE=$${d%.*}; \ - mv $${BASE##*/}.h $(TEMP_HEADERS)$${BASE##*/}.h; \ - mv $${BASE}.h $${BASE##*/}.h; \ - done; \ - $(MAKE) $(STD_LIB); \ - for d in $(STUB_OBJS); do \ - BASE=$${d%.*}; \ - mv $${BASE##*/}.h $${BASE}.h; \ - mv $(TEMP_HEADERS)$${BASE##*/}.h $${BASE##*/}.h; \ - done; \ - rm -rf temp_headers -else - $(MAKE) $(STD_LIB) -endif diff --git a/stubs/repository.c b/stubs/repository.c deleted file mode 100644 index f81520d083a..00000000000 --- a/stubs/repository.c +++ /dev/null @@ -1,4 +0,0 @@ -#include "git-compat-util.h" -#include "repository.h" - -struct repository *the_repository; diff --git a/stubs/repository.h b/stubs/repository.h deleted file mode 100644 index 18262d748e5..00000000000 --- a/stubs/repository.h +++ /dev/null @@ -1,8 +0,0 @@ -#ifndef REPOSITORY_H -#define REPOSITORY_H - -struct repository { int stub; }; - -extern struct repository *the_repository; - -#endif /* REPOSITORY_H */ diff --git a/stubs/trace2.c b/stubs/trace2.c index efc3f9c1f39..7d894822288 100644 --- a/stubs/trace2.c +++ b/stubs/trace2.c @@ -1,6 +1,10 @@ #include "git-compat-util.h" #include "trace2.h" +struct child_process { int stub; }; +struct repository { int stub; }; +struct json_writer { int stub; }; + void trace2_region_enter_fl(const char *file, int line, const char *category, const char *label, const struct repository *repo, ...) { } void trace2_region_leave_fl(const char *file, int line, const char *category, @@ -19,4 +23,5 @@ void trace2_data_intmax_fl(const char *file, int line, const char *category, const struct repository *repo, const char *key, intmax_t value) { } int trace2_is_enabled(void) { return 0; } +void trace2_counter_add(enum trace2_counter_id cid, uint64_t value) { } void trace2_collect_process_info(enum trace2_process_info_reason reason) { } diff --git a/stubs/trace2.h b/stubs/trace2.h deleted file mode 100644 index 836a14797cc..00000000000 --- a/stubs/trace2.h +++ /dev/null @@ -1,68 +0,0 @@ -#ifndef TRACE2_H -#define TRACE2_H - -struct child_process { int stub; }; -struct repository; -struct json_writer { int stub; }; - -void trace2_region_enter_fl(const char *file, int line, const char *category, - const char *label, const struct repository *repo, ...); - -#define trace2_region_enter(category, label, repo) \ - trace2_region_enter_fl(__FILE__, __LINE__, (category), (label), (repo)) - -void trace2_region_leave_fl(const char *file, int line, const char *category, - const char *label, const struct repository *repo, ...); - -#define trace2_region_leave(category, label, repo) \ - trace2_region_leave_fl(__FILE__, __LINE__, (category), (label), (repo)) - -void trace2_data_string_fl(const char *file, int line, const char *category, - const struct repository *repo, const char *key, - const char *value); - -#define trace2_data_string(category, repo, key, value) \ - trace2_data_string_fl(__FILE__, __LINE__, (category), (repo), (key), \ - (value)) - -void trace2_cmd_ancestry_fl(const char *file, int line, const char **parent_names); - -#define trace2_cmd_ancestry(v) trace2_cmd_ancestry_fl(__FILE__, __LINE__, (v)) - -void trace2_cmd_error_va_fl(const char *file, int line, const char *fmt, - va_list ap); - -#define trace2_cmd_error_va(fmt, ap) \ - trace2_cmd_error_va_fl(__FILE__, __LINE__, (fmt), (ap)) - - -void trace2_cmd_name_fl(const char *file, int line, const char *name); - -#define trace2_cmd_name(v) trace2_cmd_name_fl(__FILE__, __LINE__, (v)) - -void trace2_thread_start_fl(const char *file, int line, - const char *thread_base_name); - -#define trace2_thread_start(thread_base_name) \ - trace2_thread_start_fl(__FILE__, __LINE__, (thread_base_name)) - -void trace2_thread_exit_fl(const char *file, int line); - -#define trace2_thread_exit() trace2_thread_exit_fl(__FILE__, __LINE__) - -void trace2_data_intmax_fl(const char *file, int line, const char *category, - const struct repository *repo, const char *key, - intmax_t value); - -#define trace2_data_intmax(category, repo, key, value) \ - trace2_data_intmax_fl(__FILE__, __LINE__, (category), (repo), (key), \ - (value)) - -enum trace2_process_info_reason { - TRACE2_PROCESS_INFO_STARTUP, - TRACE2_PROCESS_INFO_EXIT, -}; -int trace2_is_enabled(void); -void trace2_collect_process_info(enum trace2_process_info_reason reason); - -#endif /* TRACE2_H */ diff --git a/wrapper.c b/wrapper.c index 9eae4a8b3a0..e6facc5ff0c 100644 --- a/wrapper.c +++ b/wrapper.c @@ -5,7 +5,6 @@ #include "abspath.h" #include "parse.h" #include "gettext.h" -#include "repository.h" #include "strbuf.h" #include "trace2.h"
Thanks for resolving the conflicts on master. I should've rebased before sending out this v2 since it's built off of 2.41 with some of my other patch cleanup series.
Calvin Wan <calvinwan@google.com> writes: > Thanks for resolving the conflicts on master. I should've rebased > before sending out this v2 since it's built off of 2.41 with some of > my other patch cleanup series. I think the freeze period before the release would be a good time to rebuild on an updated base to prepare v3 for posting. Thanks.
Original cover letter: https://lore.kernel.org/git/20230627195251.1973421-1-calvinwan@google.com/ I have taken this series out of RFC since there weren't any significant concerns with the overall concept and design of this series. This reroll incorporates some smaller changes such as dropping the "push pager dependency" patch in favor of stubbing it out. The main change this reroll cleans up the Makefile rules and stubs, as suggested by Phillip Wood (appreciate the help on this one)! This series has been rebased onto 1fc548b2d6a: The sixth batch Originally this series was built on other patches that have since been merged, which is why the range-diff is shown removing many of them. Calvin Wan (6): hex-ll: split out functionality from hex wrapper: remove dependency to Git-specific internal file config: correct bad boolean env value error message parse: create new library for parsing strings and env values git-std-lib: introduce git standard library git-std-lib: add test file to call git-std-lib.a functions Documentation/technical/git-std-lib.txt | 191 ++++++++++++++++++++ Makefile | 41 ++++- attr.c | 2 +- color.c | 2 +- config.c | 173 +----------------- config.h | 14 +- entry.c | 5 + entry.h | 6 + git-compat-util.h | 7 +- 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 +- stubs/pager.c | 6 + stubs/pager.h | 6 + stubs/trace2.c | 27 +++ symlinks.c | 2 + t/Makefile | 4 + t/helper/test-env-helper.c | 2 +- t/stdlib-test.c | 231 ++++++++++++++++++++++++ unpack-trees.c | 2 +- url.c | 2 +- urlmatch.c | 2 +- wrapper.c | 9 +- wrapper.h | 5 - write-or-die.c | 2 +- 38 files changed, 824 insertions(+), 287 deletions(-) create mode 100644 Documentation/technical/git-std-lib.txt create mode 100644 hex-ll.c create mode 100644 hex-ll.h create mode 100644 parse.c create mode 100644 parse.h create mode 100644 stubs/pager.c create mode 100644 stubs/pager.h create mode 100644 stubs/trace2.c create mode 100644 t/stdlib-test.c Range-diff against v2: 1: 121788f263 < -: ---------- strbuf: clarify API boundary 2: 5e91404ecd < -: ---------- strbuf: clarify dependency 3: 5c05f40181 < -: ---------- abspath: move related functions to abspath 4: e1addc77e5 < -: ---------- credential-store: move related functions to credential-store file 5: 62e8c42f59 < -: ---------- object-name: move related functions to object-name 6: 0abba57acb < -: ---------- path: move related function to path 7: d33267a390 < -: ---------- strbuf: remove global variable 8: 665d2c2089 < -: ---------- init-db: document existing bug with core.bare in template config 9: 68d0a8ff16 < -: ---------- init-db: remove unnecessary global variable 10: 8c8ec85507 < -: ---------- init-db, clone: change unnecessary global into passed parameter 11: d555e2b365 < -: ---------- setup: adopt shared init-db & clone code 12: 689a7bc8aa < -: ---------- read-cache: move shared commit and ls-files code 13: 392f8e75b7 < -: ---------- add: modify add_files_to_cache() to avoid globals 14: 49ce237013 < -: ---------- read-cache: move shared add/checkout/commit code 15: c5d8370d40 < -: ---------- statinfo: move stat_{data,validity} functions from cache/read-cache 16: 90a72b6f86 < -: ---------- run-command.h: move declarations for run-command.c from cache.h 17: f27516c780 < -: ---------- name-hash.h: move declarations for name-hash.c from cache.h 18: 895c38a050 < -: ---------- sparse-index.h: move declarations for sparse-index.c from cache.h 19: 8678d4ad20 < -: ---------- preload-index.h: move declarations for preload-index.c from elsewhere 20: 4a463abaae < -: ---------- diff.h: move declaration for global in diff.c from cache.h 21: 3440e762c7 < -: ---------- merge.h: move declarations for merge.c from cache.h 22: e70853e398 < -: ---------- repository.h: move declaration of the_index from cache.h 23: ccd2014d73 < -: ---------- read-cache*.h: move declarations for read-cache.c functions from cache.h 24: d3a482afa9 < -: ---------- cache.h: remove this no-longer-used header 25: eaa087f446 < -: ---------- log-tree: replace include of revision.h with simple forward declaration 26: 5d2b0a9c75 < -: ---------- repository: remove unnecessary include of path.h 27: 250f83014e < -: ---------- diff.h: remove unnecessary include of oidset.h 28: d0f9913958 < -: ---------- list-objects-filter-options.h: remove unneccessary include 29: 03a2b2a515 < -: ---------- builtin.h: remove unneccessary includes 30: 15edc22d00 < -: ---------- git-compat-util.h: remove unneccessary include of wildmatch.h 31: e4e1bec8bd < -: ---------- merge-ll: rename from ll-merge 32: 9185495fd0 < -: ---------- khash: name the structs that khash declares 33: 15fb05e453 < -: ---------- object-store-ll.h: split this header out of object-store.h 34: 2608fe4b23 < -: ---------- hash-ll, hashmap: move oidhash() to hash-ll 35: 5e8dc5b574 < -: ---------- fsmonitor-ll.h: split this header out of fsmonitor.h 36: 37d32fc3fd < -: ---------- git-compat-util: move strbuf.c funcs to its header 37: 6ed19d5fe2 < -: ---------- git-compat-util: move wrapper.c funcs to its header 38: 555d1b8942 < -: ---------- sane-ctype.h: create header for sane-ctype macros 39: 72d591e282 < -: ---------- kwset: move translation table from ctype 40: 5d1dc2a118 < -: ---------- common.h: move non-compat specific macros and functions 41: 33e07e552e < -: ---------- git-compat-util: move usage.c funcs to its header 42: 417a8aa733 < -: ---------- treewide: remove unnecessary includes for wrapper.h 43: 65e35d00c1 < -: ---------- common: move alloc macros to common.h 44: 78634bc406 ! 1: 2f99eb2ca4 hex-ll: split out functionality from hex @@ hex.h +#include "hex-ll.h" /* - * Try to read a SHA1 in hexadecimal format from the 40 characters -@@ hex.h: int get_oid_hex(const char *hex, struct object_id *sha1); + * Try to read a hash (specified by the_hash_algo) in hexadecimal +@@ hex.h: int get_oid_hex(const char *hex, struct object_id *oid); /* Like get_oid_hex, but for an arbitrary hash algorithm. */ int get_oid_hex_algop(const char *hex, struct object_id *oid, const struct git_hash_algo *algop); 45: 21ec1d276e ! 2: 7b2d123628 object: move function to object.c @@ Metadata Author: Calvin Wan <calvinwan@google.com> ## Commit message ## - object: move function to object.c + wrapper: remove dependency to Git-specific internal file - While remove_or_warn() is a simple ternary operator to call two other - wrapper functions, it creates an unnecessary dependency to object.h in - wrapper.c. Therefore move the function to object.[ch] where the concept - of GITLINKs is first defined. + 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. - ## object.c ## -@@ object.c: void parsed_object_pool_clear(struct parsed_object_pool *o) - FREE_AND_NULL(o->object_state); - FREE_AND_NULL(o->shallow_stat); + ## entry.c ## +@@ entry.c: void unlink_entry(const struct cache_entry *ce, const char *super_prefix) + return; + schedule_dir_for_removal(ce->name, ce_namelen(ce)); } + +int remove_or_warn(unsigned int mode, const char *file) @@ object.c: void parsed_object_pool_clear(struct parsed_object_pool *o) + return S_ISGITLINK(mode) ? rmdir_or_warn(file) : unlink_or_warn(file); +} - ## object.h ## -@@ object.h: void clear_object_flags(unsigned flags); - */ - void repo_clear_commit_marks(struct repository *r, unsigned int flags); + ## entry.h ## +@@ entry.h: int fstat_checkout_output(int fd, const struct checkout *state, struct stat *st) + void update_ce_after_write(const struct checkout *state, struct cache_entry *ce, + struct stat *st); +/* + * Calls the correct function out of {unlink,rmdir}_or_warn based on @@ object.h: void clear_object_flags(unsigned flags); + */ +int remove_or_warn(unsigned int mode, const char *path); + - #endif /* OBJECT_H */ + #endif /* ENTRY_H */ ## wrapper.c ## @@ 46: 41dcf8107c = 3: b37beb206a config: correct bad boolean env value error message 47: 3e800a41c4 ! 4: 3a827cf45c parse: create new library for parsing strings and env values @@ Commit message 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. + 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]. Move general string and env parsing functions from config.[ch] to parse.[ch]. @@ config.c: static int git_parse_source(struct config_source *cs, config_fn_t fn, - return 1; -} - - static int reader_config_name(struct config_reader *reader, const char **out); - static int reader_origin_type(struct config_reader *reader, - enum config_origin_type *type); -@@ config.c: ssize_t git_config_ssize_t(const char *name, const char *value) + NORETURN + static void die_bad_number(const char *name, const char *value, + const struct key_value_info *kvi) +@@ config.c: ssize_t git_config_ssize_t(const char *name, const char *value, return ret; } @@ config.c: static enum fsync_component parse_fsync_components(const char *var, co - return -1; -} - - int git_config_bool_or_int(const char *name, const char *value, int *is_bool) + int git_config_bool_or_int(const char *name, const char *value, + const struct key_value_info *kvi, int *is_bool) { - int v = git_parse_maybe_bool_text(value); @@ config.c: void git_global_config(char **user_out, char **xdg_out) *xdg_out = xdg_config; } @@ config.c: void git_global_config(char **user_out, char **xdg_out) ## config.h ## @@ - #include "hashmap.h" #include "string-list.h" + #include "repository.h" - +#include "parse.h" 48: 7a4a088bc3 < -: ---------- date: push pager.h dependency up 49: c9002734d0 ! 5: f8e4ac50a0 git-std-lib: introduce git standard library @@ Documentation/technical/git-std-lib.txt (new) +Rationale behind Git Standard Library +================ + -+The rationale behind Git Standard Library essentially is the result of -+two observations within the Git codebase: every file includes -+git-compat-util.h which defines functions in a couple of different -+files, and wrapper.c + usage.c have difficult-to-separate circular -+dependencies with each other and other files. ++The rationale behind what's in and what's not in the Git Standard ++Library essentially is the result of two observations within the Git ++codebase: every file includes git-compat-util.h which defines functions ++in a couple of different files, and wrapper.c + usage.c have ++difficult-to-separate circular dependencies with each other and other ++files. + +Ubiquity of git-compat-util.h and circular dependencies +======== @@ Documentation/technical/git-std-lib.txt (new) + - low-level git/* files with functions defined in git-compat-util.h + (ctype.c) + - compat/* -+ - stubbed out dependencies in stubs/ (stubs/repository.c, stubs/trace2.c) ++ - stubbed out dependencies in stubs/ (stubs/pager.c, stubs/trace2.c) + +There are other files that might fit this definition, but that does not +mean it should belong in git-std-lib.a. Those files should start as +their own separate library since any file added to git-std-lib.a loses +its flexibility of being easily swappable. + -+Wrapper.c and usage.c have dependencies on repository and trace2 that are ++Wrapper.c and usage.c have dependencies on pager and trace2 that are +possible to remove at the cost of sacrificing the ability for standard Git +to be able to trace functions in those files and other files in git-std-lib.a. +In order for git-std-lib.a to compile with those dependencies, stubbed out @@ Documentation/technical/git-std-lib.txt (new) +usage.c +utf8.c +wrapper.c -+stubs/repository.c -+stubs/trace2.c +relevant compat/ files + ++When these files are compiled together with the following files (or ++user-provided files that provide the same functions), they form a ++complete library: ++stubs/pager.c ++stubs/trace2.c ++ +Pitfalls +================ + @@ Makefile: LIB_OBJS += write-or-die.o +LIB_OBJS += utf8.o +LIB_OBJS += wrapper.o + -+ifdef STUB_REPOSITORY -+STUB_OBJS += stubs/repository.o -+endif -+ +ifdef STUB_TRACE2 +STUB_OBJS += stubs/trace2.o +endif + ++ifdef STUB_PAGER ++STUB_OBJS += stubs/pager.o ++endif ++ +LIB_OBJS += $(STUB_OBJS) +endif @@ Makefile: ifdef FSMONITOR_OS_SETTINGS NO_TCLTK = NoThanks endif @@ Makefile: clean: profile-clean coverage-clean cocciclean - $(RM) po/git.pot po/git-core.pot $(RM) git.res $(RM) $(OBJECTS) + $(RM) headless-git.o - $(RM) $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB) $(REFTABLE_TEST_LIB) + $(RM) $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB) $(REFTABLE_TEST_LIB) $(STD_LIB_FILE) $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) @@ Makefile: $(FUZZ_PROGRAMS): all +### Libified Git rules + +# git-std-lib -+# `make git-std-lib GIT_STD_LIB=YesPlease STUB_REPOSITORY=YesPlease STUB_TRACE2=YesPlease` ++# `make git-std-lib.a GIT_STD_LIB=YesPlease STUB_TRACE2=YesPlease STUB_PAGER=YesPlease` +STD_LIB = git-std-lib.a + +$(STD_LIB): $(LIB_OBJS) $(COMPAT_OBJS) $(STUB_OBJS) + $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^ -+ -+TEMP_HEADERS = temp_headers/ -+ -+git-std-lib: -+# Move headers to temporary folder and replace them with stubbed headers. -+# After building, move headers and stubbed headers back. -+ifneq ($(STUB_OBJS),) -+ mkdir -p $(TEMP_HEADERS); \ -+ for d in $(STUB_OBJS); do \ -+ BASE=$${d%.*}; \ -+ mv $${BASE##*/}.h $(TEMP_HEADERS)$${BASE##*/}.h; \ -+ mv $${BASE}.h $${BASE##*/}.h; \ -+ done; \ -+ $(MAKE) $(STD_LIB); \ -+ for d in $(STUB_OBJS); do \ -+ BASE=$${d%.*}; \ -+ mv $${BASE##*/}.h $${BASE}.h; \ -+ mv $(TEMP_HEADERS)$${BASE##*/}.h $${BASE##*/}.h; \ -+ done; \ -+ rm -rf temp_headers -+else -+ $(MAKE) $(STD_LIB) -+endif ## git-compat-util.h ## @@ git-compat-util.h: static inline int noop_core_config(const char *var UNUSED, @@ git-compat-util.h: const char *inet_ntop(int af, const void *src, char *dst, siz #endif +#endif - /* - * Limit size of IO chunks, because huge chunks only cause pain. OS X -@@ git-compat-util.h: int git_access(const char *path, int mode); - # endif - #endif + static inline size_t st_add(size_t a, size_t b) + { +@@ git-compat-util.h: static inline int is_missing_file_error(int errno_) + return (errno_ == ENOENT || errno_ == ENOTDIR); + } +#ifndef GIT_STD_LIB int cmd_main(int, const char **); @@ git-compat-util.h: int git_access(const char *path, int mode); /* * You can mark a stack variable with UNLEAK(var) to avoid it being - ## stubs/repository.c (new) ## + ## stubs/pager.c (new) ## @@ -+#include "git-compat-util.h" -+#include "repository.h" ++#include "pager.h" + -+struct repository *the_repository; ++int pager_in_use(void) ++{ ++ return 0; ++} - ## stubs/repository.h (new) ## + ## stubs/pager.h (new) ## @@ -+#ifndef REPOSITORY_H -+#define REPOSITORY_H ++#ifndef PAGER_H ++#define PAGER_H + -+struct repository { int stub; }; ++int pager_in_use(void); + -+extern struct repository *the_repository; -+ -+#endif /* REPOSITORY_H */ ++#endif /* PAGER_H */ ## stubs/trace2.c (new) ## @@ +#include "git-compat-util.h" +#include "trace2.h" + ++struct child_process { int stub; }; ++struct repository { int stub; }; ++struct json_writer { int stub; }; ++ +void trace2_region_enter_fl(const char *file, int line, const char *category, + const char *label, const struct repository *repo, ...) { } +void trace2_region_leave_fl(const char *file, int line, const char *category, @@ stubs/trace2.c (new) + const struct repository *repo, const char *key, + intmax_t value) { } +int trace2_is_enabled(void) { return 0; } ++void trace2_counter_add(enum trace2_counter_id cid, uint64_t value) { } +void trace2_collect_process_info(enum trace2_process_info_reason reason) { } - ## stubs/trace2.h (new) ## -@@ -+#ifndef TRACE2_H -+#define TRACE2_H -+ -+struct child_process { int stub; }; -+struct repository; -+struct json_writer { int stub; }; -+ -+void trace2_region_enter_fl(const char *file, int line, const char *category, -+ const char *label, const struct repository *repo, ...); -+ -+#define trace2_region_enter(category, label, repo) \ -+ trace2_region_enter_fl(__FILE__, __LINE__, (category), (label), (repo)) -+ -+void trace2_region_leave_fl(const char *file, int line, const char *category, -+ const char *label, const struct repository *repo, ...); -+ -+#define trace2_region_leave(category, label, repo) \ -+ trace2_region_leave_fl(__FILE__, __LINE__, (category), (label), (repo)) -+ -+void trace2_data_string_fl(const char *file, int line, const char *category, -+ const struct repository *repo, const char *key, -+ const char *value); -+ -+#define trace2_data_string(category, repo, key, value) \ -+ trace2_data_string_fl(__FILE__, __LINE__, (category), (repo), (key), \ -+ (value)) -+ -+void trace2_cmd_ancestry_fl(const char *file, int line, const char **parent_names); -+ -+#define trace2_cmd_ancestry(v) trace2_cmd_ancestry_fl(__FILE__, __LINE__, (v)) -+ -+void trace2_cmd_error_va_fl(const char *file, int line, const char *fmt, -+ va_list ap); -+ -+#define trace2_cmd_error_va(fmt, ap) \ -+ trace2_cmd_error_va_fl(__FILE__, __LINE__, (fmt), (ap)) -+ -+ -+void trace2_cmd_name_fl(const char *file, int line, const char *name); -+ -+#define trace2_cmd_name(v) trace2_cmd_name_fl(__FILE__, __LINE__, (v)) -+ -+void trace2_thread_start_fl(const char *file, int line, -+ const char *thread_base_name); -+ -+#define trace2_thread_start(thread_base_name) \ -+ trace2_thread_start_fl(__FILE__, __LINE__, (thread_base_name)) -+ -+void trace2_thread_exit_fl(const char *file, int line); -+ -+#define trace2_thread_exit() trace2_thread_exit_fl(__FILE__, __LINE__) -+ -+void trace2_data_intmax_fl(const char *file, int line, const char *category, -+ const struct repository *repo, const char *key, -+ intmax_t value); -+ -+#define trace2_data_intmax(category, repo, key, value) \ -+ trace2_data_intmax_fl(__FILE__, __LINE__, (category), (repo), (key), \ -+ (value)) -+ -+enum trace2_process_info_reason { -+ TRACE2_PROCESS_INFO_STARTUP, -+ TRACE2_PROCESS_INFO_EXIT, -+}; -+int trace2_is_enabled(void); -+void trace2_collect_process_info(enum trace2_process_info_reason reason); -+ -+#endif /* TRACE2_H */ -+ - ## symlinks.c ## @@ symlinks.c: void invalidate_lstat_cache(void) reset_lstat_cache(&default_cache); @@ symlinks.c: int lstat_cache_aware_rmdir(const char *path) return ret; } +#endif + + ## wrapper.c ## +@@ + #include "abspath.h" + #include "parse.h" + #include "gettext.h" +-#include "repository.h" + #include "strbuf.h" + #include "trace2.h" + 50: 0bead8f980 ! 6: 7840e1830a git-std-lib: add test file to call git-std-lib.a functions @@ t/stdlib-test.c (new) + struct strbuf sb3 = STRBUF_INIT; + struct string_list list = STRING_LIST_INIT_NODUP; + char *buf = "foo"; -+ struct strbuf_expand_dict_entry dict[] = { -+ { "foo", NULL, }, -+ { "bar", NULL, }, -+ }; + int fd = open("/dev/null", O_RDONLY); + + fprintf(stderr, "calling strbuf functions\n"); @@ t/stdlib-test.c (new) + strbuf_add_commented_lines(sb, "foo", 3, '#'); + strbuf_commented_addf(sb, '#', "%s", "foo"); + // strbuf_vaddf() called by strbuf_addf() -+ strbuf_expand(sb, "%s", strbuf_expand_literal_cb, NULL); -+ strbuf_expand(sb, "%s", strbuf_expand_dict_cb, &dict); -+ // strbuf_expand_literal_cb() called by strbuf_expand() -+ // strbuf_expand_dict_cb() called by strbuf_expand() + strbuf_addbuf_percentquote(sb, &sb3); + strbuf_add_percentencode(sb, "foo", STRBUF_ENCODE_SLASH); + strbuf_fread(sb, 0, stdin);
Calvin Wan <calvinwan@google.com> writes: > I have taken this series out of RFC since there weren't any significant > concerns with the overall concept and design of this series. This reroll > incorporates some smaller changes such as dropping the "push pager > dependency" patch in favor of stubbing it out. The main change this > reroll cleans up the Makefile rules and stubs, as suggested by > Phillip Wood (appreciate the help on this one)! What is your plan for the "config-parse" stuff? The "create new library" step in this series seem to aim for the same goal in a different ways. > This series has been rebased onto 1fc548b2d6a: The sixth batch > > Originally this series was built on other patches that have since been > merged, which is why the range-diff is shown removing many of them. Good. Previous rounds did not really attract much interest from the public if I recall correctly. Let's see how well this round fares. > Documentation/technical/git-std-lib.txt | 191 ++++++++++++++++++++ It is interesting to see that there is no "std.*lib\.c" in the set of source files, or "std.*lib\.a" target in the Makefile.
Junio C Hamano <gitster@pobox.com> writes: > Calvin Wan <calvinwan@google.com> writes: > >> I have taken this series out of RFC since there weren't any significant >> concerns with the overall concept and design of this series. This reroll >> incorporates some smaller changes such as dropping the "push pager >> dependency" patch in favor of stubbing it out. The main change this >> reroll cleans up the Makefile rules and stubs, as suggested by >> Phillip Wood (appreciate the help on this one)! > > What is your plan for the "config-parse" stuff? The "create new library" > step in this series seem to aim for the same goal in a different ways. Actually, this one is far less ambitious in touching "config" subsystem, in that it only deals with parsing strings as values. The other one knows how a config file is laid out, what key the value we are about to read is expected for, etc., and it will benefit by having the "parse" code separated out by this series, but they are more or less orthogonal. Queued. Thanks.