Skip to content

Fix default features in dependency#282

Merged
dtolnay merged 2 commits intodtolnay:masterfrom
gui1117:gui-fix-default-features
Jul 29, 2024
Merged

Fix default features in dependency#282
dtolnay merged 2 commits intodtolnay:masterfrom
gui1117:gui-fix-default-features

Conversation

@gui1117
Copy link
Copy Markdown
Contributor

@gui1117 gui1117 commented Jul 29, 2024

If in a crate Cargo.toml there is a dependency with default-features = true and workspace = true
If in the workspace Cargo.toml the depended crate is written with default-features = false

Then we must serialize the default-features = true when doing the manifest for the test.

Otherwise the test will run with default-features = false which is not the expected behavior.

Copy link
Copy Markdown
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dtolnay dtolnay merged commit aa567ec into dtolnay:master Jul 29, 2024
@gui1117 gui1117 deleted the gui-fix-default-features branch July 29, 2024 00:37
@gui1117
Copy link
Copy Markdown
Contributor Author

gui1117 commented Jul 29, 2024

Thank you for the crate

@Guiguiprim
Copy link
Copy Markdown

Hello !

I think there is an issue with this.

My Cargo.toml only contains workspace = true (no default-features), but the workspace Cargo.toml does contains default-features = false.
As far as I understand if the field is not present it defaults as true, so with this change we wrongly set default-features = true.

Extract from my use case:

[dependencies.ed25519-compact]
default-features = true
workspace = true

[workspace.dependencies.ed25519-compact]
version = "2.1.1"
default-features = false
features = ["std", "x25519"]

In my setup, the CI use --locked --offline when running the tests (crates are fetch in a previous step) and as a result the build fails with missing ct-codecs (which came with the pem default feature).

@gui1117
Copy link
Copy Markdown
Contributor Author

gui1117 commented Jul 31, 2024

Indeed maybe default features should be an option of boolean.

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.

3 participants