threads / patch / 58519

patchabspath.h is created.

Subject: [PATCH] abspath.h is created.

## tl;dr

5 messages between Sep 27, 2022 and Sep 28, 2022. Diffs are folded; open one to read it.

replies: 4people: 4as markdown or json

skrab-sah via GitGitGadget· Sep 27, 2022, 16:08 UTC · lore
From: skrab-sah <skrab.sah@gmail.com>

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

Signed-off-by: skrab-sah <skrab.sah@gmail.com>
---
    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
Show changes to 3 files +20 −20

abspath.c, abspath.h, cache.h

diff --git a/abspath.c b/abspath.c
index 39e06b58486..1c163bbe651 100644
--- a/abspath.c
+++ b/abspath.c
@@ -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;
diff --git a/abspath.h b/abspath.h
new file mode 100644
index 00000000000..edebc3a53ba
--- /dev/null
+++ b/abspath.h
@@ -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);
diff --git a/cache.h b/cache.h
index 26ed03bd6de..e226dbcc7d5 100644
--- a/cache.h
+++ b/cache.h
@@ -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
-- 
gitgitgadget
Junio C Hamano· Sep 28, 2022, 00:40 UTC · re: skrab-sah via GitGitGadget · lore

Re: [PATCH] abspath.h is created.

"skrab-sah via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 7 quoted lines
> diff --git a/abspath.h b/abspath.h
> new file mode 100644
> index 00000000000..edebc3a53ba
> --- /dev/null
> +++ b/abspath.h
> @@ -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.

Skrab Sah· Sep 28, 2022, 07:32 UTC · re: Junio C Hamano · lore

Re: [PATCH] abspath.h is created.

What is wrong here and what should i do to make it correct.
On Wed, 28 Sept 2022 at 06:11, Junio C Hamano <gitster@pobox.com> wrote:
Show 16 quoted lines
>
> "skrab-sah via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > diff --git a/abspath.h b/abspath.h
> > new file mode 100644
> > index 00000000000..edebc3a53ba
> > --- /dev/null
> > +++ b/abspath.h
> > @@ -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.
Taylor Blau· Sep 28, 2022, 15:39 UTC · re: Skrab Sah · lore

Re: [PATCH] abspath.h is created.

Hi Skrab,
On Wed, Sep 28, 2022 at 01:02:22PM +0530, Skrab Sah wrote:
> 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
Junio C Hamano· Sep 28, 2022, 17:58 UTC · re: Skrab Sah · lore

Re: [PATCH] abspath.h is created.

Skrab Sah <skrab.sah@gmail.com> writes:
> 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.

← back to recent threads