-
Notifications
You must be signed in to change notification settings - Fork 8.1k
WIP: Fix libpsl-native to build and pass tests on FreeBSD #7208
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
Conversation
| } | ||
|
|
||
| // TODO: Real of effective user ID? | ||
| return GetPwUid(oldp.ki_uid); |
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.
ki_uid returns effective user id, where as ki_ruid returns real user id and ki_svuid returns saved effective user id. Which one should we look for in this case?
| if (lstat(path, &buf) == -1) | ||
| { | ||
| // TODO: throw error on path doesn't exist? | ||
| return false; |
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.
If path does not exist, is returning false enough?
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.
The unit test test-isfile.cpp currently expects false for a file that does not exit. See https://github.com/PowerShell/PowerShell/blob/master/src/libpsl-native/test/test-isfile.cpp
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.
Also, I would suggest testing for path before using it; something like the following:
if (path == NULL || lstat(path, &buf) == -1)
|
Did we migrate the psl to https://github.com/PowerShell/PowerShell-Native? |
|
After merging #7129 we will no longer use |
|
@adityapatwardhan Can we answer @iSazonov 's question? When should we stop taking PRs here? |
|
@mateusrodrigues Could you submit this PR to https://github.com/PowerShell/PowerShell-Native. We are moving the native code to a separate repo. Thanks for your contribution. |
|
@adityapatwardhan just moved the PR over |
|
@mateusrodrigues Thanks! I will close this PR. |
PR Summary
As cited in #7207, these are the changes I've made to libpsl-native to fix all tests on FreeBSD. The only two files that needed changes were
src/getuserfrompid.cppandsrc/isfile.cpp:isfile.cpp, I've changed the logic to verify whether something is a file or not by using a macro available insys/stat.h. This changed allowedIsFileTest.RootIsFileto pass.getuserfrompid.cpp, a FreeBSD-specific logic wasn't defined, so NULL was being returned every time. Therefore, I've added a logic similar to the macOS one with a subtle difference in the return statement. This allowedGetUserFromPid.Successto pass.PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:to the beginning of the title and remove the prefix when the PR is ready.[feature]if the change is significant or affects feature tests