The current parsing logic for the target string assumes it follows the
format `<kind>-<triple>-<target id>:<feature>`, such as
`hipv4-amdgcn-amd-amdhsa-gfx1030:+xnack`.
Specifically, it assumes that `<target id>` does not contain any `-`,
relying on `rsplit` for parsing.
However, this assumption breaks for AMDGPU's generic targets, which may
contain one or more `-`, such as `gfx10-3-generic` or `gfx12-generic`.
As a result, the existing approach using `rstrip` is no longer reliable.
This patch reworks the parsing logic to handle target strings more
robustly, including support for generic targets.
The bundler now strictly requires a 4-field target triple.
Additionally, a new Python helper function has been added to `config.py`
to normalize the target triple into the 4-field format when it is not,
ensuring tests pass reliably.
Extending the conditionals in `AugmentRegisterInfo` to support
alternative names for lldb.
Fixes#124023
There is an exception with register `X8` which is not covered here but
more details can be found in the issue
https://github.com/llvm/llvm-project/issues/127900.
I got a report that downstream this test failed and the cause was that
it took longer than the 1 second we expected to run one of the test
cases.
This test doesn't need to be that specific, so I am updating all the
time regexes to be the same one that allows 0-9 any number of digits,
requires a decimal point, then 0-9 any number of digits for the final
part.
BusyBox `cat` has yet another slight variation of error formatting:
```console
$ cat --help 2>&1 | head -1
BusyBox v1.37.0 (2024-09-30 10:39:57 UTC) multi-call binary.
$ cat does-not-exist
cat: can't open 'does-not-exist': No such file or directory
```
Rather than extend the test result regex with a third case,
recognise that we only really care about the filename and errno string.
Weaken the regex to ignore all "noise" around the filename.
Note this also corrects what looks like a bug with the previous regex.
Previously, the `cannot open does-not-exist` alternate did not assert
the following errno message. This was introduced in
https://reviews.llvm.org/D60553 (apparently) due to differences in the
`cat` command on AIX. That bug doesn't include the specific
AIX output, so it's unclear if this omission was intended.
- Add option (--report-failures-only) to generate a reduced report for
lit tests that only includes failing tests
- This is a continuation of proposed patches by @gregbedwell here:
- https://reviews.llvm.org/D143516
- https://reviews.llvm.org/D143519
---------
Co-authored-by: Greg Bedwell <greg.bedwell@sony.com>
Co-authored-by: James Henderson <James.Henderson@sony.com>
I was too hasty landing an option whose only known use at this time is
LLVM's own CI.
We may be able to remove it before the next branch that would be the
next llvm-lit release outside of llvm, but the timing may not work out.
So I am hiding the option in case that were to happen.
This is to address the latest lit regressions
https://lab.llvm.org/buildbot/#/builders/64/builds/1285 caused by using
the internal lit shell. This change will limit using the internal lit
shell to TableGen on AIX so we do not hit these regressions.
The initial version of this feature would use the output file name
if it could, but in switching to temp files I forgot to replicate that
behaviour.
What happens now is we always use a tempfile name and the output
path is a template for that. I think the current behaviour
still makes sense so I'm just correcting the documentation.
When running a build like:
```
ninja check-clang check-llvm
```
Prior to my changes you ended up with one results file, in this specific case Junit XML:
```
results.xml
```
This would only include the last set of tests lit ran, which were for
llvm. To get around this, many CI systems will run one check target,
move the file away, then run another, somehow propgating the return code
as well.
```
rectode=0
for target in targets:
ninja target
retcode=$?
mv results.xml results-${target}.xml
<report the overall return code>
```
I want to use something like this Buildkite reporting plugin in CI, which needs to have all the results available:
https://buildkite.com/docs/agent/v3/cli-annotate#using-annotations-to-report-test-results
Modifying CI's build scripts for Windows and Linux is a lot of work. So
my changes instead make lit detect an existing result file and modify
the file name to find a new file to write to. Now you will get:
```
results.xml results.<tempfile generated value>.xml
```
This will work for all result file types since I'm doing it in the base
Report class. Now you've got separate files, it's easy to collect them
with `<path>/*.xml`.
Note that the `<tempfile generated value>` is not ordered.
The time for all testsuites will always exist because lit
measures it itself. For a given testsuite, I guess that it
can be None if for example the suite is empty.
Encountered several testcase failures when running `ninja check-all`. It
was due to the full path name were shown in the error message instead of
the binary name, and therefore causing the check string mismatch.
The machine was running CentOS 9 with binfmt_misc setup that uses
qemu-aarch64 (8.1.2). Built and ran the unittest as aarch64 host
(through qemu user).
Co-authored-by: h2h <h2h@meta.com>
Line ending policies were changed in the parent, dccebddb3b80. To make
it easier to resolve downstream merge conflicts after line-ending
policies are adjusted this is a separate whitespace-only commit. If you
have merge conflicts as a result, you can simply `git add --renormalize
-u && git merge --continue` or `git add --renormalize -u && git rebase
--continue` - depending on your workflow.
Historically, we've not automatically enforced how git tracks line
endings, but there are many, many commits that "undo" unintended CRLFs
getting into history.
`git log --pretty=oneline --grep=CRLF` shows nearly 100 commits
involving reverts of CRLF making its way into the index and then
history. As far as I can tell, there are none the other way round except
for specific cases like `.bat` files or tests for parsers that need to
accept such sequences.
Of note, one of the earliest of those listed in that output is:
```
commit 9795860250734e5c2a879546c534e35d9edd5944
Author: NAKAMURA Takumi <geek4civic@gmail.com>
Date: Thu Feb 3 11:41:27 2011 +0000
cmake/*: Add svn:eol-style=native and fix CRLF.
llvm-svn: 124793
```
...which introduced such a defacto policy for subversion.
With old versions of git, it's been a bit of a crap-shoot whether
enforcing storing line endings in the history will upset checkouts on
machines where such line endings are the norm. Indeed many users have
enforced that git checks out the working copy according to a global or
per-user config via core crlf, or core autocrlf.
For ~8 years now[1], however, git has supported the ability to "do as
the Romans do" on checkout, but internally store subsets of text files
with line-endings specified via a system of patterns in the
`.gitattributes` file. Since we now have this ability, and we've been
specifying attributes for various binary files, I think it makes sense
to rid us of all that work converting things "back", and just let git
handle the local checkout. Thus the new toplevel policy here is
* text=auto
In simple terms this means "unless otherwise specified, convert all
files considered "text" files to LF in the project history, but check
them out as expected on the local machine. What is "expected on the
local machine" is dependent on configuration and default.
For those files in the repository that *do* need CRLF endings, I've
adopted a policy of `eol=crlf` which means that git will store them in
history with LF, but regardless of user config, they'll be checked out
in tree with CRLF.
Finally, existing files have been "corrected" in history via `git add
--renormalize .`
End users should *not* need to adjust their local git config or
workflow.
[1]: git 2.10 was released with fixed support for fine-grained
line-ending tracking that respects user-config *and* repo policy. This
can be considered the point at which git will respect both the user's
local working tree preference *and* the history as specified by the
maintainers. See
https://github.com/git/git/blob/master/Documentation/RelNotes/2.10.0.txt#L248
for the release note.
Currently we write out a time taken to run all test suites:
```
<testsuites time="8.28">
```
And one for each test:
```
<testcase classname="lldb-shell.Breakpoint" name="breakpoint-command.test" time="2.38"/>
```
However, the schema says there should be one for each suite and test,
but none for testsuites:
cfa434d4b8/JUnit.xsd (L216)
I'm leaving the `testsuites` time in though because no one has
complained so far, and someone out there probably has a script relying
on it by now. Most XML tools handle unknown attributes quite well
anyway.
I'm adding a per testsuite time to comply with the schema and maybe be
more compatible with other JUnit tools.
```
<testsuite name="lldb-shell" ... time="12.34">
```
The test suite time is the sum of the time taken for all tests in the
suite. This will ignore some overhead in setting up the suite, and means
that the sum of the times for all individual suites may not equal the
`testsuites` time.
As we're usually focusing on the execution time of particular tests, not
lit's book keeping, I think this is a reasonable choice.
This is an alternate implementation of a patch proposed by @preames in
<https://reviews.llvm.org/D128840>. As noted there, when running
non-native binaries with binfmt_misc and qemu-user you typically need to
set some environment variables (at least, QEMU_LD_PREFIX), but lit
strips them by default. This patch adds what I think are the two main
ones to the list of those that aren't stripped. It does so in a place
that applies to all lit test suites (rather than just LLVM's), and as
can be seen from the other env vars in `pass_vars` I think there's
already plenty of precedent for passing through environment variables
known to be potentially useful to LLVM developers.
This adds a flag to lit for detecting and updating failing tests when
possible to do so automatically. The flag uses a plugin architecture
where config files can add additional auto-updaters for the types of
tests in the test suite. When a test fails with `--update-tests` enabled
lit passes the test RUN invocation and output to each registered test
updater until one of them signals that it updated the test (or all test
updaters have been run). As such it is the responsibility of the test
updater to only update tests where it is reasonably certain that it will
actually fix the test, or come close to doing so.
Initially adds support for UpdateVerifyTests and UpdateTestChecks. The
flag is currently only implemented for lit's internal shell, so
`--update-tests` implies `LIT_USE_INTERNAL_SHELL=1`.
Builds on work in #97369Fixes#81320
…tting" (#108104)"
This recommits 0f56ba13bff7ab72bfafcf7c5cf9e5b8bd16d895 (reverted by
6007ad79afeffb1288781b4a7241290386293aff). In the original patch
llvm/utils/lit/tests/escape-color.py failed on Windows because it diffed
llvm-lit output with a file containing '\n' newlines rather than '\r\n'.
This issue is avoided by calling 'diff --strip-trailing-cr'.
Original description below:
Test output that carried color across newlines previously resulted in
the formatting around the output also being colored. Detect the current
ANSI color and reset it when printing formatting, and then reapply it.
As an added bonus an unterminated color code is also detected,
preventing it from leaking out into the rest of the terminal.
Fixes#106633
Test output that carried color across newlines previously resulted in
the formatting around the output also being colored. Detect the current
ANSI color and reset it when printing formatting, and then reapply it.
As an added bonus an unterminated color code is also detected,
preventing it from leaking out into the rest of the terminal.
Fixes#106633
This reverts commit 5af4ba2684b9b59de3bf8135f62e05ab68cfc489.
The previous patch was missing the closing parenthesis `)` in the
`CHECK` statement in the `llvm/utils/lit/tests/shtest-glob.py` file:
`# CHECK: FAIL: shtest-glob :: glob-mkdir.txt ({{[^)]*}}`
This issue broke some build bots. This patch corrects the `CHECK`
statement by adding the closing parenthesis:
`# CHECK: FAIL: shtest-glob :: glob-mkdir.txt ({{[^)]*}})`
This patch introduces a precommit test to verify the current behavior of
glob expansion in lit's internal shell. The motivation for this test
stems from an issue encountered during the BOLT test suite when running
with the lit internal shell using the command:
`LIT_USE_INTERNAL_SHELL=1 ninja check-bolt`
During execution, the following error was observed:
```
File "/usr/local/google/home/harinidonthula/llvm-project/llvm/utils/lit/lit/TestRunner.py", line 416, in executeBuiltinEcho
stdout.write(encode(maybeUnescape(args[-1])))
TypeError: string argument expected, got 'GlobItem'
```
The `executeBuiltinEcho` function in the lit testing framework expects a
string to be passed to `stdout.write`, but it received a `GlobItem`
object instead. This precommit test is designed to check the current
behavior where the glob pattern isn't correctly expanded, leading to
this `TypeError`.
While this patch doesn't fix the issue, it helps in understanding and
verifying the current behavior. The feedback I received from this
[PR](https://github.com/llvm/llvm-project/pull/105925) suggests using
`cmd.args = expand_glob_expressions(cmd.args, shenv.cwd)` to match the
behavior of `executeBuiltinMkdir` and `executeBuiltinRm`, but it is
recognized that the internal shell should ideally expand globs before
calling any built-in command.
**Request for Feedback:**
I'm looking for feedback on how to improve this precommit test,
specifically regarding the handling and expansion of glob patterns for
commands like mkdir and rm within the internal shell. Currently, the
args are expanded at the beginning of these functions, which should
ensure proper glob expansion. However, I'd appreciate guidance on
whether I should write additional tests to verify that mkdir and rm are
handling glob expansions correctly.
If such tests are recommended, I would also appreciate advice on the
best approach to implement them, considering the existing framework and
the way glob expansion is expected to function in the internal shell.
Should these tests confirm that the current implementation passes, or
are there specific edge cases I should be aware of?
**Next Steps:**
In my follow-up PR, I plan to address the UNRESOLVED error by expanding
the entire command, ensuring correct and consistent behavior across all
commands. The current test checks for an unresolved issue with the glob
expansion, specifically looking for a `TypeError` due to an unexpanded
`GlobItem`. This will be updated to reflect the correct behavior once
the issue is resolved.
This change is relevant for [[RFC] Enabling the Lit Internal Shell by
Default](https://discourse.llvm.org/t/rfc-enabling-the-lit-internal-shell-by-default/80179/3)
This patch addresses an issue with lit's internal shell when env is
without any arguments, it fails with exit code 127 because `env`
requires a subcommand. This patch addresses the issue by encoding the
command to properly return environment variables even when no arguments
are provided.
The error occurred when running the command
` LIT_USE_INTERNAL_SHELL=1 ninja check-llvm`.
fixes: #102383
This is part of the test cleanups proposed in the RFC: [[RFC] Enabling
the Lit Internal Shell by
Default](https://discourse.llvm.org/t/rfc-enabling-the-lit-internal-shell-by-default/80179)
from PEP8
(https://peps.python.org/pep-0008/#programming-recommendations):
> Comparisons to singletons like None should always be done with is or
is not, never the equality operators.
Co-authored-by: Eisuke Kawashima <e-kwsm@users.noreply.github.com>
This patch fixes the incorrect usage of lit's built-in `export` command.
There is a typo in raising the error itself where the error being raised
had the wrong number of parameters passed in.
Fixes https://github.com/llvm/llvm-project/issues/102386.
On macOS the dynamic loader prunes dyld specific environment variables
such as `DYLD_INSERT_LIBRARIES`, `DYLD_LIBRARY_PATH`, etc. If these are
set in the lit config it's safe to assume that the user actually wanted
their subprocesses to run with these variables, versus the python
interpreter that gets executed with them before they are pruned. This
change exports all known variables in the shell script instead of
relying on them being passed through.
Substitutions can be added in a couple different ways; they can be added
via the calling python scripts by adding entries to the
config.substitutions dictionary, or via DEFINE lines in the scripts
themselves.
The substitution strings passed to Python's re classes are interpreted
so that backslashes expand to escape sequences, and literal backslashes
need to be escaped.
On Unix, the script defined substitutions don't (usually, so far)
contain backslashes - but on Windows, they often do, due to paths
containing backslashes. This lead to a Windows specific escaping of
backslashes before doing Python re substitutions - since
7c9eab8fef0ed79a5911d21eb97b6b0fa9d39f82. There's nothing inherently
Windows specific about this though - any intended literal backslashes in
the substitution strings need to be escaped; this is how the Python re
API works.
The DEFINE lines were added later, and in order to cope with
backslashes, escaping of backslashes was added in the SubstDirective
class in TestRunner, applying to DEFINE lines in the tests only.
The fact that the escaping right before passing to the Python re API was
done conditionally on Windows led to two inconsistencies:
- DEFINE lines in the tests that contain backslashes got double
backslashes on Windows. (This was visible as a FIXME in
llvm/utils/lit/tests/Inputs/shtest-define/value-escaped.txt.)
- Script provided substitutions containing backslashes did not work on
Unix, but they did work on Windows.
By removing the escaping from SubstDirective and escaping it
unconditionally in the processLine function, before feeding the
substitutions to Python's re classes, we should have consistent
behaviour across platforms, and get rid of the FIXME in the lit test.
This fixes issues with substitutions containing backslashes on Unix
platforms, as encountered in PR #86649.
When a test case inside of a gtest suite fails, we report a failure
which causes the entire `ninja check-lldb` invocation to fail, even if
the outer test case is marked as XFAIL - each test case result is
reported as its own lit test run. This PR updates lit so it checks
whether each test case's parent test suite is XFAIL before setting the
status to FAIL.
This is especially problematic because the failing tests can't manually
be marked as XFAIL, due to
https://github.com/llvm/llvm-project/issues/102264.
Fixes https://github.com/llvm/llvm-project/issues/102265
### Repro instructions
1. Modify any gtest test case to generate a failure.
2. Mark the outer lit test with XFAIL using either `--xfail-tests` flag
or `LIT_XFAIL` env var.
3. Run the tests
4. Observe the lit test is XFAIL as expected, but the failed child test
cases show up as separate failures.
Co-authored-by: kendal <kendal@thebrowser.company>
When using the lit internal shell with the command:
```
LIT_USE_INTERNAL_SHELL=1 ninja check-compiler-rt
```
The follow error is encountered:
```
File "TestRunner.py", line 770, in _executeShCmd
inproc_builtin = inproc_builtins.get(args[0], None)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: unhashable type: 'GlobItem'
```
This error is in a compiler-rt file:
```
TestCases/Linux/long-object-path.cpp
```
This error occurs because `args[0]` is of type `GlobItem`, which is not
hashable, leading to a `TypeError` when it is passed in
`inproc_builtins.get()`. To resolve this issue, I have updated the
implementation to ensure that `args[0]` is hashable before it is used in
`inproc_builtins`.
fixes: #102389
[link to
RFC](https://discourse.llvm.org/t/rfc-enabling-the-lit-internal-shell-by-default/80179)
This patch separates the lit tests that check for the functionality of
lit's built-in cat command into its own test file and folder. This is a
prerequisite for https://github.com/llvm/llvm-project/pull/101530.
When working on very busy systems, check-offload frequently fails many
tests with this diagnostic:
```
clang: error: cannot determine amdgcn architecture: /tmp/llvm/build/bin/amdgpu-arch: Child timed out: ; consider passing it via '-march'
```
This patch accepts the environment variable
`CLANG_TOOLCHAIN_PROGRAM_TIMEOUT` to set the timeout. It also increases
the timeout from 10 to 60 seconds.
FileCheck has special handline for the `-EMPTY` suffix, that should
match empty lines. Overloading the suffix can be a source of confusion
when reading tests. Additionally, the current implementation seems to
match the following expressions, which appears to be a bug in FileCheck.
LLVM lit assumes control of the test parallelism when running a test
suite. This style of testing doesn't play nicely with build systems like
Buck or Bazel since it prefers finer grained actions on a per-test
level. In order for external build systems to control the test
parallelism, add an option to disable `.lit_test_times.txt` under the
`--skip-test-time-recording` flag, thus allowing other build systems
to determine the parallelism and avoid race conditions when writing
to that file. I went for `--skip-test-time-recording` instead of `--time-tests` in
order to preserve the original functionality of writing to `.lit_test_times.txt`
as the default behavior and only opt-in for those who do _not_ want
`.lit_test_times.txt` file.
