-
Notifications
You must be signed in to change notification settings - Fork 202
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
Fix Windows regression introduced by PR #1161 #1169
Fix Windows regression introduced by PR #1161 #1169
Conversation
Before 52108b7 this code used unshare.IsRootless() which on Windows always returns false (the behavior we want). After 52108b7, a condition was unintentionally inverted, allowing Windows to function. Commit f8f2c4f fixed the inversion, but unintentionally excluded Windows since it used == 0 instead of <= 0 (Windows returns -1) Move the logic behind a function with a comment since the Windows path is a bit exotic. In the future, the Windows path should likely be refactored to be more intuitive; however, this will get things working for now. Signed-off-by: Jason T. Greene <jason.greene@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, n1hility The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/lgtm I will have to cut another release once this merges. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/hold cancel |
Before 52108b7 this code used unshare.IsRootless() which on Windows always returns false (the behavior we want).
After 52108b7, a condition was unintentionally inverted, allowing Windows to function.
Commit f8f2c4f fixed the inversion, but unintentionally excluded Windows since it used == 0 instead of <= 0 (Windows returns -1)
Move the logic behind a function with a comment since the Windows path is a bit exotic.
In the future, the Windows path should likely be refactored to be more intuitive; however, this will get things working for now.