[swift-build-dev] [swift-evolution] [Draft] Package Manager Manifest API Redesign
Ankit Aggarwal
ankit_aggarwal at apple.com
Mon Feb 27 04:21:15 CST 2017
Hi David,
Thanks for the feedback! Comments inline:
On Sun, Feb 26, 2017 at 5:08 AM, David Hart via swift-build-dev <
swift-build-dev at swift.org> wrote:
> Was looking forward to this :) here are my comments:
>
> On 25 Feb 2017, at 01:35, Rick Ballard via swift-evolution <
> swift-evolution at swift.org> wrote:
>
> Hi all,
>
> Ankit, Daniel, Anders, Boris and I have a draft proposal in progress for a
> Package.swift manifest API redesign for the Package Manager. We'll welcome
> comments or discussion at this time. My hope is that we can get this
> polished up and ready for evolution within the next week or so, but we'll
> see how the conversation goes!
>
> You can see the proposal in progress at https://github.com/
> aciidb0mb3r/swift-evolution/blob/manifest-api-redesign/
> proposals/xxxx-package-manager-manifest-api-redesign.md. I'm also
> including the current version inline in this email.
>
> Thanks,
>
> - Rick
>
> # Package Manager Manifest API Redesign
>
> * Proposal: [SE-XXXX](xxxx-package-manager-manifest-api-redesign.md)
> * Author: [Ankit Aggarwal](https://github.com/aciidb0mb3r)
> * Review Manager: TBD
> * Status: **Discussion**
>
> ## Introduction
>
> This is a proposal for redesigning the `Package.swift` manifest APIs
> provided
> by Swift Package Manager.
> This proposal only redesigns the existing public APIs and does not add any
> new functionality; any API to be added for new functionality will happen in
> separate proposals.
>
> ## Motivation
>
> The `Package.swift` manifest APIs were designed prior to the [API Design
> Guidelines] (https://swift.org/documentation/api-design-guidelines/), and
> their
> design was not reviewed by the evolution process. Additionally, there are
> several small areas which can be cleaned up to make the overall API more
> "Swifty".
>
> We would like to redesign these APIs as necessary to provide clean,
> conventions-compliant APIs that we can rely on in the future. Because we
> anticipate that the user community for the Swift Package Manager will grow
> considerably in Swift 4, we would like to make these changes now, before
> more packages are created using the old API.
>
> ## Proposed solution
>
> Note: Access modifier is omitted from the diffs and examples for brevity.
> The
> access modifier is `public` for all APIs unless specified.
>
> * Remove `successor()` and `predecessor()` from `Version`.
>
> These methods neither have well defined semantics nor are used a lot
> (internally or publicly). For e.g., the current implementation of
> `successor()` always just increases the patch version.
>
>
> <details>
> <summary>View diff</summary>
> <p>
> ```diff
> struct Version {
> - func successor() -> Version
>
> - func predecessor() -> Version
> }
> ```
> </p></details>
>
> * Make all properties of `Package` and `Target` mutable.
>
> Currently, `Package` has three immutable and four mutable properties,
> and
> `Target` has one immutable and one mutable property. We propose to make
> all
> properties mutable to allow complex customization on the package object
> after initial declaration.
>
> <details>
> <summary>View diff and example</summary>
> <p>
>
> Diff:
> ```diff
> final class Target {
> - let name: String
> + var name: String
> }
>
> final class Package {
> - let name: String
> + var name: String
>
> - let pkgConfig: String?
> + var pkgConfig: String?
>
> - let providers: [SystemPackageProvider]?
> + var providers: [SystemPackageProvider]?
> }
> ```
>
> Example:
> ```swift
> let package = Package(
> name: "FooPackage",
> targets: [
> Target(name: "Foo", dependencies: ["Bar"]),
> ]
> )
>
> #if os(Linux)
> package.targets[0].dependencies = ["BarLinux"]
> #endif
> ```
> </p></details>
>
> * Change `Target.Dependency` enum cases to lowerCamelCase.
>
> According to API design guidelines, everything other than types should
> be in lowerCamelCase.
>
> <details>
> <summary>View diff and example</summary>
> <p>
>
> Diff:
> ```diff
> enum Dependency {
> - case Target(name: String)
> + case target(name: String)
>
> - case Product(name: String, package: String?)
> + case product(name: String, package: String?)
>
> - case ByName(name: String)
> + case byName(name: String)
> }
> ```
>
> Example:
> ```diff
> let package = Package(
> name: "FooPackage",
> targets: [
> Target(
> name: "Foo",
> dependencies: [
> - .Target(name: "Bar"),
> + .target(name: "Bar"),
>
> - .Product(name: "SwiftyJSON", package: "SwiftyJSON"),
> + .product(name: "SwiftyJSON", package: "SwiftyJSON"),
> ]
> ),
> ]
> )
> ```
> </p></details>
>
> * Add default parameter to the enum case `Target.Dependency.product`.
>
> The associated value `package` in the (enum) case `product`, is an
> optional
> `String`. It should have the default value `nil` so clients don't need
> to
> write it if they prefer using explicit enum cases but don't want to
> specify
> the package name i.e. it should be possible to write `.product(name:
> "Foo")` instead of `.product(name: "Foo", package: nil)`.
>
> If
> [SE-0155](https://github.com/apple/swift-evolution/
> blob/master/proposals/0155-normalize-enum-case-representation.md)
> is accepted, we can directly add a default value. Otherwise, we will
> use a
> static factory method to provide default value for `package`.
>
> * Upgrade `SystemPackageProvider` enum to a struct.
>
> This enum allows SwiftPM System Packages to emit hints in case of build
> failures due to absence of a system package. Currently, only one system
> package per system packager can be specified. We propose to allow
> specifying multiple system packages by replacing the enum with this
> struct:
>
> ```swift
> public struct SystemPackageProvider {
> enum PackageManager {
> case apt
> case brew
> }
>
> /// The system package manager.
> let packageManager: PackageManager
>
> /// The array of system packages.
> let packages: [String]
>
> init(_ packageManager: PackageManager, packages: [String])
> }
> ```
>
> <details>
> <summary>View diff and example</summary>
> <p>
>
> Diff:
> ```diff
> -enum SystemPackageProvider {
> - case Brew(String)
> - case Apt(String)
> -}
>
> +struct SystemPackageProvider {
> + enum PackageManager {
> + case apt
> + case brew
> + }
> +
> + /// The system package manager.
> + let packageManager: PackageManager
> +
> + /// The array of system packages.
> + let packages: [String]
> +
> + init(_ packageManager: PackageManager, packages: [String])
> +}
> ```
>
> Example:
>
> ```diff
> let package = Package(
> name: "Copenssl",
> pkgConfig: "openssl",
> providers: [
> - .Brew("openssl"),
> + SystemPackageProvider(.brew, packages: ["openssl"]),
>
> - .Apt("openssl-dev"),
> + SystemPackageProvider(.apt, packages: ["openssl",
> "libssl-dev"]),
> ]
> )
> ```
> </p></details>
>
>
> Why not keep the enum and change the associated type to a String array?
>
>
True, we could do that but we'd be repeating that information in every
SystemPackager we add. Converting to a struct makes it easier to scale.
> * Remove implicit target dependency rule for test targets.
>
> There is an implicit test target dependency rule: a test target
> "FooTests"
> implicity depends on a target "Foo", if "Foo" exists and "FooTests"
> doesn't
> explicitly declare any dependency. We propose to remove this rule
> because:
>
> 1. It is a non obvious "magic" rule that has to be learned.
> 2. It is not possible for "FooTests" to remove dependency on "Foo" while
> having no other (target) dependency.
> 3. It makes real dependencies less discoverable.
> 4. It may cause issues when we get support for mechanically editing
> target
> dependencies.
>
> * Introduce an "identity rule" to determine if an API should use an
> initializer
> or a factory method:
>
>
> Could you explain this rule in more detail. What is an identity in this
> case? I'm confused.
>
This is similar to the rule we use to decide if something should be a
struct or a class. If you're forming a concrete object, that would be an
identity. Consider these two examples:
1. Target and its dependencies:
Target(name: "Foo", dependencies: [.target(name: "Bar")])
Here the target Foo is being constructed, so an initializer is used. The
target Bar is being referred in Foo's dependencies so that uses a factory
method.
2. Product and product dependencies in targets:
When constructing the product, the initializer should be used:
.Library(name: "FooLib", targets: ["Foo", "Utility"])
And while referring to the product, like in target dependency, factory
method should be used:
Target(name: "Foo", dependencies: [.product(name: "FooLib")])
> Under this rule, an entity having an identity, will use a type
> initializer
> and everything else will use factory methods. `Package`, `Target` and
> `Product` are identities. However, a product referenced in a target
> dependency is not an identity.
>
> This means the `Product` enum should be converted into an identity. We
> propose to introduce a `Product` class with two subclasses: `Executable`
> and `Library`. These subclasses will be nested inside `Product` class
> instead of being a top level declaration in the module. The major
> advantage of nesting is that we get a namespace for products and it is
> easy
> to find all the supported products when the product types grows to a
> large
> number. A downside of nesting is that the product initializers will
> have to
> used with the dot notation (e.g.: `.Executable(name: "tool", targets:
> ["tool"])`) which is a little awkward because we expect factory methods
> to
> use the dots.
>
> They will be defined as follow:
>
> ```swift
> /// Represents a product.
> class Product {
>
> /// The name of the product.
> let name: String
>
> /// The names of the targets in this product.
> let targets: [String]
>
> private init(name: String, targets: [String]) {
> self.name = name
> self.targets = targets
> }
>
> /// Represents an executable product.
> final class Executable: Product {
>
> /// Creates an executable product with given name and targets.
> override init(name: String, targets: [String])
> }
>
> /// Represents a library product.
> final class Library: Product {
> /// The type of library product.
> enum LibraryType: String {
> case `static`
> case `dynamic`
> }
>
> /// The type of the library.
> ///
> /// If the type is unspecified, package manager will
> automatically choose a type.
> let type: LibraryType?
>
> /// Creates a library product.
> init(name: String, type: LibraryType? = nil, targets: [String])
> }
> }
> ```
>
> <details>
> <summary>View example</summary>
> <p>
>
> Example:
>
> ```swift
> let package = Package(
> name: "Foo",
> target: [
> Target(name: "Foo", dependencies: ["Utility"]),
> Target(name: "tool", dependencies: ["Foo"]),
> ],
> products: [
> .Executable(name: "tool", targets: ["tool"]),
> .Library(name: "Foo", targets: ["Foo"]),
> .Library(name: "FooDy", type: .dynamic, targets: ["Foo"]),
> ]
> )
> ```
> </p></details>
>
>
> This API looks very weird: the leading dog is usually used for enum cases
> and static members. Using it with a type means that the capitalization
> looks very out of place.
>
>
Yes, as mentioned in the proposal we think the dot and capitalization
following it looks out of place here but we really think that the products
should be under a namespace because the types supported product might grow
to a substantial number in future. Adding namespace using nesting solves
this issue nicely.
Another advantage would be getting free autocomplete support for products
in IDEs or text editors which supports SourceKit. Right now there is none
which supports autocomplete for the manifest file but since SourceKit is
cross platform, it should be possible to create a plugin in future.
> * Special syntax for version initializers.
>
> A simplified summary of what is commonly supported in other package
> managers:
>
> | Package Manager | x-ranges | tilde (`~` or `~>`) | caret
> (`^`) |
> |-----------------|---------------|----------------------
> ---|---------------|
> | npm | Supported | Allows patch-level changes if a
> minor version is specified on the comparator. Allows minor-level changes if
> not. | patch and minor updates |
> | Cargo | Supported | Same as above | Same as
> above |
> | CocoaPods | Not supported | Same as above | Not
> supported |
> | Carthage | Not supported | patch and minor updates | Not
> supported |
>
> Some general observations:
>
> * Every package manager we looked at for this supports the tilde `~`
> operator in some form.
> * The widely accepted suggestion on how to constrain your versions is
> "use
> `~>`, it does the right thing".
> * It's not clear to us why this has so much traction as "the right
> thing", as it can
> prevent upgrades that should be compatible (one minor version to next
> minor version).
> * Most users may not really understand `~`, and just use it per
> recommendations.
> See e.g. how Google created a [6-minute instructional video](
> https://www.youtube.com/watch?v=x4ARXyovvPc)
> about this operator for CocoaPods.
> * A lot of people even explicitly set a single exact version simply
> because
> they don't know better. This leads to "dependency hell" (unresolvable
> dependencies
> due to conflicting requirements for a package in the dependency
> graph).
> * The Swift Package Manager will probably have many novice users,
> because it
> comes built-in to Swift.
> * We think caret `^` has the right behaviour most of the time. That is,
> you
> should be able to specify a minimum version, and you should be
> willing to let
> your package use anything after that up to the next major version.
> This policy
> works if packages correctly follow semantic versioning, and it
> prevents "dependency
> hell" by expressing permissive constraints.
> * We also think caret `^` is syntactically non-obvious, and we'd prefer
> a syntax
> that doesn't require reading a manual for novices to understand, even
> if that
> means we break with the syntactic convention established by the other
> package managers which
> support caret `^`.
> * We'd like a convenient syntax for caret `^`, but to still support the
> use
> case that tilde `~` is used for; but tilde `~` (or a single exact
> version) should
> be less convenient than caret `^`, to encourge permissive dependency
> constraints.
>
> What we propose:
>
> * We will introduce a factory method which takes a lower bound version
> and
> forms a range that goes upto the next major version (i.e. caret).
>
> ```swift
> // 1.0.0 ..< 2.0.0
> .package(url: "/SwiftyJSON", from: "1.0.0"),
>
> // 1.2.0 ..< 2.0.0
> .package(url: "/SwiftyJSON", from: "1.2.0"),
>
> // 1.5.8 ..< 2.0.0
> .package(url: "/SwiftyJSON", from: "1.5.8"),
> ```
>
> * We will introduce a factory method which takes `VersionSetSpecifier`,
> to
> conveniently specify common ranges.
>
> `VersionSetSpecifier` is an enum defined as follows:
>
> ```swift
> enum VersionSetSpecifier {
> case exact(Version)
> case range(Range<Version>)
>
> /// Creates a specifier for an exact version.
> static func only(_ version: Version) -> VersionSetSpecifier
>
> /// Creates a specified for a range starting at the given lower
> bound
> /// and going upto next major version.
> static func uptoNextMajor(_ version: Version) ->
> VersionSetSpecifier
>
> /// Creates a specified for a range starting at the given lower
> bound
> /// and going upto next minor version.
> static func uptoNextMinor(_ version: Version) ->
> VersionSetSpecifier
> }
> ```
>
> Examples:
>
> ```swift
> // 1.5.8 ..< 2.0.0
> .package(url: "/SwiftyJSON", .uptoNextMajor("1.5.8")),
>
> // 1.5.8 ..< 1.6.0
> .package(url: "/SwiftyJSON", .uptoNextMinor("1.5.8")),
>
> // 1.5.8
> .package(url: "/SwiftyJSON", .only("1.5.8")),
> ```
>
> * This will also give us ability to add more complex features in future:
>
> Examples:
>
> Note that we're not actually proposing these as part of this proposal.
>
>
> ```swift
> .package(url: "/SwiftyJSON", .uptoNextMajor("1.5.8").
> excluding("1.6.4")),
>
> .package(url: "/SwiftyJSON", .only("1.5.8", "1.6.3")),
>
> ```
>
> * We will introduce a factory method which takes `Range<Version>`, to
> specify
> arbitrary open range.
>
> ```swift
> // Constraint to an arbitrary open range.
> .package(url: "/SwiftyJSON", "1.2.3"..<"1.2.6"),
> ```
>
> * We will introduce a factory method which takes
> `ClosedRange<Version>`, to specify
> arbitrary closed range.
>
> ```swift
> // Constraint to an arbitrary closed range.
> .package(url: "/SwiftyJSON", "1.2.3"..."1.2.8"),
> ```
>
> * We will remove all of the current factory methods:
>
> ```swift
> // Constraint to a major version.
> .Package(url: "/SwiftyJSON", majorVersion: 1),
>
> // Constraint to a major and minor version.
> .Package(url: "/SwiftyJSON", majorVersion: 1, minor: 2),
>
> // Constraint to an exact version.
> .Package(url: "/SwiftyJSON", "1.2.3"),
>
> // Constraint to an arbitrary range.
> .Package(url: "/SwiftyJSON", versions: "1.2.3"..<"1.2.6"),
>
> // Constraint to an arbitrary closed range.
> .Package(url: "/SwiftyJSON", versions: "1.2.3"..."1.2.8"),
> ```
>
>
> I'm ver happy with the versioning part of this proposal :-)
>
>
Great!
> * Adjust order of parameters on `Package` class:
>
> We propose to reorder the parameters of `Package` class to: `name`,
> `pkgConfig`, `products`, `dependencies`, `targets`,
> `compatibleSwiftVersions`.
>
> The rationale behind this reorder is that the most interesting parts of
> a
> package are its product and dependencies, so they should be at the top.
> Targets are usually important during development of the package.
> Placing
> them at the end keeps it easier for the developer to jump to end of the
> file to access them. Note that the compatibleSwiftVersions property
> will likely
> be removed once we support Build Settings, but that will be discussed
> in a separate proposal.
>
>
> I would have liked this proposal to suggest modifying the API so the order
> is insignificant. While ordering feels important for me when calling a
> function or initializer with few arguments (like .package(url: "", from:
> "1.4.5")), the arguments to the Package function seem like a list of
> configuration options and shouldn't have a fixed order. My suggestion was
> to remove all arguments but the name and adds a configuration closure:
>
> let package = Package(name: "paper") {
> $0.products = [...]
> $0.dependencies = [...]
> }
>
>
It will be possible to avoid using the initializer because all the
properties will be made mutable. However I think if majority of packages
uses the initializer and thus have a consistent ordering, it will be easier
for other developers to read manifests on Github (or similar).
PS: The closure syntax can also be added using extension in the manifest
itself if someone really wants to use it.
> <details>
> <summary>View example</summary>
> <p>
>
> Example:
>
> ```swift
> let package = Package(
> name: "Paper",
> products: [
> .Executable(name: "tool", targets: ["tool"]),
> .Libary(name: "Paper", type: .static, targets: ["Paper"]),
> .Libary(name: "PaperDy", type: .dynamic, targets: ["Paper"]),
> ],
> dependencies: [
> .package(url: "http://github.com/SwiftyJSON", from: "1.2.3"),
> .package(url: "../CHTTPParser", .uptoNextMinor("2.2.0")),
> .package(url: "http://some/other/lib", .only("1.2.3")),
> ]
> targets: [
> Target(
> name: "tool",
> dependencies: [
> "Paper",
> "SwiftyJSON"
> ]),
> Target(
> name: "Paper",
> dependencies: [
> "Basic",
> .target(name: "Utility"),
> .product(name: "CHTTPParser"),
> ])
> ]
> )
> ```
> </p></details>
>
> * Eliminate exclude in future (via custom layouts feature).
>
> We expect to remove the `exclude` property after we get support for
> custom
> layouts. The exact details will be in the proposal of that feature.
>
> ## Impact on existing code
>
> The above changes will be implemented only in the new Package Description
> v4
> library. The v4 runtime library will release with Swift 4 and packages
> will be
> able to opt-in into it as described by
> [SE-0152](https://github.com/apple/swift-evolution/blob/
> master/proposals/0152-package-manager-tools-version.md).
>
> There will be no automatic migration feature for updating the manifests
> from v3
> to v4. To indicate the replacements of old APIs, we will annotate them
> using
> the `@unavailable` attribute where possible. Unfortunately, this will not
> cover
> all the changes for e.g. rename of the target dependency enum cases.
>
> All new packages created with `swift package init` command in Swift 4 tools
> will by default to use the v4 manifest. It will be possible to switch to v3
> manifest version by changing the tools version using `swift package
> tools-version --set 3.1`. However, the manifest will needed to be
> adjusted to
> use the older APIs manually.
>
> Unless declared in the manifest, existing packages automatically default
> to the Swift 3 minimum tools version; since the Swift 4 tools will also
> include
> the v3 manifest API, they will build as expected.
>
> A package which needs to support both Swift 3 and Swift 4 tools will need
> to
> stay on the v3 manifest API and support the Swift 3 language version for
> its
> sources, using the API described in the proposal
> [SE-0151](https://github.com/apple/swift-evolution/blob/
> master/proposals/0151-package-manager-swift-language-
> compatibility-version.md).
>
> An existing package which wants to use the new v4 manifest APIs will need
> to bump its
> minimum tools version to 4.0 or later using the command `$ swift package
> tools-version
> --set-current`, and then modify the manifest file with the changes
> described in
> this proposal.
>
> ## Alternatives considered
>
> * Add variadic overloads.
>
> Adding variadic overload allows omitting parenthesis which leads to less
> cognitive load on eyes, especially when there is only one value which
> needs
> to be specified. For e.g.:
>
> Target(name: "Foo", dependencies: "Bar")
>
> might looked better than:
>
> Target(name: "Foo", dependencies: ["Bar"])
>
> However, plurals words like `dependencies` and `targets` imply a
> collection
> which implies brackets. It also makes the grammar wrong. Therefore, we
> reject this option.
>
> * Version exclusion.
>
> It is not uncommon to have a specific package version break something,
> and
> it is undesirable to "fix" this by adjusting the range to exclude it
> because this overly constrains the graph and can prevent picking up the
> version with the fix.
>
> This is desirable but it should be proposed separately.
>
> * Inline package declaration.
>
> We should probably support declaring a package dependency anywhere we
> support spelling a package name. It is very common to only have one
> target
> require a dependency, and annoying to have to specify the name twice.
>
> This is desirable but it should be proposed separately.
>
> _______________________________________________
> swift-evolution mailing list
> swift-evolution at swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution
>
>
> One thing that still really bothers me about the API is the inconsistency
> in leading dots and capitalization. Should a novice (or an expert) have to
> remember the following different writings:
>
> Target(name: "Foo", dependencies: ["Utility"])
> .package(url: "http://github.com/SwiftyJSON", from: "1.2.3")
> .Library(name: "Paper", type: .static, targets: ["Paper"])
>
> I understand the arguments brought forward in the proposal. But from a
> package author point of view, it's not obvious why Target is capitalized,
> why package is lowercase with a leading dot and why Library is capitalized
> with a leading dot. It looks confusing and inconsistent, which makes it
> less pleasant to read and harder to remember.
>
> Could we push for more consistency by having everything b lowercased with
> a leading dot? It does force us to introduce a Target factory method, to
> revert SystemPackageProvider back to an enum, to revert products back to an
> enum. But I think it's worth it.
>
It is true that it might not be obvious when to use what initially, but we
think this is a good rule to create a distinction between constructing
things and referring to things. A downside of lowercasing everything would
be: it might seem like the references support all the parameters that
constructor supports. e.g. .target(name: "Foo", dependencies: [
.target(name: "Bar", dependencies: ["Baz"]) ])
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-build-dev/attachments/20170227/8c70d847/attachment.html>
More information about the swift-build-dev
mailing list