teemperor requested changes to this revision.
teemperor added inline comments.
This revision now requires changes to proceed.


================
Comment at: 
lldb/test/API/functionalities/dyld-launch-linux/TestDyldLaunchLinux.py:13
+
+    def test(self):
+        self.build()
----------------
Could we just make the test Linux-specific? I doubt we'll ever find a 
`/lib64/ld-linux-x86-64.so.2` on macOS. In theory you could also move the build 
behind the file-existence check, but then the test will PASS instead of 
UNSUPPORTED (and the later better fits the situation that this doesn't run 
because it isn't supported).

Also if you add `no_debug_info_test` then we only run this test once (which 
makes the test about 3x faster).

```
lang=Python
     @skipIf(oslist=no_match(['linux']))
     @no_debug_info_test
     def test(self):
```


================
Comment at: 
lldb/test/API/functionalities/dyld-launch-linux/TestDyldLaunchLinux.py:15
+        self.build()
+        exe = "/lib64/ld-linux-x86-64.so.2"
+        if(os.path.exists(exe)):
----------------
Could we make this based on a list of files? There will be people with slightly 
different paths/file names so I anyway anticipate that we get more candidates 
to check here:

```
lang=python
        candidates = [
            "/lib64/ld-linux-x86-64.so.2",
            "/usr/lib/ld-linux-x86-64.so.2"
        ]
        exe = next((c for c in candidates if os.path.exists(c)), None)
        if not exe:
            return
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108061

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to