Pass all packages to conversion-gen

As per #96015 and other reports, conversion-gen does the wrong thing if
dependent packages are not ALSO being re-generated.  It creates new
versions of generated files that have missing conversions.

This change passes all packages as "extras" which will be parsed but not
regenerated (default already does exactly this).
This commit is contained in:
Tim Hockin 2020-10-30 15:30:11 -07:00
parent ad6a2af7d8
commit 54e2748e13
4 changed files with 27 additions and 12 deletions

View File

@ -318,8 +318,9 @@ DEFAULTER_DIRS := $(shell \
| xargs -n1 dirname \
| LC_ALL=C sort -u \
)
DEFAULTER_FILES := $(addsuffix /$(DEFAULTER_FILENAME), $(DEFAULTER_DIRS))
DEFAULTER_EXTRA_PEER_PKGS := \
$(addprefix $(PRJ_SRC_PATH)/, $(DEFAULTER_DIRS))
# Reset the list of packages that need generation.
$(shell mkdir -p $$(dirname $(META_DIR)/$(DEFAULTER_GEN)))
@ -338,7 +339,7 @@ gen_defaulter: $(DEFAULTER_GEN) $(META_DIR)/$(DEFAULTER_GEN).todo
--v $(KUBE_VERBOSE) \
--logtostderr \
-i "$$pkgs" \
--extra-peer-dirs $$(echo $(addprefix $(PRJ_SRC_PATH)/, $(DEFAULTER_DIRS)) | sed 's/ /,/g') \
--extra-peer-dirs $$(echo $(DEFAULTER_EXTRA_PEER_PKGS) | sed 's/ /,/g') \
-O $(DEFAULTER_BASENAME) \
"$$@"; \
fi
@ -359,9 +360,9 @@ $(foreach dir, $(DEFAULTER_DIRS), $(eval \
$(META_DIR)/$(DEFAULTER_GEN).todo: $(DEFAULTER_FILES)
$(DEFAULTER_FILES): $(DEFAULTER_GEN)
if [[ "$(DBG_CODEGEN)" == 1 ]]; then \
if [[ "$(DBG_CODEGEN)" == 1 ]]; then \
echo "DBG: defaulter needed $(@D):"; \
ls -lf --full-time $@ $? || true; \
ls -lf --full-time $@ $? || true; \
fi
echo $(PRJ_SRC_PATH)/$(@D) >> $(META_DIR)/$(DEFAULTER_GEN).todo
@ -421,9 +422,12 @@ CONVERSION_DIRS := $(shell \
| xargs -n1 dirname \
| LC_ALL=C sort -u \
)
CONVERSION_FILES := $(addsuffix /$(CONVERSION_FILENAME), $(CONVERSION_DIRS))
CONVERSION_EXTRA_PEER_DIRS := k8s.io/kubernetes/pkg/apis/core,k8s.io/kubernetes/pkg/apis/core/v1,k8s.io/api/core/v1
CONVERSION_EXTRA_PEER_PKGS := \
k8s.io/kubernetes/pkg/apis/core \
k8s.io/kubernetes/pkg/apis/core/v1 \
k8s.io/api/core/v1
CONVERSION_EXTRA_PKGS := $(addprefix $(PRJ_SRC_PATH)/, $(CONVERSION_DIRS))
# Reset the list of packages that need generation.
$(shell mkdir -p $$(dirname $(META_DIR)/$(CONVERSION_GEN)))
@ -439,7 +443,8 @@ gen_conversion: $(CONVERSION_GEN) $(META_DIR)/$(CONVERSION_GEN).todo
echo "DBG: running $(CONVERSION_GEN) for $$pkgs"; \
fi; \
./hack/run-in-gopath.sh $(CONVERSION_GEN) \
--extra-peer-dirs $(CONVERSION_EXTRA_PEER_DIRS) \
--extra-peer-dirs $$(echo $(CONVERSION_EXTRA_PEER_PKGS) | sed 's/ /,/g') \
--extra-dirs $$(echo $(CONVERSION_EXTRA_PKGS) | sed 's/ /,/g') \
--v $(KUBE_VERBOSE) \
--logtostderr \
-i "$$pkgs" \

View File

@ -30,13 +30,11 @@ source "${KUBE_ROOT}/hack/lib/init.sh"
kube::util::ensure_clean_working_dir
_tmpdir="$(kube::realpath "$(mktemp -d -t verify-generated-files.XXXXXX)")"
kube::util::trap_add "rm -rf ${_tmpdir}" EXIT
_tmp_gopath="${_tmpdir}/go"
_tmp_kuberoot="${_tmp_gopath}/src/k8s.io/kubernetes"
mkdir -p "${_tmp_kuberoot}/.."
cp -a "${KUBE_ROOT}" "${_tmp_kuberoot}/.."
git worktree add "${_tmp_kuberoot}"
kube::util::trap_add "git worktree remove -f ${_tmp_kuberoot} && rm -rf ${_tmpdir}" EXIT
cd "${_tmp_kuberoot}"
# clean out anything from the temp dir that's not checked in

View File

@ -42,6 +42,11 @@ type CustomArgs struct {
// generator pick up manually written conversion funcs from external packages.
ExtraPeerDirs []string
// Additional dirs to parse and load, but not consider for peers. This is
// useful when packages depend on other packages and want to call
// conversions across them.
ExtraDirs []string
// SkipUnsafe indicates whether to generate unsafe conversions to improve the efficiency
// of these operations. The unsafe operation is a direct pointer assignment via unsafe
// (within the allowed uses of unsafe) and is equivalent to a proposed Golang change to
@ -67,6 +72,8 @@ func (ca *CustomArgs) AddFlags(fs *pflag.FlagSet) {
"Comma-separated list of apimachinery import paths which are considered, after tag-specified peers, for conversions. Only change these if you have very good reasons.")
pflag.CommandLine.StringSliceVar(&ca.ExtraPeerDirs, "extra-peer-dirs", ca.ExtraPeerDirs,
"Application specific comma-separated list of import paths which are considered, after tag-specified peers and base-peer-dirs, for conversions.")
pflag.CommandLine.StringSliceVar(&ca.ExtraDirs, "extra-dirs", ca.ExtraDirs,
"Application specific comma-separated list of import paths which are loaded and considered for callable conversions, but are not considered peers for conversion.")
pflag.CommandLine.BoolVar(&ca.SkipUnsafe, "skip-unsafe", ca.SkipUnsafe,
"If true, will not generate code using unsafe pointer conversions; resulting code may be slower.")
}

View File

@ -260,11 +260,13 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat
continue
}
skipUnsafe := false
extraDirs := []string{}
if customArgs, ok := arguments.CustomArgs.(*conversionargs.CustomArgs); ok {
if len(peerPkgs) > 0 {
peerPkgs = append(peerPkgs, customArgs.BasePeerDirs...)
peerPkgs = append(peerPkgs, customArgs.ExtraPeerDirs...)
}
extraDirs = customArgs.ExtraDirs
skipUnsafe = customArgs.SkipUnsafe
}
@ -296,9 +298,12 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat
for i := range peerPkgs {
peerPkgs[i] = genutil.Vendorless(peerPkgs[i])
}
for i := range extraDirs {
extraDirs[i] = genutil.Vendorless(extraDirs[i])
}
// Make sure our peer-packages are added and fully parsed.
for _, pp := range peerPkgs {
for _, pp := range append(peerPkgs, extraDirs...) {
context.AddDir(pp)
p := context.Universe[pp]
if nil == p {