Skip to content
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

[libcxx] Require qemu-system-arm for armv7m builder #77067

Merged
merged 1 commit into from Jan 8, 2024

Conversation

DavidSpickett
Copy link
Collaborator

And add a check in the python script that the binary given to --qemu actually exists. Otherwise you get a generic Python error:

 # .---command stderr------------
 # | Traceback (most recent call last):
 # |   File "/home/david.spickett/modules-llvm-project/libcxx/utils/qemu_baremetal.py", line 70, in <module>
 # |     exit(main())
 # |   File "/home/david.spickett/modules-llvm-project/libcxx/utils/qemu_baremetal.py", line 66, in main
 # |     os.execvp(qemu_commandline[0], qemu_commandline)
 # |   File "/usr/lib/python3.8/os.py", line 568, in execvp
 # |     _execvpe(file, args)
 # |   File "/usr/lib/python3.8/os.py", line 610, in _execvpe
 # |     raise last_exc
 # |   File "/usr/lib/python3.8/os.py", line 601, in _execvpe
 # |     exec_func(fullname, *argrest)
 # | FileNotFoundError: [Errno 2] No such file or directory
 # `-----------------------------
 # error: command failed with exit status: 1

When it tries to run the entire command later.

For the builder, it's only ever going to use qemu-system-arm so error at config time if it's not there.

And add a check in the python script that the binary given to
`--qemu` actually exists. Otherwise you get a generic Python error:
```
 # .---command stderr------------
 # | Traceback (most recent call last):
 # |   File "/home/david.spickett/modules-llvm-project/libcxx/utils/qemu_baremetal.py", line 70, in <module>
 # |     exit(main())
 # |   File "/home/david.spickett/modules-llvm-project/libcxx/utils/qemu_baremetal.py", line 66, in main
 # |     os.execvp(qemu_commandline[0], qemu_commandline)
 # |   File "/usr/lib/python3.8/os.py", line 568, in execvp
 # |     _execvpe(file, args)
 # |   File "/usr/lib/python3.8/os.py", line 610, in _execvpe
 # |     raise last_exc
 # |   File "/usr/lib/python3.8/os.py", line 601, in _execvpe
 # |     exec_func(fullname, *argrest)
 # | FileNotFoundError: [Errno 2] No such file or directory
 # `-----------------------------
 # error: command failed with exit status: 1
```
When it tries to run the entire command later.

For the builder, it's only ever going to use qemu-system-arm
so error at config time if it's not there.
@DavidSpickett DavidSpickett requested a review from a team as a code owner January 5, 2024 09:55
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jan 5, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 5, 2024

@llvm/pr-subscribers-libcxx

Author: David Spickett (DavidSpickett)

Changes

And add a check in the python script that the binary given to --qemu actually exists. Otherwise you get a generic Python error:

 # .---command stderr------------
 # | Traceback (most recent call last):
 # |   File "/home/david.spickett/modules-llvm-project/libcxx/utils/qemu_baremetal.py", line 70, in &lt;module&gt;
 # |     exit(main())
 # |   File "/home/david.spickett/modules-llvm-project/libcxx/utils/qemu_baremetal.py", line 66, in main
 # |     os.execvp(qemu_commandline[0], qemu_commandline)
 # |   File "/usr/lib/python3.8/os.py", line 568, in execvp
 # |     _execvpe(file, args)
 # |   File "/usr/lib/python3.8/os.py", line 610, in _execvpe
 # |     raise last_exc
 # |   File "/usr/lib/python3.8/os.py", line 601, in _execvpe
 # |     exec_func(fullname, *argrest)
 # | FileNotFoundError: [Errno 2] No such file or directory
 # `-----------------------------
 # error: command failed with exit status: 1

When it tries to run the entire command later.

For the builder, it's only ever going to use qemu-system-arm so error at config time if it's not there.


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

2 Files Affected:

  • (modified) libcxx/cmake/caches/Armv7M-picolibc.cmake (+1-1)
  • (modified) libcxx/utils/qemu_baremetal.py (+6)
diff --git a/libcxx/cmake/caches/Armv7M-picolibc.cmake b/libcxx/cmake/caches/Armv7M-picolibc.cmake
index 91cc32fd376e30..3ab80b960ed44a 100644
--- a/libcxx/cmake/caches/Armv7M-picolibc.cmake
+++ b/libcxx/cmake/caches/Armv7M-picolibc.cmake
@@ -39,4 +39,4 @@ set(LIBUNWIND_ENABLE_THREADS OFF CACHE BOOL "")
 set(LIBUNWIND_IS_BAREMETAL ON CACHE BOOL "")
 set(LIBUNWIND_REMEMBER_HEAP_ALLOC ON CACHE BOOL "")
 set(LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
-find_program(QEMU_SYSTEM_ARM qemu-system-arm)
+find_program(QEMU_SYSTEM_ARM qemu-system-arm REQUIRED)
diff --git a/libcxx/utils/qemu_baremetal.py b/libcxx/utils/qemu_baremetal.py
index aaf5b84489066d..126031bbb19c49 100755
--- a/libcxx/utils/qemu_baremetal.py
+++ b/libcxx/utils/qemu_baremetal.py
@@ -16,6 +16,7 @@
 import argparse
 import os
 import sys
+import shutil
 
 
 def main():
@@ -32,8 +33,13 @@ def main():
     parser.add_argument("test_binary")
     parser.add_argument("test_args", nargs=argparse.ZERO_OR_MORE, default=[])
     args = parser.parse_args()
+
+    if not shutil.which(args.qemu):
+        sys.exit(f"Failed to find QEMU binary from --qemu value: '{args.qemu}'")
+
     if not os.path.exists(args.test_binary):
         sys.exit(f"Expected argument to be a test executable: '{args.test_binary}'")
+
     qemu_commandline = [
         args.qemu,
         "-chardev",

Copy link
Contributor

@domin144 domin144 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
good error messages are helpful

Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM!

@DavidSpickett DavidSpickett merged commit acbb491 into llvm:main Jan 8, 2024
44 checks passed
@DavidSpickett DavidSpickett deleted the libcxx-qemu branch January 8, 2024 08:55
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
And add a check in the python script that the binary given to `--qemu`
actually exists. Otherwise you get a generic Python error:
```
 # .---command stderr------------
 # | Traceback (most recent call last):
 # |   File "/home/david.spickett/modules-llvm-project/libcxx/utils/qemu_baremetal.py", line 70, in <module>
 # |     exit(main())
 # |   File "/home/david.spickett/modules-llvm-project/libcxx/utils/qemu_baremetal.py", line 66, in main
 # |     os.execvp(qemu_commandline[0], qemu_commandline)
 # |   File "/usr/lib/python3.8/os.py", line 568, in execvp
 # |     _execvpe(file, args)
 # |   File "/usr/lib/python3.8/os.py", line 610, in _execvpe
 # |     raise last_exc
 # |   File "/usr/lib/python3.8/os.py", line 601, in _execvpe
 # |     exec_func(fullname, *argrest)
 # | FileNotFoundError: [Errno 2] No such file or directory
 # `-----------------------------
 # error: command failed with exit status: 1
```
When it tries to run the entire command later.

For the builder, it's only ever going to use qemu-system-arm so error at
config time if it's not there.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants