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

Features fail to contain optional dependencies that got enabled indirectly #146

Closed
nagisa opened this issue Aug 13, 2020 · 4 comments · Fixed by #185
Closed

Features fail to contain optional dependencies that got enabled indirectly #146

nagisa opened this issue Aug 13, 2020 · 4 comments · Fixed by #185

Comments

@nagisa
Copy link
Contributor

nagisa commented Aug 13, 2020

Consider the following Cargo.toml:

[package]
name = "test-num-features"
version = "0.1.0"
edition = "2018"

[dependencies]
num = { version = "=0.3", default-features = false, features = ["std"] }

It currently fails to build despite succeeding with cargo. The reason is because when crate2nix is building num-rational it somehow drops the feature for an optional num-bigint dependency. This optional dependency ends up being enabled indirectly, through num depending on both num-rational and num-bigint.

Both cargo metadata and the generated Cargo.nix contains num-bigint in the resolvedDefaultFeatures:

{
      "num-rational" = rec {
        crateName = "num-rational";
        version = "0.3.0";
        edition = "2018";
        sha256 = "0f41j1l1kn5jj36a8xdy8kv242wlwq0ka578vm8gnb1n1wvdgd55";
        authors = [<snip>];
        dependencies = [<snip>];
        buildDependencies = [<snip>];
        features = {
          "default" = [ "num-bigint-std" "std" ];
          "num-bigint-std" = [ "num-bigint/std" ];
          "std" = [ "num-integer/std" "num-traits/std" ];
        };
        resolvedDefaultFeatures = [ "num-bigint" "num-bigint-std" "std" ];
      };
}

But the final invocation of buildRustCrate ends up not being provided the feature in the end. And so num-rational is compiled without a --cfg feature="num-bigint" flag.

I admit that this is the first time I’m reading templates/nix/crate2nix/default.nix at all seriously, but from what I can tell it just throws away the information cargo already computed and attempts to compute its own feature sets. Is that correct? Why?

@kolloch
Copy link
Collaborator

kolloch commented Feb 27, 2021

Hmmm, it is a bit weird because there is even a unit test for something very similar:

https://github.com/kolloch/crate2nix/blob/master/crate2nix/templates/nix/crate2nix/tests/packageFeatures.nix#L98-L104

 "pkg_num" = {
      crateName = "num";
      dependencies = [
        {
          name = "num-bigint";
          packageId = "pkg_num_bigint";
          usesDefaultFeatures = false;
          optional = true;
        }
      ];
      features = {
        "default" = [ "std" ];
        "std" = [ "num-bigint/std" ];
      };
    };

@pandaman64
Copy link
Contributor

When Cargo see num-bigint/std, it implicitly adds num-bigint feature to the rustc invocation.
Although the expected feature set in the test matches the explicitly specified ones, it misses implicit features in this case.

For more detail, please see #165.

@Fuuzetsu
Copy link
Contributor

Fuuzetsu commented Apr 8, 2021

Is there any work-around for this or planned work? I am hitting this exact issue on this exact crate.

@Fuuzetsu
Copy link
Contributor

Fuuzetsu commented Apr 8, 2021

For us, I did something like this:

  cargoNix = pkgs.callPackage ./nix/Cargo.nix {
    inherit nixpkgs;
    buildRustCrate = null;
    defaultCrateOverrides = pkgs.defaultCrateOverrides // {
      num-rational = attrs:
        attrs // {
          # https://github.com/kolloch/crate2nix/issues/146
          features = attrs.features ++ [ "num-bigint" ];
        };
    };

pandaman64 added a commit to pandaman64/crate2nix that referenced this issue Apr 22, 2021
When Cargo sees a dependency feature `opt_crate/feat` for an optional dependency `opt_crate`,
it implicitly adds `opt_crate` to the feature set for the current crate.
For example, `num-rational` crate relies on this behavior for importing `num-bigint`, which is optional.
This patch lets crate2nix mimic this behavior.

Closes nix-community#146 and nix-community#182.
kolloch pushed a commit that referenced this issue Apr 24, 2021
When Cargo sees a dependency feature `opt_crate/feat` for an optional dependency `opt_crate`,
it implicitly adds `opt_crate` to the feature set for the current crate.
For example, `num-rational` crate relies on this behavior for importing `num-bigint`, which is optional.
This patch lets crate2nix mimic this behavior.

Closes #146 and #182.
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

Successfully merging a pull request may close this issue.

4 participants