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

4 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:57:46

Ramsay Jones [off-list ref] 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.

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

From: Ramsay Jones <hidden>
Date: 2016-06-15 22:57:49

Junio C Hamano wrote:
Ramsay Jones [off-list ref] writes:
quoted
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

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:57:58

Ramsay Jones [off-list ref] 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.

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

From: Ramsay Jones <hidden>
Date: 2016-06-15 22:58:00

Junio C Hamano wrote:
Ramsay Jones [off-list ref] writes:
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help