Lib-ify functions in builtin.h

PATCH Superseded Denton Liu 2 participants 7 messages Mar 21, 2020 – Mar 24, 2020 Kbuild

Share & tools

Denton Liu

author

2020-03-21 10:29 UTC #0 lore raw

cover Lib-ify functions in builtin.h

In builtin.h, there exist a couple of functions that are very lib-ish. Extract these functions into their own files which should make more organizational sense.

Denton Liu (2):
Lib-ify fmt-merge-msg
Lib-ify prune-packed

 Makefile                |   2 +
 builtin.h               |  16 -
 builtin/fmt-merge-msg.c | 655 +---------------------------------------
 builtin/prune-packed.c  |  44 +--
 builtin/prune.c         |   1 +
 builtin/repack.c        |   1 +
 fmt-merge-msg.c         | 653 +++++++++++++++++++++++++++++++++++++++
 fmt-merge-msg.h         |  13 +
 prune-packed.c          |  43 +++
 prune-packed.h          |   9 +
 10 files changed, 724 insertions(+), 713 deletions(-)
 create mode 100644 fmt-merge-msg.c
 create mode 100644 prune-packed.c
 create mode 100644 prune-packed.h

Signature · 1 line

2.25.0.114.g5b0ca878e0

Denton Liu

author

2020-03-21 10:29 UTC #1 lore raw

PATCH 1/2 Lib-ify fmt-merge-msg

In builtin.h, there exists the distinctly lib-ish function fmt_merge_msg(). Extract this function into fmt-merge-msg.c so that related definitions can exist clearly in their own header file.

While we're at it, clean up #includes that are unused.

This patch is best viewed with --color-moved.

Signed-off-by Denton Liu <[email protected]>

---
 Makefile                |   1 +
 builtin.h               |  11 -
 builtin/fmt-merge-msg.c | 655 +---------------------------------------
 fmt-merge-msg.c         | 653 +++++++++++++++++++++++++++++++++++++++
 fmt-merge-msg.h         |  13 +
 5 files changed, 668 insertions(+), 665 deletions(-)
 create mode 100644 fmt-merge-msg.c

Makefile +1 -0

index 9804a0758b..04e3cc179e 100644
@@ -886,6 +886,7 @@ LIB_OBJS += ewah/ewah_rlw.o
 LIB_OBJS += exec-cmd.o
 LIB_OBJS += fetch-negotiator.o
 LIB_OBJS += fetch-pack.o
+LIB_OBJS += fmt-merge-msg.o
 LIB_OBJS += fsck.o
 LIB_OBJS += fsmonitor.o
 LIB_OBJS += gettext.o

builtin.h +0 ~~-11~~

index 2b25a80cde..6101d98661 100644
@@ -94,8 +94,6 @@
  * command.
  */
 
-#define DEFAULT_MERGE_LOG_LEN 20
-
 extern const char git_usage_string[];
 extern const char git_more_info_string[];

@@ -104,15 +102,6 @@ extern const char git_more_info_string[];
 
 void prune_packed_objects(int);
 
-struct fmt_merge_msg_opts {
-	unsigned add_title:1,
-		credit_people:1;
-	int shortlog_len;
-};
-
-int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
-		  struct fmt_merge_msg_opts *);
-
 /**
  * If a built-in has DELAY_PAGER_CONFIG set, the built-in should call this early
  * when it wishes to respect the `pager.foo`-config. The `cmd` is the name of

builtin/fmt-merge-msg.c +1 ~~-654~~

667 lines not on this page · raw message

fmt-merge-msg.c new file +653 -0

653 lines not on this page · raw message

fmt-merge-msg.h +13 -0

index 01e3aa88c5..f2ab0e0085 100644
@@ -1,7 +1,20 @@
 #ifndef FMT_MERGE_MSG_H
 #define FMT_MERGE_MSG_H
 
+#include "strbuf.h"
+
+#define DEFAULT_MERGE_LOG_LEN 20
+
+struct fmt_merge_msg_opts {
+	unsigned add_title:1,
+		credit_people:1;
+	int shortlog_len;
+};
+
 extern int merge_log_config;
 int fmt_merge_msg_config(const char *key, const char *value, void *cb);
+int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
+		  struct fmt_merge_msg_opts *);
+
 
 #endif /* FMT_MERGE_MSG_H */

Signature · 1 line

2.25.0.114.g5b0ca878e0

Denton Liu

author

2020-03-21 10:29 UTC #2 lore raw

PATCH 2/2 Lib-ify prune-packed

In reply to Denton Liu

In builtin.h, there exists the distinctly lib-ish function prune_packed_objects(). Extract this function into prune-packed.c so that related definitions can exist clearly in their own header file.

While we're at it, clean up #includes that are unused.

This patch is best viewed with --color-moved.

Signed-off-by Denton Liu <[email protected]>

---
 Makefile               |  1 +
 builtin.h              |  5 -----
 builtin/prune-packed.c | 44 +-----------------------------------------
 builtin/prune.c        |  1 +
 builtin/repack.c       |  1 +
 prune-packed.c         | 43 +++++++++++++++++++++++++++++++++++++++++
 prune-packed.h         |  9 +++++++++
 7 files changed, 56 insertions(+), 48 deletions(-)
 create mode 100644 prune-packed.c
 create mode 100644 prune-packed.h

Makefile +1 -0

index 04e3cc179e..608afd58b1 100644
@@ -952,6 +952,7 @@ LIB_OBJS += progress.o
 LIB_OBJS += promisor-remote.o
 LIB_OBJS += prompt.o
 LIB_OBJS += protocol.o
+LIB_OBJS += prune-packed.o
 LIB_OBJS += quote.o
 LIB_OBJS += range-diff.o
 LIB_OBJS += reachable.o

builtin.h +0 -5

index 6101d98661..a5ae15bfe5 100644
@@ -97,11 +97,6 @@
 extern const char git_usage_string[];
 extern const char git_more_info_string[];
 
-#define PRUNE_PACKED_DRY_RUN 01
-#define PRUNE_PACKED_VERBOSE 02
-
-void prune_packed_objects(int);
-
 /**
  * If a built-in has DELAY_PAGER_CONFIG set, the built-in should call this early
  * when it wishes to respect the `pager.foo`-config. The `cmd` is the name of

builtin/prune-packed.c +1 ~~-43~~

55 lines not on this page · raw message

builtin/prune.c +1 -0

index 2b76872ad2..fd9acc7222 100644
@@ -6,6 +6,7 @@
 #include "reachable.h"
 #include "parse-options.h"
 #include "progress.h"
+#include "prune-packed.h"
 #include "object-store.h"
 
 static const char * const prune_usage[] = {

builtin/repack.c +1 -0

index 0781763b06..1b686ee9ce 100644
@@ -10,6 +10,7 @@
 #include "argv-array.h"
 #include "midx.h"
 #include "packfile.h"
+#include "prune-packed.h"
 #include "object-store.h"
 #include "promisor-remote.h"

prune-packed.c new file +43 -0

43 lines not on this page · raw message

prune-packed.h new file +9 -0

new file mode 100644
index 0000000000..936fa9df23
@@ -0,0 +1,9 @@
+#ifndef PRUNE_PACKED_H
+#define PRUNE_PACKED_H
+
+#define PRUNE_PACKED_DRY_RUN 01
+#define PRUNE_PACKED_VERBOSE 02
+
+void prune_packed_objects(int);
+
+#endif

Signature · 1 line

2.25.0.114.g5b0ca878e0

Junio C Hamano

2020-03-21 19:24 UTC #3 lore raw

In reply to Denton Liu

Quoted 3 lines · Denton Liu

In builtin.h, there exist a couple of functions that are very lib-ish. Extract these functions into their own files which should make more organizational sense.

"very lib-ish" feels a bit subjective, but I agree that among those functions that are currently only allowed to be called from a built-in command (hence, cannot be called from anything in libgit.a or non-builtin commands like transports), there may be some funtions that may make sense to be callable by non-builtin users of libgit.a (e.g. cgit perhaps?).

I do like the direction these patches are taking us to. I just do not know how best to phrase it, and having trouble coming up with a non subjective description.

Thanks.

Quoted 18 lines

Denton Liu (2):
Lib-ify fmt-merge-msg
Lib-ify prune-packed

 Makefile                |   2 +
 builtin.h               |  16 -
 builtin/fmt-merge-msg.c | 655 +---------------------------------------
 builtin/prune-packed.c  |  44 +--
 builtin/prune.c         |   1 +
 builtin/repack.c        |   1 +
 fmt-merge-msg.c         | 653 +++++++++++++++++++++++++++++++++++++++
 fmt-merge-msg.h         |  13 +
 prune-packed.c          |  43 +++
 prune-packed.h          |   9 +
 10 files changed, 724 insertions(+), 713 deletions(-)
 create mode 100644 fmt-merge-msg.c
 create mode 100644 prune-packed.c
 create mode 100644 prune-packed.h

Denton Liu

author

2020-03-24 01:08 UTC #4 lore raw

v2 cover Lib-ify functions in builtin.h

In reply to Denton Liu

Since the last revision, I tried to rewrite the commit messages to be more subjective but I was also having some trouble with it.

In builtin.h, there exist a couple of functions that are very "lib-ish". These functions can currently only be called by built-in commands but, unlike all of the other functions in the header, it does not make sense to impose this restriction as their functionality can be logically reused in libgit.

Extract these functions into their own files which should make more organizational sense.

Changes since v1:

* Rewrite commit messages to be less subjective

Denton Liu (2):
Lib-ify fmt-merge-msg
Lib-ify prune-packed

 Makefile                |   2 +
 builtin.h               |  16 -
 builtin/fmt-merge-msg.c | 655 +---------------------------------------
 builtin/prune-packed.c  |  44 +--
 builtin/prune.c         |   1 +
 builtin/repack.c        |   1 +
 fmt-merge-msg.c         | 653 +++++++++++++++++++++++++++++++++++++++
 fmt-merge-msg.h         |  13 +
 prune-packed.c          |  43 +++
 prune-packed.h          |   9 +
 10 files changed, 724 insertions(+), 713 deletions(-)
 create mode 100644 fmt-merge-msg.c
 create mode 100644 prune-packed.c
 create mode 100644 prune-packed.h

Range-diff against v1:
1: d989ab8b08 ! 1: fe76d79aa7 Lib-ify fmt-merge-msg
@@ Metadata
## Commit message ##
Lib-ify fmt-merge-msg

- In builtin.h, there exists the distinctly lib-ish function
- fmt_merge_msg(). Extract this function into fmt-merge-msg.c so that
- related definitions can exist clearly in their own header file.
+ In builtin.h, there exists the distinctly "lib-ish" function
+ fmt_merge_msg(). This function can currently only be called by built-in
+ commands but, unlike most of the other functions in the header, it does
+ not make sense to impose this restriction as the functionality can be
+ logically reused in libgit.
+
+ Extract this function into fmt-merge-msg.c so that related definitions
+ can exist clearly in their own header file.

While we're at it, clean up #includes that are unused.

2: 249dbb9420 ! 2: c6a80ce882 Lib-ify prune-packed
@@ Commit message
Lib-ify prune-packed

In builtin.h, there exists the distinctly lib-ish function
- prune_packed_objects(). Extract this function into prune-packed.c so that
- related definitions can exist clearly in their own header file.
+ prune_packed_objects(). This function can currently only be called by
+ built-in commands but, unlike all of the other functions in the header,
+ it does not make sense to impose this restriction as the functionality
+ can be logically reused in libgit.
+
+ Extract this function into prune-packed.c so that related definitions
+ can exist clearly in their own header file.

While we're at it, clean up #includes that are unused.

Signature · 1 line

2.25.0.114.g5b0ca878e0

Denton Liu

author

2020-03-24 01:08 UTC #5 lore raw

PATCH v2 1/2 Lib-ify fmt-merge-msg

In builtin.h, there exists the distinctly "lib-ish" function fmt_merge_msg(). This function can currently only be called by built-in commands but, unlike most of the other functions in the header, it does not make sense to impose this restriction as the functionality can be logically reused in libgit.

Extract this function into fmt-merge-msg.c so that related definitions can exist clearly in their own header file.

While we're at it, clean up #includes that are unused.

This patch is best viewed with --color-moved.

Signed-off-by Denton Liu <[email protected]>

---
 Makefile                |   1 +
 builtin.h               |  11 -
 builtin/fmt-merge-msg.c | 655 +---------------------------------------
 fmt-merge-msg.c         | 653 +++++++++++++++++++++++++++++++++++++++
 fmt-merge-msg.h         |  13 +
 5 files changed, 668 insertions(+), 665 deletions(-)
 create mode 100644 fmt-merge-msg.c

Makefile +1 -0

index 9804a0758b..04e3cc179e 100644
@@ -886,6 +886,7 @@ LIB_OBJS += ewah/ewah_rlw.o
 LIB_OBJS += exec-cmd.o
 LIB_OBJS += fetch-negotiator.o
 LIB_OBJS += fetch-pack.o
+LIB_OBJS += fmt-merge-msg.o
 LIB_OBJS += fsck.o
 LIB_OBJS += fsmonitor.o
 LIB_OBJS += gettext.o

builtin.h +0 ~~-11~~

index 2b25a80cde..6101d98661 100644
@@ -94,8 +94,6 @@
  * command.
  */
 
-#define DEFAULT_MERGE_LOG_LEN 20
-
 extern const char git_usage_string[];
 extern const char git_more_info_string[];

@@ -104,15 +102,6 @@ extern const char git_more_info_string[];
 
 void prune_packed_objects(int);
 
-struct fmt_merge_msg_opts {
-	unsigned add_title:1,
-		credit_people:1;
-	int shortlog_len;
-};
-
-int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
-		  struct fmt_merge_msg_opts *);
-
 /**
  * If a built-in has DELAY_PAGER_CONFIG set, the built-in should call this early
  * when it wishes to respect the `pager.foo`-config. The `cmd` is the name of

builtin/fmt-merge-msg.c +1 ~~-654~~

667 lines not on this page · raw message

fmt-merge-msg.c new file +653 -0

653 lines not on this page · raw message

fmt-merge-msg.h +13 -0

index 01e3aa88c5..f2ab0e0085 100644
@@ -1,7 +1,20 @@
 #ifndef FMT_MERGE_MSG_H
 #define FMT_MERGE_MSG_H
 
+#include "strbuf.h"
+
+#define DEFAULT_MERGE_LOG_LEN 20
+
+struct fmt_merge_msg_opts {
+	unsigned add_title:1,
+		credit_people:1;
+	int shortlog_len;
+};
+
 extern int merge_log_config;
 int fmt_merge_msg_config(const char *key, const char *value, void *cb);
+int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
+		  struct fmt_merge_msg_opts *);
+
 
 #endif /* FMT_MERGE_MSG_H */

Signature · 1 line

2.25.0.114.g5b0ca878e0

Denton Liu

author

2020-03-24 01:08 UTC #6 lore raw

PATCH v2 2/2 Lib-ify prune-packed

In reply to Denton Liu

In builtin.h, there exists the distinctly lib-ish function prune_packed_objects(). This function can currently only be called by built-in commands but, unlike all of the other functions in the header, it does not make sense to impose this restriction as the functionality can be logically reused in libgit.

Extract this function into prune-packed.c so that related definitions can exist clearly in their own header file.

While we're at it, clean up #includes that are unused.

This patch is best viewed with --color-moved.

Signed-off-by Denton Liu <[email protected]>

---
 Makefile               |  1 +
 builtin.h              |  5 -----
 builtin/prune-packed.c | 44 +-----------------------------------------
 builtin/prune.c        |  1 +
 builtin/repack.c       |  1 +
 prune-packed.c         | 43 +++++++++++++++++++++++++++++++++++++++++
 prune-packed.h         |  9 +++++++++
 7 files changed, 56 insertions(+), 48 deletions(-)
 create mode 100644 prune-packed.c
 create mode 100644 prune-packed.h

Makefile +1 -0

index 04e3cc179e..608afd58b1 100644
@@ -952,6 +952,7 @@ LIB_OBJS += progress.o
 LIB_OBJS += promisor-remote.o
 LIB_OBJS += prompt.o
 LIB_OBJS += protocol.o
+LIB_OBJS += prune-packed.o
 LIB_OBJS += quote.o
 LIB_OBJS += range-diff.o
 LIB_OBJS += reachable.o

builtin.h +0 -5

index 6101d98661..a5ae15bfe5 100644
@@ -97,11 +97,6 @@
 extern const char git_usage_string[];
 extern const char git_more_info_string[];
 
-#define PRUNE_PACKED_DRY_RUN 01
-#define PRUNE_PACKED_VERBOSE 02
-
-void prune_packed_objects(int);
-
 /**
  * If a built-in has DELAY_PAGER_CONFIG set, the built-in should call this early
  * when it wishes to respect the `pager.foo`-config. The `cmd` is the name of

builtin/prune-packed.c +1 ~~-43~~

55 lines not on this page · raw message

builtin/prune.c +1 -0

index 2b76872ad2..fd9acc7222 100644
@@ -6,6 +6,7 @@
 #include "reachable.h"
 #include "parse-options.h"
 #include "progress.h"
+#include "prune-packed.h"
 #include "object-store.h"
 
 static const char * const prune_usage[] = {

builtin/repack.c +1 -0

index 0781763b06..1b686ee9ce 100644
@@ -10,6 +10,7 @@
 #include "argv-array.h"
 #include "midx.h"
 #include "packfile.h"
+#include "prune-packed.h"
 #include "object-store.h"
 #include "promisor-remote.h"

prune-packed.c new file +43 -0

43 lines not on this page · raw message

prune-packed.h new file +9 -0

new file mode 100644
index 0000000000..936fa9df23
@@ -0,0 +1,9 @@
+#ifndef PRUNE_PACKED_H
+#define PRUNE_PACKED_H
+
+#define PRUNE_PACKED_DRY_RUN 01
+#define PRUNE_PACKED_VERBOSE 02
+
+void prune_packed_objects(int);
+
+#endif

Signature · 1 line

2.25.0.114.g5b0ca878e0