Was looking forward to this :) here are my comments:
> On 25 Feb 2017, at 01:35, Rick Ballard via swift-evolution
> <swift-evolution@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?
> * 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.
> 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.
> * 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 :-)
> * 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 = [...]
}
> <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@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._______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution
