Skip to content

[NFC, Incremental] Test effect of a minor version number change on the priors #729

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.

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

Merged
merged 3 commits into from
Jun 25, 2021

Conversation

davidungar
Copy link
Contributor

@davidungar davidungar commented Jun 24, 2021

Test effect of a minor version number change on the priors, and also add the needed affordances.
Also increment the minor version to reflect the omission of the no-longer-needed serialized inputDependencySourceMap.

Required before landing #728

@davidungar davidungar requested review from CodaFi and artemcm June 24, 2021 19:42
@davidungar
Copy link
Contributor Author

@swift-ci please test

@@ -218,9 +218,14 @@ extension IncrementalCompilationState.IncrementalDependencyAndInputSetup {
do {
graphIfPresent = try ModuleDependencyGraph.read( from: dependencyGraphPath, info: self)
}
catch ModuleDependencyGraph.ReadError.mismatchedSerializedGraphVersion {
diagnosticEngine.emit(
warning: "Will not do cross-module incremental builds, wrong version of priors at '\(dependencyGraphPath)'")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be helpful to print out the two versions, to give developers a better idea of what "wrong" means here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea! Will, do.

on: localFileSystem,
compilerVersion: compilerVersion,
mockSerializedGraphVersion: incrementedVersion)
// Reset mod time to priors modTime on newly-written priors
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear what this comment is referring to.

Copy link
Contributor Author

@davidungar davidungar Jun 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that was a marker for a future PR, but best to remove it for now.

@davidungar
Copy link
Contributor Author

@swift-ci please test

@davidungar davidungar merged commit 4408f19 into swiftlang:main Jun 25, 2021
@davidungar davidungar deleted the increment-minor branch July 3, 2021 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants