-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
fix(publish): Don't strip non-dev features #14325
Merged
Merged
Commits on Jul 30, 2024
-
Configuration menu - View commit details
-
Copy full SHA for 0ece9b9 - Browse repository at this point
Copy the full SHA 0ece9b9View commit details -
Configuration menu - View commit details
-
Copy full SHA for 691c5a2 - Browse repository at this point
Copy the full SHA 691c5a2View commit details -
Configuration menu - View commit details
-
Copy full SHA for 9e730ec - Browse repository at this point
Copy the full SHA 9e730ecView commit details -
refactor(package): Make target filtering optional
We'll be doing a second `prepare_for_publish` call that doesn't need the filtering, we don't have access to the `included`, and we don't want the warnings duplicated.
Configuration menu - View commit details
-
Copy full SHA for 53fe2b6 - Browse repository at this point
Copy the full SHA 53fe2b6View commit details -
Configuration menu - View commit details
-
Copy full SHA for dc9014d - Browse repository at this point
Copy the full SHA dc9014dView commit details -
fix(publish): Don't strip non-dev features
First, we added support for stripping of local-only dev-dependencies. This was dual-implemented for `Cargo.toml` and `Summary`. This left off stripping of `dep/feature` that reference dev-dependencies (enabling features within dev-dependencies). When we fixed this, we again dual-implemented it. The `Cargo.toml` version was correct but the `Summary` version was instead stripping too many features, particularly features that reference renamed dependencies. We didn't have tests for this case and it wasn't caught earlier because crates.io re-generates the `Summary` from `Cargo.toml`, ignoring what we post. That makes this only show up with custom registries that trust what Cargo posts. Rather than fixing the `Summary` generation, I remove the dual-implementation and instead generate the `Summary` from the published `Cargo.toml`. Unfortunately, we don't have access directly to the packaged `Cargo.toml`. It could be passed around and I originally did so, hoping to remove use of the local `Package`. However, the local `Package` is needed for things like reading the `README`. So I scaled back and isolate the change to only what needs it. This also makes it easier for `prepare_transmit` callers. Fully open to someone exploring removing this extra `prepare_for_publish` in the future. Fixes rust-lang#14321
Configuration menu - View commit details
-
Copy full SHA for eddf7b7 - Browse repository at this point
Copy the full SHA eddf7b7View commit details
Commits on Jul 31, 2024
-
Configuration menu - View commit details
-
Copy full SHA for fb41fc3 - Browse repository at this point
Copy the full SHA fb41fc3View commit details -
Configuration menu - View commit details
-
Copy full SHA for 7c17862 - Browse repository at this point
Copy the full SHA 7c17862View commit details
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.