JamitLabs / Accio

A dependency manager driven by SwiftPM that works for iOS/tvOS/watchOS/macOS projects.
MIT License
660 stars 32 forks source link

Rebuild dependencies when Swift version changed #63

Closed fredpi closed 5 years ago

fredpi commented 5 years ago

60 introduced a bug that causes dependencies not to be rebuilt or downloaded from cache although the Swift version changed.

To fix this, the Swift versions in the cached manifest should be parsed and compared against the current Swift version. If there's a difference, Accio shouldn't return saying Found all required build products specified in resolved manifest in cache – skipping checkout & integration process ...

Jeehut commented 5 years ago

Are you sure that we have this bug? Cause in this line there is an early return in case that any of the required frameworks is not cached yet and given that we use the current Swift version here when building the cache path. If anything I think that solving #62 will also fix this, then.

fredpi commented 5 years ago

@Dschee I'm also experiencing this when running Accio with the changes from #62, so it's not solved via that PR.

I looked up the code and found the issue. It is with the cacheFileSubPath of CachedFrameworkProduct, implemented as follows:

struct CachedFrameworkProduct: Codable {
    let swiftVersion: String
    let libraryName: String
    let commitHash: String
    let platform: String

    var cacheFileSubPath: String {
        return "\(swiftVersion)/\(libraryName)/\(commitHash)/\(platform).zip"
    }
}

As the swiftVersion is actually stored in the Manifest json, it's this cached Swift version that is used to copy the framework and check for the existence of the files (therefore not triggering the check you referenced). It should instead use the current Swift version, as the Package may have been resolved with Swift 5.0.1, while the user is now running Swift 5.1.

Is it needed to store a resolved manifest json for each Swift version? (like, is it possible that the resolving result is different for different Swift versions?) In that case, we shouldn't store the swiftVersion in the manifest json, but rather include it in the file name.

Otherwise, I propose to just drop the swiftVersion property from the CachedFrameworkProduct and use the current Swift version for path calculation instead.