mirror of
https://github.com/k3s-io/kubernetes.git
synced 2025-07-23 19:56:01 +00:00
Makefile: use $$ in define blocks
Make's "define" feature (macros) is subtle and it took me a long time to
convince myself this all works. In particular, we (prior to this commit)
are terribly inconsistent about the use of `$` vs `$$`. We mostly get
away with it because the "variables" are more like "constants", but the
inconsistency trips up some things. For example, using `$(shell)`
inside a macro will run at macro expansion time rather than when the
resulting make code is executed.
For a contrived, but concrete example, derived from our Makefile:
```
define MACRO
ifeq ($(DBG),1)
$(warning dbg is $(DBG))
endif
endef # macro
TGTS=a b c
$(foreach pfx, $(TGTS), $(eval $(MACRO)))
default:
@echo $@
```
yields:
```
$ make
Makefile:8: dbg is
Makefile:8: dbg is
Makefile:8: dbg is
default
$ make DBG=1
Makefile:8: dbg is 1
Makefile:8: dbg is 1
Makefile:8: dbg is 1
default
```
This is because `$(warning)` is evaluated as the macro is expanded.
Replace that with `$(shell)` and you can see how you might end up
running a bunch of things you didn't need to run. The fix is:
```
define MACRO
ifeq ($(DBG),1)
$$(warning dbg is $$(DBG))
endif
endef # macro
TGTS=a b c
$(foreach pfx, $(TGTS), $(eval $(MACRO)))
default:
@echo $@
```
which yields:
```
$ make
default
$ make DBG=1
Makefile:8: dbg is 1
Makefile:8: dbg is 1
Makefile:8: dbg is 1
default
```
We COULD have only changed `$(warning)` to `$$(warning)` and left
`$(DBG)` alone, because that's a cheap expansion. I chose NOT to do
that here because it requires brainpower to think about this all, and it
seems easier to set a simple rule: inside a `define`/`endef` block, you
always use `$$` unless you KNOW that you NEED expansion-time evaluation
(as in the `$(prefix)` in this commit, which is effectively an argument
to the macros).
This commit is contained in:
Tim Hockin
parent
b4f7da1ec8
commit
04a1d20deb
@ -576,19 +576,20 @@ SAMPLEAPISERVER_OPENAPI_OUTPUT_PKG := staging/src/k8s.io/sample-apiserver/pkg/ge
|
||||
OPENAPI_TARGETS := KUBE AGGREGATOR APIEXTENSIONS CODEGEN SAMPLEAPISERVER
|
||||
|
||||
# Find all the directories that request openapi generation.
|
||||
# - required input: $(prefix)
|
||||
define OPENAPI_DIR_DEF
|
||||
ifeq ($$(DBG_MAKEFILE),1)
|
||||
$$(warning ***** finding all +k8s:openapi-gen tags for $(prefix))
|
||||
endif
|
||||
$(prefix)_OPENAPI_DIRS := $(shell \
|
||||
grep --color=never -l '+k8s:openapi-gen=' $($(prefix)_OPENAPI_TAG_FILES) \
|
||||
| xargs -n1 dirname \
|
||||
| LC_ALL=C sort -u \
|
||||
$(prefix)_OPENAPI_DIRS := $$(shell \
|
||||
grep --color=never -l '+k8s:openapi-gen=' $$($(prefix)_OPENAPI_TAG_FILES) \
|
||||
| xargs -n1 dirname \
|
||||
| LC_ALL=C sort -u \
|
||||
)
|
||||
ifeq ($(DBG_MAKEFILE),1)
|
||||
$$(warning ***** found $(shell echo $($(prefix)_OPENAPI_TAG_FILES) | wc -w) +k8s:openapi-gen tagged dirs for $(prefix))
|
||||
ifeq ($$(DBG_MAKEFILE),1)
|
||||
$$(warning ***** found $$(shell echo $$($(prefix)_OPENAPI_TAG_FILES) | wc -w) +k8s:openapi-gen tagged dirs for $(prefix))
|
||||
endif
|
||||
endef
|
||||
endef # OPENAPI_DIR_DEF
|
||||
$(foreach prefix, $(OPENAPI_TARGETS), $(eval $(OPENAPI_DIR_DEF)))
|
||||
|
||||
# Compute all openapi output file names
|
||||
@ -597,6 +598,7 @@ $(foreach prefix, $(OPENAPI_TARGETS), $(eval
|
||||
))
|
||||
|
||||
# For each openapi target compute the spec
|
||||
# - required input: $(prefix)
|
||||
define OPENAPI_TARGETS_DEF
|
||||
# For each dir in $(prefix)_OPENAPI_DIRS, this establishes a dependency
|
||||
# between the output file and the input files that should trigger a rebuild.
|
||||
@ -606,40 +608,40 @@ define OPENAPI_TARGETS_DEF
|
||||
#
|
||||
# The 'eval' is needed because this has a different RHS for each LHS, and
|
||||
# would otherwise produce results that make can't parse.
|
||||
$(foreach dir, $($(prefix)_OPENAPI_DIRS), $(eval \
|
||||
$($(prefix)_OPENAPI_OUTFILE): $(GODEPS_$(PRJ_SRC_PATH)/$(dir)) \
|
||||
$$(foreach dir, $$($(prefix)_OPENAPI_DIRS), $$(eval \
|
||||
$$($(prefix)_OPENAPI_OUTFILE): $$(GODEPS_$$(PRJ_SRC_PATH)/$$(dir)) \
|
||||
))
|
||||
|
||||
# When UPDATE_API_KNOWN_VIOLATIONS is set to be true, let the generator to write
|
||||
# updated API violations to the known API violation exceptions list.
|
||||
ifeq ($(UPDATE_API_KNOWN_VIOLATIONS),true)
|
||||
$(prefix)_REPORT_FILENAME := $($(prefix)_KNOWN_VIOLATION_FILENAME)
|
||||
ifeq ($$(UPDATE_API_KNOWN_VIOLATIONS),true)
|
||||
$(prefix)_REPORT_FILENAME := $$($(prefix)_KNOWN_VIOLATION_FILENAME)
|
||||
# When UPDATE_API_KNOWN_VIOLATIONS is set to be true, touch the exceptions
|
||||
# list so that the $(prefix)_OPENAPI_OUTFILE target re-run instead of being cached.
|
||||
$$(shell touch $($(prefix)_KNOWN_VIOLATION_FILENAME))
|
||||
$$(shell touch $$($(prefix)_KNOWN_VIOLATION_FILENAME))
|
||||
else
|
||||
$(prefix)_REPORT_FILENAME := $(OUT_DIR)/$(prefix)_violations.report
|
||||
$(prefix)_REPORT_FILENAME := $$(OUT_DIR)/$(prefix)_violations.report
|
||||
endif
|
||||
|
||||
# How to regenerate open-api code. This emits a single file for all results.
|
||||
# The Make rule fails if generated API rule violation report differs from the
|
||||
# checked-in violation file, and prints error message to request developer to
|
||||
# fix either the API source code, or the known API rule violation file.
|
||||
$$($(prefix)_OPENAPI_OUTFILE): $(OPENAPI_GEN) $($(prefix)_KNOWN_VIOLATION_FILENAME)
|
||||
kube::log::status "Generating openapi code for $(prefix)"; \
|
||||
./hack/run-in-gopath.sh $(OPENAPI_GEN) \
|
||||
--v $(KUBE_VERBOSE) \
|
||||
--logtostderr \
|
||||
-i $$$$(echo $(addprefix $(PRJ_SRC_PATH)/, $($(prefix)_OPENAPI_DIRS)) | sed 's/ /,/g') \
|
||||
-p $(PRJ_SRC_PATH)/$($(prefix)_OPENAPI_OUTPUT_PKG) \
|
||||
-O $(OPENAPI_BASENAME) \
|
||||
-h $(BOILERPLATE_FILENAME) \
|
||||
-r $$($(prefix)_REPORT_FILENAME) \
|
||||
$$($(prefix)_OPENAPI_OUTFILE): $$(OPENAPI_GEN) $$($(prefix)_KNOWN_VIOLATION_FILENAME)
|
||||
kube::log::status "Generating openapi code for $(prefix)"; \
|
||||
./hack/run-in-gopath.sh $$(OPENAPI_GEN) \
|
||||
--v $$(KUBE_VERBOSE) \
|
||||
--logtostderr \
|
||||
-i $$$$(echo $$(addprefix $(PRJ_SRC_PATH)/, $$($(prefix)_OPENAPI_DIRS)) | sed 's/ /,/g') \
|
||||
-p $$(PRJ_SRC_PATH)/$$($(prefix)_OPENAPI_OUTPUT_PKG) \
|
||||
-O $$(OPENAPI_BASENAME) \
|
||||
-h $$(BOILERPLATE_FILENAME) \
|
||||
-r $$($(prefix)_REPORT_FILENAME) \
|
||||
"$$$$@"
|
||||
test -f $($(prefix)_KNOWN_VIOLATION_FILENAME) || touch $($(prefix)_KNOWN_VIOLATION_FILENAME)
|
||||
diff $$($(prefix)_REPORT_FILENAME) $($(prefix)_KNOWN_VIOLATION_FILENAME) || \
|
||||
(echo -e $(call API_RULE_CHECK_FAILURE_MESSAGE,$(prefix),$($(prefix)_KNOWN_VIOLATION_FILENAME)); exit 1)
|
||||
endef
|
||||
test -f $$($(prefix)_KNOWN_VIOLATION_FILENAME) || touch $$($(prefix)_KNOWN_VIOLATION_FILENAME)
|
||||
diff $$($(prefix)_REPORT_FILENAME) $$($(prefix)_KNOWN_VIOLATION_FILENAME) || \
|
||||
(echo -e $$(call API_RULE_CHECK_FAILURE_MESSAGE,$(prefix),$$($(prefix)_KNOWN_VIOLATION_FILENAME)); exit 1)
|
||||
endef # OPENAPI_TARGETS_DEF
|
||||
$(foreach prefix, $(OPENAPI_TARGETS), $(eval $(OPENAPI_TARGETS_DEF)))
|
||||
|
||||
# This rule is the user-friendly entrypoint for openapi generation.
|
||||
|
||||
Loading…
Reference in New Issue
Block a user