[PATCH/RFR&A] Do not rename read-only files during a push

[PATCH/RFR&A] Do not rename read-only files during a push

[Junio C Hamano]

This is on the "merged to 'master' soon" list, but has a small amend by me
(namely, chmod of final pack is done only when we are writing the final
pack, i.e. reading from stdin) to fix breakages observed in tests.  It
would be nice to get a final Ack before moving it to 'master'.

-- >8 --

From: Petr Baudis <pasky@suse.cz>

Win32 does not allow renaming read-only files (at least on a Samba
share), making push into a local directory to fail. Thus, defer
the chmod() call in index-pack.c:final() only after
move_temp_to_file() was called.

Signed-off-by: Petr Baudis <pasky@suse.cz>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 index-pack.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/index-pack.c b/index-pack.c
index d3a4d31..aec11cb 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -790,7 +790,6 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
 		err = close(output_fd);
 		if (err)
 			die("error while closing pack file: %s", strerror(errno));
-		chmod(curr_pack_name, 0444);
 	}
 
 	if (keep_msg) {
@@ -824,8 +823,9 @@ 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");
 	}
+	if (from_stdin)
+		chmod(final_pack_name, 0444);
 
-	chmod(curr_index_name, 0444);
 	if (final_index_name != curr_index_name) {
 		if (!final_index_name) {
 			snprintf(name, sizeof(name), "%s/pack/pack-%s.idx",
@@ -835,6 +835,7 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
 		if (move_temp_to_file(curr_index_name, final_index_name))
 			die("cannot store index file");
 	}
+	chmod(final_index_name, 0444);
 
 	if (!from_stdin) {
 		printf("%s\n", sha1_to_hex(sha1));
-- 
1.6.0.2.767.g8f0e

Re: [PATCH/RFR&A] Do not rename read-only files during a push

[Shawn O. Pearce]

Junio C Hamano <gitster@pobox.com> wrote:
> This is on the "merged to 'master' soon" list, but has a small amend by me
> (namely, chmod of final pack is done only when we are writing the final
> pack, i.e. reading from stdin) to fix breakages observed in tests.  It
> would be nice to get a final Ack before moving it to 'master'.

Looks fine to me.

 
> From: Petr Baudis <pasky@suse.cz>
> 
> Win32 does not allow renaming read-only files (at least on a Samba
> share), making push into a local directory to fail. Thus, defer
> the chmod() call in index-pack.c:final() only after
> move_temp_to_file() was called.
> 
> Signed-off-by: Petr Baudis <pasky@suse.cz>
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> ---
>  index-pack.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/index-pack.c b/index-pack.c
> index d3a4d31..aec11cb 100644
> --- a/index-pack.c
> +++ b/index-pack.c
> @@ -790,7 +790,6 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
>  		err = close(output_fd);
>  		if (err)
>  			die("error while closing pack file: %s", strerror(errno));
> -		chmod(curr_pack_name, 0444);
>  	}
>  
>  	if (keep_msg) {
> @@ -824,8 +823,9 @@ 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");
>  	}
> +	if (from_stdin)
> +		chmod(final_pack_name, 0444);
>  
> -	chmod(curr_index_name, 0444);
>  	if (final_index_name != curr_index_name) {
>  		if (!final_index_name) {
>  			snprintf(name, sizeof(name), "%s/pack/pack-%s.idx",
> @@ -835,6 +835,7 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
>  		if (move_temp_to_file(curr_index_name, final_index_name))
>  			die("cannot store index file");
>  	}
> +	chmod(final_index_name, 0444);
>  
>  	if (!from_stdin) {
>  		printf("%s\n", sha1_to_hex(sha1));
> -- 
> 1.6.0.2.767.g8f0e
> 

-- 
Shawn.

Re: [PATCH/RFR&A] Do not rename read-only files during a push

[Johannes Sixt]

On Sonntag, 19. Oktober 2008, Junio C Hamano wrote:
> From: Petr Baudis <pasky@suse.cz>
>
> Win32 does not allow renaming read-only files (at least on a Samba
> share), making push into a local directory to fail. Thus, defer
> the chmod() call in index-pack.c:final() only after
> move_temp_to_file() was called.

Concerning the Win32 aspect of this patch:

Acked-by: Johannes Sixt <johannes.sixt@telecom.at>

-- Hannes