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

bpf2go: Fall back to default module name when debug.ReadBuildInfo is not available #1004

Merged
merged 1 commit into from
Apr 12, 2023

Conversation

folbricht
Copy link
Contributor

Since 223b7d7, bpf2go reads the module name from debug.ReadBuildInfo(). Unfortunately this fails when bpf2go is built with bazel which doesn't add the buildinfo (there is an open issue for it), causing a failure with determine current module: missing build info.

This change sets a default if BuildInfo is not available.

@folbricht folbricht force-pushed the buildinfo-default branch 2 times, most recently from 1189f83 to 3e0ea7d Compare April 7, 2023 16:58
@florianl
Copy link
Contributor

florianl commented Apr 8, 2023

Is bazel-contrib/rules_go#3090 the issue on the bazel side, that you are referring to?

@folbricht
Copy link
Contributor Author

Yes, that's the one. There doesn't seem to be a solution for it yet.

cmd/bpf2go/tools.go Outdated Show resolved Hide resolved
@lmb
Copy link
Collaborator

lmb commented Apr 11, 2023

I don't mind merging this to get bazel going again, but I want to be very clear that we don't support build systems other than the Go toolchain. Aka we won't check for breakage as you've discovered :)

@folbricht
Copy link
Contributor Author

Not supporting bazel is definitely fair. I'm just trying to set a reasonable default so things still work for anyone stuck with a build system that doesn't provide BuildInfo.

bazel currently doesn't support debug.ReadBuildInfo. Default to
a hardcoded module name to make things work again.

Signed-off-by: folbrich <frank.olbricht@gmail.com>
Copy link
Collaborator

@lmb lmb left a comment

Choose a reason for hiding this comment

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

Thanks, added a comment and a commit description.

@lmb lmb merged commit b04b758 into cilium:master Apr 12, 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.

3 participants