threads / patch / 37754

patcht1304: Set LOGNAME even if USER is unset or null

Subject: [PATCH] t1304: Set LOGNAME even if USER is unset or null

## tl;dr

4 messages between Oct 17, 2014 and Oct 20, 2014. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

W. Trevor King· Oct 17, 2014, 21:39 UTC · lore
Avoid:
  # ./t1304-default-acl.sh
  ok 1 - checking for a working acl setup
  ok 2 - Setup test repo
  not ok 3 - Objects creation does not break ACLs with restrictive umask
  #
  #               # SHA1 for empty blob
  #               check_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391
  #
  not ok 4 - git gc does not break ACLs with restrictive umask
  #
  #               git gc &&
  #               check_perms_and_acl .git/objects/pack/*.pack
  #
  # failed 2 among 4 test(s)
  1..4

on systems where USER isn't set. It's usually set by the login process, but it isn't set when launching some Docker images. For example:

  $ docker run --rm debian env
  HOME=/
  PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
  HOSTNAME=b2dfdfe797ed

'id -u -n' has been in POSIX from Issue 2 through 2013 [1], so I don't expect compatibility issues.

[1]: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/id.html
Signed-off-by: W. Trevor King <wking@tremily.us>
---
The patch is based on the current maint branch.
Previous LOGNAME discussion:
* Michael Gruber on 2011-05-06 suggesting a discussing a whoami
  fallback [1] (but whoami isn't POSIX).
* René Scharfe on 2011-10-14 suggesting USER as a fallback for
  LOGNAME [2].
* Matthieu Moy on 2012-09-17 suggesting dropping $LOGNAME in
  favor of numerical user IDs 'id -u' for a system with multiple
  usernames sharing the same user ID [3].
Obviously, you can work around the problem with:
  # USER=$(id -u -n) ./t1304-default-acl.sh

so the question is really "Are empty-USER systems worth supporting out of the box?".

Cheers, Trevor

[1]: http://thread.gmane.org/gmane.comp.version-control.git/172883/focus=172961 [2]: http://thread.gmane.org/gmane.comp.version-control.git/183586 [3]: http://thread.gmane.org/gmane.comp.version-control.git/205690/focus=205703

 t/t1304-default-acl.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to t/t1304-default-acl.sh +1 −1
diff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh
index 79045ab..f5422f1 100755
--- a/t/t1304-default-acl.sh
+++ b/t/t1304-default-acl.sh
@@ -26,7 +26,7 @@ test_expect_success 'checking for a working acl setup' '
 
 if test -z "$LOGNAME"
 then
-	LOGNAME=$USER
+	LOGNAME="${USER:-$(id -u -n)}"
 fi
 
 check_perms_and_acl () {
-- 
2.1.0.60.g85f0837
Junio C Hamano· Oct 19, 2014, 22:49 UTC · re: W. Trevor King · lore

Re: [PATCH] t1304: Set LOGNAME even if USER is unset or null

"W. Trevor King" <wking@tremily.us> writes:
Show 37 quoted lines
> Previous LOGNAME discussion:
>
> * Michael Gruber on 2011-05-06 suggesting a discussing a whoami
>   fallback [1] (but whoami isn't POSIX).
> * René Scharfe on 2011-10-14 suggesting USER as a fallback for
>   LOGNAME [2].
> * Matthieu Moy on 2012-09-17 suggesting dropping $LOGNAME in
>   favor of numerical user IDs 'id -u' for a system with multiple
>   usernames sharing the same user ID [3].
>
> Obviously, you can work around the problem with:
>
>   # USER=$(id -u -n) ./t1304-default-acl.sh
>
> so the question is really "Are empty-USER systems worth supporting out
> of the box?".
>
> Cheers,
> Trevor
>
> [1]: http://thread.gmane.org/gmane.comp.version-control.git/172883/focus=172961
> [2]: http://thread.gmane.org/gmane.comp.version-control.git/183586
> [3]: http://thread.gmane.org/gmane.comp.version-control.git/205690/focus=205703
>
>  t/t1304-default-acl.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh
> index 79045ab..f5422f1 100755
> --- a/t/t1304-default-acl.sh
> +++ b/t/t1304-default-acl.sh
> @@ -26,7 +26,7 @@ test_expect_success 'checking for a working acl setup' '
>  
>  if test -z "$LOGNAME"
>  then
> -	LOGNAME=$USER
> +	LOGNAME="${USER:-$(id -u -n)}"

I'll queue this as-is, but it makes me wonder if we want to do this without if/then/fi, e.g.

	: ${LOGNAME:=${USER:-$(id -u -n)}

Spelling everything out with if/then/fi is obviously at the other extreme, i.e.

	if test -z "$LOGNAME"
	then
		if test -n "$USER"
                then
			LOGNAME=$USER
		else
			LOGNAME=$(id -u -n)
		fi
	fi
but it probably is a very bad idea.

More importantly, what if none of the alternatives work? I personally feel it is OK to punt and declare test_done early, instead of giving false positive breakages like you saw without this patch.

W. Trevor King· Oct 20, 2014, 15:28 UTC · re: Junio C Hamano · lore

Re: [PATCH] t1304: Set LOGNAME even if USER is unset or null

On Sun, Oct 19, 2014 at 03:49:36PM -0700, Junio C Hamano wrote:
> I'll queue this as-is, but it makes me wonder if we want to do this
> without if/then/fi, e.g.
> 
> 	: ${LOGNAME:=${USER:-$(id -u -n)}
I'm fine with that too.
> Spelling everything out with if/then/fi is obviously at the other
> extreme, i.e.
And I'm fine with this ;).
> More importantly, what if none of the alternatives work?  I
> personally feel it is OK to punt and declare test_done early,
> instead of giving false positive breakages like you saw without this
> patch.

I can put this into a v2 if you like. Which conditional syntax do you prefer?

Cheers, Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
Junio C Hamano· Oct 20, 2014, 18:34 UTC · re: W. Trevor King · lore

Re: [PATCH] t1304: Set LOGNAME even if USER is unset or null

"W. Trevor King" <wking@tremily.us> writes:
Show 20 quoted lines
> On Sun, Oct 19, 2014 at 03:49:36PM -0700, Junio C Hamano wrote:
>> I'll queue this as-is, but it makes me wonder if we want to do this
>> without if/then/fi, e.g.
>> 
>> 	: ${LOGNAME:=${USER:-$(id -u -n)}
>
> I'm fine with that too.
>
>> Spelling everything out with if/then/fi is obviously at the other
>> extreme, i.e.
>
> And I'm fine with this ;).
>
>> More importantly, what if none of the alternatives work?  I
>> personally feel it is OK to punt and declare test_done early,
>> instead of giving false positive breakages like you saw without this
>> patch.
>
> I can put this into a v2 if you like.  Which conditional syntax do you
> prefer?
Probably
    if test -z "$LOGNAME"
    then
            LOGNAME="${USER:-$(id -u -n)}"
    else
            : cannot test acl operations without a usable user name
            test_punt!
    fi

← back to recent threads