Re: [PATCH v2 1/1] t1450: use mv -f when overwriting a read only file

[Tuomas Ahola <taahol@utu.fi>]

tboegi@web.de wrote:

> From: Torsten Bögershausen <tboegi@web.de>
> 
> test case 38 in t/t1450-fsck.sh replaces a loose blob, "$tmp" below,
> with a trucated version "obj.tmp" using
> 

s/trucated/truncated/

> mv obj.tmp "$obj"
> 
> This fails on some sytems, like older MacOS.

s/sytems/systems

FWIW, it seems that the glitch was fixed in macOS 12.3 (file_cmds-353.100.22).

> `man mv`
> includes this information:
>   The following options are available:
>   -f      Do not prompt for confirmation before overwriting the destination
>            path.  (The -f option overrides any previous -i or -n options.)
> and further down:
>   If the destination path does not have a mode which permits writing, mv
>   prompts the user for confirmation as specified for the -i option.
> 
> Running the test case in debug and verbose mode `./t1450-fsck.sh -d -v`
> shows this lines (which I have shortened to fit into 72 characters)
> 
> override r--r--r--  user/staff for .git/objects/fc/a75a5be3823dc0560da5?
> (y/n [n]) not overwritten
> fca75a5be3823dc0560da57ade678f212aeb33f6
> 
> POSIX mentions that the observed
> "prompt to standard error and read a line from standard input"
> should not be done if "standard input is a terminal".

This replaced "when" with "if" but the meaning didn't change.  The observed
behavior shouldn't happen if the stdin is _not_ a terminal.

> So we may blame the "standard input is a terminal" logic to fail.

Well, macOS versions older than 12.3 just don't have that logic in mv(1).
Can nonexisting logic fail?  (If somebody is interested, the actual change
in 12.3 was

@@ -265,12 +290,12 @@ do_move(const char *from, const char *to)
 		} else if (iflg) {
 			(void)fprintf(stderr, "overwrite %s? %s", to, YESNO);
 			ask = 1;
-		} else if (access(to, W_OK) && !stat(to, &sb)) {
+		} else if (access(to, W_OK) && !stat(to, &sb) && isatty(STDIN_FILENO)) {
 			strmode(sb.st_mode, modep);
 			(void)fprintf(stderr, "override %s%s%s/%s for %s? %s",
 			    modep + 1, modep[9] == ' ' ? "" : " ",

rather simple indeed.)

> 
> Side note: Answering with 'y' to the question of `mv` like this
> (echo y | mv obj.tmp "$obj")
> makes the test case pass.
> 
> To make things not too complicated use the most portable form
> mv -f obj.tmp "$obj"
> 
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
> 
> Thanks to all reviewers.
> Changes since V1:
>   "should not be done when" -> "should not be done if"
>   Fix "mv -v": it must be "mv -f"
> 
> t/t1450-fsck.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
> index 16cc5d5dcb..335dcbf0af 100755
> --- a/t/t1450-fsck.sh
> +++ b/t/t1450-fsck.sh
> @@ -548,7 +548,7 @@ test_expect_success 'rev-list --verify-objects with truncated loose blob' '
>  		# Truncate the loose blob such that its header can still be
>  		# parsed, but reading the object data fails mid-stream.
>  		test_copy_bytes 64 <"$obj" >obj.tmp &&
> -		mv obj.tmp "$obj" &&
> +		mv -f obj.tmp "$obj" &&
>  
>  		test_must_fail git rev-list --verify-objects "$blob" 2>err &&
>  		test_grep "hash mismatch" err
> -- 
> 2.50.0.rc0.46.g7014b55638
> 

Thanks.