This will allow for configuring tests according to AIX version.
Details
- Reviewers
daltenty Mordante - Group Reviewers
Restricted Project Restricted Project Restricted Project - Commits
- rG4fc70197919c: [libc++][AIX] Add OS version to target triple
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
libcxx/utils/ci/run-buildbot | ||
---|---|---|
709 | Normally, the target triple is figured out automatically in the CMake code. I think it's LLVM_DEFAULT_TARGET_TRIPLE. Is there a reason why that triple isn't inferred properly? If it were, I think this would not be necessary. |
libcxx/utils/ci/run-buildbot | ||
---|---|---|
709 | We considered adding the AIX version to the triple in the driver, but we don't have a good way to retrieve the AIX version in the driver right now. So, we prefer to do it in the test-suite to avoid any potential problems. |
Note that the AIX build is not available at the moment. I'll mark this as needs work so it can be rebased once the builder is available again.
This seems to be the libc++ run https://buildkite.com/llvm-project/libcxx-ci/builds/29326
I'm not sure why it also runs other CIs, but that might be due to recent experiments. Maybe rebasing will solve the issue.
libcxx/utils/ci/run-buildbot | ||
---|---|---|
709 | Can you add some comment to explain this? |
libcxx/utils/ci/run-buildbot | ||
---|---|---|
711 | Hmm, I wonder if this script is really the right place for this. If you don't go through this script to do the build, or the environment like this var isn't properly set when you run it, you won't get a correct test run result. I think it would be better if this was implemented somewhere inside the lit config somehow. |
libcxx/utils/ci/run-buildbot | ||
---|---|---|
711 | I will look into this |
libcxx/test/configs/ibm-libc++-shared.cfg.in | ||
---|---|---|
10 ↗ | (On Diff #557849) | You need some kind of guard so we don't run this block on non-AIX platforms |
libcxx/test/configs/ibm-libc++-shared.cfg.in | ||
---|---|---|
7 ↗ | (On Diff #557865) | Maybe you can make this a utility function inside llvm/utils/lit/lit/util.py to avoid repeating the code? (There are similarish util functions for macos for example) |
libcxx/test/configs/ibm-libc++-shared.cfg.in | ||
---|---|---|
7 ↗ | (On Diff #557865) | I'm confused, why not move this whole complicated function? (You've only moved the target check) |
LGTM modulo one nit.
llvm/utils/lit/lit/util.py | ||
---|---|---|
13 ↗ | (On Diff #557944) | Nit: Please keep the imports in alphabetic order. |
Normally, the target triple is figured out automatically in the CMake code. I think it's LLVM_DEFAULT_TARGET_TRIPLE. Is there a reason why that triple isn't inferred properly? If it were, I think this would not be necessary.