Skip to content

Conversation

zjgarvey
Copy link
Collaborator

@zjgarvey zjgarvey commented Oct 3, 2025

This updates the docs and build_tools/write_env_file.sh to correctly identify the PYTHONPATH based on in-tree vs. out-of-tree builds.

Additionally, I've updated various build configure commands to include a directive to align Python with Python3, since having conflicting versions of python can cause issues when not explicitly setting both to the same python executable.

Resolves #4331

Update scripts and readme docs to grab PYTHONPATH from env file.

Signed-off-by: zjgarvey <[email protected]>
Signed-off-by: zjgarvey <[email protected]>
@zjgarvey zjgarvey requested review from marbre and penguin-wwy October 3, 2025 20:34
Copy link
Collaborator

@penguin-wwy penguin-wwy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Member

@marbre marbre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not familiar with what write_env_file.sh is / was doing but it is bad practice to write files during configure or build to the source directory. Any other place this could go to?

@zjgarvey zjgarvey changed the title Write .env file directly during cmake build Update docs and .env file to reflect PYTHONPATH variability Oct 6, 2025
@zjgarvey zjgarvey requested a review from marbre October 6, 2025 18:49
Copy link
Member

@marbre marbre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left one comment but I haven't tested and reasoned to much above what would be a better solution here if any.

Comment on lines +26 to +30
if [[ "${in_tree_pkg_dir}" -nt "${out_of_tree_pkg_dir}" ]]; then
python_packages_dir="${in_tree_pkg_dir}"
else
python_packages_dir="${out_of_tree_pkg_dir}"
fi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if that logic is desired here or if it should be an external argument?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whether the build was in-tree/out-of-tree?

Yeah, we could make it take a command line arg and error out if the dir is missing, but just checking both seems fine to me, and aligns with the other two scripts that need to set a PYTHONPATH.

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.

Issues building with python packages
4 participants