Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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.