Skip to content

Rebuild dependencies when Swift version changed #63

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

Closed
fredpi opened this issue Jul 1, 2019 · 2 comments · Fixed by #65
Closed

Rebuild dependencies when Swift version changed #63

fredpi opened this issue Jul 1, 2019 · 2 comments · Fixed by #65
Assignees
Labels
bug Something isn't working

Comments

@fredpi
Copy link
Contributor

fredpi commented Jul 1, 2019

#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 ...

@fredpi fredpi added the bug Something isn't working label Jul 1, 2019
@fredpi fredpi self-assigned this Jul 1, 2019
@Jeehut
Copy link
Contributor

Jeehut commented Jul 2, 2019

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
Copy link
Contributor Author

fredpi commented Jul 2, 2019

@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.

@fredpi fredpi closed this as completed in #65 Jul 4, 2019
fredpi added a commit that referenced this issue Jul 4, 2019
Use current Swift version to fetch frameworks from cached manifest
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants