Skip to content

makefile: make runc non-PHONY target #4873

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

makefile: make runc non-PHONY target #4873

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 12 additions & 15 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,16 @@ ifneq (,$(filter $(GOARCH),arm64 amd64))
LDFLAGS_STATIC := -linkmode external -extldflags -static-pie
endif
endif
# Enable static PIE binaries on supported platforms.
GO_BUILD_STATIC := $(GO) build $(TRIMPATH) $(GO_BUILDMODE_STATIC) \
$(EXTRA_FLAGS) -tags "$(BUILDTAGS) netgo osusergo" \
-ldflags "$(LDFLAGS_COMMON) $(LDFLAGS_STATIC) $(EXTRA_LDFLAGS)"

# "static" just maps to runc with a different "go build" invocation. This lets
# us avoid rebuilding if the sources haven't changed -- even with .PHONY.
.PHONY: static
static: export GO_BUILD=$(GO_BUILD_STATIC)
Copy link
Contributor

@kolyshkin kolyshkin Aug 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

nit: is export really needed here? Looks like not, since we don't use sub-make, but maybe I'm missing something.

static: runc

GPG_KEYID ?= [email protected]

# Some targets need cgo, which is disabled by default when cross compiling.
Expand All @@ -67,14 +72,13 @@ ifneq (,$(filter $(BUILDTAGS),seccomp))
seccompagent: export CGO_ENABLED=1
endif

.DEFAULT: runc

.PHONY: runc
runc: runc-bin
GO_SRC := \
go.mod go.sum \
$(shell find . -type f -name '*.go' -or -name '*.c')
Copy link
Member

Choose a reason for hiding this comment

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

And //go:embed files

runc/main.go

Line 25 in d845c4a

//go:embed VERSION

Copy link
Member Author

@cyphar cyphar Aug 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Let me cook up some awk-foo to make this work without hardcoding them...

Copy link
Member

@rata rata Sep 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@cyphar I'm not convinced we want to stop relying on the compiler for this on every runc compilation.

I'm not very familiar with uprobes, how are you attaching them?

If you really need the binary to not change, I think something like: cp runc runc-uprobes; make; <overwrite runc-uprobes only if md5 is different> is a simpler alternative that doesn't put the price of this limitation (discover files we need to trigger a recompile) on everyone else not using uprobes.

It would be great if go provided, like gcc, a feature to list the dependencies in a makefile-style fashion. Or if we can say to go build: only overwrite if there are changes. These might be good issues for go, though? I haven't searched if they already rejected something like this or what.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not very familiar with uprobes, how are you attaching them?

bpftrace -e 'uprobe:./runc:symbol { ... }'

I was wrong though -- the Go compiler doesn't replace the binary if it would be identical (the inode number is unchanged after make) so this patch is unnecessary. I'm not sure why it didn't work when I was testing it before...


.PHONY: runc-bin
runc-bin:
$(GO_BUILD) -o runc .
.DEFAULT: runc
Copy link
Contributor

Choose a reason for hiding this comment

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

This DEFAULT will be a no-op since there is another target defined before.

So just calling make will trigger the static build, rather than the current existing one.

runc: $(GO_SRC)
$(GO_BUILD) -o $@ .

.PHONY: all
all: runc memfd-bind
Expand All @@ -101,13 +105,6 @@ clean:
sudo rm -rf release
rm -rf man/man8

.PHONY: static
static: static-bin

.PHONY: static-bin
static-bin:
$(GO_BUILD_STATIC) -o runc .

.PHONY: releaseall
releaseall: RELEASE_ARGS := "-a 386 -a amd64 -a arm64 -a armel -a armhf -a ppc64le -a riscv64 -a s390x"
releaseall: release
Expand Down
Loading