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

Extract Platform to a separate crate. #7375

Merged
merged 1 commit into from
Sep 20, 2019
Merged

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Sep 18, 2019

This moves the Platform, Cfg, CfgExpr types to a new crate named "cargo-platform". The intent here is to give users of cargo_metadata a way of parsing and inspecting cargo's platform values.

Along the way, I rewrote the error handling to remove failure, and to slightly improve the output.

I'm having doubts whether or not this is a good idea. As you can see from the examples/matches.rs example, it is nontrivial to use this (which also misses cargo's config values and environment variables). I don't know if anyone will actually use this. If this doesn't seem to have value, I would suggest closing it.

I've also included a sample script, publish.py, for publishing cargo itself. I suspect it will need tweaking, but I figure it would be a start and open for feedback.

@rust-highfive
Copy link

r? @Eh2406

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 18, 2019
@ehuss
Copy link
Contributor Author

ehuss commented Sep 18, 2019

See #7376 for an alternative approach for this.

@alexcrichton
Copy link
Member

I agree that this is pretty nontrivial and probably not a great solution for cargo metadata, but for our own internal uses and code organization I'd be in favor of splitting out the crate (and agressively doing so for other things we can find as well!).

Do you think, though, that even for pure Cargo-only purposes this still isn't worth it?

@ehuss
Copy link
Contributor Author

ehuss commented Sep 19, 2019

If there are no objections, I'm fine with splitting it out.

Before merging, I'd like to get feedback from someone (@pietroalbini ?) on the publish script and whether it will be useful. Some notes on it:

  • I can rewrite it in another language if needed, but I'm most comfortable with python.
  • Before it is used the first time (on 1.40), someone will need to manually publish the first version of cargo-platform.
  • It assumes you run in a clean environment (i.e., it does not include --allow-dirty). Not sure if that's ok.
  • Does the dry-run pass seem OK?
  • Anything else it should do?
  • The "already published" check is there under the assumption that some crates like cargo-publish are expected to change very rarely. From my testing, it seems to work well enough.

@pietroalbini
Copy link
Member

cc @Mark-Simulacrum who's going to manage the 1.38 release.

@Mark-Simulacrum
Copy link
Member

I don't mind having it but so long as the number of crates is less than 5 or so I'm fine with manual publishing - it's not too hard to cd around and run cargo publish. The main "slowness" comes from needing to wait for the publish verification to complete so that I know I'm not doing anything wrong :)

@alexcrichton
Copy link
Member

@bors: r+

Ok sounds good! Let's go ahead and merge this and we can figure out publication closer to the release itself if anything becomes a snag.

@bors
Copy link
Collaborator

bors commented Sep 20, 2019

📌 Commit 57c96c1 has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 20, 2019
@bors
Copy link
Collaborator

bors commented Sep 20, 2019

⌛ Testing commit 57c96c1 with merge 7ab4778...

bors added a commit that referenced this pull request Sep 20, 2019
Extract Platform to a separate crate.

This moves the `Platform`, `Cfg`, `CfgExpr` types to a new crate named "cargo-platform".  The intent here is to give users of `cargo_metadata` a way of parsing and inspecting cargo's platform values.

Along the way, I rewrote the error handling to remove `failure`, and to slightly improve the output.

I'm having doubts whether or not this is a good idea.  As you can see from the `examples/matches.rs` example, it is nontrivial to use this (which also misses cargo's config values and environment variables).  I don't know if anyone will actually use this.  If this doesn't seem to have value, I would suggest closing it.

I've also included a sample script, `publish.py`, for publishing cargo itself.  I suspect it will need tweaking, but I figure it would be a start and open for feedback.
@bors
Copy link
Collaborator

bors commented Sep 20, 2019

☀️ Test successful - checks-azure
Approved by: alexcrichton
Pushing 7ab4778 to master...

@bors bors merged commit 57c96c1 into rust-lang:master Sep 20, 2019
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Sep 24, 2019
11 commits in b6c6f685b38d523580813b0031677c2298f458ea..aa6b7e01abce30091cc594cb23a15c46cead6e24
2019-09-19 21:10:09 +0000 to 2019-09-24 17:19:12 +0000
- Fix interpretation of `--features a b` on the CLI (rust-lang/cargo#7419)
- Update env_logger requirement from 0.6.0 to 0.7.0 (rust-lang/cargo#7422)
- Update some unstable docs (rust-lang/cargo#7407)
- Fix xcompile tests. (rust-lang/cargo#7408)
- -Ztimings: Fix more scale problems. (rust-lang/cargo#7403)
- Fix some rendering issues with -Ztimings. (rust-lang/cargo#7397)
- -Ztimings: show max jobs/cpus (rust-lang/cargo#7398)
- Fix -Ztimings with doc tests. (rust-lang/cargo#7395)
- Add documentation for the -Zdoctest-xcompile feature (rust-lang/cargo#7391)
- Fix integration tests waiting for binaries to finish. (rust-lang/cargo#7394)
- Extract Platform to a separate crate. (rust-lang/cargo#7375)
Centril added a commit to Centril/rust that referenced this pull request Sep 25, 2019
…tsakis

Update cargo

11 commits in b6c6f685b38d523580813b0031677c2298f458ea..aa6b7e01abce30091cc594cb23a15c46cead6e24
2019-09-19 21:10:09 +0000 to 2019-09-24 17:19:12 +0000
- Fix interpretation of `--features a b` on the CLI (rust-lang/cargo#7419)
- Update env_logger requirement from 0.6.0 to 0.7.0 (rust-lang/cargo#7422)
- Update some unstable docs (rust-lang/cargo#7407)
- Fix xcompile tests. (rust-lang/cargo#7408)
- -Ztimings: Fix more scale problems. (rust-lang/cargo#7403)
- Fix some rendering issues with -Ztimings. (rust-lang/cargo#7397)
- -Ztimings: show max jobs/cpus (rust-lang/cargo#7398)
- Fix -Ztimings with doc tests. (rust-lang/cargo#7395)
- Add documentation for the -Zdoctest-xcompile feature (rust-lang/cargo#7391)
- Fix integration tests waiting for binaries to finish. (rust-lang/cargo#7394)
- Extract Platform to a separate crate. (rust-lang/cargo#7375)
@ehuss ehuss added this to the 1.40.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants