Re: [PATCH v3 5/6] git-std-lib: introduce git standard library
From: Jonathan Tan <hidden>
Date: 2023-09-15 18:41:57
Calvin Wan [off-list ref] writes:
quoted hunk ↗ jump to hunk
diff --git a/Makefile b/Makefile index 9226c719a0..0a2d1ae3cc 100644 --- a/Makefile +++ b/Makefile@@ -669,6 +669,7 @@ FUZZ_PROGRAMS = GIT_OBJS = LIB_OBJS = SCALAR_OBJS = +STUB_OBJS = OBJECTS = OTHER_PROGRAMS = PROGRAM_OBJS =
I don't think stubs should be compiled into git-std-lib.a - I would expect a consumer of this library to be able to specify their own implementations if needed (e.g. their own trace2).
quoted hunk ↗ jump to hunk
@@ -956,6 +957,7 @@ COCCI_SOURCES = $(filter-out $(THIRD_PARTY_SOURCES),$(FOUND_C_SOURCES)) LIB_H = $(FOUND_H_SOURCES) +ifndef GIT_STD_LIB LIB_OBJS += abspath.o LIB_OBJS += add-interactive.o LIB_OBJS += add-patch.o@@ -1196,6 +1198,27 @@ LIB_OBJS += write-or-die.o LIB_OBJS += ws.o LIB_OBJS += wt-status.o LIB_OBJS += xdiff-interface.o +else ifdef GIT_STD_LIB +LIB_OBJS += abspath.o +LIB_OBJS += ctype.o +LIB_OBJS += date.o +LIB_OBJS += hex-ll.o +LIB_OBJS += parse.o +LIB_OBJS += strbuf.o +LIB_OBJS += usage.o +LIB_OBJS += utf8.o +LIB_OBJS += wrapper.o
This means that LIB_OBJS (in this patch, used both by git-std-lib and as part of compiling the regular Git binary) can differ based on the GIT_STD_LIB variable. It does seem that we cannot avoid GIT_STD_LIB for now, because the git-std-lib can only be compiled without GETTEXT (so we need a variable to make sure that none of these .o files are compiled with GETTEXT), but we should still minimize the changes between compiling with GIT_STD_LIB and without it, at least to minimize future work. Could we have two separate lists? So, leave LIB_OBJS alone and make a new STD_LIB_OBJS.
quoted hunk ↗ jump to hunk
diff --git a/git-compat-util.h b/git-compat-util.h index 3e7a59b5ff..14bf71c530 100644 --- a/git-compat-util.h +++ b/git-compat-util.h@@ -455,8 +455,8 @@ static inline int noop_core_config(const char *var UNUSED, #define platform_core_config noop_core_config #endif +#if !defined(__MINGW32__) && !defined(_MSC_VER) && !defined(GIT_STD_LIB) int lstat_cache_aware_rmdir(const char *path); -#if !defined(__MINGW32__) && !defined(_MSC_VER) #define rmdir lstat_cache_aware_rmdir #endif
I think we still want to keep the idea of "the code should still be good even if we have no use for git-std-lib" as much as possible, so could we stub lstat_cache_aware_rmdir() instead? We could have a new git-compat- util-stub.c (or whatever we want to call it).
quoted hunk ↗ jump to hunk
@@ -966,9 +966,11 @@ const char *inet_ntop(int af, const void *src, char *dst, size_t size); #endif #ifdef NO_PTHREADS +#ifdef GIT_STD_LIB #define atexit git_atexit int git_atexit(void (*handler)(void)); #endif +#endif static inline size_t st_add(size_t a, size_t b) {
Same for git_atexit().
quoted hunk ↗ jump to hunk
@@ -1462,14 +1464,17 @@ static inline int is_missing_file_error(int errno_) return (errno_ == ENOENT || errno_ == ENOTDIR); } +#ifndef GIT_STD_LIB int cmd_main(int, const char **); /* * Intercept all calls to exit() and route them to trace2 to * optionally emit a message before calling the real exit(). */ + int common_exit(const char *file, int line, int code); #define exit(code) exit(common_exit(__FILE__, __LINE__, (code))) +#endif /* * You can mark a stack variable with UNLEAK(var) to avoid it being
And for common_exit(). As for cmd_main(), that seems to be a convenience so that we can link common_main.o with various other files (e.g. http-backend.c). I think the right thing to do is to define a new cmd-main.h that declares only cmd_main(), and then have only the files that need it (common_main.c and all the files that define cmd_main()) include it. This cleanup patch can be done before this patch. I think this is a good change that we would want even without libification.