[PATCH] D118468: [cross-project-tests] XFAIL llgdb-tests when gdb can't read clang's DWARF

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.

SGTM with a couple of nits. From the discussion on discourse, I think it's a deliberate choice that there's going to be incompatible configurations out there, in which case as you say the least-worst-option is xfailing these tests.

In addition, I think it'll be worth printing a warning to the developer when the tests start running that because their gdb version is out of date, they won't get full test coverage. That gives developers a sporting chance to spot the problem, otherwise we're silently suppressing information.



================
Comment at: cross-project-tests/lit.cfg.py:212-215
+  try:
+    gdb_vers_lines = subprocess.check_output(['gdb', '--version']).decode().splitlines()
+  except FileNotFoundError:
+    return None # No gdb found.
----------------
Would this benefit from a pokemon exception handler? Given that we're calling another process, there's a nonzero possibility we'll get a CalledProcessError or some other unexpected behaviour. I'd suggest that if gdb itself is broken, we should catch the resulting exceptions here, return None, and let the gdb tests run and possibly fail. IMO, that's better than presenting an exception in llvm-lit to the developer.


================
Comment at: cross-project-tests/lit.cfg.py:220
+    return None
+  return gdb_vers_lines[0].strip().partition('GNU gdb (GDB) ')[2]
+
----------------
Having the same length-check for `[2]` as on the earlier few lines would future proof this code.


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

https://reviews.llvm.org/D118468