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

Cgo fixes #169

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Cgo fixes #169

wants to merge 3 commits into from

Conversation

izissise
Copy link
Contributor

@izissise izissise commented Oct 26, 2023

Hello this is two simple fix that make go_rules work better in cgo mode

Add _module parameter generated by please_go

see #164 (comment)

Remove -Wno-error, include PKG_DIR

Fix #149

cc_library include . (-I .) but cgo put all files in PKG_DIR

@Tatskaari
Copy link
Contributor

Tatskaari commented Oct 27, 2023

Hey! Thanks for the PR. I think we should pass the _module arg to the underlying go_library call so that gets plumed through?

@izissise
Copy link
Contributor Author

Done

@Tatskaari
Copy link
Contributor

Tatskaari commented Oct 27, 2023

Hey! Sorry I missed the removal of -Wno-error. Why did you want to change this behavior? We're compiling a lot of generated code here and I'm a little worried it's going to end up failing fairly regularly.

@izissise
Copy link
Contributor Author

izissise commented Oct 28, 2023 via email

@peterebden
Copy link
Contributor

Makes sense. The removal of -Wno-error is really a breaking change though so maybe we should be careful with it - I'm not sure we should just add it to a patch release.

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.

cgo_library adds -Wno-error
3 participants