Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failed test 'Default user_name should be set from system' #473

Closed
eserte opened this issue Jun 5, 2019 · 12 comments

Comments

Projects
None yet
3 participants
@eserte
Copy link

commented Jun 5, 2019

On some systems t/base.t fails like this:

#   Failed test 'Default user_name should be set from system'
#   at t/base.t line 56.
# Looks like you failed 1 test of 189.
t/base.t ............ 
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/189 subtests 

This seems to happen on older Debian (wheezy) and CentOS 6 + 7.

@theory theory self-assigned this Jun 5, 2019

@theory theory added the bug label Jun 5, 2019

@theory

This comment has been minimized.

Copy link
Collaborator

commented Jun 5, 2019

Hrm. Didn't fail for me in a CentOS 7 build. What's the output of

perl  -E 'say "Getlogin: ", getlogin; say "Getpwuid: ", scalar getpwuid( $< );'
@eserte

This comment has been minimized.

Copy link
Author

commented Jun 6, 2019

$ perl -E 'say "Getlogin: ", getlogin; say "Getpwuid: ", scalar getpwuid( $< );'
Getlogin: eserte
Getpwuid: cpansand

Probably both values are different because I change user here using sudo -u cpansand -i.

@theory

This comment has been minimized.

Copy link
Collaborator

commented Jun 6, 2019

Okay, what about

 perl -MUser::pwent -E 'say User::pwent::getpwnam(getlogin)->gecos'
@eserte

This comment has been minimized.

Copy link
Author

commented Jun 7, 2019

Prints an empty line on the three mentioned systems (CentOS 6 + 7, Debian/wheezy).

@matthieu-foucault

This comment has been minimized.

Copy link

commented Jun 7, 2019

I have similar results with an ArchLinux build

@theory

This comment has been minimized.

Copy link
Collaborator

commented Jun 9, 2019

Huh. What about

perl -MUser::pwent -E 'say q{"}, (User::pwent::getpwnam(getlogin)->gecos)[0], q{"}'

Or, more specifically to how Sqitch works:

perl -MUser::pwent -E 'my $sysname = getlogin; my $name = User::pwent::getpwnam($sysname) || do { say $sysname; exit }; say q{"}, ($name->gecos)[0], q{"}'
@matthieu-foucault

This comment has been minimized.

Copy link

commented Jun 9, 2019

Both return an empty string ("")

@theory

This comment has been minimized.

Copy link
Collaborator

commented Jun 9, 2019

Oh, of course, I'm an idiot. This should output a proper username, then:

 perl -MUser::pwent -E 'my $sysname = getlogin; my $name = User::pwent::getpwnam($sysname) || do { say $sysname; exit }; say q{"}, ($name->gecos)[0] || $sysname, q{"}'
@theory

This comment has been minimized.

Copy link
Collaborator

commented Jun 9, 2019

And this should fix the issue:

--- a/lib/App/Sqitch.pm
+++ b/lib/App/Sqitch.pm
@@ -94,8 +94,9 @@ has user_name => (
             }
             require User::pwent;
             my $name = User::pwent::getpwnam($sysname) || return $sysname;
+            $name = ($name->gecos)[0] || return $sysname;
             require Encode::Locale;
-            return Encode::decode( locale => ($name->gecos)[0] );
+            return Encode::decode( locale => $name );
         };
     }
 );
@theory

This comment has been minimized.

Copy link
Collaborator

commented Jun 9, 2019

Does that fix is for you, @matthieu-foucault?

@matthieu-foucault

This comment has been minimized.

Copy link

commented Jun 9, 2019

Yes, all the tests passed with that fix, thanks!

theory added a commit that referenced this issue Jun 9, 2019

Properly fall back on username.
When the full name of the user is not set by the system. Resolves #473.
@theory

This comment has been minimized.

Copy link
Collaborator

commented Jun 9, 2019

Awesome, thanks.

@theory theory added this to the v1.0.1 milestone Jun 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.