[SR-237] Merge build-script-impl into build-script

[gottesmm]

Previous ID	SR-237
Radar	None
Original Reporter	@gottesmm
Type	Improvement
Status	Reopened
Resolution

Additional Detail from JIRA

Votes	5
Component/s	Compiler
Labels	Improvement
Assignee	None
Priority	Medium

md5: 4ed4f6ff1e049872fedd73313bcf3878

relates to:

• SR-9031 Make build-script multiplatform

Issue Description:

Historically, the reason why there is build-script and build-script-impl is that original build-script-impl was build-script but we needed a better interface on top that suggested we wanted something in python (ala build-script), not bash (ala build-script-impl).

One thing that has been on the back burner for a while is to port build-script-impl to python and merge it into build-script so we only have one build-script.

[belkadan]

+1. Note that any work here should start by preserving all the existing semantics of build-script-impl. We can work on stripping things out after that.

[modocache]

I agree that this would be an improvement. I think we should merge `utils/SwiftBuildSupport.py`, `utils/build-script`, and `utils/build-script-impl` into a Python package. The package structure would allow us to split the functionality out into separate files with single responsibilities.

[modocache]

I've begun working on this:

• Determine number of parallel jobs in Python: [build-script] Do not determine build jobs in impl #760

• Find paths to clang, clang++ in Python: [build-script] Determine HOST_CC in build-script #761

The second pull request begins work on my suggestion to move all build-related logic to its own Python module. Feedback welcome!

[swift-ci]

Comment by Ron Pinz (JIRA)

In order to help facilitate this endeavor I will be submitting pull requests that restructure and harden the 'utils/build-script-impl' script. The current state of the script is a patchwork of changes that at times approaches being difficult to follow and was likely a contributing factor to this decision.

[modocache]

rpinz (JIRA User): Awesome!

Just to give you some additional context on what (little) I've done so far, I also recently submitted #792 My strategy has been to move logic out of `build-script-impl` piece by piece, starting from the top of the file. In that vein, my next set of changes will involve moving setting the default value for CMAKE_INSTALL_PREFIX and setting the deployment target options.

Once the majority of the script is moved to Python, I'd love to use a spiffy abstraction to emit the CMake build command--perhaps a factory object that vends a CMake configuration, which is then responsible for translating its attributes into a build invocation?

configuration = CMakeConfiguration().installPrefix('/usr').deploymentTarget('linux-armv7')
subprocess.check_call(configuration.command)

Of course these are just some ideas I had, feel free to take whichever direction you think is best. Would love to collaborate if possible, though!

[swift-ci]

Comment by Jay Buffington (JIRA)

Since CentOS 6.x still ships with python 2.6, it would be nice to not use python 2.7 or higher features.

[modocache]

jaybuff (JIRA User) Oh wow, good point! I wonder if maybe we should track that in a dedicated issue--I'm sure many build scripts don't work on Python 2.6. For example, I'm pretty sure that non-indexed string formatting (that is, "{}".format(value), without a 0 in between the {}) isn't supported in Python 2.6, but if you git grep "{}.*".format", you'll find a ton of places it's being used.

I think opening a dedicated issue, or discussing it on the mailing list, is the best way to go. We'll also need to make sure that it's tested somehow--it's very easy for developers to break Python for versions of the interpreter that they don't use themselves.

[swift-ci]

Comment by Jay Buffington (JIRA)

Thanks @modocache! I noticed the format one as well as subprocess using check_output, which is a 2.7 feature.

[lplarson]

@modocache: Any update on this?

[modocache]

It's moving along very slowly, meanwhile things get added to build-script-impl little by little. I think of the migration process as being composed of the following steps:

1. Move everything that isn't building/testing/installing out of build-script-impl. Things like creating a toolchain and running package tests on it, for example, should be moved to Python.

2. Move installing into Python.

3. Move testing into Python.

4. Move building into Python.

I think ideally the Python script could use some sort of abstraction responsible for generating an array of shell commands necessary to do any of the above steps.

Feel free to grab the task from me--I'm interested in working on it but am busy with other things just this moment.

[rintaro]

I'm working on this, but it would be not so fast.

[rintaro]

I posted a PR on #2190

You can try it locally by:

swift$ git fetch origin pull/2190/head:SR-237-rewrite
swift$ git checkout SR-237-rewrite
swift$ utils-experimental/build-script -RT

[rintaro]

PR #2190 was withdrawn.

We are doing incremental migration.
After several gardening works, I've started migration:

• Migrate COMMON_CMAKE_OPTIONS and BUILD_ARGS calculation: [build-script][SR-237] Migrate calculation of COMMON_CMAKE_OPTIONS and BUILD_ARGS to Python #2295 -> MERGED

• Migrate --skip-{variant}, --skip-build-{variant} arguments: [SR-237][build-script] Migrate --skip-{variant} --skip-build-{variant} arguments #2562 -> MERGED

• Migrate Ninja build:
  [SR-237][build-script] Migrate Ninja build to Python #2685 -> MERGED

Next action planned:

• Migrate num_llvm_parallel_lto_link_jobs() and num_swift_parallel_lto_link_jobs() calculation

• Migrate SWIFT_STDLIB_TARGETS, SWIFT_BENCHMARK_TARGETS, SWIFT_RUN_BENCHMARK_TARGETS, and SWIFT_TEST_TARGETS calculation

[ddunbar]

Does anyone have outstanding patches on this not reflected in current PRs? I would like to, maybe, contribute to this as part of my "Toolchain-based build process" proposal, but I don't want to do any work which is going to conflict with existing progress here.

[rintaro]

I don't have.
Current related PR in flight is
#2804

[ddunbar]

I posted the first part of one approach to allowing us to move the high-level control loop into `build-script`, here:
#2844

The basic idea is we would compute the high-level list of "actions" to perform in `build-script`, then invoke the `build-script-impl` once per action (in the right order). If we do that, it is then easy to incrementally move individual actions from the sequence to being entirely done via the `build-script`.

[modocache]

Various people have worked on this for half a year – I don't think it's a "starter bug", so I removed the label. 🙂

[gottesmm]

I just added code in #3965 to enable individual product cmake options to be ported to build-script. A large portion of build-script-impl involves these product specific arguments, so we should be able to thin build-script-impl considerably and remove a bunch of build-script-impl options.

[gottesmm]

Question to everyone watching this. Given that amount of work that everyone has done so far, I wonder if it makes sense to split this SR up into multiple SRs. I think some of them should be starter bugs such as thinning out build-script-impl via the option I just added... We could have migrating individual parts of the code into separate SRs...

Thoughts?

[gottesmm]

I have started to move some flags from build-script-impl => the product specific cmake support I added.

I am only going to do a few to provide examples for other people to use/copy/paste/etc.

[modocache]

Sorry for the delayed response. An enthusiastic +1 from me to split this up into several smaller tasks. Thanks, @gottesmm!!

[MaxDesiatov]

Hi d066z (JIRA User), I see this has been resolved, but build-script-impl is still present in the compiler repository. Would you be able to clarify the status of this? Isn't build-script-impl supposed to be removed after its functionality has been replaced by the Python build-script, or is my understanding of how this is supposed to work wrong?

[MaxDesiatov]

Reopening this as confirmed with one of the members of the Swift team. This hasn't been resolved yet.