Re: What's cooking in git/spearce.git (Oct 2008, #02; Sun, 12)

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 13, 2008, 03:49 UTC
Message-ID: <7vprm5duhq.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <7v3aj1fdw2.fsf@gitster.siamese.dyndns.org>

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

Show 12 quoted lines

> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
>> * pb/rename-rowin32 (Fri Oct 3 12:20:43 2008 +0200) 1 commit
>>  - Do not rename read-only files during a push
>>
>> Supposedly fixes pack file renames on Windows, but it makes the
>> test suite fail on Linux.  I haven't debugged why yet.
>
> I am kind of surprised that it actually passes the test on Windows, which
> implies that these cats shown in the patch to fix this breakage below do
> not honor ro-ness of the file, which in turn makes me doubt if making the
> resulting packfiles read-only has any effect on that platform whatsoever..

Actually, the patch I posted was wrong and shouldn't be applied, as the original breakage is a sign that the externally observable behaviour of the command has changed, and the patch was just hiding it under the rug.

The thing is that 8c76006 (Do not rename read-only files during a push, 2008-10-03) makes unnecessary chmod() when there is no rename is involved, namely when it is used to index an existing packfile on the filesystem.

I think the attached patch is a better fix, to be squashed in.

Note: I now have the pumpkin, so you do not have to apply and push the
results out --- this is just me showing a proposed change as usual.

 index-pack.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git c/index-pack.c w/index-pack.c
index 2c69f08..aec11cb 100644
--- c/index-pack.c
+++ w/index-pack.c
@@ -823,7 +823,8 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
 		if (move_temp_to_file(curr_pack_name, final_pack_name))
 			die("cannot store pack file");
 	}
-	chmod(final_pack_name, 0444);
+	if (from_stdin)
+		chmod(final_pack_name, 0444);
 
 	if (final_index_name != curr_index_name) {
 		if (!final_index_name) {