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

Make cargo a workspace #11851

Merged
merged 5 commits into from
Apr 15, 2023
Merged

Conversation

weihanglo
Copy link
Member

@weihanglo weihanglo commented Mar 14, 2023

What does this PR try to resolve?

The first step of making cargo a workspace.

Benefits:

How should we test and review this PR?

Please review it commit by commit. A companion PR is here rust-lang/rust#109133, and should be reviewed together.

Unresolved issues

To limit the scope of this pull request, the following issues are intentionally left unresolved. They will be addressed right after this pull request gets merged.

  • Make benches/capture and benches/capture workspace members. (Addressed with 2cf9718)
  • Make crates/resolver-tests a workspace member. (Addressed with Update proptest #11886)
  • Fix clippy warnings and re-enable clippy check in CI for all workspace members.
  • Fix rustdoc warnings and re-enable rustdoc check in CI for all workspace members.
  • Fix linkchecker.sh warnings in CI (Make cargo a workspace #11851 (comment))
  • Leverage workspace flag --workspace when running cargo build or cargo test, instead of using flag -p.
  • Leverage glob syntax when probing members in [workspace] in Cargo.toml (i.e., crates/*).

Additional information

This depends on prior works from @Muscraft and @ehuss. Credits to them!

@rustbot
Copy link
Collaborator

rustbot commented Mar 14, 2023

r? @epage

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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 14, 2023
@epage
Copy link
Contributor

epage commented Mar 14, 2023

Should benches/capture, benches/capture, crates/resolver-tests be a part of the workspace?

imo everything buildable should be in it

Should we fix clippy warning in this pull request or in a follow-up one? I tend to the latter

I agree that it should be separate. Sometimes, I'll actually do it before by making a commit on top that fixes them but then cherry-pick that into a separate branch. However, I'm not saying we have to do this and we can instead pick it up later

.github/workflows/main.yml Outdated Show resolved Hide resolved
.github/workflows/main.yml Show resolved Hide resolved
.github/workflows/main.yml Show resolved Hide resolved
Cargo.toml Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
@weihanglo weihanglo force-pushed the make-cargo-a-workspace branch 2 times, most recently from db3a838 to 712e7af Compare March 15, 2023 12:07
epage added a commit to epage/cargo that referenced this pull request Mar 15, 2023
This is to help prepare for checking for doc warnings across the entire
workspace being created in rust-lang#11851

`Mutation` was made `pub`, along with its fields, but they aren't
actually usable with anything, so I went and made it private to match
what its documentation references
@epage epage mentioned this pull request Mar 15, 2023
bors added a commit that referenced this pull request Mar 15, 2023
docs: Address warnings

This is to help prepare for checking for doc warnings across the entire workspace being created in #11851

`Mutation` was made `pub`, along with its fields, but they aren't actually usable with anything, so I went and made it private to match what its documentation references
@weihanglo weihanglo force-pushed the make-cargo-a-workspace branch from 712e7af to 54810df Compare March 17, 2023 13:07
Cargo.lock Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
@weihanglo weihanglo force-pushed the make-cargo-a-workspace branch from 54810df to b36c0b2 Compare March 19, 2023 03:55
@weihanglo weihanglo marked this pull request as ready for review March 19, 2023 03:55
@Rustin170506 Rustin170506 mentioned this pull request Mar 23, 2023
@weihanglo weihanglo force-pushed the make-cargo-a-workspace branch 2 times, most recently from 68399a1 to f725d3d Compare March 23, 2023 09:48
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated
Comment on lines 17 to 18
# For linkchecker (downloaded during CI) and semver-check
# TODO: consider move semver-check to crates/ folder
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the linkcheck.sh script need to be updated to support running from a directory separate from the book?

Copy link
Member Author

Choose a reason for hiding this comment

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

It definitely could. I just postpone it a bit to make this PR easier to get merged. I'll add this suggestion to TODO as well.

.github/workflows/main.yml Outdated Show resolved Hide resolved
@weihanglo weihanglo force-pushed the make-cargo-a-workspace branch from 9eed0c5 to 80d4306 Compare March 26, 2023 23:28
weihanglo and others added 5 commits April 12, 2023 11:30
Some dependencies in `resolver-tests` do not have any license
information. This prevent it from being a member when integrating in
rust-lang/rust. Will figure it out after.

Co-authored-by: Scott Schafer <[email protected]>
Co-authored-by: Eric Huss <[email protected]>
This is primarily for the release process of rust-lang/rust.

Note that in rustc-worksace-hack[1] it enable http2 via libnghttp2,
cargo probably needs to enable it to compile in rust-lang/rust.

[1]: https://github.com/rust-lang/rust/blob/992d154f3a84cc8abcefcf6e6cf3698e4821b506/src/tools/rustc-workspace-hack/Cargo.toml#L77

Co-authored-by: Scott Schafer <[email protected]>
Co-authored-by: Eric Huss <[email protected]>
Co-authored-by: Scott Schafer <[email protected]>
Co-authored-by: Eric Huss <[email protected]>
Co-authored-by: Scott Schafer <[email protected]>
Co-authored-by: Eric Huss <[email protected]>
Co-authored-by: Scott Schafer <[email protected]>
Co-authored-by: Eric Huss <[email protected]>
@weihanglo weihanglo force-pushed the make-cargo-a-workspace branch from 80d4306 to aaca5a0 Compare April 12, 2023 10:33
@weihanglo
Copy link
Member Author

@ehuss, I guess we get a green light from t-bootstrap (rust-lang/rust#109133 (comment)). Do we want to move forward?

@ehuss
Copy link
Contributor

ehuss commented Apr 12, 2023

I'm a little concerned about trying to land this just before the beta branch. I suspect this will be a pretty major change, and may have a high risk that it will encounter CI issues or other regressions we'll need to deal with. Would you mind waiting a few days until that happens?

And, BTW, would you mind doing a submodule update todoy or tomorrow to include #11960 so we don't need to backport it after the beta branch? I can do it if needed. And also maybe consider #11878?

@weihanglo
Copy link
Member Author

Sure for all. I'll take a look at #11878 today.

@ehuss
Copy link
Contributor

ehuss commented Apr 15, 2023

@weihanglo It looks like the bump has landed. Whenever you are ready, feel free to r=me this PR and the rust-lang/rust one as well.

@weihanglo
Copy link
Member Author

Thanks!

@bors r=ehuss

@bors
Copy link
Contributor

bors commented Apr 15, 2023

📌 Commit aaca5a0 has been approved by ehuss

It is now in the queue for this repository.

@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 Apr 15, 2023
@bors
Copy link
Contributor

bors commented Apr 15, 2023

⌛ Testing commit aaca5a0 with merge 39116cc...

@bors
Copy link
Contributor

bors commented Apr 15, 2023

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing 39116cc to master...

@bors bors merged commit 39116cc into rust-lang:master Apr 15, 2023
@weihanglo weihanglo deleted the make-cargo-a-workspace branch April 15, 2023 21:13
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 16, 2023
…ehuss

Make cargo a workspace

8 commits in 7bf43f028ba5eb1f4d70d271c2546c38512c9875..39116ccc9b420a883a98a960f0597f9cf87414b8
2023-04-10 16:01:41 +0000 to 2023-04-15 20:24:15 +0000

- Make cargo a workspace (rust-lang/cargo#11851)
- Fix flaky not_found_permutations test. (rust-lang/cargo#11976)
- Use restricted Damerau-Levenshtein algorithm (rust-lang/cargo#11963)
- Correct the bug report for `cargo clippy --fix` (rust-lang/cargo#11882)
- Stabilize `cargo logout` (rust-lang/cargo#11950)
- Add more information to HTTP errors to help with debugging. (rust-lang/cargo#11878)
- Use registry.default for login/logout (rust-lang/cargo#11949)
- Change -C to be unstable (rust-lang/cargo#11960)

---

### What does this PR try to resolve?

Making cargo a workspace.

Why doing this?

* `rustc-workspace-hack` is primarily for sharing dependencies between rls and cargo, as rls previously depends on cargo. After rls retired, it is no longer the case sharing dependencies.
* It's q bit painful that cargo needs to deal with some dependency and licensing complexities. For example, rust-lang#108665 failed because of the interaction bewteen `windows-sys` and `raw-dylib`. It currenctly blocks cargo's feature `-Zgitxodie` from moving forward.
* See rust-lang/cargo#11851

### Benchmark result

I've done a simple benchmark on both keeping or removing entire `rustc-workspace-hack`. It had no significant difference. Both took ~2m30s to finish `./x.py build -j8 src/tools/cargo src/tools/rls src/tools/clippy src/tools/miri src/tools/rustfmt`. Environment info:

```
host: aarch64-apple-darwin
os: Mac OS 13.2.1 [64-bit]
```

A sophisticated benchmark may be needed.

### Additional information

This depends on prior works from `@Muscraft` and `@ehuss.` Credits to them!
@ehuss ehuss added this to the 1.71.0 milestone Apr 17, 2023
weihanglo added a commit to weihanglo/rust that referenced this pull request Apr 20, 2023
Since rust-lang/cargo#11851, Cargo became a Cargo workspace of
itself. However, since `src/tools/linkchecker` cannot run inside
a workspace, Cargo needs a workaround that excludes `src/doc`
from workspace member probing.

To remove this hack, this PR adds a new optional argument `--path`
for `linkchecker.sh`. With this new argument, `linkchecker.sh` can
be run from a directory separate from the book. This also benefits
other projects using linkchecker, as they can run it under target
directory or any other directory, reducing leftover.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 20, 2023
linkchecker: running from a directory separate from the book

Since rust-lang/cargo#11851, Cargo became a Cargo workspace of
itself. However, since `src/tools/linkchecker` cannot run inside
a workspace, Cargo needs a workaround that excludes `src/doc`
from workspace member probing.

To remove this hack, this PR adds a new optional argument `--path`
for `linkchecker.sh`. With this new argument, `linkchecker.sh` can
be run from a directory separate from the book. This also benefits
other projects using linkchecker, as they can run it under target
directory or any other directory, reducing leftover.
bors added a commit that referenced this pull request May 3, 2023
chore: Use `[workspace.dependencies]`

### What does this PR try to resolve?
Keeping shared dependencies in sync. It does this by utilizing workspace inheritance and adding `[workspace.dependencies]`.

### Additional information
The ability to use workspace inheritance was added in #11851, as that PR added a workspace for `cargo`.

The changes were made by a tool I wrote to assist in finding duplicate dependencies.
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.

6 participants