Skip to content

fix: add OS check to pdeathsig#8

Open
Davincible wants to merge 2 commits into
masterfrom
windows
Open

fix: add OS check to pdeathsig#8
Davincible wants to merge 2 commits into
masterfrom
windows

Conversation

@Davincible

Copy link
Copy Markdown
Owner

Fix Windows/ Mac support

On linux with pdeathsig it will automatically kill the child processes, that is currently not implemented on mac/linux

@Davincible Davincible mentioned this pull request Feb 10, 2023
@mihaiav

mihaiav commented Feb 10, 2023

Copy link
Copy Markdown
Contributor

@Davincible that's not working on MacOS for obvious reasons. You can try to build for MacOS using the GOOS=darwin and GOARCH=amd64 and still get the error.
I believe using build tags like in my PR is the idiomatic way to support different platforms. As Go is a compiled language you cannot reference platform specific code (i.e. *syscall.SysProcAttr) and ignore it at runtime. You have to use build tags. It's the only way.

Even if that code would somehow compile(it's impossible) you still try to create a frame buffer (newFrameBuffer) that will fail on both MacOS and Windows because none of them support Xvfb.

env GOOS=darwin GOARCH=amd64 go build

./chrome.go:158:22: cmd.SysProcAttr.Pdeathsig undefined (type *syscall.SysProcAttr has no field or method Pdeathsig)
./display.go:83:20: xvfb.SysProcAttr.Pdeathsig undefined (type *syscall.SysProcAttr has no field or method Pdeathsig)

@yms2772

yms2772 commented Feb 11, 2023

Copy link
Copy Markdown

Unless specify a build tag, Windows or MacOS will read that code unconditionally, and inevitably, errors occur.

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.

3 participants