Thread (72 messages) 72 messages, 8 authors, 2024-02-29

Re: [PATCH v3 5/6] git-std-lib: introduce git standard library

From: Phillip Wood <hidden>
Date: 2023-09-11 21:38:36

Hi Calvin

On 08/09/2023 18:44, Calvin Wan wrote:
quoted hunk ↗ jump to hunk
+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
It is still not clear to me how re-using LIB_OBJS like this is compatible with building libgit.a and git-stb-lib.a in a single make process c.f. [1].
quoted hunk ↗ jump to hunk
+ifdef GIT_STD_LIB
+	BASIC_CFLAGS += -DGIT_STD_LIB
+	BASIC_CFLAGS += -DNO_GETTEXT
As I've said before [2] I think that being able to built git-std-lib.a with gettext support is a prerequisite for using it to build git (just like trace2 support is). If we cannot build git using git-std-lib then the latter is likely to bit rot and so I don't think git-std-lib should be merged until there is a demonstration of building git using it.

quoted hunk ↗ jump to hunk
+### Libified Git rules
+
+# git-std-lib
+# `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) $@ $^
This is much nicer that the previous version.
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 thought we'd agreed that this represents a change in behavior that should be fixed c.f. [2]
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().
   */
+
Nit: this blank line seems unnecessary
quoted hunk ↗ jump to hunk
  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
diff --git a/stubs/pager.c b/stubs/pager.c
quoted hunk ↗ jump to hunk
diff --git a/stubs/pager.h b/stubs/pager.h
new file mode 100644
index 0000000000..b797910881
--- /dev/null
+++ b/stubs/pager.h
@@ -0,0 +1,6 @@
+#ifndef PAGER_H
+#define PAGER_H
+
+int pager_in_use(void);
+
+#endif /* PAGER_H */
Is this file actually used for anything? pager_in_use() is already declared in pager.h in the project root directory.
quoted hunk ↗ jump to hunk
diff --git a/wrapper.c b/wrapper.c
index 7da15a56da..eeac3741cf 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -5,7 +5,6 @@
  #include "abspath.h"
  #include "parse.h"
  #include "gettext.h"
-#include "repository.h"
It is probably worth splitting this change out with a commit message explaining why the include is unneeded.

This is looking good, it would be really nice to see a demonstration of building git using git-std-lib (with gettext support) in the next iteration.

Best Wishes

Phillip


[1] https://lore.kernel.org/git/a0f04bd7-3a1e-b303-fd52-eee2af4d38b3@gmail.com/ (local)
[2] https://lore.kernel.org/git/CAFySSZBMng9nEdCkuT5+fc6rfFgaFfU2E0NP3=jUQC1yRcUE6Q@mail.gmail.com/ (local)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help