Make native debugger tests more stable in the CI - #13485
Conversation
ghost
mentioned this pull request
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
Looking good, should work much better in the long term.
Thanks for these fixes!
Can you use a more descriptive title please? We'd like our PR titles to be a very short summary of the work. How about
|
tmcgilchrist
changed the title
| - id: normal | ||
| name: normal | ||
| dependencies: texlive-latex-extra texlive-fonts-recommended hevea sass | ||
| dependencies: texlive-latex-extra texlive-fonts-recommended hevea sass gdb lldb python3-lldb-14 |
There was a problem hiding this comment.
Is the -14 necessary? Does python3-lldb work? (it would simplify updating when gha moves to a newer Ubuntu)
There was a problem hiding this comment.
This should become just gdb lldb when we start using Ubuntu 24.04.
| run: | | ||
| sudo apt-get update -y && sudo apt-get install -y ${{ matrix.dependencies }} | ||
| # Work around lldb on Ubuntu 22.04 issue https://github.com/llvm/llvm-project/issues/55575 | ||
| sudo ln -s /usr/lib/llvm-14/lib/python3.10/dist-packages/lldb/* /usr/lib/python3/dist-packages/lldb/ |
There was a problem hiding this comment.
Is there any way you could avoid hard-coding llvm-14 and python-3.10 and retrieve the LLVM and Python versions from the system instead?
There was a problem hiding this comment.
unless we expect this bug to be fixed for upcoming llvm/python3 releases and using this work-around would actually become a problem on its own
There was a problem hiding this comment.
The bug is fixed in 24.04 LTS so this is a temporary work around until we update CI to use that version. Based on actions/runner-images#10636 that will be between now and the October 30 2024. At which point this will break and we can remove it.
|
|
||
| # Replace line numbers in runtime files - one rule for lldb, one for gdb | ||
| # (it would be better to only match on runtime/*.c, but gsub does not | ||
| # handle ERE) |
There was a problem hiding this comment.
out of curiosity, what's ERE?
There was a problem hiding this comment.
Extended Regular Expressions.
I wanted to use something like (runtime/.*.c):[0-9]+ and replace with \1:XX so that only runtime files match.
Let this work with the one true awk used on macOS
OCaml will name mangle symbols with an extra underscore plus a number that is not reproducable or stable across builds. Trimming this avoids issues with symbol names inadvertently changing.
313a536 to
e6e417d
Compare
|
I would like @shindere to review this. As a meta comment, moving the testing logic into Python provides a better experience and allows more customisation than using the CLI for either debugger. |
|
Tim McGilchrist (2024/09/30 20:59 -0700):
I would like @shindere to review this.
Many thanks for having given me theopportunity to do so!
This looks excellent to me.
I only have one question: why do you specifically install
`python3-lldb-14`? Wouldn't it be more robust to installl `python3-lldb`
(so without the -14 suffix)? Would that maybe solve the issue youwere
referring to, llvm/llvm-project#55575 ?
|
|
@shindere Installing This is a temporary work around until CI updates to Ubuntu 24.04 LTS, which based on actions/runner-images#10636 that will be between now and the October 30 2024. At which point this will break installing that package and we can remove this work around. |
|
Okay many thanks for the clarification, @tmcgilchrist.
|
|
I’m now seeing 24.04 being rolled out, which makes the CI test fail. Unfortunately, they seem to be doing this progressively from now to mid January: actions/runner-images#10636. Maybe we should force ubuntu-24.04 until this is stable? |
|
Maybe I missed something: I tried to apply the changes as mentioned here for when 24.04 would be rolled out. |
|
#13665 upgrades Ubuntu to 24.04, I had a workaround for a packaging issue with lldb in Ubuntu 22.04 which is now unnecessary. Moving more of the tests into python scripting should be more stable across lldb versions. I'm currently working on other things but hope to get back to this next year. |
5 participants
Improvement upon #13477
First change is #13484 plus various fixes to make the tests more stable across CI.Superseded by #13488.