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

Get UID/GID for current user in write file_test.go #228

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jakefhyde
Copy link
Collaborator

This can just get merged whenever, it's just to enable these tests to run locally. I've confirmed they work both with go test ./... and make test.

@jakefhyde jakefhyde requested review from snasovich and a team April 2, 2025 19:38
HarrisonWAffel
HarrisonWAffel previously approved these changes Apr 2, 2025
jiaqiluo
jiaqiluo previously approved these changes Apr 2, 2025
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we drop the build arg to allow it to run on arm64 now?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jakefhyde, I think this question is still valid. Please let me know if I misunderstand anything.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me try and see if it works

@HarrisonWAffel
Copy link
Contributor

Ah shoot, I forgot this test is shared between both platforms. On windows u.Uid is going to return a security identifier (e.g. S-1-5-21-788787744-2135942908-530990601-500) and not a simple integer, so atoi will fail. We should either do a GOOS check to conditionally pass the current uid and guid, or create platform specific test files (though they would be nearly identical).

@jakefhyde
Copy link
Collaborator Author

Ah shoot, I forgot this test is shared between both platforms. On windows u.Uid is going to return a security identifier (e.g. S-1-5-21-788787744-2135942908-530990601-500) and not a simple integer, so atoi will fail. We should either do a GOOS check to conditionally pass the current uid and guid, or create platform specific test files (though they would be nearly identical).

I'll opt for the former (for now).

@jakefhyde
Copy link
Collaborator Author

@HarrisonWAffel actually from looking at the docs, it appears it does nothing if uid and gid are -1 (means no change) so that might be a better option for the test. This will also be useful for any files the system-agent creates. Also FWIW it doesn't error when installed as a systemd service because they run as root (ID 0).

@jakefhyde jakefhyde dismissed stale reviews from jiaqiluo and HarrisonWAffel via cc8842d April 3, 2025 00:15
@jakefhyde jakefhyde force-pushed the get-uid-gid-for-current-user-in-test branch from b01b2e5 to cc8842d Compare April 3, 2025 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants