Re: [PATCH v2] Makefile.llvm: simplify LLVM build

[Nathan Chancellor]

On Mon, Mar 30, 2020 at 11:58:19AM -0700, Nick Desaulniers wrote:
> On Sat, Mar 28, 2020 at 6:57 PM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote:
> >
> > I also had planned to provide a single switch to change
> > all the tool defaults to LLVM.
> >
> > So, supporting 'LLVM' is fine, but I'd rather want this
> > look symmetrical, and easy to understand.
> >
> > CPP        = $(CC) -E
> > ifneq ($(LLVM),)
> 
> Yes, a simple if statement is much simpler than the overly complex patch I had.
> 
> > CC         = $(LLVM_DIR)clang
> 
> Do we need $LLVM_DIR? Shouldn't users just have that in their $PATH?
> 
> Also, I think we need to support suffixed binaries, as debian
> distributes these with version suffixes, as Nathan points out.  Or do
> the debian packages install suffixed binaries AND path versioned
> non-suffixed binaries?

I think the idea here is that ultimately, the suffixed versions of clang
that Debian has in /usr/bin are symlinks to binaries in
/usr/lib/llvm-#/bin; as a result, a user could say
LLVM_DIR=/usr/lib/llvm-#/bin/ and all of those tools would be picked up
automatically. I am not really sure what is better.

I'll try to have some other comments by later today/tonight.

> > LD         = $(LLVM_DIR)ld.lld
> > AR         = $(LLVM_DIR)llvm-ar
> > NM         = $(LLVM_DIR)llvm-nm
> > OBJCOPY    = $(LLVM_DIR)llvm-objcopy
> > OBJDUMP    = $(LLVM_DIR)llvm-objdump
> > READELF    = $(LLVM_DIR)llvm-readelf
> > OBJSIZE    = $(LLVM_DIR)llvm-size
> > STRIP      = $(LLVM_DIR)llvm-strip
> > else
> > CC         = $(CROSS_COMPILE)gcc
> > LD         = $(CROSS_COMPILE)ld
> > AR         = $(CROSS_COMPILE)ar
> > NM         = $(CROSS_COMPILE)nm
> > OBJCOPY    = $(CROSS_COMPILE)objcopy
> > OBJDUMP    = $(CROSS_COMPILE)objdump
> > READELF    = $(CROSS_COMPILE)readelf
> > OBJSIZE    = $(CROSS_COMPILE)size
> > STRIP      = $(CROSS_COMPILE)strip
> > endif
> >
> >
> >
> > I attached two patches.
> > Comments appreciated.
> 
> I'm not sure the second one that recommends changing cc/c++ is the way
> to go; I think it might harm hermeticity.

Agreed. I do not modify my host system at all for this project, just
relying on PATH modification. In theory, we can still override HOSTCC
and HOSTCXX but that would defeat the purpose of that patch.

Cheers,
Nathan