Re: [RFC PATCH 2/7] t0301: remove trailing / for dir creation

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 26, 2018, 06:40 UTC
Message-ID: <xmqqpnusxrxx.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <fdf6f2ec6296cf48f5e5ac5edc5fe93adca77f32.1543143503.git.carenas@gmail.com>

Carlo Marcelo Arenas Belón  <carenas@gmail.com> writes:

> the semantics of how mkdir -p should work, specially when using -m are
> not standard and in this case NetBSD will assume that the permision
> should not be changed, breaking the test

This does not explain, except for the fuzzy "in this case", why we want to lose trailing slash at all. If what you wanted to say was

    On NetBSD, "mkdir -p -m $bits path/to/dir/" ignores the
    permission bits when creating the directory component 'dir', but
    without the trailing slash at the end of "dir/", it works as
    expected.

then that would be an understandable justification for the patch. If you meant to say something else, then I couldn't read it from what you wrote, so the log message needs updating to help future readers.

> -p is technically not needed either, but will be cleared in a future
> patch eventhough it could be considered an alternative fix

I haven't seen the steps 3-7/7 yet, but if they remove "-p", then "but" would be a strange thing to say here. If they indeed do, then perhaps:

    The '-p' option is not needed in thse cases, as we know $HOME/
    exists in the test environment and we are creating a new
    directory directly under it.  It will be removed in a future
    patch in the series.  Removing '-p' could be an alternative fix,
    as the command then works as expected even on NetBSD with the
    trailing slash after directory name.

On the other hand, if future changes make it necessary to create two or more levels here, then

    At this point in the series, '-p' is not needed in thse cases,
    as we know $HOME/ exists in the test environment and we are
    creating .git-credential-cache directory directly under it.
    However, in later patches, we'll make it necessary for these
    'mkdir -p' to create two or more levels, so removing '-p' would
    be an alternative fix for this step, but would not work for the
    series as a whole.

would also make sense. I simply do not have enough information yet to tell which at this point in the series.

Show 34 quoted lines

> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>  t/t0301-credential-cache.sh | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/t/t0301-credential-cache.sh b/t/t0301-credential-cache.sh
> index fd92533acf..9529c612af 100755
> --- a/t/t0301-credential-cache.sh
> +++ b/t/t0301-credential-cache.sh
> @@ -77,9 +77,9 @@ test_expect_success "use custom XDG_CACHE_HOME even if xdg socket exists" '
>  test_expect_success 'use user socket if user directory exists' '
>  	test_when_finished "
>  		git credential-cache exit &&
> -		rmdir \"\$HOME/.git-credential-cache/\"
> +		rmdir \"\$HOME/.git-credential-cache\"
>  	" &&
> -	mkdir -p -m 700 "$HOME/.git-credential-cache/" &&
> +	mkdir -p -m 700 "$HOME/.git-credential-cache" &&
>  	check approve cache <<-\EOF &&
>  	protocol=https
>  	host=example.com
> @@ -92,10 +92,10 @@ test_expect_success 'use user socket if user directory exists' '
>  test_expect_success SYMLINKS 'use user socket if user directory is a symlink to a directory' '
>  	test_when_finished "
>  		git credential-cache exit &&
> -		rmdir \"\$HOME/dir/\" &&
> +		rmdir \"\$HOME/dir\" &&
>  		rm \"\$HOME/.git-credential-cache\"
>  	" &&
> -	mkdir -p -m 700 "$HOME/dir/" &&
> +	mkdir -p -m 700 "$HOME/dir" &&
>  	ln -s "$HOME/dir" "$HOME/.git-credential-cache" &&
>  	check approve cache <<-\EOF &&
>  	protocol=https