Re: [PATCH v2 2/2] Handle more file writes correctly in shared repos

Johannes Schindelin [off-list ref] writes:

On Thu, 7 Jan 2016, Junio C Hamano wrote:

quoted

Johannes Schindelin [off-list ref] writes:

quoted

- git apply, when writing rejected hunks

Today I may try to apply and leave hello.c.rej; tomorrow you may try to
apply a different patch and get rejection for the same file.  And you
would not be able to if my umask is 077.

As I just wrote in my reply to Peff: my experience with .rej files is that
I want to inspect them, and then delete them once I addressed them. I do
not want anybody to interfere with that, as the presence of .rej files
serves also as a TODO list.

That argues for protecting .rej files against overwriting by myself,
too, which means (1) we do not want to loosen it by using
fopen_for_writing(), and (2) relying on permission bits and
ownership is not sufficient, i.e. just using regular fopen(3) is
wrong.

I think it is correct not to touch this codepath in this patch,
because of the above two reasons, but more simply and generally, it
is correct not to touch this codepath because core.sharedRepository
is not about working tree files, and .rej is a file you use in your
working tree.

As the log messages are often used to guide future developers, I
think the log message of this commit should mention that criterion.
It would cover multiple codepaths you listed in your proposed log
message in a more generic way, helping other people to reason about
when they see other instances of fopen(..., "w") and wonder if it
should become fopen_for_writing().

Thanks.

Hi Junio,

On Fri, 8 Jan 2016, Junio C Hamano wrote:

I think it is correct not to touch this codepath in this patch,
because of the above two reasons, but more simply and generally, it
is correct not to touch this codepath because core.sharedRepository
is not about working tree files, and .rej is a file you use in your
working tree.

I am happy to adjust the log message, but I am pretty certain that the
`core.sharedRepository` setting actually also affects the working tree. At
least in my hands, calling

	git clone -c core.sharedRepository=group . test-shared

results in all of the working tree files being group-writable.

Therefore I am not convinced this is a sound line of reasoning.

Ciao,
Dscho