Skip to content
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

Feature flag platforms #88

Closed
Sytten opened this issue Nov 16, 2022 · 10 comments
Closed

Feature flag platforms #88

Sytten opened this issue Nov 16, 2022 · 10 comments

Comments

@Sytten
Copy link

Sytten commented Nov 16, 2022

Currently all platforms are enabled by default and pulling their dependencies.
I recently tried upgrading to 1.0.53 and it added a bunch of things for haiku that I really don't care about since that's not a target for our software.
It would be nice to feature gate each platform.

@astraw
Copy link
Member

astraw commented Nov 16, 2022

This shouldn't be happening. Dependencies are pulled in only for the target being built. Can you provide steps to reproduce?

@Kijewski
Copy link
Collaborator

It's the same problem as in #61. Dependencies that are only needed in other platforms are still downloaded and evaluated, but they don't get compiled. That is a shortcoming of cargo that we can do nothing about.

You have to file an issue with rust-lang/rust if you want to see this problem fixed, but I daresay it's already known.

@Sytten
Copy link
Author

Sytten commented Nov 16, 2022

Make sense @Kijewski. There is one exception though.
If you put the dependencies as optional and enable them with a feature flag (and default features to all platforms).

@Kijewski
Copy link
Collaborator

But that would break all the usages, that only indirectly depend on iana-time-zone, e.g. through chrono. IMO this bug has to be fixed in cargo itself, and working around the problem downstream does only hurts usability.

@Sytten
Copy link
Author

Sytten commented Nov 16, 2022

Only if you don't add those feature flags in the default features no?

@Kijewski
Copy link
Collaborator

By default it should work everywhere, for every target platform, but maybe the number of supported platforms could be restricted by using no-default-features = true.

But unless I'm mistaken, the wasm support pulls in far more dependencies than the haiku support.

@Sytten
Copy link
Author

Sytten commented Nov 16, 2022

Yeah that was the idea, yeah it's really not great and it's really not your fault. I am just a bit paranoid with my lockfile :)

@astraw
Copy link
Member

astraw commented Nov 20, 2022

Thanks all for the discussion. I am closing this for now.

@astraw astraw closed this as completed Nov 20, 2022
lopopolo added a commit that referenced this issue May 10, 2023
We have gotten several inbound tickets with users expressing concern
over the inclusion of `cxx` to support the haiku platform. Additionally,
for my personal use, I would prefer to be able to prune things like
`wasm-bindgen` from my lockfile when not expressly enabling Wasm
platform support, similar to the approach the `getrandom` crate takes.

This commit adds cargo features which allow disabling platform support
for each platform that requires a third party dependency. Currently,
that set of platforms is:

- android
- all apple targets
- haiku
- wasm
- windows

This commit adds a new default feature, `platform-all`, which retains
the existing behavior of all platforms being enabled by default.
`platform-all` is a meta feature which enables all of the other platform
support features.

For example, to disable haiku support, users would add the following to
their `Cargo.toml`:

```toml
iana-time-zone = { version = "0.2", default-features = false, features = ["platform-android", "platform-apple", "platform-wasm", "platform-windows"] }
```

The addition of a new default feature is a breaking change because it
may break the build for folks currently depending on this crate with
`default-features = false`.

I'm not sure if this is the direction we want to go, but it would
address the ask from these tickets:

- #89
- #88

I know that cargo won't build deps unless compiling for a particular
platform, but practically, most users don't want or need Haiku or Wasm
support. These targets have heavy dependencies which end up in the lock
file. Deps in the lock file have a carrying cost in terms of supply
chain risk and need for patching/updating.
@lopopolo
Copy link
Collaborator

hi @Sytten, following up here, we significantly reduced the impact of iana-time-zone-haiku on lockfile weight with this PR: #106.

Let us know if this resolves your concerns!

@Sytten
Copy link
Author

Sytten commented May 15, 2023

Ha thanks! I will give it is a shot. cc is acceptable as a dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants