Build script invocation helper #2919
Conversation
|
@swift-ci please Python lint |
|
@swift-ci please smoke test |
|
/cc @gribozavr @rintaro @karwa |
|
Yep, I certainly support the intent of this - will need to look at the code later. One problem I was encountering with my changes was that we can't really test the argument processing because it's all in one mega function which also calls build-script-impl. The way I would like to see this going is that BuildScriptInvocation parses out a set of lists from its parameters:
Then we loop over hosts(, targets) & products and call a configure()/build()/test() function. |
ddunbar
force-pushed
the
build-script-invocation-helper
branch
from
39e34fa to
46c3384
Compare
|
@swift-ci please smoke test |
|
Since this has gotten conflicted, I'm just going to merge this with #2880, which I have refactored to include it. |
- This is an object designed to represent a single invocation of the build script, primarily for use in decomposing the main program flow into its distinct conceptual pieces.
|
Actually, reopening this. I remember the reason I factored it out was because I was hoping to land it ASAP to limit the window of merge conflicts. |
ddunbar
force-pushed
the
build-script-invocation-helper
branch
from
46c3384 to
bc0f921
Compare
ddunbar
force-pushed
the
build-script-invocation-helper
branch
from
bc0f921 to
754521b
Compare
|
@swift-ci please Python lint |
|
@ddunbar LGTM! I verified with manual inspection that this refactoring does not drop any code. |
| if not os.path.exists(self.workspace.source_dir("ninja")): | ||
| diagnostics.fatal( | ||
| "can't find source directory for ninja " | ||
| "(tried %s)" % (self.workspace.source_dir("ninja"))) |
There was a problem hiding this comment.
This check was deferred after (possible) shell.rmtree(build_dir).
We should perform every validation before any destructive operation.
Anyway, I'm OK with it for now. build-script-impl still have many validations.
When we migrate them, we should rearrange them (including this).
What's in this pull request?
This factors out a
BuildScriptInvocationhelper object and then decomposes the high-level operation into the conceptual phases mentioned in #2804.This should make it more obvious what the high-level flow of the script is, and where new code belongs while respecting the conceptual divisions.
Part of SR-237.
Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
Validation Testing
Lint Testing
Note: Only members of the Apple organization can trigger swift-ci.