[GSoC][PATCH 0/4] teach git repo info to handle path keys

[GSoC][PATCH 0/4] teach git repo info to handle path keys

[K Jayatheerth]

Hi!

The first and second patches are self-explanatory, so I will
focus more on the third and fourth patches, which introduce the
path-related fields to `git repo info`.

In the last discussion [1] we had on the mailing list about paths
in repo info, we didn't reach a definitive conclusion, but
adding both options made the most sense based on the feedback.

So in patches 3 and 4, we add both `path.<field>.absolute` and
`path.<field>.relative` for `gitdir` and `commondir`. Initially,
it was proposed by Ayush to use `path.absolute.<field>`, but
this would break the lexicographical order of the internal field
array. I tweaked it to place the variant at the end as a suffix instead.

There are still a few open questions that should be addressed
by the community. I am tagging members who were involved in the
previous discussions:

Justin Tobler, Lucas Seiki Oshiro, Junio, Phillip Wood,
brian m. carlson, and Ayush Jha.

Apologies if I missed anyone; I included everyone who reviewed
or participated in the discussions of Eslam's and Lucas's
patches.

Questions:

1. Should there still be a --path-format flag?
2. Should we consider a default option?
   Currently we have path.gitdir.absolute; should we consider
   an option where a plain path.gitdir returns some default?
   If yes:
     2.1 Should we keep the default the same as rev-parse? Or
         should either relative or absolute be the default?
     2.2 When printing using --all, should the default be
         printed, or should we print both absolute and
         relative?
3. Is printing both absolute and relative in a single call
   using --all acceptable?
   If no:
     3.1 What's a better approach?

I have discussed these changes with both Justin and Lucas
internally. This series is presented to gather opinions from the
wider community before moving forward.

K Jayatheerth (4):
  path: add strbuf_add_path for formatting paths
  rev-parse: use strbuf_add_path for path formatting
  repo: add path.gitdir with absolute and relative suffix formatting
  repo: add path.commondir with absolute and relative suffix formatting

 Documentation/git-repo.adoc |  15 ++++++
 builtin/repo.c              |  50 ++++++++++++++++++
 builtin/rev-parse.c         | 100 ++++++++----------------------------
 path.c                      |  58 +++++++++++++++++++++
 path.h                      |  16 ++++++
 t/t1900-repo-info.sh        |  32 ++++++++++++
 6 files changed, 192 insertions(+), 79 deletions(-)

-- 
2.54.0

[GSoC][PATCH 1/4] path: add strbuf_add_path for formatting paths

[K Jayatheerth]

The `print_path()` function in `builtin/rev-parse.c` contains
logic for formatting paths as either absolute or relative based on user
preferences and default behaviors. However, this logic is currently
locked inside `rev-parse` and writes directly to stdout using `puts()`.

To allow other builtins (such as the new `git repo` command) to utilize
this same path-formatting logic, extract the core algorithm into a new
string-builder function, `strbuf_add_path()`, in `path.c`.

Additionally, extract the associated enums (`format_type` and
`default_type`), and prefix them with `path_` (e.g., `path_format_type`)
to safely expose them in `path.h` without polluting the global namespace.

Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
---
 path.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 path.h | 16 ++++++++++++++++
 2 files changed, 74 insertions(+)

diff --git a/path.c b/path.c
index d7e17bf174..914812320f 100644
--- a/path.c
+++ b/path.c
@@ -1579,6 +1579,64 @@ char *xdg_cache_home(const char *filename)
 	return NULL;
 }
 
+void strbuf_add_path(struct strbuf *sb, const char *path, const char *prefix,
+		     enum path_format_type format, enum path_default_type def)
+{
+	char *cwd = NULL;
+
+	/*
+	 * We don't ever produce a relative path if prefix is NULL, so set the
+	 * prefix to the current directory so that we can produce a relative
+	 * path whenever possible. If we're using RELATIVE_IF_SHARED mode, then
+	 * we want an absolute path unless the two share a common prefix, so don't
+	 * set it in that case, since doing so causes a relative path to always
+	 * be produced if possible.
+	 */
+	if (!prefix && (format != PATH_FORMAT_DEFAULT || def != PATH_DEFAULT_RELATIVE_IF_SHARED))
+		prefix = cwd = xgetcwd();
+
+	if (format == PATH_FORMAT_DEFAULT && def == PATH_DEFAULT_UNMODIFIED) {
+		/* Case 1: Return the path exactly as-is without modifications */
+		strbuf_addstr(sb, path);
+	} else if (format == PATH_FORMAT_RELATIVE ||
+		   (format == PATH_FORMAT_DEFAULT && def == PATH_DEFAULT_RELATIVE)) {
+		/*
+		 * Case 2: Explicitly or implicitly relative.
+		 * inside relative_path(), both targets must be absolute paths
+		 * to compute a reliable relative tracking offset.
+		 */
+		struct strbuf buf = STRBUF_INIT, realbuf = STRBUF_INIT, prefixbuf = STRBUF_INIT;
+
+		if (!is_absolute_path(path)) {
+			strbuf_realpath_forgiving(&realbuf, path, 1);
+			path = realbuf.buf;
+		}
+		if (!is_absolute_path(prefix)) {
+			strbuf_realpath_forgiving(&prefixbuf, prefix, 1);
+			prefix = prefixbuf.buf;
+		}
+
+		strbuf_addstr(sb, relative_path(path, prefix, &buf));
+
+		strbuf_release(&buf);
+		strbuf_release(&realbuf);
+		strbuf_release(&prefixbuf);
+	} else if (format == PATH_FORMAT_DEFAULT && def == PATH_DEFAULT_RELATIVE_IF_SHARED) {
+		/* Case 3: Relative format if they share a common root pathway */
+		struct strbuf buf = STRBUF_INIT;
+		strbuf_addstr(sb, relative_path(path, prefix, &buf));
+		strbuf_release(&buf);
+	} else {
+		/* Case 4: Forced absolute / canonical format optimization */
+		struct strbuf buf = STRBUF_INIT;
+		strbuf_realpath_forgiving(&buf, path, 1);
+		strbuf_addbuf(sb, &buf);
+		strbuf_release(&buf);
+	}
+
+	free(cwd);
+}
+
 REPO_GIT_PATH_FUNC(squash_msg, "SQUASH_MSG")
 REPO_GIT_PATH_FUNC(merge_msg, "MERGE_MSG")
 REPO_GIT_PATH_FUNC(merge_rr, "MERGE_RR")
diff --git a/path.h b/path.h
index 0434ba5e07..b9b626ce4a 100644
--- a/path.h
+++ b/path.h
@@ -262,6 +262,22 @@ enum scld_error safe_create_leading_directories_no_share(char *path);
 int safe_create_file_with_leading_directories(struct repository *repo,
 					      const char *path);
 
+enum path_format_type {
+	PATH_FORMAT_DEFAULT,
+	PATH_FORMAT_RELATIVE,
+	PATH_FORMAT_CANONICAL
+};
+
+enum path_default_type {
+	PATH_DEFAULT_RELATIVE,
+	PATH_DEFAULT_RELATIVE_IF_SHARED,
+	PATH_DEFAULT_CANONICAL,
+	PATH_DEFAULT_UNMODIFIED
+};
+
+void strbuf_add_path(struct strbuf *buf, const char *path, const char *prefix,
+		     enum path_format_type format, enum path_default_type def);
+
 # ifdef USE_THE_REPOSITORY_VARIABLE
 #  include "strbuf.h"
 #  include "repository.h"
-- 
2.54.0

[GSoC][PATCH 2/4] rev-parse: use strbuf_add_path for path formatting

[K Jayatheerth]

Now that the core path-formatting logic has been abstracted into
strbuf_add_path() inside path.c, remove the duplicate localized
implementation from builtin/rev-parse.c.

Drop the local format_type and default_type enums from the builtin, and
update print_path() to act as a light wrapper around the new shared
strbuf engine. Update cmd_rev_parse() to use the new path_ format and
default enum types exposed via path.h.

Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
---
 builtin/rev-parse.c | 100 ++++++++++----------------------------------
 1 file changed, 21 insertions(+), 79 deletions(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 218b5f34d6..812cfd55ad 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -632,73 +632,15 @@ static void handle_ref_opt(const char *pattern, const char *prefix)
 	clear_ref_exclusions(&ref_excludes);
 }
 
-enum format_type {
-	/* We would like a relative path. */
-	FORMAT_RELATIVE,
-	/* We would like a canonical absolute path. */
-	FORMAT_CANONICAL,
-	/* We would like the default behavior. */
-	FORMAT_DEFAULT,
-};
-
-enum default_type {
-	/* Our default is a relative path. */
-	DEFAULT_RELATIVE,
-	/* Our default is a relative path if there's a shared root. */
-	DEFAULT_RELATIVE_IF_SHARED,
-	/* Our default is a canonical absolute path. */
-	DEFAULT_CANONICAL,
-	/* Our default is not to modify the item. */
-	DEFAULT_UNMODIFIED,
-};
-
-static void print_path(const char *path, const char *prefix, enum format_type format, enum default_type def)
+static void print_path(const char *path, const char *prefix,
+		       enum path_format_type format, enum path_default_type def)
 {
-	char *cwd = NULL;
-	/*
-	 * We don't ever produce a relative path if prefix is NULL, so set the
-	 * prefix to the current directory so that we can produce a relative
-	 * path whenever possible.  If we're using RELATIVE_IF_SHARED mode, then
-	 * we want an absolute path unless the two share a common prefix, so don't
-	 * set it in that case, since doing so causes a relative path to always
-	 * be produced if possible.
-	 */
-	if (!prefix && (format != FORMAT_DEFAULT || def != DEFAULT_RELATIVE_IF_SHARED))
-		prefix = cwd = xgetcwd();
-	if (format == FORMAT_DEFAULT && def == DEFAULT_UNMODIFIED) {
-		puts(path);
-	} else if (format == FORMAT_RELATIVE ||
-		  (format == FORMAT_DEFAULT && def == DEFAULT_RELATIVE)) {
-		/*
-		 * In order for relative_path to work as expected, we need to
-		 * make sure that both paths are absolute paths.  If we don't,
-		 * we can end up with an unexpected absolute path that the user
-		 * didn't want.
-		 */
-		struct strbuf buf = STRBUF_INIT, realbuf = STRBUF_INIT, prefixbuf = STRBUF_INIT;
-		if (!is_absolute_path(path)) {
-			strbuf_realpath_forgiving(&realbuf, path,  1);
-			path = realbuf.buf;
-		}
-		if (!is_absolute_path(prefix)) {
-			strbuf_realpath_forgiving(&prefixbuf, prefix, 1);
-			prefix = prefixbuf.buf;
-		}
-		puts(relative_path(path, prefix, &buf));
-		strbuf_release(&buf);
-		strbuf_release(&realbuf);
-		strbuf_release(&prefixbuf);
-	} else if (format == FORMAT_DEFAULT && def == DEFAULT_RELATIVE_IF_SHARED) {
-		struct strbuf buf = STRBUF_INIT;
-		puts(relative_path(path, prefix, &buf));
-		strbuf_release(&buf);
-	} else {
-		struct strbuf buf = STRBUF_INIT;
-		strbuf_realpath_forgiving(&buf, path, 1);
-		puts(buf.buf);
-		strbuf_release(&buf);
-	}
-	free(cwd);
+	struct strbuf sb = STRBUF_INIT;
+
+	strbuf_add_path(&sb, path, prefix, format, def);
+	puts(sb.buf);
+
+	strbuf_release(&sb);
 }
 
 int cmd_rev_parse(int argc,
@@ -717,7 +659,7 @@ int cmd_rev_parse(int argc,
 	const char *name = NULL;
 	struct strbuf buf = STRBUF_INIT;
 	int seen_end_of_options = 0;
-	enum format_type format = FORMAT_DEFAULT;
+	enum path_format_type format = PATH_FORMAT_DEFAULT;
 
 	show_usage_if_asked(argc, argv, builtin_rev_parse_usage);
 
@@ -798,7 +740,7 @@ int cmd_rev_parse(int argc,
 				print_path(repo_git_path_replace(the_repository, &buf,
 								 "%s", argv[i + 1]), prefix,
 						format,
-						DEFAULT_RELATIVE_IF_SHARED);
+						PATH_DEFAULT_RELATIVE_IF_SHARED);
 				i++;
 				continue;
 			}
@@ -820,9 +762,9 @@ int cmd_rev_parse(int argc,
 				if (!arg)
 					die(_("--path-format requires an argument"));
 				if (!strcmp(arg, "absolute")) {
-					format = FORMAT_CANONICAL;
+					format = PATH_FORMAT_CANONICAL;
 				} else if (!strcmp(arg, "relative")) {
-					format = FORMAT_RELATIVE;
+					format = PATH_FORMAT_RELATIVE;
 				} else {
 					die(_("unknown argument to --path-format: %s"), arg);
 				}
@@ -985,7 +927,7 @@ int cmd_rev_parse(int argc,
 			if (!strcmp(arg, "--show-toplevel")) {
 				const char *work_tree = repo_get_work_tree(the_repository);
 				if (work_tree)
-					print_path(work_tree, prefix, format, DEFAULT_UNMODIFIED);
+					print_path(work_tree, prefix, format, PATH_DEFAULT_UNMODIFIED);
 				else
 					die(_("this operation must be run in a work tree"));
 				continue;
@@ -993,7 +935,7 @@ int cmd_rev_parse(int argc,
 			if (!strcmp(arg, "--show-superproject-working-tree")) {
 				struct strbuf superproject = STRBUF_INIT;
 				if (get_superproject_working_tree(&superproject))
-					print_path(superproject.buf, prefix, format, DEFAULT_UNMODIFIED);
+					print_path(superproject.buf, prefix, format, PATH_DEFAULT_UNMODIFIED);
 				strbuf_release(&superproject);
 				continue;
 			}
@@ -1028,18 +970,18 @@ int cmd_rev_parse(int argc,
 				const char *gitdir = getenv(GIT_DIR_ENVIRONMENT);
 				char *cwd;
 				int len;
-				enum format_type wanted = format;
+				enum path_format_type wanted = format;
 				if (arg[2] == 'g') {	/* --git-dir */
 					if (gitdir) {
-						print_path(gitdir, prefix, format, DEFAULT_UNMODIFIED);
+						print_path(gitdir, prefix, format, PATH_DEFAULT_UNMODIFIED);
 						continue;
 					}
 					if (!prefix) {
-						print_path(".git", prefix, format, DEFAULT_UNMODIFIED);
+						print_path(".git", prefix, format, PATH_DEFAULT_UNMODIFIED);
 						continue;
 					}
 				} else {		/* --absolute-git-dir */
-					wanted = FORMAT_CANONICAL;
+					wanted = PATH_FORMAT_CANONICAL;
 					if (!gitdir && !prefix)
 						gitdir = ".git";
 					if (gitdir) {
@@ -1055,11 +997,11 @@ int cmd_rev_parse(int argc,
 				strbuf_reset(&buf);
 				strbuf_addf(&buf, "%s%s.git", cwd, len && cwd[len-1] != '/' ? "/" : "");
 				free(cwd);
-				print_path(buf.buf, prefix, wanted, DEFAULT_CANONICAL);
+				print_path(buf.buf, prefix, wanted, PATH_DEFAULT_CANONICAL);
 				continue;
 			}
 			if (!strcmp(arg, "--git-common-dir")) {
-				print_path(repo_get_common_dir(the_repository), prefix, format, DEFAULT_RELATIVE_IF_SHARED);
+				print_path(repo_get_common_dir(the_repository), prefix, format, PATH_DEFAULT_RELATIVE_IF_SHARED);
 				continue;
 			}
 			if (!strcmp(arg, "--is-inside-git-dir")) {
@@ -1089,7 +1031,7 @@ int cmd_rev_parse(int argc,
 				if (the_repository->index->split_index) {
 					const struct object_id *oid = &the_repository->index->split_index->base_oid;
 					const char *path = repo_git_path_replace(the_repository, &buf, "sharedindex.%s", oid_to_hex(oid));
-					print_path(path, prefix, format, DEFAULT_RELATIVE);
+					print_path(path, prefix, format, PATH_DEFAULT_RELATIVE);
 				}
 				continue;
 			}
-- 
2.54.0

[GSoC][PATCH 3/4] repo: add path.gitdir with absolute and relative suffix formatting

[K Jayatheerth]

Introduce path-related metadata fields to `git repo info` by adding
explicit `path.gitdir.absolute` and `path.gitdir.relative` keys. This
replaces dynamic prefix parsing machinery with individual, predictable
lexicographically-sorted keys that map directly to dedicated formatting
callbacks.

To calculate paths relative to the current working directory, update
`builtin/repo.c` to include `setup.h` and supply `startup_info->prefix`
to the path-formatting engine. Both explicit variants automatically
populate bulk dumps via `--all` and output predictably under `--keys`.

Update `t/t1900-repo-info.sh` to use a modernized, function-based loop
helper (`test_repo_info_path`) and `test_grep` to cleanly assert separate
path variation lookups.

Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
---
 Documentation/git-repo.adoc |  6 ++++++
 builtin/repo.c              | 26 ++++++++++++++++++++++++++
 t/t1900-repo-info.sh        | 31 +++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+)

diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
index 42262c1983..a0dca7ce88 100644
--- a/Documentation/git-repo.adoc
+++ b/Documentation/git-repo.adoc
@@ -104,6 +104,12 @@ values that they return:
 `object.format`::
 	The object format (hash algorithm) used in the repository.
 
+`path.gitdir.absolute`::
+	The canonical absolute path to the Git repository directory (the `.git` directory).
+
+`path.gitdir.relative`::
+	The path to the Git repository directory relative to the current working directory.
+
 `references.format`::
 	The reference storage format. The valid values are:
 +
diff --git a/builtin/repo.c b/builtin/repo.c
index 71a5c1c29c..c141ef892a 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -7,12 +7,14 @@
 #include "hex.h"
 #include "odb.h"
 #include "parse-options.h"
+#include "path.h"
 #include "path-walk.h"
 #include "progress.h"
 #include "quote.h"
 #include "ref-filter.h"
 #include "refs.h"
 #include "revision.h"
+#include "setup.h"
 #include "strbuf.h"
 #include "string-list.h"
 #include "shallow.h"
@@ -75,6 +77,28 @@ static int get_object_format(struct repository *repo, struct strbuf *buf)
 	return 0;
 }
 
+static int get_path_gitdir_absolute(struct repository *repo, struct strbuf *buf)
+{
+	const char *git_dir = repo_get_git_dir(repo);
+
+	if (!git_dir)
+		return error(_("unable to get git directory"));
+
+	strbuf_add_path(buf, git_dir, startup_info->prefix, PATH_FORMAT_CANONICAL, PATH_DEFAULT_UNMODIFIED);
+	return 0;
+}
+
+static int get_path_gitdir_relative(struct repository *repo, struct strbuf *buf)
+{
+	const char *git_dir = repo_get_git_dir(repo);
+
+	if (!git_dir)
+		return error(_("unable to get git directory"));
+
+	strbuf_add_path(buf, git_dir, startup_info->prefix, PATH_FORMAT_RELATIVE, PATH_DEFAULT_UNMODIFIED);
+	return 0;
+}
+
 static int get_references_format(struct repository *repo, struct strbuf *buf)
 {
 	strbuf_addstr(buf,
@@ -87,6 +111,8 @@ static const struct repo_info_field repo_info_field[] = {
 	{ "layout.bare", get_layout_bare },
 	{ "layout.shallow", get_layout_shallow },
 	{ "object.format", get_object_format },
+	{ "path.gitdir.absolute", get_path_gitdir_absolute },
+	{ "path.gitdir.relative", get_path_gitdir_relative },
 	{ "references.format", get_references_format },
 };
 
diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
index 39bb77dda0..7c7dfbb052 100755
--- a/t/t1900-repo-info.sh
+++ b/t/t1900-repo-info.sh
@@ -155,4 +155,35 @@ test_expect_success 'git repo info -h shows only repo info usage' '
 	test_grep ! "git repo structure" actual
 '
 
+test_repo_info_path () {
+	field_name=$1
+	expect_relative=$2
+
+	test_expect_success "query individual key: path.$field_name.absolute" '
+		(
+			cd test-repo/sub &&
+			expect_absolute=$(cd .. && pwd)/.git &&
+			echo "path.$field_name.absolute=$expect_absolute" >expect &&
+			git repo info path.$field_name.absolute >actual &&
+			test_cmp expect actual
+		)
+	'
+
+	test_expect_success "query individual key: path.$field_name.relative" '
+		(
+			cd test-repo/sub &&
+			echo "path.$field_name.relative=$expect_relative" >expect &&
+			git repo info path.$field_name.relative >actual &&
+			test_cmp expect actual
+		)
+	'
+}
+
+test_expect_success 'setup test repository layout for path fields' '
+	git init test-repo &&
+	mkdir -p test-repo/sub
+'
+
+test_repo_info_path 'gitdir' '../.git'
+
 test_done
-- 
2.54.0

[GSoC][PATCH 4/4] repo: add path.commondir with absolute and relative suffix formatting

[K Jayatheerth]

Introduce `path.commondir.absolute` and `path.commondir.relative` keys
to `git repo info`. These track the repository's common directory path,
extending the path metadata engine alongside the existing `gitdir` fields.

Update `repo_info_field` to store the new keys in proper lexicographical
order to protect binary search operations, and expand the test matrix in
`t/t1900-repo-info.sh` to validate separate queries, bulk dumps, and
key listings.

Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
---
 Documentation/git-repo.adoc |  9 +++++++++
 builtin/repo.c              | 24 ++++++++++++++++++++++++
 t/t1900-repo-info.sh        |  1 +
 3 files changed, 34 insertions(+)

diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
index a0dca7ce88..ed7d80c690 100644
--- a/Documentation/git-repo.adoc
+++ b/Documentation/git-repo.adoc
@@ -104,6 +104,15 @@ values that they return:
 `object.format`::
 	The object format (hash algorithm) used in the repository.
 
+`path.commondir.absolute`::
+	The canonical absolute path to the Git repository's common
+	directory (the shared `.git` directory containing objects,
+	refs, and global configuration).
+
+`path.commondir.relative`::
+	The path to the Git repository's common directory relative to
+	the current working directory.
+
 `path.gitdir.absolute`::
 	The canonical absolute path to the Git repository directory (the `.git` directory).
 
diff --git a/builtin/repo.c b/builtin/repo.c
index c141ef892a..be24a5a8e8 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -77,6 +77,28 @@ static int get_object_format(struct repository *repo, struct strbuf *buf)
 	return 0;
 }
 
+static int get_path_commondir_absolute(struct repository *repo, struct strbuf *buf)
+{
+	const char *common_dir = repo_get_common_dir(repo);
+
+	if (!common_dir)
+		return error(_("unable to get common directory"));
+
+	strbuf_add_path(buf, common_dir, startup_info->prefix, PATH_FORMAT_CANONICAL, PATH_DEFAULT_UNMODIFIED);
+	return 0;
+}
+
+static int get_path_commondir_relative(struct repository *repo, struct strbuf *buf)
+{
+	const char *common_dir = repo_get_common_dir(repo);
+
+	if (!common_dir)
+		return error(_("unable to get common directory"));
+
+	strbuf_add_path(buf, common_dir, startup_info->prefix, PATH_FORMAT_RELATIVE, PATH_DEFAULT_UNMODIFIED);
+	return 0;
+}
+
 static int get_path_gitdir_absolute(struct repository *repo, struct strbuf *buf)
 {
 	const char *git_dir = repo_get_git_dir(repo);
@@ -111,6 +133,8 @@ static const struct repo_info_field repo_info_field[] = {
 	{ "layout.bare", get_layout_bare },
 	{ "layout.shallow", get_layout_shallow },
 	{ "object.format", get_object_format },
+	{ "path.commondir.absolute", get_path_commondir_absolute },
+	{ "path.commondir.relative", get_path_commondir_relative },
 	{ "path.gitdir.absolute", get_path_gitdir_absolute },
 	{ "path.gitdir.relative", get_path_gitdir_relative },
 	{ "references.format", get_references_format },
diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
index 7c7dfbb052..dd2706e1f7 100755
--- a/t/t1900-repo-info.sh
+++ b/t/t1900-repo-info.sh
@@ -184,6 +184,7 @@ test_expect_success 'setup test repository layout for path fields' '
 	mkdir -p test-repo/sub
 '
 
+test_repo_info_path 'commondir' '../.git'
 test_repo_info_path 'gitdir' '../.git'
 
 test_done
-- 
2.54.0

Re: [GSoC][PATCH 0/4] teach git repo info to handle path keys

[Lucas Seiki Oshiro]

> 1. Should there still be a --path-format flag?

If you specify "absolute" and "relative" in the keys, it won't
make sense to use it.

> 2. Should we consider a default option?

Some pros and cons:

- Pro: some values make more sense to be in absolute or relative
  format
- Pro: it's boring to always add `.(relative|absolute)` to the
  paths
- Con: it will be perpetuating what git-rev-parse does, and we
  don't git-repo-info to be git-rev-parse with a different
  interface. It's our chance to learn with [1] for example.
- Con: the user will need if the value is relative or absolute

> 3. Is printing both absolute and relative in a single call
>   using --all acceptable?

If you're providing both keys, I think it's not only acceptable
but mandatory. `--all` should mean "all", not "all, but ...".

> I have discussed these changes with both Justin and Lucas
> internally. This series is presented to gather opinions from the
> wider community before moving forward.

I probably sent the same comments internally, but I'm sending
here to share my opinions with the rest of the community ;-)

[1] fac60b8925 (rev-parse: add option for absolute or relative path formatting, 2020-12-13)

Re: [GSoC][PATCH 3/4] repo: add path.gitdir with absolute and relative suffix formatting

[Lucas Seiki Oshiro]

> +test_repo_info_path () {
> + field_name=$1
> + expect_relative=$2
> +
> + test_expect_success "query individual key: path.$field_name.absolute" '
> + (
> + cd test-repo/sub &&
> + expect_absolute=$(cd .. && pwd)/.git &&

Note that this semi-hardcoded path won't work for other values (e.g.
top level dir, superproject working tree). This needs to be a parameter
just like `expect_relative`

Re: [GSoC][PATCH 4/4] repo: add path.commondir with absolute and relative suffix formatting

[Lucas Seiki Oshiro]

> diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
> index 7c7dfbb052..dd2706e1f7 100755
> --- a/t/t1900-repo-info.sh
> +++ b/t/t1900-repo-info.sh
> @@ -184,6 +184,7 @@ test_expect_success 'setup test repository layout for path fields' '
> mkdir -p test-repo/sub
> '
> 
> +test_repo_info_path 'commondir' '../.git'
> test_repo_info_path 'gitdir' '../.git'

I was thinking here, maybe you need to take a look at
git-rev-parse's tests and check what are the corner cases.

For example, `git rev-parse --git-common-dir` documentation
says:

    --git-common-dir:
        Show $GIT_COMMON_DIR if defined, else $GIT_DIR

This way, you should take a look on how git-rev-parse tests
test those two branches (GIT_COMMON_DIR and GIT_DIR).

Re: [GSoC][PATCH 4/4] repo: add path.commondir with absolute and relative suffix formatting

[Lucas Seiki Oshiro]

> diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
> index 7c7dfbb052..dd2706e1f7 100755
> --- a/t/t1900-repo-info.sh
> +++ b/t/t1900-repo-info.sh
> @@ -184,6 +184,7 @@ test_expect_success 'setup test repository layout for path fields' '
> mkdir -p test-repo/sub
> '
> 
> +test_repo_info_path 'commondir' '../.git'
> test_repo_info_path 'gitdir' '../.git'

I was thinking here, maybe you need to take a look at
git-rev-parse's tests and check what are the corner cases.

For example, `git rev-parse --git-common-dir` documentation
says:

    --git-common-dir:
        Show $GIT_COMMON_DIR if defined, else $GIT_DIR

This way, you should take a look on how git-rev-parse tests
test those two cases (GIT_COMMON_DIR and GIT_DIR) and do
something similar here.

Re: [GSoC][PATCH 0/4] teach git repo info to handle path keys

[Lucas Seiki Oshiro]

Nitpick: use [GSoC PATCH] instead of [GSoC][PATCH] as prefix.
Use --subject-prefix='GSoC PATCH' in git-send-email or
git-format-patch or set the configuration variable
`format.subjectPrefix` to that until you finish your GSoC:

$ git config --local format.subjectPrefix 'GSoC PATCH'

Re: [GSoC][PATCH 0/4] teach git repo info to handle path keys

[Junio C Hamano]

K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:

> Hi!
>
> The first and second patches are self-explanatory, so I will
> focus more on the third and fourth patches, which introduce the
> path-related fields to `git repo info`.

I sense that a paragraph or two is missing before that.  The purpose
of your cover letter is to _sell_ your changes, explain what it is
about, and get your target audience interested enough to read the
patches.  The above goes totally backwards---your readers do not yet
know what the series is about, they haven't decided if it is worth
their time to read it, and you are telling "go read first two
yourself, I am not going to tell you what they are about"?

> In the last discussion [1] we had on the mailing list about paths
> in repo info, we didn't reach a definitive conclusion, but
> adding both options made the most sense based on the feedback.

And again, you are excluding those who were not in, or do not
remember, what was discussed in "the last discussion".

Re: [GSoC][PATCH 3/4] repo: add path.gitdir with absolute and relative suffix formatting

[Junio C Hamano]

Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> writes:

>> +test_repo_info_path () {
>> + field_name=$1
>> + expect_relative=$2
>> +
>> + test_expect_success "query individual key: path.$field_name.absolute" '
>> + (
>> + cd test-repo/sub &&
>> + expect_absolute=$(cd .. && pwd)/.git &&
>
> Note that this semi-hardcoded path won't work for other values (e.g.
> top level dir, superproject working tree). This needs to be a parameter
> just like `expect_relative`

Good thinking.  Thanks for carefully reading.

Re: [GSoC][PATCH 1/4] path: add strbuf_add_path for formatting paths

[Phillip Wood]

On 01/06/2026 16:19, K Jayatheerth wrote:
> 
> diff --git a/path.h b/path.h
> index 0434ba5e07..b9b626ce4a 100644
> --- a/path.h
> +++ b/path.h
> @@ -262,6 +262,22 @@ enum scld_error safe_create_leading_directories_no_share(char *path);
>   int safe_create_file_with_leading_directories(struct repository *repo,
>   					      const char *path);
>   
> +enum path_format_type {
> +	PATH_FORMAT_DEFAULT,
> +	PATH_FORMAT_RELATIVE,
> +	PATH_FORMAT_CANONICAL
> +};
> +
> +enum path_default_type {
> +	PATH_DEFAULT_RELATIVE,
> +	PATH_DEFAULT_RELATIVE_IF_SHARED,
> +	PATH_DEFAULT_CANONICAL,
> +	PATH_DEFAULT_UNMODIFIED
> +};
> +
> +void strbuf_add_path(struct strbuf *buf, const char *path, const char *prefix,
> +		     enum path_format_type format, enum path_default_type def);

This API is very specific to rev-parse and to me at least it is hard to 
understand. I think it would be clearer if we had a single enum 
describing the desired format and let the rev-parse code worry about 
passing the appropriate value based on the options the user passed.

enum path_format {
	PATH_FORMAT_ABSOLUTE,
	PATH_FORMAT_CANONICAL,
	PATH_FORMAT_RELATIVE,
	PATH_FORMAT_RELATIVE_IF_SHARED	PATH_FORMAT_UNMODIFIED,
};

void format_path(struct strbuf *buf, const char *path,
		 const char *prefix, enum path_format format);

We tend to avoid adding "strbuf_" to the beginning of functions these 
days when they're adding things to a strbuf. This function also needs 
some documentation explaining what the arguments are.

Thanks

Phillip

Re: [GSoC][PATCH 0/4] teach git repo info to handle path keys

[Phillip Wood]

On 01/06/2026 16:19, K Jayatheerth wrote:
> 
> So in patches 3 and 4, we add both `path.<field>.absolute` and
> `path.<field>.relative` for `gitdir` and `commondir`. Initially,
> it was proposed by Ayush to use `path.absolute.<field>`, but
> this would break the lexicographical order of the internal field
> array. I tweaked it to place the variant at the end as a suffix instead.

I don't understand the comment about breaking the lexicographical order, 
surely it only breaks if the new items are added out of order? Why can't 
we have

	path.absolute.commondir
	path.absolute.gitdir
	path.relative.commondir
	path.relative.gitdir

?

Thanks

Phillip

> There are still a few open questions that should be addressed
> by the community. I am tagging members who were involved in the
> previous discussions:
> 
> Justin Tobler, Lucas Seiki Oshiro, Junio, Phillip Wood,
> brian m. carlson, and Ayush Jha.
> 
> Apologies if I missed anyone; I included everyone who reviewed
> or participated in the discussions of Eslam's and Lucas's
> patches.
> 
> Questions:
> 
> 1. Should there still be a --path-format flag?
> 2. Should we consider a default option?
>     Currently we have path.gitdir.absolute; should we consider
>     an option where a plain path.gitdir returns some default?
>     If yes:
>       2.1 Should we keep the default the same as rev-parse? Or
>           should either relative or absolute be the default?
>       2.2 When printing using --all, should the default be
>           printed, or should we print both absolute and
>           relative?
> 3. Is printing both absolute and relative in a single call
>     using --all acceptable?
>     If no:
>       3.1 What's a better approach?
> 
> I have discussed these changes with both Justin and Lucas
> internally. This series is presented to gather opinions from the
> wider community before moving forward.
> 
> K Jayatheerth (4):
>    path: add strbuf_add_path for formatting paths
>    rev-parse: use strbuf_add_path for path formatting
>    repo: add path.gitdir with absolute and relative suffix formatting
>    repo: add path.commondir with absolute and relative suffix formatting
> 
>   Documentation/git-repo.adoc |  15 ++++++
>   builtin/repo.c              |  50 ++++++++++++++++++
>   builtin/rev-parse.c         | 100 ++++++++----------------------------
>   path.c                      |  58 +++++++++++++++++++++
>   path.h                      |  16 ++++++
>   t/t1900-repo-info.sh        |  32 ++++++++++++
>   6 files changed, 192 insertions(+), 79 deletions(-)
>