Skip to content

[lldb][test] Add --target-os argument to dotest.py #93887

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

dzhidzhoev
Copy link
Member

Makefile.rules uses HOST_OS and OS variables for determining host and target OSes for LLDB API tests compilation.
HOST_OS is determined using uname -s command, and OS is defaulted to HOST_OS.
This argument allows to pass the target OS to the make invocation for the case of Windows-to-Linux cross-testing.

The argument can be set using LLDB_TEST_USER_ARGS argument:

cmake ...
-DLLDB_TEST_USER_ARGS="...;--target-os;Linux;..."
...

Makefile.rules uses HOST_OS and OS variables for determining host and
target OSes for LLDB API tests compilation.
HOST_OS is determined using `uname -s` command, and OS is defaulted to
HOST_OS.
This argument allows to pass the target OS to the make invocation for
the case of Windows-to-Linux cross-testing.

The argument can be set using `LLDB_TEST_USER_ARGS` argument:
```
cmake ...
-DLLDB_TEST_USER_ARGS="...;--target-os;Linux;..."
...
```
@llvmbot
Copy link
Member

llvmbot commented May 30, 2024

@llvm/pr-subscribers-lldb

Author: Vladislav Dzhidzhoev (dzhidzhoev)

Changes

Makefile.rules uses HOST_OS and OS variables for determining host and target OSes for LLDB API tests compilation.
HOST_OS is determined using uname -s command, and OS is defaulted to HOST_OS.
This argument allows to pass the target OS to the make invocation for the case of Windows-to-Linux cross-testing.

The argument can be set using LLDB_TEST_USER_ARGS argument:

cmake ...
-DLLDB_TEST_USER_ARGS="...;--target-os;Linux;..."
...

Full diff: https://github.com/llvm/llvm-project/pull/93887.diff

5 Files Affected:

  • (modified) lldb/packages/Python/lldbsuite/test/builders/builder.py (+9)
  • (modified) lldb/packages/Python/lldbsuite/test/configuration.py (+1)
  • (modified) lldb/packages/Python/lldbsuite/test/dotest.py (+3)
  • (modified) lldb/packages/Python/lldbsuite/test/dotest_args.py (+9)
  • (modified) lldb/packages/Python/lldbsuite/test/lldbtest.py (+4)
diff --git a/lldb/packages/Python/lldbsuite/test/builders/builder.py b/lldb/packages/Python/lldbsuite/test/builders/builder.py
index 21ea3530e24fc..84ae7655e2b1e 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/builder.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/builder.py
@@ -36,6 +36,13 @@ def getArchCFlags(self, architecture):
         """Returns the ARCH_CFLAGS for the make system."""
         return []
 
+    def getTargetOsFlag(self, target_os):
+        if target_os is None:
+            target_os = configuration.target_os
+        if target_os is None:
+            return []
+        return ["OS=%s" % target_os]
+
     def getMake(self, test_subdir, test_name):
         """Returns the invocation for GNU make.
         The first argument is a tuple of the relative path to the testcase
@@ -171,6 +178,7 @@ def getBuildCommand(
         testdir=None,
         testname=None,
         make_targets=None,
+        target_os=None,
     ):
         debug_info_args = self._getDebugInfoArgs(debug_info)
         if debug_info_args is None:
@@ -182,6 +190,7 @@ def getBuildCommand(
             debug_info_args,
             make_targets,
             self.getArchCFlags(architecture),
+            self.getTargetOsFlag(target_os),
             self.getArchSpec(architecture),
             self.getCCSpec(compiler),
             self.getExtraMakeArgs(),
diff --git a/lldb/packages/Python/lldbsuite/test/configuration.py b/lldb/packages/Python/lldbsuite/test/configuration.py
index dbd4a2d72a15d..2418eb4c7cde3 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -39,6 +39,7 @@
 count = 1
 
 # The 'arch' and 'compiler' can be specified via command line.
+target_os = None
 arch = None
 compiler = None
 dsymutil = None
diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py b/lldb/packages/Python/lldbsuite/test/dotest.py
index 2e537e3fd3ce0..a9dc6361af4d6 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -312,6 +312,9 @@ def parseOptionsAndInitTestdirs():
             logging.error("No SDK found with the name %s; aborting...", args.apple_sdk)
             sys.exit(-1)
 
+    if args.target_os:
+        configuration.target_os = args.target_os
+
     if args.arch:
         configuration.arch = args.arch
     else:
diff --git a/lldb/packages/Python/lldbsuite/test/dotest_args.py b/lldb/packages/Python/lldbsuite/test/dotest_args.py
index 8b00c7a4d56e7..905b263fff2b5 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -31,6 +31,15 @@ def create_parser():
 
     # C and Python toolchain options
     group = parser.add_argument_group("Toolchain options")
+    group.add_argument(
+        "--target-os",
+        metavar="target_os",
+        dest="target_os",
+        default="",
+        help=textwrap.dedent(
+            """Specify target os for test compilation. This is useful for cross-compilation."""
+        ),
+    )
     group.add_argument(
         "-A",
         "--arch",
diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 1ad8ab6e6e462..2d219c39ad917 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1454,11 +1454,15 @@ def getDebugInfo(self):
     def build(
         self,
         debug_info=None,
+        target_os=None,
         architecture=None,
         compiler=None,
         dictionary=None,
         make_targets=None,
     ):
+        if not target_os and configuration.target_os:
+            target_os = configuration.target_os
+
         """Platform specific way to build binaries."""
         if not architecture and configuration.arch:
             architecture = configuration.arch

@labath
Copy link
Collaborator

labath commented May 31, 2024

Would it be possible to pass this automatically from the python process? Basically set OS to the value of lldbplatformutil.getPlatform() ? We could keep the existing exception for Android, although I don't think that anyone tests Android these days (your PRs sort of demonstrate that)...

@JDevlieghere
Copy link
Member

Would it be possible to pass this automatically from the python process? Basically set OS to the value of lldbplatformutil.getPlatform() ? We could keep the existing exception for Android, although I don't think that anyone tests Android these days (your PRs sort of demonstrate that)...

+1, I'd prefer to have the logic centralized in one place (i.e. Python), pass it explicitly and remove the code in Makefile.rules that uses uname.

@labath
Copy link
Collaborator

labath commented Jun 3, 2024

Agreed. The reason code is looking like it does is because at some point in (now, pretty distant) past it was possible to cd into the test directory, type make and get the test binary out. Among other problems, this has caused duplicate code for determining various properties of the build between python and make. Since we no longer support these workflow anyway, there's no reason to keep the makefile code.

@dzhidzhoev
Copy link
Member Author

Closed in favor of #96654

@dzhidzhoev dzhidzhoev closed this Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants