On Wed, Nov 11, 2015 at 10:02:50PM +0000, Petr Hosek wrote:
> Attached is an update version which doesn't use VPATH and supports both
> in-tree and out-of-tree builds.

Is there a reason you prefer not to use VPATH?

> One way to improve this even further would be to completely avoid the
> mkasmdep and instead include the architecture specific object files into
> the list of objects directly (filtering out the generic versions). Would
> that be a preferred way?

My intent is to remove the *.sub system entirely. The current idea is
to replace it with make fragments in the arch dirs that add the
implicit rules for whatever subarch dirs should be searched.

> From 1336bc5c1eddc43214559712f85702efba8aa864 Mon Sep 17 00:00:00 2001
> From: Petr Hosek <phosek@chromium.org>
> Date: Thu, 5 Nov 2015 14:51:50 -0800
> Subject: [PATCH] suport out-of-tree build
>
> this change add support for building musl outside of the source
> tree. the implementation is similar to autotools where running
> configure in a different folder creates config.mak in the current
> working directory and symlinks the makefile, which contains the
> logic for creating all necessary directories and resolving paths
> relative to the source directory. based on patch by Szabolcs Nagy.

Some quick comments here, and then more inline below:

Assuming this all works, it looks to me like you've done a remarkable
job of preserving existing behavior while making out-of-tree work.
What I'd really like to do, though, is figure out what parts of the
current build system are "unclean" for out-of-tree and improve them so
that we can simplify the build system at the same time (or at least
keep the complexity around the same level). For instance eliminating
all generated files from directories that contain non-generated files
would probably be good idea. I'm not asking you to do things like that
right away, more for input on what changes would actually be good for
achieving this so we can discuss them.

> ---
>  Makefile  | 93 +++++++++++++++++++++++++++++++++++++++++----------------------
>  configure | 24 ++++++++++++++---
>  2 files changed, 82 insertions(+), 35 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 2b21015..a022411 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -8,6 +8,7 @@
>  # Do not make changes here.
>  #
>
> +srcdir = .
>  exec_prefix = /usr/local
>  bindir = $(exec_prefix)/bin
>
> @@ -16,12 +17,24 @@ includedir = $(prefix)/include
>  libdir = $(prefix)/lib
>  syslibdir = /lib
>
> -SRCS = $(sort $(wildcard src/*/*.c arch/$(ARCH)/src/*.c))
> -OBJS = $(SRCS:.c=.o)
> +SRCS = $(sort $(wildcard $(srcdir)/src/*/*.c $(srcdir)/arch/$(ARCH)/src/*.c))
> +OBJS = $(SRCS:$(srcdir)/%.c=%.o)
>  LOBJS = $(OBJS:.o=.lo)
>  GENH = include/bits/alltypes.h
>  GENH_INT = src/internal/version.h
> -IMPH = src/internal/stdio_impl.h src/internal/pthread_impl.h src/internal/libc.h
> +IMPH = $(addprefix $(srcdir)/, src/internal/stdio_impl.h src/internal/pthread_impl.h src/internal/libc.h)
> +
> +SRCS_SUB = $(wildcard $(srcdir)/src/*/$(ARCH)$(ASMSUBARCH)/*.sub)
> +OBJS_SUB = $(foreach s,$(SRCS_SUB),$(dir $(patsubst $(srcdir)/%/,%,$(dir $(s))))$(notdir $(s:.sub=.o)))
> +LOBJS_SUB = $(OBJS_SUB:.o=.lo)
> +
> +SRCS_S = $(wildcard $(srcdir)/src/*/$(ARCH)*/*.s)
> +OBJS_S = $(foreach s,$(SRCS_S),$(dir $(patsubst $(srcdir)/%/,%,$(dir $(s))))$(notdir $(s:.s=.o)))
> +LOBJS_S = $(OBJS_S:.o=.lo)
> +
> +OBJS_C = $(filter-out $(OBJS_S) $(OBJS_SUB), $(OBJS))
> +LOBJS_C = $(OBJS_C:.o=.lo)
> +OBJS_C += crt/crt1.o crt/Scrt1.o crt/rcrt1.o crt/crti.o crt/crtn.o
>
>  LDFLAGS =
>  LDFLAGS_AUTO =
> @@ -32,7 +45,7 @@ CFLAGS_AUTO = -Os -pipe
>  CFLAGS_C99FSE = -std=c99 -ffreestanding -nostdinc
>
>  CFLAGS_ALL = $(CFLAGS_C99FSE)
> -CFLAGS_ALL += -D_XOPEN_SOURCE=700 -I./arch/$(ARCH) -I./src/internal -I./include
> +CFLAGS_ALL += -D_XOPEN_SOURCE=700 -I$(srcdir)/arch/$(ARCH) $(sort -I./src/internal -I$(srcdir)/src/internal) $(sort -I./include -I$(srcdir)/include)

I'm confused about why you are applying $(sort...) to include paths.
It seems like the resulting order may differ depending on $(srcdir).
Maybe this doesn't matter, but I'd rather not rely on that.

> +SRC_DIRS = $(sort $(dir $(OBJS) $(ALL_LIBS) $(ALL_TOOLS)) arch/$(ARCH)/bits/ crt/ include/ src/internal/)
> +
>  WRAPCC_GCC = gcc
>  WRAPCC_CLANG = clang
>
> @@ -64,6 +80,17 @@ LDSO_PATHNAME = $(syslibdir)/ld-musl-$(ARCH)$(SUBARCH).so.1
>
>  all: $(ALL_LIBS) $(ALL_TOOLS)
>
> +$(ALL_LIBS): | lib/
> +$(ALL_TOOLS): | tools/
> +$(CRT_LIBS:lib/%=crt/%): | crt/
> +$(OBJS) $(LOBJS): | $(sort $(dir $(OBJS)))
> +$(GENH): | arch/$(ARCH)/bits/
> +$(GENH_INT): | src/internal/
> +include/bits: | include
> +
> +$(SRC_DIRS):
> +     mkdir -p $@

This all looks nice.

> @@ -82,26 +109,24 @@ include/bits:
>       @test "$(ARCH)" || { echo "Please set ARCH in config.mak before running make." ; exit 1 ; }
>       ln -sf ../arch/$(ARCH)/bits $@
>
> -include/bits/alltypes.h.in: include/bits
> -
> -include/bits/alltypes.h: include/bits/alltypes.h.in include/alltypes.h.in tools/mkalltypes.sed
> -     sed -f tools/mkalltypes.sed include/bits/alltypes.h.in include/alltypes.h.in > $@
> +include/bits/alltypes.h: $(srcdir)/arch/$(ARCH)/bits/alltypes.h.in $(srcdir)/include/alltypes.h.in $(srcdir)/tools/mkalltypes.sed include/bits
> +     sed -f $(srcdir)/tools/mkalltypes.sed $(srcdir)/arch/$(ARCH)/bits/alltypes.h.in $(srcdir)/include/alltypes.h.in > $@
>
> -src/internal/version.h: $(wildcard VERSION .git)
> -     printf '#define VERSION "%s"\n' "$$(sh tools/version.sh)" > $@
> +src/internal/version.h: $(wildcard $(srcdir)/VERSION $(srcdir)/.git)
> +     printf '#define VERSION "%s"\n' "$$(cd $(srcdir); sh tools/version.sh)" > $@
>
>  src/internal/version.lo: src/internal/version.h
>
> -crt/rcrt1.o src/ldso/dlstart.lo src/ldso/dynlink.lo: src/internal/dynlink.h arch/$(ARCH)/reloc.h
> +crt/rcrt1.o src/ldso/dlstart.lo src/ldso/dynlink.lo: $(srcdir)/src/internal/dynlink.h $(srcdir)/arch/$(ARCH)/reloc.h
>
> -crt/crt1.o crt/Scrt1.o crt/rcrt1.o src/ldso/dlstart.lo: $(wildcard arch/$(ARCH)/crt_arch.h)
> +crt/crt1.o crt/scrt1.o crt/rcrt1.o src/ldso/dlstart.lo: $(wildcard $(srcdir)/arch/$(ARCH)/crt_arch.h)

Just a note: this $(wildcard...) is obsolete. crt_arch.h is now mandatory.

>  MEMOPS_SRCS = src/string/memcpy.c src/string/memmove.c src/string/memcmp.c src/string/memset.c
>  $(MEMOPS_SRCS:%.c=%.o) $(MEMOPS_SRCS:%.c=%.lo): CFLAGS_ALL += $(CFLAGS_MEMOPS)
> @@ -119,34 +144,35 @@ $(CRT_LIBS:lib/%=crt/%): CFLAGS_ALL += -DCRT
>  # force the corresponding object file to be rebuilt, even if the implicit
>  # rule below goes indirectly through a .sub file.
>  define mkasmdep
> -$(dir $(patsubst %/,%,$(dir $(1))))$(notdir $(1:.s=.o)): $(1)
> +$(dir $(patsubst $(srcdir)/%/,%,$(dir $(1))))$(notdir $(1:.s=.o)): $(1)
> +$(dir $(patsubst $(srcdir)/%/,%,$(dir $(1))))$(notdir $(1:.s=.lo)): $(1)
>  endef
> -$(foreach s,$(wildcard src/*/$(ARCH)*/*.s),$(eval $(call mkasmdep,$(s))))
> +$(foreach s,$(wildcard $(srcdir)/src/*/$(ARCH)*/*.s),$(eval $(call mkasmdep,$(s))))

Was this missing the dep rules for .lo files?

> -%.o: $(ARCH)$(ASMSUBARCH)/%.sub
> +$(OBJS_SUB): %.o:
>       $(CC) $(CFLAGS_ALL_STATIC) -c -o $@ $(dir $<)$(shell cat $<)
>
> -%.o: $(ARCH)/%.s
> +$(OBJS_S): %.o:
>       $(AS_CMD) $(CFLAGS_ALL_STATIC)
>
> -%.o: %.c $(GENH) $(IMPH)
> +$(OBJS_C): %.o: $(srcdir)/%.c $(GENH) $(IMPH)
>       $(CC) $(CFLAGS_ALL_STATIC) -c -o $@ $<
>
> -%.lo: $(ARCH)$(ASMSUBARCH)/%.sub
> +$(LOBJS_SUB): %.lo:
>       $(CC) $(CFLAGS_ALL_SHARED) -c -o $@ $(dir $<)$(shell cat $<)
>
> -%.lo: $(ARCH)/%.s
> +$(LOBJS_S): %.lo:
>       $(AS_CMD) $(CFLAGS_ALL_SHARED)
>
> -%.lo: %.c $(GENH) $(IMPH)
> +$(LOBJS_C): %.lo: $(srcdir)/%.c $(GENH) $(IMPH)
>       $(CC) $(CFLAGS_ALL_SHARED) -c -o $@ $<

I don't understand these rules with two :'s. I assume it's some trick
I don't yet know. But in the case of the %.s ones, the new rules have
no %.s in them... this looks wrong, no?