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

fix undefined processExists error #87

Merged
merged 6 commits into from
May 16, 2022
Merged

Conversation

proditis
Copy link
Collaborator

@proditis proditis commented May 16, 2022

just a copy of daemon_darwin to fix daemon/daemon.go:90:5: undefined: processExists

I tried changing the headers to make the daemon_darwin be used but it kept failing.

//go:build darwin && openbsd
// +build darwin,openbsd

Is this command i'm using to cross-compile easeprobe wrong?

GOOS=openbsd GOARCH=amd64 go build -a -ldflags '-s -w -extldflags "-static"' -gcflags=-G=3 -o $PWD/build/bin/easeprobe.obsd $PWD/cmd/easeprobe

daemon/daemon.go:90:5: undefined: processExists
just a copy of daemon_darwin
@proditis proditis marked this pull request as ready for review May 16, 2022 00:19
@proditis proditis changed the title fix error undefined processExists error fix undefined processExists error May 16, 2022
@haoel
Copy link
Contributor

haoel commented May 16, 2022

This is a very good finding.

I realize I made a mistake - the file daemon_linux.go should be daemon_unix.go

  • the suffix _linux only for the Linux platform
  • the suffix _darwin only for macOS
  • the suffix _unix is for all Unix-like platform

So, I think the correct fix as below

  1. rename the daemon_linux.go to daemon_unix.go, I believe the OpenBSD and other Unix-like platforms can be compiled successfully.

  2. evaluating the daemon_unix.go whether works on the OpenBSD platform, if not, we need the daemon_openbsd.go, and exclude the openbsd in daemon_unix.go

@haoel
Copy link
Contributor

haoel commented May 16, 2022

Just Google how to test it (article). we can run the following command to check how the go build choose the file.

cd daemon
GOOS=openbsd GOARCH=amd64 go list -f '{{.GoFiles}}'

@proditis
Copy link
Collaborator Author

  • rename the daemon_linux.go to daemon_unix.go, I believe the OpenBSD and other Unix-like platforms can be compiled successfully.

Linux is not a good candidate for BSD systems since these have no /proc (mostly). In this case the best candidate is darwin as it uses closer to BSD API's.

I see two options

  1. rename the existing _openbsd to _unix and have a small duplication of code?
  2. rename the _darwin => _unix and remove _openbsd?

What do you think?

@haoel
Copy link
Contributor

haoel commented May 16, 2022

I like the 2nd one, as darwin is based on the BSD system.

And don't forget to remove the go-build tags in _linux.go, and add the correct tags in _unix.go

@proditis
Copy link
Collaborator Author

The list of files each os pulls

linux: [daemon.go daemon_linux.go]
windows: [daemon.go daemon_windows.go]
darwin: [daemon.go daemon_unix.go]
openbsd: [daemon.go daemon_unix.go]

Comment on lines 1 to 2
//go:build darwin || openbsd
// +build darwin openbsd
Copy link
Contributor

Choose a reason for hiding this comment

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

how about adding freebsd and netbsd?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🤦‍♂️ How could i ever ?!?!?! 😄 adding the rest of the family

Copy link
Collaborator Author

@proditis proditis May 16, 2022

Choose a reason for hiding this comment

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

What do you think if we make this our fall back for unix systems and simply exclude linux/windows on top ?

There are a lot more unix systems that will work with this one rather with the linux.
!linux && !windows

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the !linux && !windows

now we support more platforms out of the box
@haoel
Copy link
Contributor

haoel commented May 16, 2022

Wonderful! @proditis

@haoel haoel merged commit a0673b7 into megaease:main May 16, 2022
@proditis proditis deleted the daemon_openbsd branch May 16, 2022 02:52
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.

None yet

2 participants