[3.0] [Driver] Make sure to rebuild dependents when a dirty file fails. #3957
Conversation
…iftlang#3935) Otherwise we get into a situation like this: 1. Change made to the interface of file A.swift that also causes an error in A.swift. 2. Fixing the error in A.swift does not affect A.swift's interface. 3. File B.swift that depends on A.swift is not rebuilt, since the most recent change to A.swift did not change its interface. To fix this, mark downstream files as needing to be rebuilt even when a compilation job fails with errors. Additionally, attempt to be extra conservative when there's a crash. rdar://problem/25405605 (cherry picked from commit 848b3eb)
|
No one else works on this area of the compiler, but I'd still like someone else's eyes on the change. @bitjammer, @modocache, and/or @gribozavr, would you mind taking a look? (Feel free to ask questions.) |
|
@swift-ci Please test |
jrose-apple
changed the title
|
My pleasure! I appreciate the mention, I definitely want to get more familiar with this code. /cc @Samjohn, who expressed interested in incremental compilation. |
| if os.path.basename(primaryFile) == 'bad.swift': | ||
| sys.exit(1) | ||
| else: | ||
| sys.exit(129) | ||
|
|
||
| dir = os.path.dirname(os.path.abspath(__file__)) |
There was a problem hiding this comment.
nit-pick: dir is actually a built-in Python function. In a future commit, it might be nice to prevent the shadowing that function here.
|
sigh @Samjohn, I still owe you some more details and better tools, if not actual improvements. (Re-link to DependencyAnalysis.rst for all involved.) |
Thanks, Dmitri. (cherry picked from commit b8cc267)
| StringRef DependenciesFile = | ||
| Output.getAdditionalOutputForType(types::TY_SwiftDeps); | ||
| if (!DependenciesFile.empty() && | ||
| (ReturnCode == EXIT_SUCCESS || ReturnCode == EXIT_FAILURE)) { |
There was a problem hiding this comment.
I interpret this reference to mean that ReturnCode must always be either EXIT_SUCCESS or EXIT_FAILURE. Is there a case in which it is neither? In other words, is this check necessary?
There was a problem hiding this comment.
Yeah, it's a check for crashes. It's not the most direct way to check for abnormal exit, but it ought to work.
|
@modocache, since all your suggestions are cosmetic, does it look okay to land otherwise? @swift-ci Please test |
|
@jrose-apple Yup! Thanks again for the chance to look at it. |
(not yet reviewed)@modocacheBefore 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
A smoke test on macOS does the following:
device standard libraries are not built.
version of these tests are not run.
A smoke test on Linux does the following:
tests are not run.
Validation Testing
Lint Testing
Note: Only members of the Apple organization can trigger swift-ci.