From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/8904 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: Support for out-of-tree build Date: Thu, 19 Nov 2015 23:09:55 -0500 Message-ID: <20151120040955.GV3818@brightrain.aerifal.cx> References: <20151117220141.GD3818@brightrain.aerifal.cx> <20151117230606.GJ18372@port70.net> <20151117232711.GG3818@brightrain.aerifal.cx> <20151117234330.GH3818@brightrain.aerifal.cx> <20151118001955.GI3818@brightrain.aerifal.cx> <20151118214452.GQ3818@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1447992617 5394 80.91.229.3 (20 Nov 2015 04:10:17 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 20 Nov 2015 04:10:17 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-8917-gllmg-musl=m.gmane.org@lists.openwall.com Fri Nov 20 05:10:15 2015 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1Zzd1a-0005Oq-HY for gllmg-musl@m.gmane.org; Fri, 20 Nov 2015 05:10:14 +0100 Original-Received: (qmail 9531 invoked by uid 550); 20 Nov 2015 04:10:11 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 9507 invoked from network); 20 Nov 2015 04:10:09 -0000 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:8904 Archived-At: Thanks! Have you tested it? I get: $ mkdir foo $ cd foo $ ../configure $ make make: *** No rule to make target `obj/include/bits', needed by `obj/include/bits/alltypes.h'. Stop. I think this means there's still some cruft using the old include/bits symlink rather than the proper pathname. See below: On Thu, Nov 19, 2015 at 12:22:09AM +0000, Petr Hosek wrote: > > > clean: > > > - rm -f crt/*.o > > > + rm -f $(objdir)/crt/*.o > > > rm -f $(OBJS) > > > rm -f $(LOBJS) > > > rm -f $(ALL_LIBS) lib/*.[ao] lib/*.so > > > > This can probably just be rm -rf obj, but perhaps we should wait to > > make that change. And seeing this, I'm mildly in favor of hard-coding > > obj/ (everywhere) rather than using $(objdir)/ simply because the > > latter is really dangerous if you override it on the command line with > > an unwanted setting (like make clean objdir=/). > > That's why I avoided using rm -rf $(objdir). I can imagine the use case for > setting $(objdir) to a different location, changing this to make it > hard-coded wouldn't be a problem if you prefer. Did you mean "can't imagine a use"? To me this looks roughly like a "second level of out-of-tree support" that's probably not useful, and could be error-prone e.g. if somebody tries to get clever and use objdir=., or could be dangerous (objdir=). I think it's best just to hard-code it. > > > + > > > +$(CRT_LIBS:lib/%=$(objdir)/crt/%): CFLAGS_ALL += -DCRT > > > > I'm not clear on whether crti.o/crtn.o end up in crt/ or crt/$(ARCH)/ > > but if it's the latter we need to handle this somehow. > > These end up in crt/ even in the current upstream implementation. I don't follow. In current upstream musl, crt*.o end up in crt/ and then get copied from there to lib/. But if, after the out-of-tree patch, some object files end up in crt/$(ARCH)/ instead of crt/, we need to make sure they make it to lib/ correctly. Now for comments on the new patch: > +objdir = obj Let's remove objdir entirely and just use obj/. > -SRCS = $(sort $(wildcard src/*/*.c arch/$(ARCH)/src/*.c)) > -OBJS = $(SRCS:.c=.o) > +BASE_SRCS = $(sort $(wildcard $(srcdir)/src/*/*.c $(srcdir)/arch/$(ARCH)/src/*.c)) > +BASE_OBJS = $(patsubst $(srcdir)/%.c,%.o,$(BASE_SRCS)) > +ARCH_SRCS = $(wildcard $(srcdir)/src/*/$(ARCH)/*.s $(srcdir)/src/*/$(ARCH)$(ASMSUBARCH)/*.sub) > +ARCH_OBJS = $(patsubst $(srcdir)/%.sub,%.o,$(patsubst $(srcdir)/%.s,%.o,$(ARCH_SRCS))) > +EXCLUDE_OBJS = $(patsubst $(srcdir)/%,%,$(subst /$(ARCH)$(ASMSUBARCH)/,/,$(subst /$(ARCH)/,/,$(patsubst $(srcdir)/%,%,$(ARCH_OBJS))))) > +OBJS = $(addprefix $(objdir)/, $(filter-out $(EXCLUDE_OBJS), $(BASE_OBJS)) $(ARCH_OBJS) $(SUB_OBJS)) Not important at all, but REPLACED_OBJS might be a better name to signify to people reading the makefile what this is about. > -ARCH_INCLUDES = $(wildcard arch/$(ARCH)/bits/*.h) > -ALL_INCLUDES = $(sort $(wildcard include/*.h include/*/*.h) $(GENH) $(ARCH_INCLUDES:arch/$(ARCH)/%=include/%)) > +ARCH_INCLUDES = $(wildcard $(srcdir)/arch/$(ARCH)/bits/*.h) > +INCLUDES = $(wildcard $(srcdir)/include/*.h $(srcdir)/include/*/*.h) > +ALL_INCLUDES = $(sort $(INCLUDES:$(srcdir)/%=%) $(GENH:$(objdir)/%=%) $(ARCH_INCLUDES:$(srcdir)/arch/$(ARCH)/%=include/%)) This is confusing, but probably no worse than what was already there. > EMPTY_LIB_NAMES = m rt pthread crypt util xnet resolv dl > EMPTY_LIBS = $(EMPTY_LIB_NAMES:%=lib/lib%.a) > @@ -64,10 +71,22 @@ LDSO_PATHNAME = $(syslibdir)/ld-musl-$(ARCH)$(SUBARCH).so.1 > > all: $(ALL_LIBS) $(ALL_TOOLS) > > +$(ALL_LIBS): | lib/ > +$(ALL_TOOLS): | tools/ > +$(CRT_LIBS:lib/%=$(objdir)/crt/%): | $(objdir)/crt/ > +$(OBJS) $(LOBJS): | $(sort $(dir $(OBJS))) > +$(GENH): | $(objdir)/include/bits/ > +$(GENH_INT): | $(objdir)/src/internal/ > + > +SRC_DIRS = $(sort $(dir $(ALL_LIBS) $(ALL_TOOLS) $(OBJS) $(GENH) $(GENH_INT)) $(addprefix $(objdir)/, crt/ include/)) > + > +$(SRC_DIRS): > + mkdir -p $@ > + It looks like you're missing mkdir rules for everything but $(SRC_DIRS). > -include/bits: > - @test "$(ARCH)" || { echo "Please set ARCH in config.mak before running make." ; exit 1 ; } > - ln -sf ../arch/$(ARCH)/bits $@ Removing the bits symlink is ok, but we need to preserve the logic that prevents trying to build without ARCH set. Perhaps something like: ifeq ($(ARCH),) $(error Please run configure or set ARCH in config.mak before running make) endif just after the inclusion of config.mak. > NOSSP_SRCS = $(wildcard crt/*.c) \ > src/env/__libc_start_main.c src/env/__init_tls.c \ > src/thread/__set_thread_area.c src/env/__stack_chk_fail.c \ > src/string/memset.c src/string/memcpy.c \ > src/ldso/dlstart.c src/ldso/dynlink.c > -$(NOSSP_SRCS:%.c=%.o) $(NOSSP_SRCS:%.c=%.lo): CFLAGS_ALL += $(CFLAGS_NOSSP) > +$(NOSSP_SRCS:%.c=$(objdir)/%.o) $(NOSSP_SRCS:%.c=$(objdir)/%.lo): CFLAGS_ALL += $(CFLAGS_NOSSP) This looks much better. > -$(CRT_LIBS:lib/%=crt/%): CFLAGS_ALL += -DCRT > +$(CRT_LIBS:lib/%=$(objdir)/crt/%): CFLAGS_ALL += -DCRT > > # This incantation ensures that changes to any subarch asm files will > # force the corresponding object file to be rebuilt, even if the implicit > # rule below goes indirectly through a .sub file. > +#$(dir $(patsubst %/,%,$(dir $(1))))$(notdir $(1:.s=.o)): $(1) I suppose this comment was meant to be removed? > -%.o: $(ARCH)$(ASMSUBARCH)/%.sub > +$(objdir)/%.o: $(srcdir)/%.sub > $(CC) $(CFLAGS_ALL_STATIC) -c -o $@ $(dir $<)$(shell cat $<) Not new in your patch, but I wonder if using $$(cat $<) would make more sense than $(shell cat $<) -- does the latter risk getting evaluated even when the rule isn't being run? > -lib/%.o: crt/%.o > +lib/%.o: $(objdir)/crt/%.o > cp $< $@ I don't see how this gets arch versions of crti.o/crtn.o. I think you must be producing the dummy ones from the empty .c files, which won't work. Actually once this is all done the dummy .c files there can probably be removed, or we could remove them right away to eliminate the need for another filter-out here (but I don't think that's really needed anyway since it's a small fixed set of files we can hard-code). BTW that made me realize one semantic change in how the new build system treats arch files: src/*/$(ARCH)/*.s (and .sub and whatever other types we add) will get used now even if there's no .c file by the same name in the parent directory. This actually looks to be really nice -- we can eliminate dummy .c files that are empty and only have them when the generic implementation works for some/most archs. We can also move the arch/$(ARCH)/src/* files to appropriate src/*/$(ARCH) locations based on what code they go with. > +test -f Makefile || ln -sf $srcdir/Makefile . Is there a reason you do it this way rather than checking for srcdir=.? The reason I ask is that existence of a stray makefile that's not a symlink to the musl makefile should probably be an error here rather than silently ignored. But maybe there are more things to consider.. Rich