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

Adds optional dependencies to features when they are enabled indirectly #185

Merged
merged 2 commits into from
Apr 24, 2021

Conversation

pandaman64
Copy link
Contributor

@pandaman64 pandaman64 commented 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.

Main changes are in crate2nix/templates/nix/crate2nix/default.nix
I also modified test cases(crate2nix/templates/nix/crate2nix/tests/packageFeatures.nix) so that they match what Cargo does.

Closes #146, #182.

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
Copy link
Collaborator

kolloch commented Apr 24, 2021

Woah, thank you, @pandaman64!

Prime example of a PR I love.

  1. You clearly write what you want to achieve and what you changed. (that significantly reduces the time that I need to review it)
  2. You have nice inline comment that also explains it.
  3. You adjusted the unit tests.

🎆

@kolloch kolloch merged commit 166c04a into nix-community:master Apr 24, 2021
@Fuuzetsu
Copy link
Contributor

Thanks! Just confirming that it works for us too, I tried it today.

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 this pull request may close these issues.

Features fail to contain optional dependencies that got enabled indirectly
3 participants