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.

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

[lldb] Transfer some environment variables into the tests on Windows build host #115613

Merged
merged 2 commits into from
Nov 11, 2024

Conversation

slydiman
Copy link
Contributor

@slydiman slydiman commented Nov 9, 2024

Some API tests (compiler calls) create a lot of garbage and cause unexpected behavior in case of Windows host and Linux target, e.g.

lldb/test/API/commands/process/attach/%SystemDrive%/
lldb/test/API/functionalities/deleted-executable/%SystemDrive%/
lldb/test/API/functionalities/exec/%SystemDrive%/
lldb/test/API/functionalities/load_unload/%SystemDrive%/
lldb/test/API/functionalities/target-new-solib-notifications/%SystemDrive%/
lldb/test/API/functionalities/thread/create_after_attach/%SystemDrive%/

It can be fixed by transfer some standard Windows environment variables into API tests.

…build host

Some API tests (compiler calls) create a lot of garbage and cause unexpected behavior in case of Windows host and Linux target, e.g.
```
lldb/test/API/commands/process/attach/%SystemDrive%/
lldb/test/API/functionalities/deleted-executable/%SystemDrive%/
lldb/test/API/functionalities/exec/%SystemDrive%/
lldb/test/API/functionalities/load_unload/%SystemDrive%/
lldb/test/API/functionalities/target-new-solib-notifications/%SystemDrive%/
lldb/test/API/functionalities/thread/create_after_attach/%SystemDrive%/
```
It can be fixed by transfer some standard Windows environment variables into API tests.
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 9, 2024

@llvm/pr-subscribers-lldb

Author: Dmitry Vasilyev (slydiman)

Changes

Some API tests (compiler calls) create a lot of garbage and cause unexpected behavior in case of Windows host and Linux target, e.g.

lldb/test/API/commands/process/attach/%SystemDrive%/
lldb/test/API/functionalities/deleted-executable/%SystemDrive%/
lldb/test/API/functionalities/exec/%SystemDrive%/
lldb/test/API/functionalities/load_unload/%SystemDrive%/
lldb/test/API/functionalities/target-new-solib-notifications/%SystemDrive%/
lldb/test/API/functionalities/thread/create_after_attach/%SystemDrive%/

It can be fixed by transfer some standard Windows environment variables into API tests.


Full diff: https://github.com/llvm/llvm-project/pull/115613.diff

1 Files Affected:

  • (modified) lldb/test/API/lit.cfg.py (+19)
diff --git a/lldb/test/API/lit.cfg.py b/lldb/test/API/lit.cfg.py
index 6ef09f36a1907e..febf8dc3d19021 100644
--- a/lldb/test/API/lit.cfg.py
+++ b/lldb/test/API/lit.cfg.py
@@ -334,3 +334,22 @@ def delete_module_cache(path):
 # Propagate XDG_CACHE_HOME
 if "XDG_CACHE_HOME" in os.environ:
     config.environment["XDG_CACHE_HOME"] = os.environ["XDG_CACHE_HOME"]
+
+# Transfer some environment variables into the tests on Windows build host.
+if platform.system() == "Windows":
+    for v in [
+        "SystemDrive",
+        "SystemRoot",
+        "ALLUSERSPROFILE",
+        "APPDATA",
+        "LOCALAPPDATA",
+        "USERDNSDOMAIN",
+        "USERDOMAIN",
+        "USERNAME",
+        "USERPROFILE",
+        "USERDOMAIN_ROAMINGPROFILE",
+        "COMPUTERNAME",
+        "ProgramData",
+    ]:
+        if v in os.environ:
+            config.environment[v] = os.environ[v]

@labath
Copy link
Collaborator

labath commented Nov 11, 2024

It looks like what you need is SystemDrive. What's up with the other 10 variables? The reason we're not passing all environment variables is because we want to make the build reproducible. Passing LOCALAPPDATA makes it look like the test might actually depend on what you have in that directory, which is less than ideal. I'd like to avoid the other variables if possible.

@slydiman
Copy link
Contributor Author

Reduced the list of environment variables to SystemDrive only.
We can easily add necessary variables later.
It must fix cmake-configure step here
https://lab.llvm.org/staging/#/builders/197/builds/77

@slydiman slydiman merged commit 8941f89 into llvm:main Nov 11, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants