[Lldb-commits] [PATCH] D85539: [lldb] Extend builder to pass the TRIPLE spec to Make

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D85539#2215387 <https://reviews.llvm.org/D85539#2215387>, @labath wrote:

> The part which bugs me about this is that the first thing that Makefile.rules will do with the triple we've painstakingly constructed here is it will decompose it into individual components, and then do some more matching on them, including running some xcrun commands, which are already being run here. I do think that doing this in python is a good idea (I believe the main reason a lot of this stuff is done in make is so that running "make" would just work, but we are way past that point now), but if we're going to do that in python, how about we move all of that to python? It seems the only effect of the TRIPLE argument is to add `-target $(TRIPLE) -m$(OS)-version-min=$(VERSION)` to CFLAGS. That should be easy to achieve using the information that `construct_triple` has available.

Sounds reasonable.

> In D85539#2203251 <https://reviews.llvm.org/D85539#2203251>, @JDevlieghere wrote:
>
>> In D85539#2203247 <https://reviews.llvm.org/D85539#2203247>, @friss wrote:
>>
>>> `lldbremote.py` looks awfully Apple-specific, yet it is going to be called from the generic code. Is that not an issue?
>>
>> It's not an issue because it checks for the Apple SDK, but I could hoist that out of the helper to make it more obvious.
>
> How about making an abstract/dummy `getTripleSpec` (or whatever) method in the base builder class. Then the darwin builder could override it do to its thing, and all of this code could reside in builder_darwin.py. If any other platform decides it needs to set the triple argument, then it's likely it will do that via some completely different method.

Even though this file is called "base", there's no inheritance going on here, it's just methods and `lldbtest.py` will import the appropriate module. I considered this too, but didn't feel like rewriting those files as a classes for this small change. But if we're going to pass the `TRIPLE`, `OS` and `VERSION` separately, we might as well.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85539/new/

https://reviews.llvm.org/D85539