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

Merge upstream changes localy #3

Merged
merged 14 commits into from
Jun 9, 2023
Merged

Conversation

smortex
Copy link

@smortex smortex commented Apr 23, 2023

The upstream project has some commits missing from this fork. Syncing this
copy will fix the build of downstream projects relying on this fork instead of
upstream, e.g. choria which currently breaks on i386 because of the lack of
32d7659.

brianshumate and others added 13 commits August 20, 2017 19:20
It also works on i386 architecture.
simple numeric name check should suffice. Otherwise, it is pretty
problematic to query processes on systems with SELinux in enforcing mode
as it is complicated to open access to _all_ files inside proc.
The reason this catch-all access is required is that with os.Readdir it
needs to lstat each entry to return os.FileInfo and this requires
additional permissions on each disperate file type inside /proc which is
not easy to catch with an attribute (as it is with processes).
Update process fetcher to support SELinux-enabled systems.
Adding Power Support
Updating go version to 1.13 as the lower versions are not supported.
AddingPowerSupport_&_CI/Testing
Remove .travis.yml as Travis-CI is basically dead.
Copy link

@ripienaar ripienaar left a comment

Choose a reason for hiding this comment

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

Some minor changes

go.mod Outdated
@@ -0,0 +1,3 @@
module github.com/mitchellh/go-ps

Choose a reason for hiding this comment

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

I think this would need to change

Copy link
Author

Choose a reason for hiding this comment

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

Good catch, fixed!

@@ -1,4 +1,4 @@
# Process List Library for Go
# Process List Library for Go [![GoDoc](https://godoc.org/github.com/mitchellh/go-ps?status.png)](https://godoc.org/github.com/mitchellh/go-ps)

Choose a reason for hiding this comment

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

I think we would also need to change these urls?

Copy link
Author

Choose a reason for hiding this comment

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

Maybe the README could be replaced completely and detail the purpose of this fork?

Choose a reason for hiding this comment

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

Yeah that sounds like a good idea. Or just a more high up

@aelsabbahy
Copy link
Member

Cc: @ripienaar would love your review on this.

@aelsabbahy
Copy link
Member

Random thought, it's been a while.. but if I remember correctly, the only reason this fork exists is because of this:

mitchellh#23

If there are no other changes (this needs to be confirmed).. I wonder if there's a better way to maintain this delta.. or if someone wants to champion it getting added upstream.

@smortex
Copy link
Author

smortex commented Apr 23, 2023

I see #2 got merged in master and is not part of upstream, but it seems easy to send this as a PR upstream. I see no other changes.

My root problem seems to have been addressed 3 years ago to a branch of this repository (this explain why this is so "deja vu" for me) in #1 😄

So as far as I am concerned, being able to switch to upsteam would be great. mitchellh#23 report merge conflicts. Fixing them and adding some context to explain how to reproduce the issue being fixed might help?

@aelsabbahy
Copy link
Member

Just realized I never merged this.

Merging now, to avoid blocking anything. Let me know if there are any follow up prs related to this.

@aelsabbahy aelsabbahy merged commit 7b318e6 into goss-org:master Jun 9, 2023
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.

8 participants