[PATCH] repo: add paths.toplevel to repo info

[PATCH] repo: add paths.toplevel to repo info

[Jayesh Daga via GitGitGadget]

From: Jayesh Daga <jayeshdaga99@gmail.com>

Expose the working tree root via `git repo info` as
paths.toplevel, matching the semantics of
`git rev-parse --show-toplevel`.

For bare repositories, this value is empty, consistent
with other non-applicable fields.

This allows scripts to retrieve the repository root
through a structured interface without invoking
rev-parse.

Signed-off-by: Jayesh Daga <jayeshdaga99@gmail.com>
---
    repo: add paths.toplevel to repo info
    
    repo info currently does not expose the repository's working tree root,
    even though this information is available via repo_get_work_tree().
    
    This makes it harder for scripts to retrieve the repository root through
    a structured interface, often requiring the use of git rev-parse
    --show-toplevel.
    
    Add a new field paths.toplevel to git repo info that returns the working
    tree root. For bare repositories, this value is empty, consistent with
    other non-applicable fields.
    
    This provides a consistent and script-friendly way to query repository
    paths without invoking additional commands.
    
    Signed-off-by: Jayesh Daga jayeshdaga99@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2264%2Fjayesh0104%2Frepo-toplevel-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2264/jayesh0104/repo-toplevel-v1
Pull-Request: https://github.com/git/git/pull/2264

 builtin/repo.c       | 12 ++++++++++++
 t/t1900-repo-info.sh | 16 ++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/builtin/repo.c b/builtin/repo.c
index 71a5c1c29c..d0491f6c66 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -62,6 +62,17 @@ static int get_layout_bare(struct repository *repo UNUSED, struct strbuf *buf)
 	return 0;
 }
 
+static int get_paths_toplevel(struct repository *repo, struct strbuf *buf)
+{
+    const char *wt = repo_get_work_tree(repo);
+
+    if (!wt)
+	return -1; /* match existing error style */
+
+    strbuf_addstr(buf, wt);
+    return 0;
+}
+
 static int get_layout_shallow(struct repository *repo, struct strbuf *buf)
 {
 	strbuf_addstr(buf,
@@ -87,6 +98,7 @@ static const struct repo_info_field repo_info_field[] = {
 	{ "layout.bare", get_layout_bare },
 	{ "layout.shallow", get_layout_shallow },
 	{ "object.format", get_object_format },
+	{ "paths.toplevel", get_paths_toplevel },
 	{ "references.format", get_references_format },
 };
 
diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
index 39bb77dda0..470e06e8c2 100755
--- a/t/t1900-repo-info.sh
+++ b/t/t1900-repo-info.sh
@@ -155,4 +155,20 @@ test_expect_success 'git repo info -h shows only repo info usage' '
 	test_grep ! "git repo structure" actual
 '
 
+test_expect_success 'repo info paths.toplevel' '
+    git repo info paths.toplevel >actual &&
+    echo "paths.toplevel=$(git rev-parse --show-toplevel)" >expected &&
+    test_cmp expected actual
+'
+
+test_expect_success 'repo info paths.toplevel (bare repo)' '
+    git init --bare bare.git &&
+    (
+	cd bare.git &&
+	git repo info paths.toplevel >actual &&
+	echo "paths.toplevel=" >expected &&
+	test_cmp expected actual
+    )
+'
+
 test_done

base-commit: 256554692df0685b45e60778b08802b720880c50
-- 
gitgitgadget

Re: [PATCH] repo: add paths.toplevel to repo info

[Junio C Hamano]

"Jayesh Daga via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +static int get_paths_toplevel(struct repository *repo, struct strbuf *buf)
> +{
> +    const char *wt = repo_get_work_tree(repo);
> +
> +    if (!wt)
> +	return -1; /* match existing error style */
> +
> +    strbuf_addstr(buf, wt);
> +    return 0;
> +}

Funny indentation.  In our C codebase, one level of indent is one
horizontal tab "\t".

> @@ -87,6 +98,7 @@ static const struct repo_info_field repo_info_field[] = {
>  	{ "layout.bare", get_layout_bare },
>  	{ "layout.shallow", get_layout_shallow },
>  	{ "object.format", get_object_format },
> +	{ "paths.toplevel", get_paths_toplevel },
>  	{ "references.format", get_references_format },

Instead of adding yet another one as people think of it, can we
first take an inventory of what is available from the kitchen-sink
options of "git rev-parse" and make a list, to be compared with what
is available in repo_info_field[]?  Add that correspondence table in
a comment before the definition of this array, and it is perfectly
OK if the right hand side on many rows say "missing", like

    /*
     * rev-parse --<opt>	repo info <field>
     *
     * is-bare-repository	layout.bare
     * is-shallow-repository    layout.shallow
     * show-cdup		<missing>
     * show-prefix              <missing>
     * show-object-format       object.format
     * show-ref-format          references.format
     * ... more ...
     */

There can be two classes of <missing>, ones that we want to have but
haven't been implemented eyt, and others that we do not think we
need.

Also, we may not want to limit the existing sources of information
to "rev-parse", in which case lines of such a correspondence table
may need to be grouped by where each piece of information is found
elsewhere.

>  };
>  
> diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
> index 39bb77dda0..470e06e8c2 100755
> --- a/t/t1900-repo-info.sh
> +++ b/t/t1900-repo-info.sh
> @@ -155,4 +155,20 @@ test_expect_success 'git repo info -h shows only repo info usage' '
>  	test_grep ! "git repo structure" actual
>  '
>  
> +test_expect_success 'repo info paths.toplevel' '
> +    git repo info paths.toplevel >actual &&
> +    echo "paths.toplevel=$(git rev-parse --show-toplevel)" >expected &&
> +    test_cmp expected actual
> +'
> +
> +test_expect_success 'repo info paths.toplevel (bare repo)' '
> +    git init --bare bare.git &&
> +    (
> +	cd bare.git &&
> +	git repo info paths.toplevel >actual &&
> +	echo "paths.toplevel=" >expected &&
> +	test_cmp expected actual
> +    )
> +'
> +
>  test_done
>
> base-commit: 256554692df0685b45e60778b08802b720880c50

[PATCH v2] repo: add paths.toplevel to repo info

[Jayesh Daga via GitGitGadget]

From: Jayesh Daga <jayeshdaga99@gmail.com>

repo info currently does not expose the repository's
working tree root, even though this information is
available via `repo_get_work_tree()` and
`git rev-parse --show-toplevel`.

Add a new field `paths.toplevel` to expose this value.

While doing so, document the correspondence between
`git rev-parse` options and `repo info` fields to make
it easier to identify missing or future additions.

For bare repositories, this value is empty, consistent
with other non-applicable fields.

Signed-off-by: Jayesh Daga [jayeshdaga99@gmail.com](mailto:jayeshdaga99@gmail.com)
---
    repo: add paths.toplevel to repo info
    
    repo info currently does not expose the repository's working tree root,
    even though this information is available via repo_get_work_tree().
    
    This makes it harder for scripts to retrieve the repository root through
    a structured interface, often requiring the use of git rev-parse
    --show-toplevel.
    
    Add a new field paths.toplevel to git repo info that returns the working
    tree root. For bare repositories, this value is empty, consistent with
    other non-applicable fields.
    
    This provides a consistent and script-friendly way to query repository
    paths without invoking additional commands.
    
    Signed-off-by: Jayesh Daga jayeshdaga99@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2264%2Fjayesh0104%2Frepo-toplevel-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2264/jayesh0104/repo-toplevel-v2
Pull-Request: https://github.com/git/git/pull/2264

Range-diff vs v1:

 1:  448dfae6a1 ! 1:  05e34bfe2c repo: add paths.toplevel to repo info
     @@ Metadata
       ## Commit message ##
          repo: add paths.toplevel to repo info
      
     -    Expose the working tree root via `git repo info` as
     -    paths.toplevel, matching the semantics of
     +    repo info currently does not expose the repository's
     +    working tree root, even though this information is
     +    available via `repo_get_work_tree()` and
          `git rev-parse --show-toplevel`.
      
     +    Add a new field `paths.toplevel` to expose this value.
     +
     +    While doing so, document the correspondence between
     +    `git rev-parse` options and `repo info` fields to make
     +    it easier to identify missing or future additions.
     +
          For bare repositories, this value is empty, consistent
          with other non-applicable fields.
      
     -    This allows scripts to retrieve the repository root
     -    through a structured interface without invoking
     -    rev-parse.
     -
     -    Signed-off-by: Jayesh Daga <jayeshdaga99@gmail.com>
     +    Signed-off-by: Jayesh Daga [jayeshdaga99@gmail.com](mailto:jayeshdaga99@gmail.com)
      
       ## builtin/repo.c ##
      @@ builtin/repo.c: static int get_layout_bare(struct repository *repo UNUSED, struct strbuf *buf)


 builtin/repo.c       | 12 ++++++++++++
 t/t1900-repo-info.sh | 16 ++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/builtin/repo.c b/builtin/repo.c
index 71a5c1c29c..d0491f6c66 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -62,6 +62,17 @@ static int get_layout_bare(struct repository *repo UNUSED, struct strbuf *buf)
 	return 0;
 }
 
+static int get_paths_toplevel(struct repository *repo, struct strbuf *buf)
+{
+    const char *wt = repo_get_work_tree(repo);
+
+    if (!wt)
+	return -1; /* match existing error style */
+
+    strbuf_addstr(buf, wt);
+    return 0;
+}
+
 static int get_layout_shallow(struct repository *repo, struct strbuf *buf)
 {
 	strbuf_addstr(buf,
@@ -87,6 +98,7 @@ static const struct repo_info_field repo_info_field[] = {
 	{ "layout.bare", get_layout_bare },
 	{ "layout.shallow", get_layout_shallow },
 	{ "object.format", get_object_format },
+	{ "paths.toplevel", get_paths_toplevel },
 	{ "references.format", get_references_format },
 };
 
diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
index 39bb77dda0..470e06e8c2 100755
--- a/t/t1900-repo-info.sh
+++ b/t/t1900-repo-info.sh
@@ -155,4 +155,20 @@ test_expect_success 'git repo info -h shows only repo info usage' '
 	test_grep ! "git repo structure" actual
 '
 
+test_expect_success 'repo info paths.toplevel' '
+    git repo info paths.toplevel >actual &&
+    echo "paths.toplevel=$(git rev-parse --show-toplevel)" >expected &&
+    test_cmp expected actual
+'
+
+test_expect_success 'repo info paths.toplevel (bare repo)' '
+    git init --bare bare.git &&
+    (
+	cd bare.git &&
+	git repo info paths.toplevel >actual &&
+	echo "paths.toplevel=" >expected &&
+	test_cmp expected actual
+    )
+'
+
 test_done

base-commit: 256554692df0685b45e60778b08802b720880c50
-- 
gitgitgadget

[PATCH v3] repo: add paths.toplevel to repo info

[Jayesh Daga via GitGitGadget]

From: Jayesh Daga <jayeshdaga99@gmail.com>

repo info currently does not expose the repository's
working tree root, even though this information is
available via `repo_get_work_tree()` and
`git rev-parse --show-toplevel`.

Add a new field `paths.toplevel` to expose this value.

While doing so, document the correspondence between
`git rev-parse` options and `repo info` fields to make
it easier to identify missing or future additions.

For bare repositories, this value is empty, consistent
with other non-applicable fields.

Signed-off-by: Jayesh Daga jayeshdaga99@gmail.com
---
    repo: add paths.toplevel to repo info
    
    repo info currently does not expose the repository's working tree root,
    even though this information is available via repo_get_work_tree().
    
    This makes it harder for scripts to retrieve the repository root through
    a structured interface, often requiring the use of git rev-parse
    --show-toplevel.
    
    Add a new field paths.toplevel to git repo info that returns the working
    tree root. For bare repositories, this value is empty, consistent with
    other non-applicable fields.
    
    This provides a consistent and script-friendly way to query repository
    paths without invoking additional commands.
    
    Signed-off-by: Jayesh Daga jayeshdaga99@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2264%2Fjayesh0104%2Frepo-toplevel-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2264/jayesh0104/repo-toplevel-v3
Pull-Request: https://github.com/git/git/pull/2264

Range-diff vs v2:

 1:  05e34bfe2c ! 1:  8a58b6ce03 repo: add paths.toplevel to repo info
     @@ Commit message
          For bare repositories, this value is empty, consistent
          with other non-applicable fields.
      
     -    Signed-off-by: Jayesh Daga [jayeshdaga99@gmail.com](mailto:jayeshdaga99@gmail.com)
     +    Signed-off-by: Jayesh Daga jayeshdaga99@gmail.com
      
       ## builtin/repo.c ##
      @@ builtin/repo.c: static int get_layout_bare(struct repository *repo UNUSED, struct strbuf *buf)
     @@ builtin/repo.c: static int get_layout_bare(struct repository *repo UNUSED, struc
       
      +static int get_paths_toplevel(struct repository *repo, struct strbuf *buf)
      +{
     -+    const char *wt = repo_get_work_tree(repo);
     ++	const char *wt = repo_get_work_tree(repo);
      +
     -+    if (!wt)
     -+	return -1; /* match existing error style */
     ++	if (!wt)
     ++		return -1; /* match existing error style */
      +
     -+    strbuf_addstr(buf, wt);
     -+    return 0;
     ++	strbuf_addstr(buf, wt);
     ++	return 0;
      +}
      +
       static int get_layout_shallow(struct repository *repo, struct strbuf *buf)
       {
       	strbuf_addstr(buf,
     -@@ builtin/repo.c: static const struct repo_info_field repo_info_field[] = {
     +@@ builtin/repo.c: static int get_references_format(struct repository *repo, struct strbuf *buf)
     + 	return 0;
     + }
     + 
     ++/*
     ++ * rev-parse --<opt>        repo info <field>
     ++ *
     ++ * is-bare-repository      layout.bare
     ++ * is-shallow-repository   layout.shallow
     ++ * show-object-format      object.format
     ++ * show-ref-format         references.format
     ++ * show-toplevel           paths.toplevel
     ++ *
     ++ * show-cdup               <missing>
     ++ * show-prefix             <missing>
     ++ *
     ++ * Some <missing> entries may be candidates for future
     ++ * implementation, while others may not be needed.
     ++ */
     ++
     + /* repo_info_field keys must be in lexicographical order */
     + static const struct repo_info_field repo_info_field[] = {
       	{ "layout.bare", get_layout_bare },
       	{ "layout.shallow", get_layout_shallow },
       	{ "object.format", get_object_format },


 builtin/repo.c       | 28 ++++++++++++++++++++++++++++
 t/t1900-repo-info.sh | 16 ++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/builtin/repo.c b/builtin/repo.c
index 71a5c1c29c..8c9afc1150 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -62,6 +62,17 @@ static int get_layout_bare(struct repository *repo UNUSED, struct strbuf *buf)
 	return 0;
 }
 
+static int get_paths_toplevel(struct repository *repo, struct strbuf *buf)
+{
+	const char *wt = repo_get_work_tree(repo);
+
+	if (!wt)
+		return -1; /* match existing error style */
+
+	strbuf_addstr(buf, wt);
+	return 0;
+}
+
 static int get_layout_shallow(struct repository *repo, struct strbuf *buf)
 {
 	strbuf_addstr(buf,
@@ -82,11 +93,28 @@ static int get_references_format(struct repository *repo, struct strbuf *buf)
 	return 0;
 }
 
+/*
+ * rev-parse --<opt>        repo info <field>
+ *
+ * is-bare-repository      layout.bare
+ * is-shallow-repository   layout.shallow
+ * show-object-format      object.format
+ * show-ref-format         references.format
+ * show-toplevel           paths.toplevel
+ *
+ * show-cdup               <missing>
+ * show-prefix             <missing>
+ *
+ * Some <missing> entries may be candidates for future
+ * implementation, while others may not be needed.
+ */
+
 /* repo_info_field keys must be in lexicographical order */
 static const struct repo_info_field repo_info_field[] = {
 	{ "layout.bare", get_layout_bare },
 	{ "layout.shallow", get_layout_shallow },
 	{ "object.format", get_object_format },
+	{ "paths.toplevel", get_paths_toplevel },
 	{ "references.format", get_references_format },
 };
 
diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
index 39bb77dda0..470e06e8c2 100755
--- a/t/t1900-repo-info.sh
+++ b/t/t1900-repo-info.sh
@@ -155,4 +155,20 @@ test_expect_success 'git repo info -h shows only repo info usage' '
 	test_grep ! "git repo structure" actual
 '
 
+test_expect_success 'repo info paths.toplevel' '
+    git repo info paths.toplevel >actual &&
+    echo "paths.toplevel=$(git rev-parse --show-toplevel)" >expected &&
+    test_cmp expected actual
+'
+
+test_expect_success 'repo info paths.toplevel (bare repo)' '
+    git init --bare bare.git &&
+    (
+	cd bare.git &&
+	git repo info paths.toplevel >actual &&
+	echo "paths.toplevel=" >expected &&
+	test_cmp expected actual
+    )
+'
+
 test_done

base-commit: 256554692df0685b45e60778b08802b720880c50
-- 
gitgitgadget

Re: [PATCH v3] repo: add paths.toplevel to repo info

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1080 bytes --]

"Jayesh Daga via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Jayesh Daga <jayeshdaga99@gmail.com>
>
> repo info currently does not expose the repository's
> working tree root, even though this information is
> available via `repo_get_work_tree()` and
> `git rev-parse --show-toplevel`.
>
> Add a new field `paths.toplevel` to expose this value.
>
> While doing so, document the correspondence between
> `git rev-parse` options and `repo info` fields to make
> it easier to identify missing or future additions.
>
> For bare repositories, this value is empty, consistent
> with other non-applicable fields.
>

Don't we still have to decide how we want to support relative vs
absolute paths? [1]

Also seeing that you're a GSoC candidate and this is part of the project
that you've (and other potential contributors) applied for, our
recommendation is to not start working on a project before the selection
process.

[snip]

[1]: https://lore.kernel.org/git/CA+rGoLfbzXqP1Tw+94jMmWcSGPoefMv5E_fvwriad-O5CUeKHQ@mail.gmail.com/T/#m1e2b69b6b097c42b06088349947e43831cf5988f

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v3] repo: add paths.toplevel to repo info

[Junio C Hamano]

"Jayesh Daga via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Jayesh Daga <jayeshdaga99@gmail.com>
>
> repo info currently does not expose the repository's
> working tree root, even though this information is
> available via `repo_get_work_tree()` and
> `git rev-parse --show-toplevel`.
>
> Add a new field `paths.toplevel` to expose this value.
>
> While doing so, document the correspondence between
> `git rev-parse` options and `repo info` fields to make
> it easier to identify missing or future additions.
>
> For bare repositories, this value is empty, consistent
> with other non-applicable fields.
>
> Signed-off-by: Jayesh Daga jayeshdaga99@gmail.com
> ---

So "document the correspondence" added to the v2 iteration is still
up there.  I expect to see such a change in the patch body.

>     repo: add paths.toplevel to repo info
>     
>     repo info currently does not expose the repository's working tree root,
>     even though this information is available via repo_get_work_tree().
>     
>     This makes it harder for scripts to retrieve the repository root through
>     a structured interface, often requiring the use of git rev-parse
>     --show-toplevel.
>     
>     Add a new field paths.toplevel to git repo info that returns the working
>     tree root. For bare repositories, this value is empty, consistent with
>     other non-applicable fields.
>     
>     This provides a consistent and script-friendly way to query repository
>     paths without invoking additional commands.
>     
>     Signed-off-by: Jayesh Daga jayeshdaga99@gmail.com

This space below the three-dash is not where reiterate what you
already wrote in the commit log message.  Rather, this is a place to
give additional explanation that should not go in the commit log
message, like "the previous iteration botched X and Y which have
been corrected in this version", etc.

I see this mistake often in patches from GGG users---please double
check how to use that tool (I am not a GGG user, but I vaguely
recall that GGG duplicates what you wrote in the pull request
message after the three-dashes, so perhaps you are expected to
rewrite the pull request message every time to summarize the
differences since the last iteration before you submit a new
iteration, or something?).

> Range-diff vs v2:
>
>  1:  05e34bfe2c ! 1:  8a58b6ce03 repo: add paths.toplevel to repo info
>      @@ Commit message
>           For bare repositories, this value is empty, consistent
>           with other non-applicable fields.
>       
>      -    Signed-off-by: Jayesh Daga [jayeshdaga99@gmail.com](mailto:jayeshdaga99@gmail.com)
>      +    Signed-off-by: Jayesh Daga jayeshdaga99@gmail.com

OK.

>      -@@ builtin/repo.c: static const struct repo_info_field repo_info_field[] = {
>      +@@ builtin/repo.c: static int get_references_format(struct repository *repo, struct strbuf *buf)
>      + 	return 0;
>      + }
>      + 
>      ++/*
>      ++ * rev-parse --<opt>        repo info <field>
>      ++ *
>      ++ * is-bare-repository      layout.bare
>      ++ * is-shallow-repository   layout.shallow
>      ++ * show-object-format      object.format
>      ++ * show-ref-format         references.format
>      ++ * show-toplevel           paths.toplevel
>      ++ *
>      ++ * show-cdup               <missing>
>      ++ * show-prefix             <missing>
>      ++ *
>      ++ * Some <missing> entries may be candidates for future
>      ++ * implementation, while others may not be needed.

It is a surprisingly small set.  Did you do your own research, or
did you only copied what I gave as examples?

> +static int get_paths_toplevel(struct repository *repo, struct strbuf *buf)
> +{
> +	const char *wt = repo_get_work_tree(repo);
> +
> +	if (!wt)
> +		return -1; /* match existing error style */

That is not a very helpful comment.  It sounds more like "I do not
understand why I am supposed to return -1 here upon error, but I am
just blindly following what everybody else does" excuse, than "I
return -1 upon an error because the caller is prepared to act on it
and do X, which is what we want to see when repo_get_work_tree(repo)
returns NULL, which signals condition Y that should be reported as
such" explanation.  If the reason why returning -1 is so obvious
that we do not even have to say it (which I suspect is the case),
then future readers would appreciate if you didn't add such a
comment.  If the reason why this needs to return -1 (and not -2 or
-999 or whatever) is so subtle and needs to be explained, then the
comment does not give even a single bit of information to help them
understand why.

> +	strbuf_addstr(buf, wt);
> +	return 0;
> +}

> diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
> index 39bb77dda0..470e06e8c2 100755
> --- a/t/t1900-repo-info.sh
> +++ b/t/t1900-repo-info.sh
> @@ -155,4 +155,20 @@ test_expect_success 'git repo info -h shows only repo info usage' '
>  	test_grep ! "git repo structure" actual
>  '
>  
> +test_expect_success 'repo info paths.toplevel' '
> +    git repo info paths.toplevel >actual &&
> +    echo "paths.toplevel=$(git rev-parse --show-toplevel)" >expected &&

These seem to be SP*4 indented; one level of indent is supposed to
be a single tab, not 4-column.

You may have copied it from other tests, but the above is
done in an unusual order and I found it harder to read.

We prepare an expected output in a file called "expect" first, and
then store the output of the command we are testing in a file called
"actual", and then compare "test_cmp expect actual", in this order.

If you deliberately misspell the command to pass "--shaw-toplabel",
does it break this "echo" invocation, or will we only notice the
breakage/mistake when we check the output in the "expect" file
against 

> +    test_cmp expected actual
> +'
> +
> +test_expect_success 'repo info paths.toplevel (bare repo)' '
> +    git init --bare bare.git &&
> +    (
> +	cd bare.git &&
> +	git repo info paths.toplevel >actual &&
> +	echo "paths.toplevel=" >expected &&
> +	test_cmp expected actual
> +    )
> +'
> +
>  test_done
>
> base-commit: 256554692df0685b45e60778b08802b720880c50

Re: [PATCH v3] repo: add paths.toplevel to repo info

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> Don't we still have to decide how we want to support relative vs
> absolute paths? [1]

I suspect more than just a few requests to this command yield
path-valued response, so do we want an independent boolean "if we
are showing a path, show it in relative (yes) or absolute (no)"
option, or something?

> Also seeing that you're a GSoC candidate and this is part of the project
> that you've (and other potential contributors) applied for, our
> recommendation is to not start working on a project before the selection
> process.

Perhaps I should refrain from commenting on these patches to
discourage the authors if that is the case.  I am not raising an
objection, but do you have a pointer to the rationale behind the
recommendation?

Re: [PATCH v3] repo: add paths.toplevel to repo info

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1851 bytes --]

Junio C Hamano <gitster@pobox.com> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Don't we still have to decide how we want to support relative vs
>> absolute paths? [1]
>
> I suspect more than just a few requests to this command yield
> path-valued response, so do we want an independent boolean "if we
> are showing a path, show it in relative (yes) or absolute (no)"
> option, or something?
>

I think there was some ideas around this:

  - Have a default mode and a way to swap it. Perhaps a boolean config
    on the command level
  - Output both the relative and absolute paths. So we'd have
    paths.toplevel.absolute
    paths.toplevel.relative
    or something like that.

Either ways, I think we should finalize the decision first, that way we
don't have to have this discussion with each path-valued option.

>> Also seeing that you're a GSoC candidate and this is part of the project
>> that you've (and other potential contributors) applied for, our
>> recommendation is to not start working on a project before the selection
>> process.
>
> Perhaps I should refrain from commenting on these patches to
> discourage the authors if that is the case.  I am not raising an
> objection, but do you have a pointer to the rationale behind the
> recommendation?

Officially the GSoC rules [1] only say:

  Any work done on the Project prior to acceptance of the Project Proposal
  will not be considered for Evaluations.

So there is no rule stopping a contributor from making such changes, and
ultimately, open source projects benefit from contributions regardless
of the contributor's application status.

However it gets confusing for other contributors who have proposals for
the same topic, since they expect a certain status quo and a moving goal
post makes it a lot harder.

[1]: https://summerofcode.withgoogle.com/rules

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]