Skip to content

process: allow compiling on darwin #621

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

korniltsev
Copy link
Contributor

@korniltsev korniltsev commented Jul 16, 2025

Similar to #612

This will eventually help allowing running some unit tests (for example coredump tests) without spawning a full linux OS on non linux systems (at least darwin, maybe windows)

@korniltsev korniltsev marked this pull request as ready for review July 16, 2025 09:41
@korniltsev korniltsev requested review from a team as code owners July 16, 2025 09:41
@florianl
Copy link
Contributor

I think, there are more changes needed to reach this point:

$ GOOS=windows go build
# go.opentelemetry.io/ebpf-profiler/libpf/pfelf/internal/mmap
libpf/pfelf/internal/mmap/mmap.go:49:17: undefined: syscall.Munmap
libpf/pfelf/internal/mmap/mmap.go:116:23: undefined: syscall.Mmap
libpf/pfelf/internal/mmap/mmap.go:116:63: undefined: syscall.PROT_READ
libpf/pfelf/internal/mmap/mmap.go:116:82: undefined: syscall.MAP_SHARED
# go.opentelemetry.io/ebpf-profiler/rlimit
rlimit/rlimit.go:17:20: undefined: unix.Rlimit
rlimit/rlimit.go:18:19: undefined: unix.Rlimit
rlimit/rlimit.go:19:13: undefined: unix.RLIM_INFINITY
rlimit/rlimit.go:20:13: undefined: unix.RLIM_INFINITY
rlimit/rlimit.go:23:17: undefined: unix.Prlimit
rlimit/rlimit.go:23:33: undefined: unix.RLIMIT_MEMLOCK
rlimit/rlimit.go:28:18: undefined: unix.Setrlimit
rlimit/rlimit.go:28:33: undefined: unix.RLIMIT_MEMLOCK

@rockdaboot
Copy link
Contributor

I think, there are more changes needed to reach this point:

Maybe change the title and say non-linux open source systems!? I guess supporting Windows builds just adds more complexity without gaining any value.

@florianl
Copy link
Contributor

It is not just Windows.

$ GOOS=darwin go build
# go.opentelemetry.io/ebpf-profiler/rlimit
rlimit/rlimit.go:23:17: undefined: unix.Prlimit
$ GOOS=netbsd go build
# go.opentelemetry.io/ebpf-profiler/rlimit
rlimit/rlimit.go:23:17: undefined: unix.Prlimi

@korniltsev
Copy link
Contributor Author

I think, there are more changes needed to reach this point:

yes, they are in different packages. rlimit and processmanager/ebpf are my next targets.

I want the change to be small and self contained and revertable just in case

@rockdaboot
Copy link
Contributor

@korniltsev Please be more specific in the PR title.

@korniltsev korniltsev changed the title process: allow compiling on nonlinux systems process: allow compiling on darwin Jul 16, 2025
@korniltsev
Copy link
Contributor Author

@korniltsev Please be more specific in the PR title.

Thanks, updated

@korniltsev
Copy link
Contributor Author

I guess supporting Windows builds just adds more complexity without gaining any value.

It would gain the same value as darwin.
I have not looked into windows yet, darwin will probably be slightly easier as it does support mmap.

process/debug.go Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe rename the file to debug_linux.go

Copy link
Contributor Author

Choose a reason for hiding this comment

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

applied

@korniltsev
Copy link
Contributor Author

Just had an idea similar tohttps://github.com//pull/625. Maybe instead of introducing build tags, we can split the package - move the ptrace specific code into a separate ptrace package

@christos68k
Copy link
Member

christos68k commented Jul 16, 2025

Before approving this, I would like to have the complete picture in terms of how the code structure is changing and also the build system. In principle, I'm not against enabling limited compilation on non-Linux systems, but not if it comes with a lot of extra complexity and potential support headaches.

The profiler only works on Linux [1] and that's where the focus should be. @korniltsev If you have multiple PRs in mind, then I'd delay approving/merging until you open all of them so we can look at them as a whole.

[1] Technically it also works inside Docker-on-macOS as that spins up a Linux kernel that supports eBPF that's shared across containers running on the same macOS host. Maybe it also works on WSL2.

@korniltsev
Copy link
Contributor Author

I dont have any more new PRs in mind regarding this.

There are 2 things I want to do.

  1. There is a rlimit: allow compiling rlimit on non-linux systems #624 with an alternative proposal WIP: split processmanager/ebpf in two packages: interface and implementation #625
  2. process: allow compiling on darwin #621

Together they allow compiling and running coredump tests.

The tests fail with an unexpected SIGURG signal - that would require another fix.

Thats it. This will enable running a big and useful subset of unit tests without spawning virtual machines.

@korniltsev
Copy link
Contributor Author

korniltsev commented Jul 16, 2025

To add a bit more on my previous comment:

I see 2 possible solutions to what I am trying to achieve.

  1. guard linux specific code with go build tags
  2. Split packages both processmanager/ebpf and process in half - keep the base interfaces inplace and move the specific implementations to a separate package, for example process/ptrace and processmanager/ebpf/cilium. I kind of like this idea more now. It's should be a good change on it's own - separating concerns.

Do you have any other ideas how this could be done?

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.

4 participants