[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABBv4TZ6xny9ZNT8ctpcWaMTcGfC3bECM1Mj3Tq00mQ-LK0fwg@mail.gmail.com>
Date: Mon, 30 Nov 2015 01:25:24 +0000
From: Petr Hosek <phosek@...omium.org>
To: musl@...ts.openwall.com
Subject: Re: Support for out-of-tree build
I'm sorry about the delay, I've been away last week. I've incorporated all
your code review comments. However, I haven't replaced the use of Makefile
symlink with make -C $(srcdir) as discussed on IRC since it's not clear
whether that's the way we want to go.
On Thu, Nov 19, 2015 at 8:10 PM Rich Felker <dalias@...c.org> wrote:
> Let's remove objdir entirely and just use obj/.
>
Done.
> Not important at all, but REPLACED_OBJS might be a better name to
> signify to people reading the makefile what this is about.
>
Renamed.
> It looks like you're missing mkdir rules for everything but $(SRC_DIRS).
>
Those are the only directories which should be needed.
> 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.
>
Done.
> I suppose this comment was meant to be removed?
>
Yes, removed.
> 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?
>
That seem to work fine.
> > -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).
>
That was a mistake. I've added explicit rules for both crti.o and crtn.o.
> 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.
>
True, do you want me to remove all these files in the same or a separate
patch?
> > +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..
>
There is no particular reason, failing in case of existing Makefile sounds
like a better solution.
Content of type "text/html" skipped
View attachment "support-out-of-tree-build.patch" of type "text/x-patch" (13742 bytes)
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
