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

[W. Trevor King]

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 umas=
k
#
# # SHA1 for empty blob
# check_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8=
b29ae775ad8c2e48c5391
#
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=3D/
PATH=3D/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
HOSTNAME=3Db2dfdfe797ed

'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 <***@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=C3=A9 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=3D$(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/focu=
s=3D172961
[2]: http://thread.gmane.org/gmane.comp.version-control.git/183586
[3]: http://thread.gmane.org/gmane.comp.version-control.git/205690/focu=
s=3D205703

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=
' '
=20
if test -z "$LOGNAME"
then
- LOGNAME=3D$USER
+ LOGNAME=3D"${USER:-$(id -u -n)}"
fi
=20
check_perms_and_acl () {
--=20
2.1.0.60.g85f0837

[Junio C Hamano]

Post by W. Trevor King
* Michael Gruber on 2011-05-06 suggesting a discussing a whoami
fallback [1] (but whoami isn't POSIX).
* Ren=C3=A9 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].
# USER=3D$(id -u -n) ./t1304-default-acl.sh
so the question is really "Are empty-USER systems worth supporting ou=

t

Post by W. Trevor King
of the box?".
Cheers,
Trevor
[1]: http://thread.gmane.org/gmane.comp.version-control.git/172883/fo=

cus=3D172961

Post by W. Trevor King
[2]: http://thread.gmane.org/gmane.comp.version-control.git/183586
[3]: http://thread.gmane.org/gmane.comp.version-control.git/205690/fo=

cus=3D205703

Post by W. Trevor King
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 set=

up' '

Post by W. Trevor King
=20
if test -z "$LOGNAME"
then
- LOGNAME=3D$USER
+ LOGNAME=3D"${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:=3D${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=3D$USER
else
LOGNAME=3D$(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]

Post by Junio C Hamano
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.

Post by Junio C Hamano
Spelling everything out with if/then/fi is obviously at the other
extreme, i.e.

And I'm fine with this ;).

Post by Junio C Hamano
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

[Junio C Hamano]

Post by W. Trevor King

Post by Junio C Hamano
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.

Post by Junio C Hamano
Spelling everything out with if/then/fi is obviously at the other
extreme, i.e.

And I'm fine with this ;).

Post by Junio C Hamano
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