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.

Sign up for GitHub

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
+30 −8
Open

Fix toolchain path when invoking build_script_helper.py #41739

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d4b4c14
Fix toolchain path in build_script_helper.py invocation for benchmarks
jblache Mar 9, 2022
816b840
Fix toolchain path in build_script_helper.py invocation for swiftinspect
jblache Mar 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion benchmark/scripts/build_script_helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,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")
swiftbuild_path = os.path.join(args.toolchain, "bin", "swift-build")
perform_build(args, swiftbuild_path, "debug", "Benchmark_Onone", "-Onone")
perform_build(args, swiftbuild_path, "release", "Benchmark_Osize", "-Osize")
perform_build(args, swiftbuild_path, "release", "Benchmark_O", "-O")
Expand Down
6 changes: 3 additions & 3 deletions tools/swift-inspect/build_script_helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ def perform_build(args, swiftbuild_path):
"-Xswiftc",
"-I",
"-Xswiftc",
os.path.join(args.toolchain, 'usr', 'include', 'swift'),
os.path.join(args.toolchain, 'include', 'swift'),
"-Xswiftc",
"-L",
"-Xswiftc",
os.path.join(args.toolchain, 'usr', 'lib', 'swift', 'macosx'),
os.path.join(args.toolchain, 'lib', 'swift', 'macosx'),
"-Xswiftc",
"-lswiftRemoteMirror"
]
Expand All @@ -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 •
edited
Loading

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.

swiftbuild_path = os.path.join(args.toolchain, "bin", "swift-build")
perform_build(args, swiftbuild_path)


Expand Down
10 changes: 8 additions & 2 deletions utils/swift_build_support/swift_build_support/products/benchmarks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,17 @@ def get_dependencies(cls):


def _get_toolchain_path(host_target, product, args):
# TODO check if we should prefer using product.install_toolchain_path
# this logic initially was inside run_build_script_helper
# and was factored out so it can be used in testing as well

toolchain_path = product.host_install_destdir(host_target)
install_destdir = args.install_destdir
if swiftpm.SwiftPM.has_cross_compile_hosts(args):
install_destdir = swiftpm.SwiftPM.get_install_destdir(args,
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.


if platform.system() == 'Darwin':
# The prefix is an absolute path, so concatenate without os.path.
toolchain_path += \
Expand Down
20 changes: 18 additions & 2 deletions utils/swift_build_support/swift_build_support/products/swiftinspect.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,29 @@ def get_dependencies(cls):
swiftpm.SwiftPM]


def run_build_script_helper(host_target, product, args):
toolchain_path = args.install_destdir
def _get_toolchain_path(host_target, product, args):
# this logic initially was inside run_build_script_helper
# and was factored out so it can be used in testing as well

install_destdir = args.install_destdir
if swiftpm.SwiftPM.has_cross_compile_hosts(args):
install_destdir = swiftpm.SwiftPM.get_install_destdir(args,
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.


if platform.system() == 'Darwin':
# The prefix is an absolute path, so concatenate without os.path.
toolchain_path += \
targets.darwin_toolchain_prefix(args.install_prefix)

return toolchain_path


def run_build_script_helper(host_target, product, args):
toolchain_path = _get_toolchain_path(host_target, product, args)

# Our source_dir is expected to be './$SOURCE_ROOT/benchmarks'. That is due
# the assumption that each product is in its own build directory. This
# product is not like that and has its package/tools instead in
Expand Down