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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
