[PATCH] init: show absolute path of (re)initialized repository

[PATCH] init: show absolute path of (re)initialized repository

[Matt Hunter]

Since 20e38cba3926 (setup: split up concerns of `init_db()`,
2026-09-10), git-init produces the message:

    Initialized empty Git repository in .git/

instead of the previous

    Initialized empty Git repository in /full/path/to/repo/.git/

Prior to 20e38cba3926, the code producing this message was in function
init_db (renamed at the time to create_repository).  In that context,
the git_dir variable was updated by 'git_dir = repo_get_git_dir(repo);'
earlier in the function, to set the value used in printf.

None of the discussion [1] around the original patch indicates that this
change was intentional, so address the regression by similarly using
repo_get_git_dir in this code's new location.

In this case, git_dir is referencing owned memory that is later freed,
so declare a new variable to hold the used string.

1: https://lore.kernel.org/git/20260825-pks-odb-write-alternates-at-creation-time-v1-0-911513ba95c3@pks.im/

Signed-off-by: Matt Hunter <m@lfurio.us>
---
 builtin/init-db.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index 374ed3a74706..a2628568ff6a 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -248,18 +248,19 @@ int cmd_init_db(int argc,
 	create_object_database(the_repository, NULL);
 
 	if (!quiet) {
-		int len = strlen(git_dir);
+		const char *full_git_dir = repo_get_git_dir(the_repository);
+		int len = strlen(full_git_dir);
 
 		if (reinit)
 			printf(repo_settings_get_shared_repository(the_repository)
 			       ? _("Reinitialized existing shared Git repository in %s%s\n")
 			       : _("Reinitialized existing Git repository in %s%s\n"),
-			       git_dir, len && git_dir[len-1] != '/' ? "/" : "");
+			       full_git_dir, len && full_git_dir[len-1] != '/' ? "/" : "");
 		else
 			printf(repo_settings_get_shared_repository(the_repository)
 			       ? _("Initialized empty shared Git repository in %s%s\n")
 			       : _("Initialized empty Git repository in %s%s\n"),
-			       git_dir, len && git_dir[len-1] != '/' ? "/" : "");
+			       full_git_dir, len && full_git_dir[len-1] != '/' ? "/" : "");
 	}
 
 	free(template_dir_to_free);

base-commit: c51408889c3b372814bc20b9431c3b74d0d268eb
-- 
2.56.0.257.gf7497c7d27

Re: [PATCH] init: show absolute path of (re)initialized repository

[Patrick Steinhardt]

On Tue, Oct 20, 2026 at 01:38:47AM -0400, Matt Hunter wrote:
> Since 20e38cba3926 (setup: split up concerns of `init_db()`,
> 2026-09-10), git-init produces the message:
> 
>     Initialized empty Git repository in .git/
> 
> instead of the previous
> 
>     Initialized empty Git repository in /full/path/to/repo/.git/
> 
> Prior to 20e38cba3926, the code producing this message was in function
> init_db (renamed at the time to create_repository).  In that context,
> the git_dir variable was updated by 'git_dir = repo_get_git_dir(repo);'
> earlier in the function, to set the value used in printf.
> 
> None of the discussion [1] around the original patch indicates that this
> change was intentional, so address the regression by similarly using
> repo_get_git_dir in this code's new location.

This indeed wasn't an intentional change. I also confirmed that with
v2.55 we print the full path.

> In this case, git_dir is referencing owned memory that is later freed,
> so declare a new variable to hold the used string.

This sentence doesn't add any value as it simply explains parts that are
obvious when reading the patch. I'd just drop it.

> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index 374ed3a74706..a2628568ff6a 100644
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c
> @@ -248,18 +248,19 @@ int cmd_init_db(int argc,
>  	create_object_database(the_repository, NULL);
>  
>  	if (!quiet) {
> -		int len = strlen(git_dir);
> +		const char *full_git_dir = repo_get_git_dir(the_repository);

Previously, in `init_db()` we always assigned `git_dir =
repo_get_git_dir(repo)`, so it's clear that this change here does the
equivalent.

> +		int len = strlen(full_git_dir);

I was wondering whether we want to fix the variable type to be a
`size_t` while at it. But maybe we should just keep it as-is.

>  		if (reinit)
>  			printf(repo_settings_get_shared_repository(the_repository)
>  			       ? _("Reinitialized existing shared Git repository in %s%s\n")
>  			       : _("Reinitialized existing Git repository in %s%s\n"),
> -			       git_dir, len && git_dir[len-1] != '/' ? "/" : "");
> +			       full_git_dir, len && full_git_dir[len-1] != '/' ? "/" : "");
>  		else
>  			printf(repo_settings_get_shared_repository(the_repository)
>  			       ? _("Initialized empty shared Git repository in %s%s\n")
>  			       : _("Initialized empty Git repository in %s%s\n"),
> -			       git_dir, len && git_dir[len-1] != '/' ? "/" : "");
> +			       full_git_dir, len && full_git_dir[len-1] != '/' ? "/" : "");
>  	}

The changes look obviously good to me.

I'm a bit on the fence about whether we want to have a regression test
or not. It feels like a very specific thing in the user-facing output,
so it's a bit dubious whether it's really worth it. But on the other
hand, folks like you seem to care enough to make the test worthwhile, so
maybe we should have one?

Thanks!

Patrick

Re: [PATCH] init: show absolute path of (re)initialized repository

[Matt Hunter]

On Tue Oct 20, 2026 at 2:55 AM EDT, Patrick Steinhardt wrote:
> On Tue, Oct 20, 2026 at 01:38:47AM -0400, Matt Hunter wrote:
>> 
>> None of the discussion [1] around the original patch indicates that this
>> change was intentional, so address the regression by similarly using
>> repo_get_git_dir in this code's new location.
>
> This indeed wasn't an intentional change. I also confirmed that with
> v2.55 we print the full path.

And with 2.56 :-)  Output changed right at the beginning of this release
cycle.

>
>> In this case, git_dir is referencing owned memory that is later freed,
>> so declare a new variable to hold the used string.
>
> This sentence doesn't add any value as it simply explains parts that are
> obvious when reading the patch. I'd just drop it.

Yeah.. Will do

>
>> +		int len = strlen(full_git_dir);
>
> I was wondering whether we want to fix the variable type to be a
> `size_t` while at it. But maybe we should just keep it as-is.

It's not of consequence to this change, but I agree it would be more
correct.  I'll include it.

>
> I'm a bit on the fence about whether we want to have a regression test
> or not. It feels like a very specific thing in the user-facing output,
> so it's a bit dubious whether it's really worth it.

This is my thought exactly.

> But on the other
> hand, folks like you seem to care enough to make the test worthwhile, so
> maybe we should have one?

I could be convinced to add one.  I'll see if additional feedback comes
in before sending a new revision, but won't plan for a test as of now.

Thanks for the review

>
> Thanks!
>
> Patrick

Re: [PATCH] init: show absolute path of (re)initialized repository

[Patrick Steinhardt]

On Tue, Oct 20, 2026 at 05:13:07AM -0400, Matt Hunter wrote:
> On Tue Oct 20, 2026 at 2:55 AM EDT, Patrick Steinhardt wrote:
> > On Tue, Oct 20, 2026 at 01:38:47AM -0400, Matt Hunter wrote:
> >> 
> >> None of the discussion [1] around the original patch indicates that this
> >> change was intentional, so address the regression by similarly using
> >> repo_get_git_dir in this code's new location.
> >
> > This indeed wasn't an intentional change. I also confirmed that with
> > v2.55 we print the full path.
> 
> And with 2.56 :-)  Output changed right at the beginning of this release
> cycle.

Oh, that's something I missed. Even better then to fix this before it's
part of any public release :)

Patrick

Re: [PATCH] init: show absolute path of (re)initialized repository

[Harald Nordgren]

> I'm a bit on the fence about whether we want to have a regression test
> or not. It feels like a very specific thing in the user-facing output,
> so it's a bit dubious whether it's really worth it. But on the other
> hand, folks like you seem to care enough to make the test worthwhile, so
> maybe we should have one?

It seems self-evident that we need a regression test here. Because
it's specifically the thing that would have prevented it, and the only
thing that will prevent a new regression later on :)


Harald

Re: [PATCH] init: show absolute path of (re)initialized repository

[Patrick Steinhardt]

On Tue, Oct 20, 2026 at 02:37:05PM +0200, Harald Nordgren wrote:
> > I'm a bit on the fence about whether we want to have a regression test
> > or not. It feels like a very specific thing in the user-facing output,
> > so it's a bit dubious whether it's really worth it. But on the other
> > hand, folks like you seem to care enough to make the test worthwhile, so
> > maybe we should have one?
> 
> It seems self-evident that we need a regression test here. Because
> it's specifically the thing that would have prevented it, and the only
> thing that will prevent a new regression later on :)

I wouldn't necessarily say self-evident as the result isn't wrong. It's
still correct, even though it has changed compared to the previous
status quo.

Patrick

Re: [PATCH] init: show absolute path of (re)initialized repository

[Harald Nordgren]

On Tue, Oct 20, 2026 at 4:37 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Tue, Oct 20, 2026 at 02:37:05PM +0200, Harald Nordgren wrote:
> > > I'm a bit on the fence about whether we want to have a regression test
> > > or not. It feels like a very specific thing in the user-facing output,
> > > so it's a bit dubious whether it's really worth it. But on the other
> > > hand, folks like you seem to care enough to make the test worthwhile, so
> > > maybe we should have one?
> >
> > It seems self-evident that we need a regression test here. Because
> > it's specifically the thing that would have prevented it, and the only
> > thing that will prevent a new regression later on :)
>
> I wouldn't necessarily say self-evident as the result isn't wrong. It's
> still correct, even though it has changed compared to the previous
> status quo.

I guess it's semantics. The regression test locks down the behavior,
so it won't be accidentally changed later on.


Harald

Re: [PATCH] init: show absolute path of (re)initialized repository

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> On Tue, Oct 20, 2026 at 02:37:05PM +0200, Harald Nordgren wrote:
>> > I'm a bit on the fence about whether we want to have a regression test
>> > or not. It feels like a very specific thing in the user-facing output,
>> > so it's a bit dubious whether it's really worth it. But on the other
>> > hand, folks like you seem to care enough to make the test worthwhile, so
>> > maybe we should have one?
>> 
>> It seems self-evident that we need a regression test here. Because
>> it's specifically the thing that would have prevented it, and the only
>> thing that will prevent a new regression later on :)
>
> I wouldn't necessarily say self-evident as the result isn't wrong. It's
> still correct, even though it has changed compared to the previous
> status quo.

It is correct in the sense that it is not wrong, but we cannot deny
that it is much less useful when trying to determine where the new
repository was created while reading a transcript that contains an
invocation of the 'git init' command.  One would need to go back
and see where the user in the transcript 'cd'ed to before issuing
the command.

This episode makes me wonder what other details, not captured in our
existing tests, users find useful and have grown accustomed to.

Thanks.