Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[llvm-lit] Environment variable not found in LLVM subproject with lit internal shell #102395

Closed
Harini0924 opened this issue Aug 7, 2024 · 1 comment · Fixed by #98370
Closed

[llvm-lit] Environment variable not found in LLVM subproject with lit internal shell #102395

Harini0924 opened this issue Aug 7, 2024 · 1 comment · Fixed by #98370
Labels
test-suite

Comments

@Harini0924
Copy link
Contributor

Harini0924 commented Aug 7, 2024 •
edited
Loading

Currently, an environment variable is not recognized within the LLVM subproject, resulting in test failures when using the lit internal shell.
Error displayed:

'JITDUMPDIR=/usr/local/google/home/harinidonthula/llvm-project/build/test/ExecutionEngine/JITLink/x86-64/Output/ELF_perf.s.tmp': command not found

This error indicates that the internal shell is misinterpreting environment variable assignments as commands, leading to these failures.
@github-actions github-actions bot added clang Clang issues not falling into any other category lld labels Aug 7, 2024
This was referenced Aug 7, 2024
Merged
Merged
Merged
@connieyzhu connieyzhu mentioned this issue Aug 7, 2024
Merged
@Harini0924 Harini0924 mentioned this issue Aug 9, 2024
Merged
vitalybuka pushed a commit that referenced this issue Aug 9, 2024
@Harini0924
[compiler-rt][NFC] Replace environment variable with %t (#102197)
c69b8c4 
Certain tests within the compiler-rt subproject encountered "command not
found" errors when using lit's internal shell, particularly when trying
to use the `DIR` environment variable. When checking with the command
`LIT_USE_INTERNAL_SHELL=1 ninja check-compiler-rt`, I encountered the
following error:
```
********************
Testing: 
FAIL: SanitizerCommon-ubsan-i386-Linux :: sanitizer_coverage_trace_pc_guard-init.cpp (146 of 9570)
******************** TEST 'SanitizerCommon-ubsan-i386-Linux :: sanitizer_coverage_trace_pc_guard-init.cpp' FAILED ********************
Exit Code: 127

Command Output (stdout):
--
# RUN: at line 5
DIR=/usr/local/google/home/harinidonthula/llvm-project/build/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/ubsan-i386-Linux/Output/sanitizer_coverage_trace_pc_guard-init.cpp.tmp_workdir
# executed command: DIR=/usr/local/google/home/harinidonthula/llvm-project/build/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/ubsan-i386-Linux/Output/sanitizer_coverage_trace_pc_guard-init.cpp.tmp_workdir
# .---command stderr------------
# | 'DIR=/usr/local/google/home/harinidonthula/llvm-project/build/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/ubsan-i386-Linux/Output/sanitizer_coverage_trace_pc_guard-init.cpp.tmp_workdir': command not found
# `-----------------------------
# error: command failed with exit status: 127
```
In this patch, I resolved these issues by removing the use of the `DIR`
environment variable. Instead, the tests now directly utilize
`%t_workdir` for managing temporary directories. Additionally, I
simplified the tests by embedding the clang command arguments directly
into the test scripts, which avoids complications with environment
variable expansion under lit's internal shell.

This fix ensures that the tests run smoothly with lit's internal shell
and prevents the "command not found" errors, improving the reliability
of the test suite when executed in this environment.

fixes: #102395
[link to
RFC](https://discourse.llvm.org/t/rfc-enabling-the-lit-internal-shell-by-default/80179)
@EugeneZelenko EugeneZelenko added test-suite compiler-rt:sanitizer and removed clang Clang issues not falling into any other category lld labels Aug 9, 2024
This was referenced Aug 9, 2024
Open
Open
Open
This was referenced Aug 9, 2024
Open
Open
vitalybuka pushed a commit that referenced this issue Aug 13, 2024
@Harini0924
[llvm-lit] Fix error in compiler-rt tests when using lit internal she…
643a208 
…ll with env (#102069)

When running tests in compiler-rt using the lit internal shell with the
following command:
```
LIT_USE_INTERNAL_SHELL=1 ninja check-compiler-rt
```
one common error that occurs is:
```
'XRAY_OPTIONS=patch_premain=false:verbosity=1': command not found
```
This error, along with over 50 similar "environment variable not found"
errors, appears across 35 files in complier-rt. These errors happen
because the environment variables are not being set correctly when the
internal shell is used, leading to commands failing due to unrecognized
environment variables.

This patch addresses the issue by using the `env` command to properly
set the environment variables before running the tests. By explicitly
setting the environment variables through `env`, the internal shell can
correctly interpret and apply them, allowing the tests to pass.
fixes: #102395 
[link to
RFC](https://discourse.llvm.org/t/rfc-enabling-the-lit-internal-shell-by-default/80179)
ilovepi pushed a commit that referenced this issue Aug 21, 2024
@Harini0924
[clang] [test] Use lit Syntax for Environment Variables in Clang subp…
c975dc1 
…roject (#102647)

This patch updates the clang tests by replacing shell command
substitutions with lit-compatible syntax for setting and referencing
environment variables. Specifically, the use of shell-style variable
substitution (e.g., `DEFAULT_TRIPLE=`and `EXPECTED_RESOURCE_DIR=`) has
been replaced with `env` and `%{env}` to align with lit's internal shell
requirements. These changes ensure that environment variables are
properly set and accessed within the lit environment.

When using the lit internal shell with the command
`LIT_USE_INTERNAL_SHELL=1 ninja check-clang`, one common error
encountered is:
```
FAIL: Clang :: Driver/program-path-priority.c (19 of 20640)
******************** TEST 'Clang :: Driver/program-path-priority.c' FAILED ********************
Exit Code: 127

Command Output (stdout):
--
# RUN: at line 90
DEFAULT_TRIPLE=`/usr/local/google/home/harinidonthula/llvm-project/build/tools/clang/test/Driver/Output/program-path-priority.c.tmp/clang --version | grep "Target:" | cut -d ' ' -f2`
# executed command: 'DEFAULT_TRIPLE=`/usr/local/google/home/harinidonthula/llvm-project/build/tools/clang/test/Driver/Output/program-path-priority.c.tmp/clang' --version
# .---command stderr------------
# | 'DEFAULT_TRIPLE=`/usr/local/google/home/harinidonthula/llvm-project/build/tools/clang/test/Driver/Output/program-path-priority.c.tmp/clang': command not found
# `-----------------------------
# error: command failed with exit status: 127
```
To fix this issue, the patch replaces traditional shell substitutions
with lit's environment variable handling, ensuring compatibility with
the lit internal shell framework. This update applies to both the
handling of the `DEFAULT_TRIPLE` and `EXPECTED_RESOURCE_DIR` variables,
allowing the tests to pass when using the lit internal shell.
The patch also adds `env` to the `PWD` variable setting in the following
command to ensure the environment variable is correctly set within the
lit internal shell:
```
// RUN: %if system-linux %{ env PWD=/proc/self/cwd %clang -### -c --coverage %s -o foo/bar.o 2>&1 | FileCheck --check-prefix=PWD %s %}
```
fixes: #102395
[link to
RFC](https://discourse.llvm.org/t/rfc-enabling-the-lit-internal-shell-by-default/80179)
cjdb pushed a commit to cjdb/llvm-project that referenced this issue Aug 23, 2024
@Harini0924 @cjdb
[clang] [test] Use lit Syntax for Environment Variables in Clang subp…
e527677 
…roject (llvm#102647)

This patch updates the clang tests by replacing shell command
substitutions with lit-compatible syntax for setting and referencing
environment variables. Specifically, the use of shell-style variable
substitution (e.g., `DEFAULT_TRIPLE=`and `EXPECTED_RESOURCE_DIR=`) has
been replaced with `env` and `%{env}` to align with lit's internal shell
requirements. These changes ensure that environment variables are
properly set and accessed within the lit environment.

When using the lit internal shell with the command
`LIT_USE_INTERNAL_SHELL=1 ninja check-clang`, one common error
encountered is:
```
FAIL: Clang :: Driver/program-path-priority.c (19 of 20640)
******************** TEST 'Clang :: Driver/program-path-priority.c' FAILED ********************
Exit Code: 127

Command Output (stdout):
--
# RUN: at line 90
DEFAULT_TRIPLE=`/usr/local/google/home/harinidonthula/llvm-project/build/tools/clang/test/Driver/Output/program-path-priority.c.tmp/clang --version | grep "Target:" | cut -d ' ' -f2`
# executed command: 'DEFAULT_TRIPLE=`/usr/local/google/home/harinidonthula/llvm-project/build/tools/clang/test/Driver/Output/program-path-priority.c.tmp/clang' --version
# .---command stderr------------
# | 'DEFAULT_TRIPLE=`/usr/local/google/home/harinidonthula/llvm-project/build/tools/clang/test/Driver/Output/program-path-priority.c.tmp/clang': command not found
# `-----------------------------
# error: command failed with exit status: 127
```
To fix this issue, the patch replaces traditional shell substitutions
with lit's environment variable handling, ensuring compatibility with
the lit internal shell framework. This update applies to both the
handling of the `DEFAULT_TRIPLE` and `EXPECTED_RESOURCE_DIR` variables,
allowing the tests to pass when using the lit internal shell.
The patch also adds `env` to the `PWD` variable setting in the following
command to ensure the environment variable is correctly set within the
lit internal shell:
```
// RUN: %if system-linux %{ env PWD=/proc/self/cwd %clang -### -c --coverage %s -o foo/bar.o 2>&1 | FileCheck --check-prefix=PWD %s %}
```
fixes: llvm#102395
[link to
RFC](https://discourse.llvm.org/t/rfc-enabling-the-lit-internal-shell-by-default/80179)
@Harini0924 Harini0924 mentioned this issue Aug 24, 2024
Merged
vitalybuka pushed a commit that referenced this issue Aug 26, 2024
@Harini0924
[compiler-rt][test] Add env command to fix command not found errors…
52ae891 
… in compiler-rt with lit internal shell (#105917)

There are several files in the compiler-rt subproject that have command
not found errors. This patch uses the `env` command to properly set the
environment variables correctly when using the lit internal shell.
fixes: #102395 
[This change is relevant [RFC] Enabling the lit internal shell by
Default](https://discourse.llvm.org/t/rfc-enabling-the-lit-internal-shell-by-default/80179)
@connieyzhu connieyzhu reopened this Aug 27, 2024
@vitalybuka
Copy link
Collaborator

vitalybuka commented Aug 27, 2024 •
edited
Loading

@Harini0924 Patch descriptions should not contain "fixes: llvm#NNNN" if you are not intended to close the issue.
Something like "For llvm#NNNN" will be enough.

https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword

@Harini0924 Harini0924 changed the title [llvm-lit] lit internal shell environment variables not found across LLVM, clang, LLD, compiler-rt [llvm-lit] Environment variable not found in LLVM with lit internal shell Aug 29, 2024
@Harini0924 Harini0924 changed the title [llvm-lit] Environment variable not found in LLVM with lit internal shell [llvm-lit] Environment variable not found in LLVM subproject with lit internal shell Aug 29, 2024
@Harini0924 Harini0924 changed the title [llvm-lit] Environment variable not found in LLVM subproject with lit internal shell [llvm-lit] Environment variable not found in LLD subproject with lit internal shell Aug 29, 2024
@Harini0924 Harini0924 changed the title [llvm-lit] Environment variable not found in LLD subproject with lit internal shell [llvm-lit] Environment variable not found in LLVM subproject with lit internal shell Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
test-suite
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[LLVM][test][nfc] Updating test to work with internal shell connieyzhu/llvm-fork
4 participants
@vitalybuka @EugeneZelenko @connieyzhu @Harini0924