Add System to the toolchain #84057
Add System to the toolchain #84057
Conversation
milseman
force-pushed
the
system_toolchain
branch
from
456fa51 to
488f04e
Compare
|
|
||
| Whether this product is produced by build-script-impl. | ||
| """ | ||
| return True |
There was a problem hiding this comment.
We should avoid to implement new build products in build-script-impl unless it is absolutely necessary -- at first glance, it looks like SwiftSystem can build in the same "phase" as EarlySwiftDriver.
There was a problem hiding this comment.
Agree, we are trying to deprecate the bash script build-script-impl, not add to it. Take a look at the cmark, Testing, or massive LLVM Python product config for examples.
There was a problem hiding this comment.
I would love to not have to touch build-script-impl. Would these python files and build.ps1 then be the only things that need to be touched?
There was a problem hiding this comment.
Yes, this file would be longer, as you'd move that build config from build-script-impl here, as was done for LLVM before.
| @@ -61,7 +61,7 @@ set TMPDIR=%BuildRoot%\tmp | |||
| set NINJA_STATUS=[%%f/%%t][%%p][%%es] | |||
|
|
|||
| :: Build the -Test argument, if any, by subtracting skipped tests | |||
| set TestArg=-Test lld,lldb,swift,dispatch,foundation,xctest,swift-format,sourcekit-lsp, | |||
| set TestArg=-Test lld,lldb,swift,dispatch,system,foundation,xctest,swift-format,sourcekit-lsp, | |||
There was a problem hiding this comment.
From a quick peek at build.ps1, it looks we may need to change the Build-Foundation function to add the SwiftSystem dependency.
There was a problem hiding this comment.
Yeah, we will need to re-organise the build a bit. We will need to add a new Build-System invocations in the SDK phases (should measure with static and dynamic linking) and pass that to the Foundation build.
milseman
force-pushed
the
system_toolchain
branch
from
488f04e to
1c0a249
Compare
|
@swift-ci Please Build Toolchain |
|
@swift-ci Please Build Toolchain |
milseman
force-pushed
the
system_toolchain
branch
from
1c0a249 to
e5b2ab2
Compare
milseman
requested review from
etcwilde,
justice-adams-apple and
shahmishal
as code owners
| "--lldb-build-type", args.lldb_build_variant, | ||
| "--foundation-build-type", args.foundation_build_variant, | ||
| # TODO: Do I have the option here? | ||
| # "--swift-system-build-type", args.swift_system_build_variant, |
There was a problem hiding this comment.
I'm not sure if I should add the option here and do more plumbing, or if these arguments are just for the legacy build_script_impl path.
There was a problem hiding this comment.
Yes, these are only to pass flags to build-script-impl, which you correctly are no longer using for this new build product, so nothing to add here.
| def _for_each_host_target(self, base_target, body): | ||
| body(base_target) | ||
|
|
||
| # TODO: what do we do here? We're not built on Darwin |
There was a problem hiding this comment.
Note: I was cribbing off of SwiftTesting. Not sure whether I need all of this machinery. System won't be built on Darwin as part of the toolchain (it's like SwiftFoundation in that regard).
| override_deployment_version = None | ||
|
|
||
| # TODO: Do we need this, or should we adjust our own version? | ||
| if host_target.startswith('macosx'): |
There was a problem hiding this comment.
Similarly, should this just go away?
|
@edymtt @finagolfin I pushed updates that get us off of modifying build_script_impl. Could you take a look? |
|
@swift-ci Please Build Toolchain |
| const='Debug', | ||
| help='build the Debug variant of System') | ||
|
|
||
| option('--swift-system-tests-build-type', store('swift_system_tests_build_variant'), |
There was a problem hiding this comment.
I added this flag for Foundation only because it was super slow to build in release mode: is that a problem for System too? If not, no reason to add a separate flag for the test build variant.
| # `install_with_cmake` later which already has the same prefix. | ||
| self.cmake_options.define('CMAKE_INSTALL_PREFIX', '') | ||
|
|
||
| self.cmake_options.define('CMAKE_BUILD_TYPE', self.args.build_variant) |
There was a problem hiding this comment.
This is the default build variant, you'll want to override it with the system_build_variant flag you are adding.
4 participants
Add System to the toolchain, so that Foundation can depend on it.