[build-script] Extract most parts related to build-script-impl #23982
Conversation
drodriguez
force-pushed
the
build-script-extract-build-script-impl-helper
branch
from
3649471 to
5880b04
Compare
|
@swift-ci please test |
drodriguez
force-pushed
the
build-script-extract-build-script-impl-helper
branch
from
5880b04 to
7aac72d
Compare
|
Ready for review. Fixed the conflicts with current master, and checked it still works as intended. @swift-ci please test |
drodriguez
force-pushed
the
build-script-extract-build-script-impl-helper
branch
from
7aac72d to
f24db94
Compare
|
Build failed |
|
Build failed |
| def validate_arguments(toolchain, args): | ||
| if toolchain.cc is None or toolchain.cxx is None: | ||
| diagnostics.fatal( | ||
| "can't find clang (please install clang-3.5 or a " |
There was a problem hiding this comment.
I don't think that we should hardcode the version here, its too easy to forget to update it.
There was a problem hiding this comment.
This code is just a copy of the existing code. You are right that hardcoding '3.5' in the string is not the best idea, but I don’t think there's a good way (right now) to do it better.
The accepted versions are “hardcoded” in swift_build_support.toolchain, and they are not easy to query. I can say that 3.5 seems to still be the minimum version (but this is only the executable suffix, so…).
Most parts related to build-script-impl moved into its own type BuildScriptImplHelper. This will make moving this helper to its own file and to a builder easier later. The parts that haven't moved are the path of the script (still a constant in the main build-script file), and the invocation to check the migration parameters. Those will be dealt later. The changes move the methods that turn build-script args into build-script-impl args and creates the environment variables into this new helper. It also moves the specific of building action names and how to invoke the script from their previous places in BuildScriptInvocation into the Helper.
drodriguez
force-pushed
the
build-script-extract-build-script-impl-helper
branch
from
f24db94 to
eb818b6
Compare
|
@swift-ci please smoke test |
|
Abandoning these efforts. It might still be useful for better Windows builds for most of the community, but I will not have time to keep improving this and it doesn't seem to be a lot of interest in improving the build system in this direction. |
Most parts related to build-script-impl moved into its own type
BuildScriptImplHelper. This will make moving this helper to its own file
and to a builder easier later.
The parts that haven't moved are the path of the script (still a
constant in the main build-script file), and the invocation to check the
migration parameters. Those will be dealt later.
The changes move the methods that turn build-script args into
build-script-impl args and creates the environment variables into this
new helper. It also moves the specific of building action names and how
to invoke the script from their previous places in BuildScriptInvocation
into the Helper.
Sadly I wasn’t able to modify the resulting diff in a nice way that the implementations of
validate_arguments,apply_default_arguments,__init__andinitialize_runtime_environmentinBuildScriptInvocationdoesn’t seem to be modified. I just simply moved the code around them. At least one can see what the modifications in_convert_to_impl_argumentsand_convert_to_impl_argumentslooks like.This was part of #23257. I’m trying to split the PR in smaller ones to make the reviews easier. Other PRs in this group are #23303, #23803, #23810, #23822, #23865, #23915, #23917, and #23918.