Skip to content

Fix toolchain path when invoking build_script_helper.py #41739

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 2 commits into
base: main
Choose a base branch
from

Conversation

jblache
Copy link

@jblache jblache commented Mar 9, 2022

build_script_helper.py hardcodes a usr component in the toolchain path it builds, which broke a build with a non-/usr install destination.

Fix the script invoking build_script_helper.py to path a complete toolchain path, and remove the hardcoded usr path component. The logic to determine the toolchain path was lifted from another script, as this logic is repeated in several location, with slight variations. I picked the variant that seemed the most complete.

The first commit is tested in a Linux; the second commit fixes similar code in the vicinity and has not been exercised. It is possible that these changes break on macOS and require some slight tweaking (e.g. the toolchain path given to build_script_helper.py might need to include usr as its last path component).

jblache added 2 commits March 8, 2022 16:48
build_script_helper.py hardcodes usr as a path component to the toolchain;
pass the correct toolchain path, and remove the hardcoded usr component.
build_script_helper.py hardcodes usr as a path component to the toolchain;
pass the correct toolchain path, and remove the hardcoded usr component.
@shahmishal
Copy link
Member

@swift-ci build toolchain

@shahmishal
Copy link
Member

@swift-ci test

host_target,
product.build_dir)
toolchain_path = targets.toolchain_path(install_destdir,
args.install_prefix)
Copy link
Member

Choose a reason for hiding this comment

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

Note the prefix added for Darwin in the next couple lines, that's going to break this usual formulation there, which is why I didn't mess with it last year.

host_target,
product.build_dir)
toolchain_path = targets.toolchain_path(install_destdir,
args.install_prefix)
Copy link
Member

Choose a reason for hiding this comment

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

Try using this better two-line formulation I came up with for other build products, though as above, you will have to do something about that Darwin prefix added below too.

@@ -49,7 +49,7 @@ def main():
if not os.path.isdir(bin_dir):
os.makedirs(bin_dir)

swiftbuild_path = os.path.join(args.toolchain, "usr", "bin", "swift-build")
Copy link
Member

@finagolfin finagolfin Mar 9, 2022

Choose a reason for hiding this comment

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

Nice, I removed several of these in #30565 and related pulls a couple years ago, but I don't build these products, so it looks like these slipped through.

@gottesmm
Copy link
Contributor

Note to self, need to change the toolchain installation location.

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.

4 participants