[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN PATCH] libs: Fix unstable libs build on FreeBSD, auto-generate version-script
On Wed, Feb 15, 2023 at 04:30:43PM +0100, Jan Beulich wrote: > On 15.02.2023 16:21, Anthony PERARD wrote: > > @@ -13,6 +14,10 @@ MAJOR := $(shell $(XEN_ROOT)/version.sh > > $(XEN_ROOT)/xen/Makefile) > > endif > > MINOR ?= 0 > > > > +ifeq ($(origin version-script), undefined) > > +version-script := libxen$(LIBNAME).map.tmp > > +endif > > Such a use of $(origin ...) is pretty fragile. Maybe better use ?= ? I'm not sure why I used $(origin), I've written that more than 6 month ago, but this way of using it is actually described in the manual, when documenting ?= but with a = instead of := . So, maybe I wanted to have ?= but with immediate evaluation rather than deferred. Maybe I had issue with $(version-script) evaluating to different values. If that ok, I'd like to keep using these runes, since ?:= doesn't exist. > > @@ -72,6 +77,10 @@ headers.lst: FORCE > > @{ set -e; $(foreach h,$(LIBHEADERS),echo $(h);) } > $@.tmp > > @$(call move-if-changed,$@.tmp,$@) > > > > +libxen$(LIBNAME).map.tmp: FORCE > > + echo 'VERS_$(MAJOR).$(MINOR) { global: *; };' >.$@.tmp > > + $(call move-if-changed,.$@.tmp,$@) > > Isn't this going to get in the way of your "build everything from root" > effort, where $@ will include a path? Also do we really need .tmp.tmp > files? Good call, I'll replace that with $(@D)/.$(@F), that will be one less thing to deal with, Also, I guess just adding a dot to the filename would be enough, and we would have `mv .libxen.map.tmp libxen.map.tmp`, and no more .tmp.tmp files. > > lib$(LIB_FILE_NAME).a: $(OBJS-y) > > Seeing this right adjacent in context - any reason you use libxen$(LIBNAME) > and not the same lib$(LIB_FILE_NAME) for the base file name? That was the name used before, I guess it would be fine to rename. I just need to set $(version-script) later as $(LIB_FILE_NAME) would not be defined yet. > > @@ -120,7 +129,7 @@ TAGS: > > clean:: > > rm -rf $(TARGETS) *~ $(DEPS_RM) $(OBJS-y) $(PIC_OBJS) > > rm -f lib$(LIB_FILE_NAME).so.$(MAJOR).$(MINOR) > > lib$(LIB_FILE_NAME).so.$(MAJOR) > > - rm -f headers.chk headers.lst > > + rm -f headers.chk headers.lst libxen*.map.tmp > > If I hadn't checked, I'd have assumed that *.tmp are removed without > being named explicitly. So yes, I see the need for the addition, but then > I wonder why you don't also remove the .*.tmp.tmp file, which may be left > around if the build is interrupted at exactly the "right" time. I forgot because $(move-if-changed) remove the temporary file. I'll clean the .*.tmp file. Thanks, -- Anthony PERARD
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |