Arnavion / k8s-openapi

Rust definitions of the resource types in the Kubernetes client API
Apache License 2.0
373 stars 42 forks source link

Cannot `cargo check` a library that has `k8s-openapi` as dependency #115

Closed flavio closed 2 years ago

flavio commented 2 years ago

I've create a Rust library that depends on k8s-openapi and I'm having troubles when build-ing and check-ing it.

As documented here:

If you’re writing a library crate, your crate must not enable any features of k8s-openapi directly. The choice of which feature to enable must be left to any application crates that use your library. This ensures that all k8s-openapi-using dependencies in that application crate’s dependency graph use the same set of k8s-openapi types and are interoperable.

Following the docs I created a Cargo.toml that looks like that:

[package]
name = "k8s-openapi-lib-bug"
version = "0.1.0"
edition = "2021"

[dependencies]
k8s-openapi = "0.14.0"

[dev-dependencies]
k8s-openapi = { version = "0.14.0", default-features = false, features = ["api", "v1_23"] }

However, running cargo check results in the following error:

$ cargo check
   Compiling autocfg v1.1.0
   Compiling serde v1.0.136
    Checking tinyvec_macros v0.1.0
    Checking matches v0.1.9
    Checking unicode-bidi v0.3.7
   Compiling serde_json v1.0.79
    Checking percent-encoding v2.1.0
    Checking itoa v1.0.1
    Checking bytes v1.1.0
    Checking fnv v1.0.7
   Compiling k8s-openapi v0.14.0
    Checking ryu v1.0.9
    Checking base64 v0.13.0
    Checking tinyvec v1.5.1
    Checking form_urlencoded v1.0.1
    Checking http v0.2.6
   Compiling num-traits v0.2.14
   Compiling num-integer v0.1.44
error: failed to run custom build command for `k8s-openapi v0.14.0`

Caused by:
  process didn't exit successfully: `/home/flavio/hacking/rust/k8s-openapi-lib-bug/target/debug/build/k8s-openapi-39ca3b8b07e57bbc/build-script-build` (exit status: 101)
  --- stderr
  thread 'main' panicked at '
  None of the v1_* features are enabled on the k8s-openapi crate.

  The k8s-openapi crate requires a feature to be enabled to indicate which version of Kubernetes it should support.

  If you're using k8s-openapi in a binary crate, enable the feature corresponding to the minimum version of API server that you want to support. It may be possible that your binary crate does not directly depend on k8s-openapi. In this case, add a dependency on k8s-openapi, then enable the corresponding feature.

  If you're using k8s-openapi in a library crate, add a dev-dependency on k8s-openapi and enable one of the features there. This way the feature will be enabled when buildings tests and examples of your library, but not when building the library itself. It may be possible that your library crate does not directly depend on k8s-openapi. In this case, add a dev-dependency on k8s-openapi, then enable the corresponding feature.

  Library crates *must not* enable any features in their direct dependency on k8s-openapi, only in their dev-dependency. The choice of Kubernetes version to support should be left to the final binary crate, so only the binary crate should enable a specific feature. If library crates also enable features, it can cause multiple features to be enabled simultaneously, which k8s-openapi does not support.

  If you want to restrict your library crate to support only a single specific version or range of versions of Kubernetes, please use the k8s_* version-specific macros to emit different code based on which feature gets enabled in the end.', /home/flavio/.cargo/registry/src/github.com-1ecc6299db9ec823/k8s-openapi-0.14.0/build.rs:9:42
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
warning: build failed, waiting for other jobs to finish...
error: build failed

How to reproduce the error:

Am I doing something wrong? Thanks in advance!

olix0r commented 2 years ago

I'd bet cargo check --all-targets works. cargo check won't enable dev dependencies unless you're including tests.

Arnavion commented 2 years ago

Yes, it's expected that cargo build / cargo check will not work. You'd need at least --tests to pick up the dev dep that enables a version feature.

The only way I can think of is to define another env var like K8S_OPENAPI_CARGO_FEATURE=1.23 that mimics the actual feature, with the expectation that library users will only set it for local development.

flavio commented 2 years ago

Thanks for the quick answers. Using --tests with cargo check/cargo build solves the problem, but I noticed this doesn't work with cargo doc because this flag doesn't exist for it.

I think the environment variable approach could work :+1:

flavio commented 2 years ago

I think I've just found another workaround:

[package]
name = "k8s-openapi-lib-bug"
version = "0.1.0"
edition = "2021"

[features]
default = []
enable-k8s-openapi-version = [ "k8s-openapi/v1_23" ]

[dependencies]
k8s-openapi = { version = "0.14.0", default-features = false, features = ["api"] }

Now I can run the following commands just fine:

cargo check --features enable-k8s-openapi-version
cargo build --features enable-k8s-openapi-version
cargo doc --features enable-k8s-openapi-version

I think this solution is even better than the environment variable. How do you feel about that?

If you agree, I can document the "trick" inside of the official docs

flavio commented 2 years ago

I've just noticed that a Cargo.toml file like this one, works just fine:

[package]
name = "k8s-openapi-lib-bug"
version = "0.1.0"
edition = "2021"

[dependencies]
k8s-openapi = { version = "0.14.0", default-features = false, features = ["api"] }

[dev-dependencies]
k8s-openapi = { version = "0.14.0", default-features = false, features = ["v1_23"] }

No changes are required to execute cargo test|check|build|doc. This is great, but only works if the "main library" doesn't require k8s-openapi to have the default features turned on.

flavio commented 2 years ago

About the workaround documented in https://github.com/Arnavion/k8s-openapi/issues/115#issuecomment-1109517795

I think this solution is even better than the environment variable. How do you feel about that?

I think this solution has a workaround: doc.rs will probably fail because it will not run cargo doc with this feature enabled :thinking:

Arnavion commented 2 years ago

I think this solution has a workaround: doc.rs will probably fail because it will not run cargo doc with this feature enabled thinking

https://docs.rs/about/metadata

In any case, I don't think I'd want every k8s-openapi-using library to start adding dummy features just for this reason. That is, while a feature is a more "standard" solution than the ad-hoc build-time env var, at least the build-time env var is only noticeable to the library's devs and not to the library's users.

flavio commented 2 years ago

In any case, I don't think I'd want every k8s-openapi-using library to start adding dummy features just for this reason. That is, while a feature is a more "standard" solution than the ad-hoc build-time env var, at least the build-time env var is only noticeable to the library's devs and not to the library's users.

That's a good point :+1:

Also, thanks for the link to docs.rs metadata