When a file is changed, only mark targets that depend on it as out-of-date #1313
Conversation
|
@swift-ci Please test |
| /// necessary by scheduling re-preparation of a target where it isn't necessary. | ||
| /// | ||
| /// Returning `nil` indicates that all targets should be considered depending on the given target. | ||
| func targets(dependingOn targets: [ConfiguredTarget]) async -> [ConfiguredTarget]? |
There was a problem hiding this comment.
Would taking Target make more sense here? We don't really care about the possibly large set of configured targets as input. I'd argue that's even the case for output, but we do need the configured target eventually since that's what we track preparation of.
There was a problem hiding this comment.
Ah, Alex brought up a very valid point here. It's entirely possible that while the concept of a "target" may contain the file, the underlying target + platform may not - consider settings for including/excluding files per platform. So ConfiguredTarget really is the right abstraction here, thanks for remembering that one :).
There was a problem hiding this comment.
Added the following comment to SemanticIndexManager.filesDidChange
// Note that configured targets are the right abstraction layer here (instead of a non-configured target) because a
// build system might have targets that include different source files. Hence a source file might be in target T
// configured for macOS but not in target T configured for iOS.| // We couldn't determine which targets depend on the modified targets. Be conservative and assume all of them do. | ||
| preparationStatus = [:] |
There was a problem hiding this comment.
We could not remove targets in this case, not that it's really expected to happen anyway.
There was a problem hiding this comment.
The only case where we’d be able to not remove the target itself is if there is only a single target. Say you have a project two targets A and B and you change one file in A and one file in B then you don’t know if they depend on each other and you need to assume that they do. So you need to invalidate both.
I don’t think it’s worth optimizing for the single module case. Really, the build system should provide more accurate information here.
There was a problem hiding this comment.
Single target is the main case though 😅 I agree the buildsystem should, but asking for preparation on every edit is really pretty bad.
There was a problem hiding this comment.
In one of my follow-up PRs (not up yet), I’m re-implementing the entire up-to-date tracking of targets and that allows us to early-exit if we know that the target is up-to-date without any build system interaction.
ahoppen
force-pushed
the
only-mark-dependent-targets-as-out-of-date
branch
from
60d8a3e
to
a850cb6
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
No description provided.