On 13.12.2021 20:04, Andrew Cooper wrote: > --- a/tools/libs/libs.mk > +++ b/tools/libs/libs.mk > @@ -6,7 +6,10 @@ > # MINOR: minor version of lib (0 if empty) > > LIBNAME := $(notdir $(CURDIR)) > -MAJOR ?= $(shell $(XEN_ROOT)/version.sh $(XEN_ROOT)/xen/Makefile) > + > +ifeq ($(origin MAJOR), undefined) > +MAJOR := $(shell $(XEN_ROOT)/version.sh $(XEN_ROOT)/xen/Makefile) > +endif > MINOR ?= 0 > > SHLIB_LDFLAGS += -Wl,--version-script=libxen$(LIBNAME).map
Wouldn't it be better to move the "endif" past the setting of MINOR (which then could use := as well)? Libraries with their own versioning would imo better specify both rather than relying on getting 0 from here (which at present none of them does). Would require an adjustment to the comment at the top of libs.mk, though. And further, since you're switching to $(origin ...), wouldn't this be an opportunity to avoid stray inheriting of values from the environment, by switching to "ifneq ($(origin MAJOR), file)"? Or is there an intention of allowing such control via the environment (which would then override the versions for all libraries not explicitly setting them)? In turn I would then further wonder whether command line overrides are intended, but I guess people doing so ought to indeed get what they have asked for (all libraries versioned identically, assuming both MAJOR and MINOR get defined that way). Jan
