JamitLabs / Accio

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

Support prebuilt binaries #5

Closed fredpi closed 4 years ago

fredpi commented 5 years ago

There should be support to include prebuilt binaries. While the actual implementation will probably be quite straightforward, it is unclear how those binaries should best be specified in the Package.swift.

Jeehut commented 5 years ago

I agree that this is probably easy to add feature wise within Accio (it's just downloading a file) – but I'm not sure how we could make sure this adheres to the goal of Accio of working towards SwiftPM since I don't see any binary download feature in SwiftPM. I think what we should do is to pitch the idea of specifying binary URLs within the package manifest on Swift Evolution and see what the general feedback looks like.

As an alternative (or in the meantime) we could do something like this:

let package = Package(
    name: "NewProjectTemplate",
    products: [],
    dependencies: [
        .package(url: "https://github.com/Flinesoft/HandySwift.git", .upToNextMajor(from: "2.8.0")),
        .package(url: "https://github.com/Flinesoft/HandySwift.git", .upToNextMajor(from: "2.8.0")),
        // .binary(url: "https://building42.github.io/Specs/Carthage/iOS/Crashlytics.json", .upToNextMajor(from: "3.12.0")),
        // .binary(url: "https://building42.github.io/Specs/Carthage/iOS/Fabric.json", .upToNextMajor(from: "1.9.0")),
    ],
    targets: [
        .target(
            name: "App",
            dependencies: [
                "HandySwift",
                // "Crashlytics",
                // "Fabric",
            ],
            path: "App"
        ),
    ]
)

But please note that in this case we need to add parsing capabilities for all version specifier styles listed here (of course we could copy the code from the SwiftPM project). Also note that I'm not a fan of commenting the entries within the targets dependencies array since it's not clear why they are commented out, while I can live with commenting the .binary entries since all .binary entries need to be commented out.

Any better ideas on how we could tackle this?

Plase note that as a workaround, users of Accio can always fall back to Carthage for binaries only by keeping a Cartfile around that only includes the binary dependencies and run the Carthage commands they're used to (a build script isn't required for binary dependencies in Carthage AFAIK).

fredpi commented 5 years ago

@Dschee Thanks for your suggestions.

I think we should do both things: Pitch on Swift Evolution for Swift PM to support binaries and in the meantime also implement parsing for commented binary specifications.

However, it is unclear how to proceed, if binary specifications won't be accepted for Swift PM... Generally speaking, there are a couple of things that could improve Accio, if it wouldn't be tied close to the Swift PM. Apart from binary specifications, one of those is to allow for dependencies to refer to different versions of a framework each without having the dependency graph fail because of conflicting version requirements.

I suggest an approach where all features that are added to Accio on top of Swift PM are considered valid, if the Package.swift manifest of dependencies remains valid for use with Swift PM and if there are no accio-specific requirements for the Package.swift file of dependencies. When taking such an approach, implementing binary parsing using a commented syntax would be fine even if binary specifications will be declined for Swift PM. However, dependencies themselves wouldn't be able to include binaries, that would only be allowed top-level.

When adding binary parsing capabilities to Accio, I suggest to implement the entire parsing and dependency graph resolution logic directly in Accio, without using Swift PM.

Jeehut commented 5 years ago

Using SwiftPM as much as possible is part of this projects goals.

I don't think we should reimplement dependency graph resolution or manifest parsing. They are already implemented in SwiftPM and if the community decided that conflicting version requirements should exist, than there probably is a good reason behind that. Also note that it is currently already possible to allow different versions of a framework – the library authors can opt for from: "1.0.0" instead of the more restrictive .upToNextMajor(from: "1.0.0") if they like and fixing such issues isn't too hard, too. But making sure conflicting versions are shown as errors prevents any undefined behavior or ambiguity along the way.

The only thing I noticed about dependency resolution which I don't like is that when a target A which has B as it's subdependency is not used in an app, that SwiftPM is checking out and resolving B regardless (although it might not be needed). This could be improved in SwiftPM though so everybody (also the SwiftPM community) profits from the change.

In case you want to further discuss the Version Requirements topic, please open a separate issue for that as it doesn't have much to do with this one and people won't be able to find it easily. Also feel free to open an issue about custom dependency graph resolution in order to keep that discussion separate from here, too. If that's an approach many people think is a good idea, I'd be happy to accept PRs that add a non-SwiftPM based dependency resolution approach as an option. But as of now, I don't think that's a way we should go just to solve this issue.

Jeehut commented 5 years ago

@fredpi What is your opinion on my suggested design in https://github.com/JamitLabs/Accio/issues/5#issuecomment-479810415? I would happily review & merge a related PR – even if not all version requirement variants aren't implemented as long as this is documented well enough. Would you be up to a PR?

fredpi commented 5 years ago

@Dschee I was a bit hesitant replying as I was thinking about the general evolution of Accio and how it depends / should depend on Swift PM. What if Swift PM explicitly declines binary specifications, should Accio still support it?

I'd happily create a PR implementing binary specifications, yet I believe that we should first define in general what happens to an Accio feature implemented on top of Swift PM, if Swift PM "declines" this feature.

What do you think? Should I already begin with an implementation of binary specifications, while discussion about the general evolution strategy of Accio goes on in a dedicated thread?

mrylmz commented 5 years ago

It should be possible to resolve this issue once we are supporting swift build in Accio.

One possible solution would be to create an umbrella project with a modulemap and limit the configuration to only contain linkage information if it is possible (so that SwiftPM will not search for a System Library). Then we could lookup in a subfolder in the project like ./Binaries for libraries and do something similar to this.

For further reading: Module Map Specification

Jeehut commented 5 years ago

@fredpi While I always try to design things like they would be designed in SwiftPM to minimize the risk of having breaking changes on SwiftPM updates, Accio was also explicitly designed to fill the gaps where SwiftPM doesn't provide a solution. So if the feature was declined by the Swift community (for whatever reason) as long as having the feature improves dependency management for the community (which it does) I think we should still implement it. So, in any case, we'd have the feature independent from the decision the community makes.

Only if they decide to implement it, we would need to change our implementation to what they are doing (e.g. instead of a JSON they might specify the binaries right within the Package.swift file) – but that shouldn't stop us from implementing it now. So feel free to go ahead.

@mrylmz I'm not sure why this is only possible to resolve after we have implemented #32? From my point of view this feature is not dependent to any other feature and can be implemented in its own way with the approach I suggested above. But maybe I didn't quite understand what you mean ...

mrylmz commented 5 years ago

Well my Idea was a work-around solution to rely on the current feature set of SwiftPM without adding a new binary framework feature.

The SwiftPM does already support building c-family language library targets. We could possibly use that feature to embed binary framework support by generating correct module.modulemap files and linking the framework manually while executing the swift build command. Thats the reason why we would depend on the swift build system.

mrylmz commented 5 years ago

You can have a look ok this for example. There is only a modulemap and the correct library headers.

The missing part here is the retrieval of the binary it self that is currently made manually by the developer using brew or another dependency manager.

Jeehut commented 5 years ago

I've had a look into your linked example and as far as I understand, you have to install the library via Homebrew and then SwiftPM is only used for linking purposes, is that right? Also, the library can't simply provide binaries and links to them, but they also have to create weird modulemap files which we could try to automate, but is that going to be so easy, really? Also, how would this work without Homebrew?

I don't really see how this solves the problem better than my suggestion made in https://github.com/JamitLabs/Accio/issues/5#issuecomment-479810415.

fredpi commented 4 years ago

Closing this, as Accio is now deprecated.