Package traits

[FranzBusch]

No description provided.

[MaxDesiatov]

Suggested change

	or WASM. This proposal aims to give package authors a new tool to conditionlize
	or Wasm. This proposal aims to give package authors a new tool to conditionalize

[MaxDesiatov]

Suggested change

	`swift-collection` package. To reduce the build times and binary size impact
	`swift-collections` package. To reduce the build times and binary size impact

[MaxDesiatov]

What's the exact type here that init is called on?

[MaxDesiatov]

Suggested change

	- `--all-traits`: Enables all traits of the package.
	- `--enable-all-traits`: Enables all traits of the package.

[MaxDesiatov]

Suggested change

	- `-disable-default-traits`: Disables all default traits of the package.
	- `--disable-default-traits`: Disables all default traits of the package.

[czechboy0]

Thanks for getting the ball rolling on this. A few thoughts:

1. Do we need the concept of default traits, or could we just do away with it, and by default, each package has all traits disabled?
2. Relatedly, do traits need to be enabled explicitly, or can we make room for cross-module overlays here?
3. Traits not being considered during package resolution is surprising to me, for the OpenAPI example we'd want it to be taken into account, and written into Package.resolved.

[FranzBusch]

• Do we need the concept of default traits, or could we just do away with it, and by default, each package has all traits disabled?

After having implemented and written a few examples, I personally think that default traits hold their weight. They make it significantly easier for people to start using a package while leaving room for optimising in the important cases.

• Relatedly, do traits need to be enabled explicitly, or can we make room for cross-module overlays here?

I thought about this a bit while implementing and I think we should start with explicit traits first. Having a dependency down the chain add another dependency that suddenly enables a cross-module overlay is a completely different behaviour than traits. With traits a lot of the cross-module overlay needs can already be solved.

• Traits not being considered during package resolution is surprising to me, for the OpenAPI example we'd want it to be taken into account, and written into Package.resolved.

I think this is a future optimisation that we can do which has no impact on the user facing API. Right now it was far easier to get started making this work post dependency resolution. It also reduces the burden of re-fetching dependencies when changing the traits of the root package. This is also in-line with how platform specific dependencies work right now. I agree it would be nice to have but MO isn't mandatory to begin with.

[al45tair]

I'm still not sure traits is the right word for these. I actually do think we should call them features.

[FranzBusch]

I leave it to community wide bike shedding :D

[MaxDesiatov]

I don't find "SwiftPM features feature" phrase particularly readable. It's quite unspecific and tautological, and even Cargo itself finds this naming to be confusing in https://doc.rust-lang.org/nightly/cargo/reference/features-examples.html#nightly-only-features:

Keep in mind that the feature attribute is unrelated to Cargo features, and is used to opt-in to experimental language features.

[glbrntt]

+1 on features

[MaxDesiatov]

Another argument against "features" is searchability in documentation, imagine googling for "swift language feature" or "swiftpm feature" and trying to find relevant information on this, which would be basically impossible with this naming.

[czechboy0]

Package flags, package options.

[MaxDesiatov]

Flags put us into a corner of making them binary: enabled/disabled. One nice direction I anticipate is codifying traits in a package as RawRepresentable<String> enums, also allowing enums with associated values. Then one could create a trait with case stackSize(UInt) encoded as stackSize=42 trait for a stack size setting passed to a linker that can be useful in embedded use cases, for example. IMO from that perspective this looks even more like "package parameters" to me.

[czechboy0]

Sure, that's fine too. But you bring up a good point, and we should make that explicit. Are package traits only binary? Can non-binary traits be all combined? Even if that's a future feature to add non-binary (parameterized traits), we should make sure we make room for them now, both in API and terminology.

[al45tair]

Just for the record, the reason I dislike traits is that I don't think this would be using the word in its usual (plain English) sense. Dictionary says:

trait | treɪt, treɪ |
noun
a distinguishing quality or characteristic, typically one belonging to a person: the traditionally British trait of self-denigration.

• a genetically determined characteristic: breeders were installing some trait that allowed the crop to thrive.

and I don't think these are distinguishing qualities or characteristics of the package. Rather, they're options that can be set.

"Options" is quite a nice name for that, actually. I do quite like that one.

[gwynne]

I'd like to toss "configuration" into the pool for discussion for good measure, though I wouldn't really be against "options".

[al45tair]

I wonder whether we could have a special trait, .defaults, that you add to your enabledTraits: to specify that you want the default traits? That would get rid of disableDefaultTraits and then I think we could spell this:

dependencies: [
    .package(
        url: "https://github.com/Org/SomePackage.git",
        from: "1.0.0",
        traits: ["SomeTrait"],
     )
]

or to enable the defaults plus SomeTrait:

dependencies: [
    .package(
        url: "https://github.com/Org/SomePackage.git",
        from: "1.0.0",
        traits: [.defaults, "SomeTrait"],
     )
]

[FranzBusch]

@MaxDesiatov was suggesting something similar. We can certainly make that work. I haven't done it yet since rewiring all of this requires tens of trivial changes :D

[MaxDesiatov]

+1 to what @al45tair proposed. For me personally, singular .default reads slightly better.

[bnbarham]

Another +1 from me :)

[FranzBusch]

I have updated the proposal to use .defaults and disallowed traits to be named default or defaults (in any casing combination).

[al45tair]

IMO we definitely want some syntax for this rather than TRAIT_Foo.

[FranzBusch]

I agree. I started working on #if trait(Foo) support but wanted to draft up the proposal with the current state already.

[MaxDesiatov]

I agree with Franz that this isn't critical for the proposal as it requires additional compiler work, but certainly deserves to be mentioned in "Future Directions".

[bnbarham]

This is exactly what compilation conditions are, so it seems odd to me that we'd want another syntax for them. Is there any particular reason that we need to prefix it at all? Maybe needs to be converted to uppercase + underscored so that this would be #if FOO though.

There's also C/C++ to consider, but I also think just passing a macro definition through would be fine there too.

[FranzBusch]

I have already called out a potential future direction of #if trait(Foo). For now I would like to keep it as #if TRAIT_Foo and pitch it. I assume a lot of people of opinions about this syntax.

[al45tair]

Is there any particular reason that we need to prefix it at all?

I guess the only reason is to avoid collisions with non-trait compilation conditions, but you're right — this is what they're for anyway, so maybe we don't need a prefix at all?

[gwynne]

I question if there's also a danger of running into confusion versus #if hasFeature(Foo) (which I still generally end up discarding in favor of the #if $Foo syntax, to add yet another layer of legacy confusion).

[al45tair]

I quite like the Python TOML documentation from Hatchling — see optional dependencies, features.

[glbrntt]

Is this enforced somehow?

IMO it seems like the user experience of developing a library which offers traits should be more like availability guards than compiler guards.

[FranzBusch]

In theory we could use the API breaking change checker to check the APIs between various traits to assure nothing removes API. IMO, compiler guards are exactly what package traits are but I would love if compiler guards could be spelled nicer in the language more akin to what Rust does i.e. only at the declaration and not surrounding the whole body.

[glbrntt]

Right, I think this could be a rough edge for developers as they'd need to test with different combinations of traits enabled but maybe that's just a limitation we need to accept.

[gwynne]

I wouldn't trust the API breakage checker to check any such thing; the false positive rate of API breakage reports remains appalling (yes, bugs have been filed 🙂).

[glbrntt]

Is this 300 definitions per package or can a package only enable 300 traits across all of its dependencies?

[FranzBusch]

I clarified this slightly:

To avoid such a scenario an initial maximum
number of 300 defined traits per package is imposed

[glbrntt]

Why do we allow a digit as the first character? This seems counter to other naming conventions we have.

[FranzBusch]

AFAIK this follows the exact same enforcements as targets or products in the Package.swift. I agree that a good convention is to not use digits. The Unicode XID standard is specifically defined for parsers and something that we enforce in our tooling.

[glbrntt]

I didn't realise that was the case for targets and products. Makes sense to be consistent with them.

[MaxDesiatov]

I don't think that's a good approach. Why not restrict character names to what's possible as Swift identifiers? That would allow people to define traits as enum cases, which would improve diagnostics significantly. Currently if someone makes a typo in trait's name, we have to implement special diagnostic handling (which is hard to implement well for package manifests), and you'll get that diagnostic only when you try to resolve instead of getting a usual error inline in an IDE about nonexistent enum case.

[FranzBusch]

I think the Unicode XID start and Unicode XID continue character sets are either matching what Swift identifiers are allowed to be or a subset. However, I think this is worth confirming with somebody from the compiler.

[gwynne]

The official grammar for identifiers supports this, if I read it correctly.

[glbrntt]

Even if the trait is a default trait?

[FranzBusch]

Yes, it is never safe. An adopter might declare disableDefaultTraits: true; hence, loosing the API that got moved behind the default.

[glbrntt]

Wouldn't that be the adopter breaking the API rather than the package?

[glbrntt]

I don't find this naming very obvious when used in the example above. The only suggestions which come to mind are dependencies and subTraits, neither of which seem quite right.

[FranzBusch]

I agree. I was struggling a lot with naming this. Initial I had just made this a Dictionary<String: String> but wanted some more strongly typed.

[glbrntt]

+1 on features

[bnbarham]

There's a couple uses of environment variables in the toolchain itself that I don't see covered here:

1. Specifying different package URLs. I don't necessarily think features should cover this, but just pointing it out as something else environment variables are used for.
2. Specifying different flags to a target. This one does seem reasonable for features to cover and likely would be with the .when(traits: ["Foo"]) support, but could be called out specifically?

[FranzBusch]

Thanks for bringing this up.

1. Is interesting and we could come up with something where traits can make this work; however, I think the mirror feature or location rewrites are better suited.
2. This is totally in line with what traits can do and I will add it to this proposal

[bnbarham]

Another +1 from me :)

[bnbarham]

This is exactly what compilation conditions are, so it seems odd to me that we'd want another syntax for them. Is there any particular reason that we need to prefix it at all? Maybe needs to be converted to uppercase + underscored so that this would be #if FOO though.

There's also C/C++ to consider, but I also think just passing a macro definition through would be fine there too.

[glbrntt]

• Do we need the concept of default traits, or could we just do away with it, and by default, each package has all traits disabled?

After having implemented and written a few examples, I personally think that default traits hold their weight. They make it significantly easier for people to start using a package while leaving room for optimising in the important cases.

Can you shared some examples where you found the defaults helpful? I think there's value in being explicit about which traits are enabled so it's not clear whether defaults hold their weight or not.

[al45tair]

Can you shared some examples where you found the defaults helpful? I think there's value in being explicit about which traits are enabled so it's not clear whether defaults hold their weight or not.

Defaults are presumably required because we can't have negative traits. For example, imagine a package that does HTTP requests; it might have a trait that enables TLS, and that trait would probably be on by default since most people will want that to work. But maybe there are situations where you don't want TLS support; without negative traits, the only way to express this is to have default traits, and then allow people to not include them.

[czechboy0]

I'm still not sure I understand the use cases we're trying to cover with this implementation.

The proposal mentions the OpenAPI transports use case, but without traits being part of package resolution, we definitely cannot use it there, as otherwise adopters will get the union of all dependencies, which might be a huge graph.

The proposal also mentions that traits must be additive, and that traits cannot remove API. But the rationale behind default traits seems to be to want to remove API (the TLS example). Conceptually, I find the ability to disable default traits equivalent to having negative traits, which I thought was an anti-goal. The exact spelling is less important than whether it's possible to explicitly remove functionality by manipulating traits.

Also, I'm worried we might accidentally make properly solving cross-module overlays in a user-friendly way in the future harder, unless we clarify how they'll work both with default traits and package resolution.

To get more clarity, could you provide 1) a few examples of use cases that are the motivating ones for this proposal and 2) a few examples of use cases that will NOT be covered by this proposal, but room is being left for them for the future? I think that'll help me think only about the use cases you're aiming to solve for.

Do you believe default traits are a pure add-on? If so, would you consider excluding default traits, building this feature without them, and only adding them once we also design their interaction with cross-module overlays and package resolution? I agree with building the feature incrementally, I just see default traits as a pretty heavy feature that I both don't see fully justified, and even more, I'm worried it'll complicate adding other features we want too.

How many use cases would fall out of scope if we removed default traits from v1? Would it still be worth doing, in your opinion?

[FranzBusch]

The proposal mentions the OpenAPI transports use case, but without traits being part of package resolution, we definitely cannot use it there, as otherwise adopters will get the union of all dependencies, which might be a huge graph.

I think there is a misunderstanding. OpenAPI would declare traits such as URLSession or AHC and make non of them default traits. Users will get a larger dependency graph but at build time non of the code will be compiled. I agree that is not optimal but something that can be changed in the future. Integrating this with dependency resolution is going to make this a lot more complicated and IMO we can piecemeal adopt this.

The proposal also mentions that traits must be additive, and that traits cannot remove API. But the rationale behind default traits seems to be to want to remove API (the TLS example). Conceptually, I find the ability to disable default traits equivalent to having negative traits, which I thought was an anti-goal. The exact spelling is less important than whether it's possible to explicitly remove functionality by manipulating traits.

On a per trait basis their API changes must be additive. However, disabling a trait can and probably in a lot of cases will remove APIs. That is expected. Default traits just make it easier to keep a package configurable while catering to the default 95% use-case.

Also, I'm worried we might accidentally make properly solving cross-module overlays in a user-friendly way in the future harder, unless we clarify how they'll work both with default traits and package resolution.

I personally don't think cross-module overlays are a good thing for the package ecosystem and far better suited for the pre-built SDK ecosystem that Apple platforms offer. Traits are mostly orthogonal to this though.

Do you believe default traits are a pure add-on? If so, would you consider excluding default traits, building this feature without them, and only adding them once we also design their interaction with cross-module overlays and package resolution? I agree with building the feature incrementally, I just see default traits as a pretty heavy feature that I both don't see fully justified, and even more, I'm worried it'll complicate adding other features we want too.

I think we want default traits from the beginning. They make adopting this easier and reduce a significant pain point that will come up where every package has to spell all the traits of a package just to use the default use-case. Adding default traits later will become significantly harder since we cannot reasonably make the defaults enabled by default since users already specified what traits they want to enable. IMO we either can include default traits from the beginning or don't do them at all.

How many use cases would fall out of scope if we removed default traits from v1? Would it still be worth doing, in your opinion?

Default traits enable no additional use-case but make the developer experience significantly better in my opinion.

[czechboy0]

The proposal mentions the OpenAPI transports use case, but without traits being part of package resolution, we definitely cannot use it there, as otherwise adopters will get the union of all dependencies, which might be a huge graph.
I think there is a misunderstanding. OpenAPI would declare traits such as URLSession or AHC and make non of them default traits. Users will get a larger dependency graph but at build time non of the code will be compiled. I agree that is not optimal but something that can be changed in the future. Integrating this with dependency resolution is going to make this a lot more complicated and IMO we can piecemeal adopt this.

Actually, I wonder how this intersects with target-based dependency resolution? If a leaf package depends on a product that doesn't require a package dependency, I think it won't even be cloned. How does that apply here? cc @simonjbeaumont

[FranzBusch]

Actually, I wonder how this intersects with target-based dependency resolution? If a leaf package depends on a product that doesn't require a package dependency, I think it won't even be cloned. How does that apply here? cc

As pitched it doesn't intersect at all with target based dependency resolution since the implementation is only going to consider traits post resolution and fetching. This is mostly done due to getting something working. I outlined traits influencing dependency resolution as a future direction that doesn't require evolution. Once we do that it will just "work". This means dependency resolution gets a set of Targets and enabled traits passed. From there on it is going to take each target look at the enabled dependencies and goes down the graph.

[czechboy0]

Right, but target-based dependency resolution is already a feature in SwiftPM that works today - in other words, when you depend on a package, only the dependencies required by the products you depend on from that package get cloned (others don't). I'm wondering how package traits will affect this feature - if a trait makes a product I'm depending on include an extra dependency, will it get included? cc @MaxDesiatov do you know?

[FranzBusch]

Right, but target-based dependency resolution is already a feature in SwiftPM that works today - in other words, when you depend on a package, only the dependencies required by the products you depend on from that package get cloned (others don't). I'm wondering how package traits will affect this feature - if a trait makes a product I'm depending on include an extra dependency, will it get included? cc @MaxDesiatov do you know?

Let me try again. If a target depends on a product it will fetch that dependency. If a target depends on a product but this dependency is optional. The dependency will get fetch regardless if the trait is enabled or not for this optional dependency. However, this is just a limitation from the current implementation. There is nothing preventing us from taking traits into consideration during target based dependency resolution. I hope that makes it more clear.

[czechboy0]

It does, thanks. So basically the behavior in this v1 will result in all the dependencies fetched, as if all the traits were enabled.

If the fetching is considered an implementation detail though, I wonder whether it needs to be spelled out in the proposal like this. Maybe we should consider making the fetching work correctly part of the implementation work, even if it's split into two stages (fetching being stage 2). I really want to avoid someone having to open a new proposal that's just: "fixing the implementation of target-based dependency resolution to work with package traits". If we agree the implementation change won't need a new proposal, I think this proposal shouldn't explicitly remove it from scope, and leave it as part of the implementation plan.

[FranzBusch]

So I think we must call this out for now since otherwise users will run into this and file a bug report. This is actually quite common in evolution proposals to call out future directions that require no evolution proposal just implementation work. I added more context to the paragraph. @czechboy0 does this work for you?

The implementation to this proposal only considers traits after the
dependency resolution when constructing the module graph. This is inline with
how platform specific dependencies are currently handled. In the future, both
platform specific dependencies and traits can be taken into consideration during
dependency resolution to avoid fetching an optional dependency that is not
enabled by a trait. Changing this doesn't require a Swift evolution proposal
since it is just an implementation detail of how dependency resolution currently
works.

[czechboy0]

Yup works for me, thanks @FranzBusch 🙂

[nkcsgexi]

This looks a fantastic idea and a useful addition! I wonder what would be our story for testing support with the present of multiple traits. Is it possible that we could provide package vendors a nice testing utility for covering all possible permutations of traits?

[FranzBusch]

This looks a fantastic idea and a useful addition! I wonder what would be our story for testing support with the present of multiple traits. Is it possible that we could provide package vendors a nice testing utility for covering all possible permutations of traits?

Yes adding an swift test --all-trait-permutations is a great idea and I will add it to the proposal. Another future direction that I called out in the proposal is the compiler understanding more about traits and how they apply to public APIs. This could be used to in various places such as the API breaking change checker or in DocC when generating documentation. Another thing that would be great is if the compiler can diagnose the usage of a trait guarded API if the trait hasn't been enabled on the dependency.

However, all of this is compiler interaction is in my opinion a future direction and just having the feature with pure define based support would be incredibly valuable already

[gwynne]

Yes adding an swift test --all-trait-permutations is a great idea and I will add it to the proposal. Another future direction that I called out in the proposal is the compiler understanding more about traits and how they apply to public APIs. This could be used to in various places such as the API breaking change checker or in DocC when generating documentation. Another thing that would be great is if the compiler can diagnose the usage of a trait guarded API if the trait hasn't been enabled on the dependency.

Could this eventually expand to offering a non-underscored version of @_spi? I know the SPI concept covers more ground than "this is not guaranteed-stable API" in practice (hence the existence of @_spi_available, for example), but people are arguably misusing @_spi to that purpose in the present day; traits could potentially provide a more "correct" alternative.