Skip to content

[windows] fix missing environment variables when running check-lldb #82063

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

charles-zablit
Copy link
Contributor

Currently when running build.ps1 at desk to build and test LLDB, the tests will fail because python39.dll is not in the Path and because PYTHONHOME is not set.

This PR adds python39.dll to the Path before running the tests and passes PYTHONHOME to lit through LLDB_TEST_USER_ARGS. Simply setting the PYTHONHOME env variable is not enough as lit reset the env before running the tests.

@charles-zablit charles-zablit requested a review from compnerd as a code owner June 6, 2025 15:25
@charles-zablit charles-zablit changed the title [windows] fix missing environment variable when running check-lldb [windows] fix missing environment variables when running check-lldb Jun 6, 2025
@charles-zablit
Copy link
Contributor Author

@swift-ci please test

@JDevlieghere
Copy link
Contributor

This makes sense. I think passing PYTHONHOME should be meaningful on all platforms, so instead of doing this in build.ps1, I think we can do this upstream, and have CMake configure that property in lit.site.cfg.in, similar to config.python_executable.

Not only does that avoid diverging the platforms and swift repo, it also has the benefit of being a bit more "hermetical" if the test python is different. I'm not sure if this is even possible here, but build-script for example often runs under a different python than what we build lldb against.

@charles-zablit
Copy link
Contributor Author

I opened llvm/llvm-project#143183 with the changes to the lldb lit config. We still need the env:Path change in this patch for lldb to correctly find python.dll. Or maybe we want to do that in lit as well?

@compnerd
Copy link
Member

compnerd commented Jun 6, 2025

I opened llvm/llvm-project#143183 with the changes to the lldb lit config. We still need the env:Path change in this patch for lldb to correctly find python.dll. Or maybe we want to do that in lit as well?

I think that is best handled outside of lit - this location is something specific to the build setup. It is possible to place the extracted python anywhere. If we want to do this through lit, it should be wired through CMake and through CMake into lit.

Michael137 pushed a commit to llvm/llvm-project that referenced this pull request Jun 9, 2025
…3183)

When testing LLDB, we want to make sure to use the same Python as the
one we used to build it.

This patch used the CMake variable `Python3_ROOT_DIR` to set the
`PYTHONHOME` env variable in LLDB lit tests, in order to ensure of this.

Please see swiftlang/swift#82063 for the
original issue.
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jun 9, 2025
…esting (#143183)

When testing LLDB, we want to make sure to use the same Python as the
one we used to build it.

This patch used the CMake variable `Python3_ROOT_DIR` to set the
`PYTHONHOME` env variable in LLDB lit tests, in order to ensure of this.

Please see swiftlang/swift#82063 for the
original issue.
Co-authored-by: Saleem Abdulrasool <[email protected]>
charles-zablit added a commit to swiftlang/llvm-project that referenced this pull request Jun 9, 2025
…m#143183)

When testing LLDB, we want to make sure to use the same Python as the
one we used to build it.

This patch used the CMake variable `Python3_ROOT_DIR` to set the
`PYTHONHOME` env variable in LLDB lit tests, in order to ensure of this.

Please see swiftlang/swift#82063 for the
original issue.
@charles-zablit
Copy link
Contributor Author

I think that is best handled outside of lit - this location is something specific to the build setup. It is possible to place the extracted python anywhere. If we want to do this through lit, it should be wired through CMake and through CMake into lit.

What I have in mind is to do something like this:

# lit.cfg.py
config.environment["PATH"] = os.path.pathsep.join(config.python_root_dir, config.environment.get("PATH", ""))

And config.python_root_dir is the CMake variable Python3_ROOT_DIR.
This would mean that the setting is wired through CMake and then through lit.

What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants