threads / patch / 15961

patchDo not rename read-only files during a push

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

## tl;dr

3 messages between Oct 19, 2008 and Oct 19, 2008. Diffs are folded; open one to read it.

replies: 2people: 3as markdown or json

Junio C Hamano· Oct 19, 2008, 07:07 UTC · lore

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(-)
Show changes to index-pack.c +3 −2
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 O. Pearce· Oct 19, 2008, 17:02 UTC · re: Junio C Hamano · lore

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

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.
 
Show 47 quoted lines
> 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.
Johannes Sixt· Oct 19, 2008, 17:41 UTC · re: Junio C Hamano · lore

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

On Sonntag, 19. Oktober 2008, Junio C Hamano wrote:
Show 6 quoted lines
> 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

← back to recent threads