Skip to content
This repository was archived by the owner on Sep 3, 2024. It is now read-only.

Get user name by getpwuid instead of environment variable USER #150

Merged
merged 15 commits into from
Jun 3, 2024

Conversation

youpong
Copy link
Collaborator

@youpong youpong commented May 14, 2024

Description

getlogin(2) does not return the expected value.
Instead, getpwuid(3) can be used to retrieve the user name.

To test this, I've prepared a repository youpong/getuser.
See function test_getlogin() for getlogin(2) behavior, function getuser() for an example implementation in getpwuid(3).
You can see the results of the multi-platform behavior in the GitHub Actions log.

Related Issue

@youpong
Copy link
Collaborator Author

youpong commented May 14, 2024

Unit tests are failing.
The reason is that ubuntu:latest in Github Actions is still using 22.04 LTS, so the std::format adopted in standard C++20 is not available.

@youpong
Copy link
Collaborator Author

youpong commented May 14, 2024

clang++ (not g++) on Ubuntu 22.04 LTS can use std::format.
I would like to propose to use clang++ for unit test.

@youpong
Copy link
Collaborator Author

youpong commented May 14, 2024

Coverage testing has failed. This is because it uses g++.

Maybe Ubuntu 24.04 LTS will be available in a few months, so it's a not bad idea not to take action.
What do you think?

@TanmayPatil105
Copy link
Owner

I believe we would want procfetch to be compatible with older distributions too, including the tests!

@youpong
Copy link
Collaborator Author

youpong commented May 14, 2024

I think that being available in various distributions, old and new, is not about being able to build, but about having binaries available.
And while unit testing should be checked with every commit, coverage testing should be enough to get a trend every few months.

@youpong
Copy link
Collaborator Author

youpong commented May 15, 2024

Thank you for information. I'm traveling for a couple of days. I'll take a look later.

@TanmayPatil105
Copy link
Owner

We'll also need to update the unit test images to 24.04.

@youpong
Copy link
Collaborator Author

youpong commented May 22, 2024

Added ubuntu-24.04 as a runner host for unit testing.
ubuntu-latest is left as it is.
It is OK if the test succeeds on ubuntu-24.04, even if it fails on ubuntu-latest.

Update ubuntu-latset to ubuntu-24.04 on unit testing.

The OS version of ubuntu-latest is 22.04 now, but will be 24.04 in a few months. We will remove update ubuntu-24.04 to ubuntu-latest on unit testing and coverage testing at that time.

@youpong youpong requested a review from TanmayPatil105 May 25, 2024 00:53
@youpong youpong requested a review from TanmayPatil105 June 3, 2024 02:12
@youpong youpong requested a review from TanmayPatil105 June 3, 2024 04:53
@TanmayPatil105 TanmayPatil105 merged commit 6f066ec into TanmayPatil105:main Jun 3, 2024
4 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants