[Incremental] Write build record & test #298
Conversation
| @swift-ci please test |
davidungar force-pushed the incremental-10-9c-write-build-rec branch from 9adcb5d to 81d55fa Compare davidungar force-pushed the incremental-10-9c-write-build-rec branch from a9c7850 to 9bc4730 Compare davidungar changed the title | @swift-ci please test |
davidungar force-pushed the incremental-10-9c-write-build-rec branch from 87fe8ca to 07b2b17 Compare | @swift-ci please test |
Sources/SwiftDriver/Incremental Compilation/BuildRecordInfo.swift Outdated Show resolved Hide resolved
Sources/SwiftDriver/Incremental Compilation/BuildRecordInfo.swift Outdated Show resolved Hide resolved
Sources/SwiftDriver/Incremental Compilation/InputIInfoMap.swift Outdated Show resolved Hide resolved
| public init?(identifier: String) { | ||
| switch identifier { | ||
| case "": self = .upToDate | ||
| case "!dirty": self = .needsCascadingBuild | ||
| case "!private": self = .needsNonCascadingBuild | ||
| default: return nil | ||
| } | ||
| assert(self.identifier == identifier, "Ensure reversibility") | ||
| } |
There was a problem hiding this comment.
We'd lose the nicer argument label, but making Status a string-backed enum might be worth it to maintain consistency with the identifier property below
There was a problem hiding this comment.
I'm not sure how to do that, given that there are two statuses with the same string:
case .needsCascadingBuild, .newlyAdded: return "!dirty" There was a problem hiding this comment.
Oh yeah, I missed that, thanks! This looks good to me 👍
| catch let InputInfoMap.Errors.notAbsolutePath(p) { | ||
| diagnosticEngine.emit( | ||
| .warning_could_not_write_build_record_not_absolutePath(p)) | ||
| return | ||
| } |
There was a problem hiding this comment.
Are there any build systems that rely on using a relative build record path in the output file path right now? (Bazel comes to mind as a possibility, but I'm not sure) Instead of failing here, we could try to resolve the path against the file system's cwd
There was a problem hiding this comment.
Good idea, but I think it's OK.
If I read the following code correctly (in Driver.swift):
if let workingDirectory = self.workingDirectory { self.outputFileMap = outputFileMap?.resolveRelativePaths(relativeTo: workingDirectory) } else { self.outputFileMap = outputFileMap } the warning will only happen if there is no working directory and the map contains a relative path.
There was a problem hiding this comment.
Driver.workingDirectory will only be non-nil if the user explicitly passes one with -working-directory, so we can still end up in this situation if the file system has one but the user didn't pass the command line option. That said, this is a pretty rare edge case and I don't think we need to handle it perfectly right now, so I'm fine with leaving this as-is
Sources/SwiftDriver/Incremental Compilation/InputIInfoMap.swift Outdated Show resolved Hide resolved
| @swift-ci please test |
| @swift-ci please test |
There was a problem hiding this comment.
@davidungar Looks good to me, thanks!
Sorry if I sound like a broken record about the CMake stuff, I've broken the build more times than I'd care to admit by forgetting to update it 🙂
| @@ -0,0 +1,245 @@ | |||
| //===--------------- BuildRecord.swift - Swift Input File Info Map -------===// | |||
There was a problem hiding this comment.
The CMakeLists.txt entry for InputInfoMap will need to be renamed to BuildRecord
| @@ -0,0 +1,148 @@ | |||
| //===--------------- BuildRecordInfo.swift --------------------------------===// | |||
There was a problem hiding this comment.
This will need a new entry in CMakeLists.txt
There was a problem hiding this comment.
Thank you--yes, I remembered that too late, and I really appreciate the reminder. I will fix those.
| @swift-ci please test |
2 participants
Adds
BuildRecordInfoto hold information needed to read and write build records. Writes build records whether incremental or not. Reads when incremental. Tests reading and writing.