-
Notifications
You must be signed in to change notification settings - Fork 5
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 how we find parent process #472
Conversation
$releaseUrl = gh release create "$env:RELEASE_TAG" --repo Brightspace/bmx --draft --prerelease --generate-notes --title "Release $env:RELEASE_TAG" | ||
$releaseUrl = gh release create "$env:RELEASE_TAG" --repo Brightspace/bmx --draft --prerelease --generate-notes --title "Release $env:RELEASE_TAG" --target $env:GITHUB_SHA | ||
} else { | ||
$releaseUrl = gh release create "$env:RELEASE_TAG" --repo Brightspace/bmx --draft --generate-notes --title "Release $env:RELEASE_TAG" | ||
$releaseUrl = gh release create "$env:RELEASE_TAG" --repo Brightspace/bmx --draft --generate-notes --title "Release $env:RELEASE_TAG" --target $env:GITHUB_SHA |
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.
unrelated fix - our release job only worked on the main branch.
internal partial class WindowsParentProcess { | ||
internal partial class ParentProcess { |
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.
Consolidated both the previous Windows and Unix classes under the same one now.
The high level method GetParentProcessName
has identical logic.
if( parentPid == -1 ) { | ||
return null; | ||
} |
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.
This is new.
GetWindowsParentProcessId
returns -1 on error. We should check that, instead of passing it to GetProcessById
, which will throw.
We have default fallback behaviour on each OS, so throwing doesn't make sense.
[LibraryImport( "ntdll.dll", EntryPoint = "NtQueryInformationProcess" )] | ||
internal static partial int NtQueryInformationProcess( |
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.
Moved below, as per our convention to put public/high-level methods first, and private/low-level/helper methods after.
Also changed it to private
, as it's not used elsewhere.
Why
Turns out BusyBox only implements a small portion of the POSIX spec for its ps, so our code that gets the parent process (and hence
bmx print
) never worked on Alpine...(I guess no one ever used
bmx print
on alpine...)Switch to use native API call, which is actually easier.
Ticket
https://desire2learn.atlassian.net/browse/VUL-403