llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.26k stars 12.09k forks source link

[lit] Lit's builtin `env` command fails to match environment variables case insensitively on Windows #115584

Open tahonermann opened 2 weeks ago

tahonermann commented 2 weeks ago

LLVM's lit utility includes a builtin implementation of the env command in llvm/utils/lit/lit/TestRunner.py; see here and here. The builtin implementation does not match the case-insensitive behavior that would be expected on Windows.

The issue can be demonstrated by adding the following test to an llvm-project workspace and running it as shown. A couple of notes about the test:

  1. The test uses cmd /c set as a workaround for https://github.com/llvm/llvm-project/issues/115578. Once that issue is fixed, the cmd /c set portion of the command line can be removed and the builtin env functionality should suffice.
  2. The test is specific to Windows. I tried adding REQUIRES: system-windows to it, but that didn't work; the test was still skipped as unsupported for me. I'm guessing there is some relevant lit.cfg magic that is necessary to make that work right. More work will be necessary for this test to be checked in as a regression test.
  3. If this test does end up getting used as a regression test, llvm/utils/lit/tests/shtest-env-positive.py should be updated to validate it in the same manner it does for other tests.
  4. Setting an environment variable that is already set changes the case of the variable. This differs from the behavior of the cmd.exe set command and is related to why the unset case doesn't work. The builtin implementation uses a Python dictionary keyed by the (case-sensitive) variable name. Unsetting by a case-mismatched name fails because the code doesn't attempt to match case; the preset variable therefore remains in the dictionary.
> type llvm/utils/lit/tests/Inputs/shtest-env-positive/env-case-insensitive.txt
## Tests env command for preset variables and case sensitive handling.

## Check and make sure preset environment variable were set in lit.cfg.
#
# RUN: env cmd /c set | FileCheck --check-prefix=CHECK-ENV-PRESET %s
#
## Check set of environment variable by lowercase name.
#
# RUN: env foo=changed cmd /c set | FileCheck --check-prefix=CHECK-ENV-SET-LC %s
## Check unset of environment variable by lowercase name.
#
# RUN: env -u foo cmd /c set | FileCheck --check-prefix=CHECK-ENV-UNSET-LC %s

# CHECK-ENV-PRESET: FOO=1
# CHECK-ENV-PRESET: BAR=2

# CHECK-ENV-SET-LC-NOT: FOO=
# CHECK-ENV-SET-LC: foo=changed
# CHECK-ENV-SET-LC: BAR=2

# CHECK-ENV-UNSET-LC-NOT: FOO=
# CHECK-ENV-UNSET-LC: BAR=2

> python llvm-lit.py -a llvm/utils/lit/tests/Inputs/shtest-env-positive/env-case-insensitive.txt
-- Testing: 1 tests, 1 workers --
FAIL: shtest-env :: env-case-insensitive.txt (1 of 1)
******************** TEST 'shtest-env :: env-case-insensitive.txt' FAILED ********************
Exit Code: 1
...
# RUN: at line 9
env foo=changed cmd /c set | FileCheck --check-prefix=CHECK-ENV-SET-LC C:\Users\thonerma\llvm-project\llvm\utils\lit\tests\Inputs\shtest-env-positive\env-case-insensitive.txt
# executed command: env foo=changed cmd /c set
# executed command: FileCheck --check-prefix=CHECK-ENV-SET-LC 'C:\Users\thonerma\llvm-project\llvm\utils\lit\tests\Inputs\shtest-env-positive\env-case-insensitive.txt'
# .---command stderr------------
# | C:\Users\thonerma\llvm-project\llvm\utils\lit\tests\Inputs\shtest-env-positive\env-case-insensitive.txt:17:25: error: CHECK-ENV-SET-LC-NOT: excluded string found in input
# | # CHECK-ENV-SET-LC-NOT: FOO=
# |                         ^
# | <stdin>:13:1: note: found here
# | FOO=1
# | ^~~~
...
# | Input was:
# | <<<<<<
...
# |        13: FOO=1
# | not:17     !~~~   error: no match expected
...
# | >>>>>>
# `-----------------------------
# error: command failed with exit status: 1
...
Failed Tests (1):
  shtest-env :: env-case-insensitive.txt
...

Emulating case insensitive matching is important for testing some environment variables. For example, MSVC environments have an environment variable named VCToolsInstallDir that Clang consults (see findVCToolChainViaEnvironment() here. Lit's testing configuration allows this environment variable to be observed in tests, but for some reason, it spells the variable name as VCToolsinstallDir (note the lowercase "i" in "install"); see here. When lit loads variables from the environment, it ends up using its own form of the variable name. Attempts to unset it for testing purposes using the name as MSVC sets it (VCToolsInstallDir with an uppercase "I") fails to do so. The case-mismatched name in TestingConfig.py should be fixed as part of resolving this issue.

tahonermann commented 2 weeks ago

@Harini0924, you might be interested in this issue given your recent work on the builtin env implementation.