threads / patch / 34154

patch, 2 partsshow-ref.c: Add missing call to git_config()

Subject: [PATCH 1/2] show-ref.c: Add missing call to git_config()

## tl;dr

5 messages between Jun 15, 2013 and Jul 4, 2013. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Ramsay Jones· Jun 15, 2013, 20:29 UTC · lore

At present, 'git show-ref' ignores any attempt to set config variables (e.g. core.checkstat) from the command line using the -c option to git. In order to enable such usage, add the missing call to git_config() from cmd_show_ref().

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 builtin/show-ref.c | 2 ++
 1 file changed, 2 insertions(+)

Show changes to builtin/show-ref.c +2 −0

diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 8d9b76a..95f5ca9 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -191,6 +191,8 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
 	if (argc == 2 && !strcmp(argv[1], "-h"))
 		usage_with_options(show_ref_usage, show_ref_options);
 
+	git_config(git_default_config, NULL);
+
 	argc = parse_options(argc, argv, prefix, show_ref_options,
 			     show_ref_usage, PARSE_OPT_NO_INTERNAL_HELP);

-- 
1.8.3

Junio C Hamano· Jun 17, 2013, 03:05 UTC · re: Ramsay Jones · lore

Re: [PATCH 1/2] show-ref.c: Add missing call to git_config()

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

> At present, 'git show-ref' ignores any attempt to set config
> variables (e.g. core.checkstat) from the command line using
> the -c option to git.

I think what you really want to see is not giving "-c" and have it honored.

	"git show-ref" does not honor configuration variables at
	all, but at least core configuration variables read by
	git_default_config (most importantly core.checkstat) should
	be read and honored, because ...

would be more appropriate. What are the variables that matter to show-ref, and what are the reasons why they should be honored? I thought show-ref was just a way to enumerate refs, and does not read the index nor checks if there are modifications in the working tree, so I do not see any reason offhand for it to honor core.checkstat (and I think that is the reason why we don't have the call there in the current code).

Exactly the same comment applies to 2/2.

Note that I am _not_ opposing these changes. You brought them up because you saw some reason why these should honor some core variables. I just want that reason to be explained in the log for the future developers.

Ramsay Jones· Jun 18, 2013, 18:38 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/2] show-ref.c: Add missing call to git_config()

Junio C Hamano wrote:

Show 21 quoted lines

> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> 
>> At present, 'git show-ref' ignores any attempt to set config
>> variables (e.g. core.checkstat) from the command line using
>> the -c option to git.
> 
> I think what you really want to see is not giving "-c" and have it
> honored.
> 
> 	"git show-ref" does not honor configuration variables at
> 	all, but at least core configuration variables read by
> 	git_default_config (most importantly core.checkstat) should
> 	be read and honored, because ...
> 
> would be more appropriate.  What are the variables that matter to
> show-ref, and what are the reasons why they should be honored?  I
> thought show-ref was just a way to enumerate refs, and does not read
> the index nor checks if there are modifications in the working tree,
> so I do not see any reason offhand for it to honor core.checkstat
> (and I think that is the reason why we don't have the call there in
> the current code).

:-D Yes, you caught me!

These patches *may* not be necessary, prior to Michael's "reference related races" series. Specifically, the introduction of the stat_validity_check() function to the reference handling API.

This means that the behaviour 'git show-ref' is now affected by several config variables, including core.checkstat. I haven't spent any time auditing the code, but the list would include (at least) core.trustctime, core.filemode, core.checkstat, core.ignorecygwinfstricks, ...

> 
> Exactly the same comment applies to 2/2.

ditto

> Note that I am _not_ opposing these changes.  You brought them up
> because you saw some reason why these should honor some core
> variables.  I just want that reason to be explained in the log for
> the future developers.

Yes, I will send a v2 (soon-ish, I hope).

ATB, Ramsay Jones

Junio C Hamano· Jul 1, 2013, 21:56 UTC · re: Ramsay Jones · lore

Re: [PATCH 1/2] show-ref.c: Add missing call to git_config()

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

> Yes, I will send a v2 (soon-ish, I hope).

Ping?

No need to hurry, but just to make sure this didn't disappear from everybody's radar.

Thanks.

Ramsay Jones· Jul 4, 2013, 18:25 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/2] show-ref.c: Add missing call to git_config()

Junio C Hamano wrote:

Show 8 quoted lines

> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> 
>> Yes, I will send a v2 (soon-ish, I hope).
> 
> Ping?
> 
> No need to hurry, but just to make sure this didn't disappear from
> everybody's radar.

Yep, this is still on my TODO list.

Sorry for being tardy on these patches. :(

ATB, Ramsay Jones