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

Use dep: syntax to avoid implicit features. #1099

Merged
merged 1 commit into from
Jun 21, 2024

Conversation

waywardmonkeys
Copy link
Contributor

The only real externally visible feature should be parallel and not the implicit features for the optional dependencies.

The only real externally visible feature should be `parallel` and
not the implicit features for the optional dependencies.
@waywardmonkeys
Copy link
Contributor Author

With this, adding cc as a dependency results in:

      Adding cc (local) to build-dependencies
             Features:
             - parallel

rather than:

      Adding cc v1.0.99 to build-dependencies
             Features:
             - jobserver
             - libc
             - once_cell
             - parallel

Copy link
Collaborator

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

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

Thank you!

Nice improvements to feature documentation

@NobodyXu NobodyXu merged commit 7ff340c into rust-lang:main Jun 21, 2024
24 checks passed
@waywardmonkeys waywardmonkeys deleted the use-dep-syntax branch June 21, 2024 13:38
@rafales
Copy link

rafales commented Jul 12, 2024

Guys, this broke a lot of code because solana-program has been using jobserver dependency for a long time.

@NobodyXu
Copy link
Collaborator

Guys, this broke a lot of code because solana-program has been using jobserver dependency for a long time.

In that case it's a no-op, enabling jobserver does not enable the parallel compilation, it's still compiled in sequential, just with one extra dependency doing nothing.

@waywardmonkeys
Copy link
Contributor Author

@NobodyXu It looks like they enabled both parallel and jobserver:

https://github.com/solana-labs/solana/blob/27eff8408b7223bb3c4ab70523f8a8dca3ca6645/sdk/program/Cargo.toml#L92

Now this would fail since there is no jobserver feature.

Maybe cc should add one that does nothing, but there's no means for deprecating a feature either. I could submit a PR that does one of:

jobserver = ["parallel"]

or

jobserver = []

And perhaps we could emit a warning when cfg(feature = "jobserver")

@NobodyXu
Copy link
Collaborator

Right yeah that makes sense, I would accept a PR for it.

@waywardmonkeys
Copy link
Contributor Author

I'll do this ... but I notice that it has been 3 weeks and only apparently one thing is broken and I don't see that anyone in that community has reported an issue or filed a PR to fix their usage of cc.

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