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

Update bpf2go and stringer go:generate statements across the codebase #1140

Merged
merged 2 commits into from
Sep 27, 2023

Conversation

ti-mo
Copy link
Collaborator

@ti-mo ti-mo commented Sep 27, 2023

#1047 added env support to bpf2go, so use it.

Also cleaned up the stringer statements that would otherwise pick an old version from my GOBIN.

This would pick an old, incompatible version from my GOBIN instead.

Signed-off-by: Timo Beckers <timo@isovalent.com>
…rate

This patch removes the -cc and -cflags from all 'go:generate' statements across
the codebase. Instead, ambiently provide BPF2GO_ env vars to the 'make' call
in container-all.

This allows running `go generate ./...` on distributions where clang binaries
don't have their version numbers suffixed.

Signed-off-by: Timo Beckers <timo@isovalent.com>
@ti-mo ti-mo requested a review from lmb September 27, 2023 14:09
@ti-mo ti-mo merged commit 0667fa5 into cilium:main Sep 27, 2023
11 checks passed
@ti-mo ti-mo deleted the tb/update-gogen branch September 27, 2023 14:48
@@ -81,9 +81,6 @@ all: format $(addsuffix -el.elf,$(TARGETS)) $(addsuffix -eb.elf,$(TARGETS)) gene
ln -srf testdata/loader-$(CLANG)-el.elf testdata/loader-el.elf
ln -srf testdata/loader-$(CLANG)-eb.elf testdata/loader-eb.elf

# $BPF_CLANG is used in go:generate invocations.
generate: export BPF_CLANG := $(CLANG)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn't this break make generate outside of container?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not for me, it defaults to clang and llvm-strip. I also sent a patch that removed a hardcoded clang-14 somewhere.

@@ -1,6 +1,6 @@
package asm

//go:generate stringer -output alu_string.go -type=Source,Endianness,ALUOp
//go:generate go run golang.org/x/tools/cmd/stringer@latest -output alu_string.go -type=Source,Endianness,ALUOp
Copy link
Collaborator

Choose a reason for hiding this comment

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

How does this work without adding a dependency on stringer to go.mod?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The version is explicitly mentioned, so I assume that's all it needs to know. It doesn't add anything to go.mod/sum.

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.

2 participants