Skip to content

Make emit-module-separately the default incremental build mode #730

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

Jump to bottom
Merged
merged 6 commits into from
Sep 9, 2021
Merged

Make emit-module-separately the default incremental build mode #730

merged 6 commits into from
Sep 9, 2021

Conversation

xymus
Copy link
Contributor

@xymus xymus commented Jun 24, 2021

Making emit-module-separately the default build mode will improve the reliability of incremental builds. This mode builds the module files in a distinct job which only parses the module API and doesn't use the merge-module phase.

In support, this PR fixes a few issues and update a lot of tests expecting the precise behavior of merge-module.

rdar://77225764

@xymus
Copy link
Contributor Author

xymus commented Jun 24, 2021

@swift-ci Please test

@xymus xymus force-pushed the emit-module-default branch from ab1f911 to a0c35b4 Compare June 25, 2021 18:44
@xymus
Copy link
Contributor Author

xymus commented Jun 25, 2021

@swift-ci Please test

@xymus xymus mentioned this pull request Jul 1, 2021
Merged
@xymus
Copy link
Contributor Author

xymus commented Jul 1, 2021

Does that work here?

swiftlang/swift#38211
@swift-ci Please test

@artemcm
Copy link
Contributor

artemcm commented Jul 1, 2021

Does that work here?

apple/swift#38211
@swift-ci Please test

I don't think so.
It definitely works the other way around though, from the apple/swift repo CI, which will also run swift-driver tests.

@xymus xymus mentioned this pull request Aug 10, 2021
Merged
@xymus xymus force-pushed the emit-module-default branch from a0c35b4 to 69a858f Compare August 18, 2021 21:52
@xymus
Copy link
Contributor Author

xymus commented Aug 18, 2021

Rebasing on top of #790. I would expect only some incremental build tests to fail until swiftlang/swift#38939 lands.

@swift-ci Please test

@xymus
Copy link
Contributor Author

xymus commented Aug 19, 2021

@swift-ci Please test

This was referenced Aug 19, 2021
Draft
Merged
@xymus
Copy link
Contributor Author

xymus commented Aug 24, 2021

@swift-ci Please test

@xymus
Copy link
Contributor Author

xymus commented Aug 27, 2021

The toolchain should have been updated with recent compiler fixes.

@swift-ci Please test

@xymus xymus force-pushed the emit-module-default branch from 69a858f to d64c34a Compare August 30, 2021 22:04
@xymus
Copy link
Contributor Author

xymus commented Aug 30, 2021

@swift-ci Please test

@shahmishal
Copy link
Member

@swift-ci test

@artemcm
Copy link
Contributor

artemcm commented Sep 1, 2021

@nkcsgexi , this above run is with an updated CI config to always use the snapshot toolchain's swift-frontend and now we are seeing:

error: generate-api-baseline command failed with exit code 1 (use -v to see invocation)
/Users/buildnode/jenkins/workspace/pr-swift-driver-macos/branch-main/swift-driver/Sources/SwiftDriverExecution/MultiJobExecutor.swift:307: error: -[SwiftDriverTests.APIDigesterTests testAPIComparisonEndToEnd] : failed: caught error: "fatalError"

Could you please take a look?

xymus added 2 commits September 1, 2021 09:29
@xymus
Make emit-module-separately the default incremental build mode
74aee03 
Making emit-module-separately the default build mode will make
incremental build more reliable. This mode build the module files in a
distinct job that only parses the module API and doesn't use the
merge-module phase.

rdar://77225764
@xymus
[Tests] Update tests for emit-module-separately by default
21bd47d
@xymus xymus force-pushed the emit-module-default branch from d64c34a to 21bd47d Compare September 1, 2021 16:30
@xymus
Copy link
Contributor Author

xymus commented Sep 1, 2021

The last macOS job with the SwiftDriverTests.APIDigesterTests failure is at https://ci.swift.org/job/pr-swift-driver-macos/1445/

Running again after removing the debug print.
@swift-ci Please test

@nkcsgexi
Copy link
Contributor

nkcsgexi commented Sep 1, 2021

Skipping the test here: #818

@xymus
Copy link
Contributor Author

xymus commented Sep 2, 2021

@swift-ci Please test

@xymus
Copy link
Contributor Author

xymus commented Sep 9, 2021

@swift-ci Please test

@xymus
Copy link
Contributor Author

xymus commented Sep 9, 2021

Good point and it's cleaner too. Added a commit with @artemcm's suggestion.

@swift-ci Please test

@xymus xymus marked this pull request as ready for review September 9, 2021 16:45
@xymus
Copy link
Contributor Author

xymus commented Sep 9, 2021

Marking as ready for review as the compiler-side tests are looking promising: swiftlang/swift#38454

@xymus
Copy link
Contributor Author

xymus commented Sep 9, 2021

Added support for the symbol-graphs flag causing the failure of the list test SymbolGraph/EmitWhileBuilding.swift.

@swift-ci Please test

@xymus
Copy link
Contributor Author

xymus commented Sep 9, 2021

The compiler-side tests are green too. Time to merge this.

@xymus xymus merged commit af42b87 into swiftlang:main Sep 9, 2021
@artemcm
Copy link
Contributor

artemcm commented Sep 9, 2021

:shipit: 🍾

@artemcm artemcm mentioned this pull request Oct 8, 2021
Merged
@xymus xymus deleted the emit-module-default branch May 26, 2022 23:40
@xymus xymus mentioned this pull request Apr 3, 2023
Closed
This was referenced Apr 4, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@artemcm artemcm artemcm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@xymus @artemcm @shahmishal @nkcsgexi