Makefile: add RUNC_BUILDTAGS, deprecate EXTRA_BUILDTAGS#5171
Makefile: add RUNC_BUILDTAGS, deprecate EXTRA_BUILDTAGS#5171lifubang merged 1 commit intoopencontainers:mainfrom
Conversation
542b1eb to
af92711
Compare
rata
left a comment
There was a problem hiding this comment.
Awesome, thanks! I've been playing a little here, works like a charm! :)
|
I like this quite a lot, my only concern is that packagers probably rely on |
| BUILDTAGS := seccomp urfave_cli_no_docs libpathrs | ||
| BUILDTAGS += $(EXTRA_BUILDTAGS) | ||
| # Tags prefixed with - in RUNC_BUILDTAGS are removed from BUILDTAGS; others are added. | ||
| BUILDTAGS_REMOVE := $(patsubst -%,%,$(filter -%,$(RUNC_BUILDTAGS))) |
There was a problem hiding this comment.
In order for tab-completion (when passing things via command-line arguments) to work, you need to define this variable. RUNC_BUILDTAGS ?= is enough.
There was a problem hiding this comment.
Added.
Wonder what tab-completion are you using? I have bash-completion-2.16-2.fc43.noarch and it only completes make targets for me.
af92711 to
c76fe00
Compare
I was thinking about adding a deprecation message -- the problem is no one will see it. Let's try it though; added :) |
c76fe00 to
e33b1cd
Compare
|
@cyphar ptal |
A bit of history. EXTRA_BUILDTAGS was introduced in commit dac4171, as a quick way to add some extra Go build tags to the runc build. Later, commit 767bc00 changed Makefile to not get EXTRA_TAGS from the shell environment, as the name is quite generic and some unrelated environment variable with that name can affect runc build. While such change does make sense, it makes it more complicated to pass build tags in CI and otherwise (see e.g. commit 0e1fe36). Moreover, runc build uses some Go build tags by default (via Makefile), and while it is easy to add more build tags (via EXTRA_BUILDTAGS), in order to remove some existing tags one has to redefine BUILDTAGS from scratch, which is not very convenient (again, see commit 0e1fe36 which gets the current value of BUILDTAGS from the Makefile in order to remove a single tag). To handle all of the above, let's do this: - implement RUNC_BUILDTAGS, fixing the issue of not-so-unique name; - allow to get RUNC_BUILDTAGS from shell environment; - implement a feature to remove a build tag from default set by prefixing it with "-" (as in RUNC_BUILDTAGS="-seccomp"); - document all this in README; - make CI use the new feature; - keep EXTRA_BUILDTAGS for backward compatibility, add a make warning and a TODO to remove it for runc 1.6. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
e33b1cd to
d8c62c7
Compare
Looks like we’ve addressed all of Cyphar’s suggestions -- ready to merge! |
|
Yeah (post-merge) LGTM. |
A bit of history. EXTRA_BUILDTAGS was introduced in commit dac4171, as a quick way to add some extra Go build tags to the runc build.
Later, commit 767bc00 changed Makefile to not get EXTRA_TAGS from the shell environment, as the name is quite generic and some unrelated environment variable with that name can affect runc build. While such change does make sense, it makes it more complicated to pass build tags in CI and otherwise (see e.g. commit 0e1fe36).
Moreover, runc build uses some Go build tags by default (via Makefile), and while it is easy to add more build tags (via EXTRA_BUILDTAGS), in order to remove some existing tags one has to redefine BUILDTAGS from scratch, which is not very convenient (again, see commit 0e1fe36 which gets the value of BUILDTAGS from the Makefile in order to remove a single tag).
To handle all of the above, let's do this:
and a TODO to remove it for runc 1.6.
Fixes: #5165.