Skip to contentSkip to conversation

abspath.h is created.

PATCH In review skrab-sah via GitGitGadget 4 participants 5 messages Sep 27, 2022 – Sep 28, 2022

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 abspath.h is created.

From: skrab-sah <[email protected]>

replaced declaration of abspath.c from cache.h to abspath.h. abspath.h is generated by using makeheaders tool.

---
abspath.h file is generated by makeheaders tool

1. we don't need to commit the file.
2. added routin for abspath.c in Makefile.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1345%2Fskrab-sah%2Fmaster-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1345/skrab-sah/master-v1
Pull-Request: https://github.com/git/git/pull/1345

 abspath.c | 10 ++++++++++
 abspath.h |  9 +++++++++
 cache.h   | 21 +--------------------
 3 files changed, 20 insertions(+), 20 deletions(-)
 create mode 100644 abspath.h
abspath.c +10 -0
index 39e06b58486..1c163bbe651 100644
@@ -262,6 +262,16 @@ char *absolute_pathdup(const char *path)
 	return strbuf_detach(&sb, NULL);
 }
 
+/*
+ * Concatenate "prefix" (if len is non-zero) and "path", with no
+ * connecting characters (so "prefix" should end with a "/").
+ * Unlike prefix_path, this should be used if the named file does
+ * not have to interact with index entry; i.e. name of a random file
+ * on the filesystem.
+ *
+ * The return value is always a newly allocated string (even if the
+ * prefix was empty).
+ */
 char *prefix_filename(const char *pfx, const char *arg)
 {
 	struct strbuf path = STRBUF_INIT;
abspath.h new file +9 -0
new file mode 100644
index 00000000000..edebc3a53ba
@@ -0,0 +1,9 @@
+/* This file was automatically generated.  Do not edit! */
+#undef INTERFACE
+char *prefix_filename(const char *pfx,const char *arg);
+char *absolute_pathdup(const char *path);
+const char *absolute_path(const char *path);
+char *real_pathdup(const char *path,int die_on_error);
+char *strbuf_realpath_forgiving(struct strbuf *resolved,const char *path,int die_on_error);
+char *strbuf_realpath(struct strbuf *resolved,const char *path,int die_on_error);
+int is_directory(const char *path);
cache.h +1 -20
index 26ed03bd6de..e226dbcc7d5 100644
@@ -646,18 +646,6 @@ const char *setup_git_directory(void);
 char *prefix_path(const char *prefix, int len, const char *path);
 char *prefix_path_gently(const char *prefix, int len, int *remaining, const char *path);
 
-/*
- * Concatenate "prefix" (if len is non-zero) and "path", with no
- * connecting characters (so "prefix" should end with a "/").
- * Unlike prefix_path, this should be used if the named file does
- * not have to interact with index entry; i.e. name of a random file
- * on the filesystem.
- *
- * The return value is always a newly allocated string (even if the
- * prefix was empty).
- */
-char *prefix_filename(const char *prefix, const char *path);
-
 int check_filename(const char *prefix, const char *name);
 void verify_filename(const char *prefix,
 		     const char *name,
@@ -1299,14 +1287,7 @@ static inline int is_absolute_path(const char *path)
 {
 	return is_dir_sep(path[0]) || has_dos_drive_prefix(path);
 }
-int is_directory(const char *);
-char *strbuf_realpath(struct strbuf *resolved, const char *path,
-		      int die_on_error);
-char *strbuf_realpath_forgiving(struct strbuf *resolved, const char *path,
-				int die_on_error);
-char *real_pathdup(const char *path, int die_on_error);
-const char *absolute_path(const char *path);
-char *absolute_pathdup(const char *path);
+#include "abspath.h"
 const char *remove_leading_path(const char *in, const char *prefix);
 const char *relative_path(const char *in, const char *prefix, struct strbuf *sb);
 int normalize_path_copy_len(char *dst, const char *src, int *prefix_len);

base-commit: 4fd6c5e44459e6444c2cd93383660134c95aabd1

Signature · 1 line
gitgitgadget
#1 lore raw

abspath.h line 1

Quoted 7 lines · skrab-sah via GitGitGadget
abspath.h new file +1 -0
new file mode 100644
index 00000000000..edebc3a53ba
@@ -0,0 +1,9 @@
+/* This file was automatically generated.  Do not edit! */

No thanks.

I suspect this change is taking us in a wrong direction. People grep for function and struct declarations in <*.h> files and expect to find the associated comments.

#2 lore raw

What is wrong here and what should i do to make it correct.

Quoted 16 lines · Junio C Hamano

abspath.h line 1

Quoted 7 lines · skrab-sah via GitGitGadget
abspath.h new file +1 -0
new file mode 100644
index 00000000000..edebc3a53ba
@@ -0,0 +1,9 @@
+/*  This file was automatically generated.  Do not edit! */

No thanks.

I suspect this change is taking us in a wrong direction. People grep for function and struct declarations in <*.h> files and expect to find the associated comments.

#3 lore raw

Hi Skrab,

Quoted 1 line · Skrab Sah

What is wrong here and what should i do to make it correct.

I suspect that Junio's concern is that it appears the tool you're using to auto-generate these .h files moves the comments away from the declaration and into the header.

I imagine that such a change would be improved if:

- You kept the comment attached to the function declaration (and ensured that it was present in the header file).

- And we dropped the "this file is auto-generated by ..." comment, since I don't imagine that folks will be interested in adding a new tool to the development toolchain.

Thanks.

Taylor

#4 lore raw
In reply to Skrab Sah
Quoted 1 line · Skrab Sah

What is wrong here and what should i do to make it correct.

Almost everything in what the patch is wrong.

* It creates <abspath.h> as a tracked file, but has a funny comment with control-G in it that says "do not edit"

* It is entirely unclear how the contents of <abspath.h> is meant to evolve, if the contributors are forbidden to edit it, and without any instruction how to update it.

* The new <abspath.h> lost all function comments, grouping by category, etc. that were in the original <cache.h>.

* There is an "#undef INTERFACE" that has no use to us, those who develop Git, for no clearly explained reason.

* The proposed log message says what it did, but it does not even try to justify why it is a good idea to do so.

* There is no need to create a new <abspath.h> in the first place. If we were in an alternate world where we did not have <abspath.c>, it may be reasonable to add <abspath.h> along with it, instead of adding declarations for new functions and types in <cache.h>. But the difference falls into "once it is in, it is not worth the patch churn to change it" category.

The easiest way to make it correct is to drop it, I guess.

Thanks.