[lldb] Extend builder to pass the TRIPLE spec to Make

[JDevlieghere]

Summary

This is needed to build the test binaries for iOS. This is part of upstreaming the last missing pieces to run the on-device test suite with the upstream repository.

[friss]

lldbremote.py looks awfully Apple-specific, yet it is going to be called from the generic code. Is that not an issue?

[JDevlieghere]

In 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.

[JDevlieghere]

• Simplify things a bit more
• Add a comment explaining that the triple spec is only relevant with an Apple SDK

[labath]

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.

In D85539#2203251, @JDevlieghere wrote:

In 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.

lldb/packages/Python/lldbsuite/test/lldbremote.py

38–40

when dealing with triples, I believe this field is usually called the "os", not "platform".

[JDevlieghere]

In 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, @JDevlieghere wrote:

In 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.

[labath]

In D85539#2216197, @JDevlieghere wrote:

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.

I'd just send the final ARCH_CFLAGS value. Then the Android.rules makefile could be ported to python as well.

[JDevlieghere]

Reimplement based on passing the ARCH_CFLAGS to Make

[JDevlieghere]

lldb/packages/Python/lldbsuite/builders/builder.py

224 ↗

(On Diff #286431)

This should've been part of D86174, I'll fix that before updating the patch/landing.

[jasonmolenda]

This looks good to me, getting any logic out of the Makefile templates that we can, that's a good improvement for maintainability.

[labath]

lldb/packages/Python/lldbsuite/builders/builder.py

201–202 ↗

(On Diff #286455)

Instead of a new method for each variable any subclass might want to set it would probably be better to just have an getExtraMakeArgs method that each subclass can return whatever it wants. ARCH_CFLAGS is fairly generic, so it may make sense to keep that separate, but DSYMUTIL and CODESIGN (and maybe SDKROOT) sound like they could go into the "extra" category.

[jankratochvil]

It broke the Debian buildbot: http://lab.llvm.org:8011/builders/lldb-x86_64-debian/builds/16095
And also Fedora (although there the commit looks as a different one): http://lab.llvm.org:8014/builders/lldb-x86_64-fedora?numbuilds=10000

lldb/packages/Python/lldbsuite/test/lldbremote.py

+from lldbsuite.test import configuration
+import re
+import subprocess
+
+REMOTE_PLATFORM_NAME_RE = re.compile(r"^remote-(.+)$")
+
+def remote_platform_to_triple_os_and_sdk_name(platform_name):
+    match = REMOTE_PLATFORM_NAME_RE.match(platform_name)
+    if match is None:
+        return None
+
+    triple_platform = match.group(1)
+    if triple_platform == "ios":
+        # The iOS SDK does not follow the platform name.
+        sdk_name = "iphoneos"
+    else:
+        # All other SDKs match the platform name.
+        sdk_name = triple_platform
+    return triple_platform, sdk_name
+
+
+def construct_triple(platform_name, apple_sdk, architecture):
+    """Return a fabricated triple for a given platform and architecture."""
+    if platform_name is None and architecture is None and apple_sdk is None:
+        return None
+
+    if architecture is None:
+        architecture = subprocess.check_output(['machine'])
+
+    if platform_name:
+        # Pull the platform name out of the remote platform description.
+        triple_platform, sdk_name = remote_platform_to_triple_os_and_sdk_name(
+            platform_name)
+        if triple_platform is None or sdk_name is None:
+            return None
+    elif apple_sdk:
+        triple_platform = apple_sdk[:apple_sdk.find('.')]
+        if triple_platform == 'iphoneos':
+            triple_platform = 'ios'
+        sdk_name = apple_sdk
+    else:
+        return None
+
+    # Grab the current SDK version number, which will be used in the triple.
+    version_output = subprocess.check_output(
+        ["xcrun", "--sdk", sdk_name, "--show-sdk-version"]).rstrip().decode('utf-8')
+    if version_output is None:
+        return None
+
+    return architecture + "-apple-" + triple_platform + version_output

lldb/packages/Python/lldbsuite/test/plugins/builder_base.py

 import os
 import platform
 import subprocess
 import sys
 
 # Our imports
 import lldbsuite.test.lldbtest as lldbtest
 import lldbsuite.test.lldbutil as lldbutil
+import lldbsuite.test.lldbremote as lldbremote
 from lldbsuite.test import configuration
 from lldbsuite.test_event import build_exception
 
 
 def getArchitecture():
     """Returns the architecture in effect the test suite is running with."""
     return configuration.arch if configuration.arch else ""
 
 
+def getEffectiveArchitecture(architecture):
+    """Return the given architecture if specified, or the value of the architecture in effect."""
+    if architecture is not None:
+        return architecture
+    else:
+        return getArchitecture()
+
+
 def getCompiler():
     """Returns the compiler in effect the test suite is running with."""
     compiler = configuration.compiler if configuration.compiler else "clang"
     compiler = lldbutil.which(compiler)
     return os.path.abspath(compiler)
 
 
 def getArchFlag():
def getArchSpec(architecture):
     """
     arch = architecture if architecture else None
     if not arch and configuration.arch:
         arch = configuration.arch
 
     return ("ARCH=" + arch) if arch else ""
 
 
+def getTripleSpec(architecture):
+    """
+    Helper function to return the key-value string to specify the triple
+    used for the make system.
+    """
+    triple = lldbremote.construct_triple(
+        configuration.lldb_platform_name,
+        configuration.apple_sdk,
+        getEffectiveArchitecture(architecture))
+    return ("TRIPLE=" + triple) if triple else ""
+
+
 def getCCSpec(compiler):
     """
     Helper function to return the key-value string to specify the compiler
     used for the make system.
     """
     cc = compiler if compiler else None
     if not cc and configuration.compiler:
         cc = configuration.compiler
def buildDefault(
         dictionary=None,
         testdir=None,
         testname=None):
     """Build the binaries the default way."""
     commands = []
     commands.append(getMake(testdir, testname) +
                     ["all",
                      getArchSpec(architecture),
+                     getTripleSpec(architecture),
                      getCCSpec(compiler),
                      getDsymutilSpec(),
                      getSDKRootSpec(),
                      getModuleCacheSpec(),
                      getCmdLine(dictionary)])
 
     runBuildCommands(commands, sender=sender)
 
     # True signifies that we can handle building default.
     return True
 
 
 def buildDwarf(
         sender=None,
         architecture=None,
         compiler=None,
         dictionary=None,
         testdir=None,
         testname=None):
     """Build the binaries with dwarf debug info."""
     commands = []
     commands.append(getMake(testdir, testname) +
                     ["MAKE_DSYM=NO",
                      getArchSpec(architecture),
+                     getTripleSpec(architecture),
                      getCCSpec(compiler),
                      getDsymutilSpec(),
                      getSDKRootSpec(),
                      getModuleCacheSpec(),
                      getCmdLine(dictionary)])
 
     runBuildCommands(commands, sender=sender)
     # True signifies that we can handle building dwarf.
     return True
 
 
 def buildDwo(
         sender=None,
         architecture=None,
         compiler=None,
         dictionary=None,
         testdir=None,
         testname=None):
     """Build the binaries with dwarf debug info."""
     commands = []
     commands.append(getMake(testdir, testname) +
                     ["MAKE_DSYM=NO",
                      "MAKE_DWO=YES",
                      getArchSpec(architecture),
+                     getTripleSpec(architecture),
                      getCCSpec(compiler),
                      getDsymutilSpec(),
                      getSDKRootSpec(),
                      getModuleCacheSpec(),
                      getCmdLine(dictionary)])
 
     runBuildCommands(commands, sender=sender)
     # True signifies that we can handle building dwo.
     return True
 
 
 def buildGModules(
         sender=None,
         architecture=None,
         compiler=None,
         dictionary=None,
         testdir=None,
         testname=None):
     """Build the binaries with dwarf debug info."""
     commands = []
     commands.append(getMake(testdir, testname) +
                     ["MAKE_DSYM=NO",
                      "MAKE_GMODULES=YES",
                      getArchSpec(architecture),
+                     getTripleSpec(architecture),
                      getCCSpec(compiler),
                      getDsymutilSpec(),
                      getSDKRootSpec(),
                      getModuleCacheSpec(),
                      getCmdLine(dictionary)])
 
     runBuildCommands(commands, sender=sender)
     # True signifies that we can handle building with gmodules.
     return True
 
 
 def cleanup(sender=None, dictionary=None):
     """Perform a platform-specific cleanup after the test."""
     return True

lldb/packages/Python/lldbsuite/test/plugins/builder_darwin.py

def buildDsym(
         dictionary=None,
         testdir=None,
         testname=None):
     """Build the binaries with dsym debug info."""
     commands = []
     commands.append(getMake(testdir, testname) +
                     ["MAKE_DSYM=YES",
                      getArchSpec(architecture),
+                     getTripleSpec(architecture),
                      getCCSpec(compiler),
                      getDsymutilSpec(),
                      getSDKRootSpec(),
                      getModuleCacheSpec(),
                      "all",
                      getCmdLine(dictionary)])
 
     runBuildCommands(commands, sender=sender)
 
     # True signifies that we can handle building dsym.
     return True