Skip to content

Add workspace to manifest #6733

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Feb 16, 2021
Merged

Add workspace to manifest #6733

merged 1 commit into from
Feb 16, 2021

Conversation

camsteffen
Copy link
Contributor

changelog: none

This fixes the dogfood test to include clippy_lints (regression from #6687).

r? @flip1995

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 12, 2021
@camsteffen
Copy link
Contributor Author

I went on a long research trait to figure out the build errors. I found dtolnay/trybuild#13 and rust-lang/rust#73603 to be interesting. @Manishearth do you think it is possible that using an up-to-date compiletest would fix the build errors here? Is it feasible to do rust-lang/rust#73603 soon or is that a lot of work?

@Manishearth
Copy link
Member

Note that there's an active Clippy PR to update our compiletest dependency

But I would dearly love to split compiletest out, yes. I think it's feasible but a bunch of work

@camsteffen
Copy link
Contributor Author

Note that there's an active Clippy PR to update our compiletest dependency

Oh! Can you link to it? I suppose I should at least wait for that.

I think it's feasible but a bunch of work

Is there a more feasible version where compiletest is included just like any rustc_* module?

@flip1995
Copy link
Member

The compiletest update was already merged: #6743 Not sure if it includes the features we want/need for this PR though.

@Manishearth
Copy link
Member

Manishearth commented Feb 16, 2021

@camsteffen hmm, unsure about rustc_. It's worth a try, I don't think it will work automatically but it might be doable with some work.

But really I feel like it's better for everyone if compiletest is moved out of tree. But you should still feel free to move on making the other thing happen!

@camsteffen
Copy link
Contributor Author

camsteffen commented Feb 16, 2021

Excluding mini-macro fixed the build errors. I'm not sure why RUSTC_WORKSPACE_WRAPPER=.. cargo .. produced a different target for mini-macro 🤷. Maybe something to do with the fact that it's a proc-macro.

Excluding clippy-dev is necessary to avoid

error: current package believes it's in a workspace when it's not

@camsteffen
Copy link
Contributor Author

A possible follow-up change is to change our automated tests to use cargo test --all and remove the clippy_lints and rustc_tools_util tests?

bors added a commit that referenced this pull request Feb 16, 2021
Test workspace at once

changelog: none

Follow-up to #6733
@bors bors merged commit 9bcb257 into rust-lang:master Feb 16, 2021
@camsteffen
Copy link
Contributor Author

Whoops didn't mean to pull a fast one but this got merged in #6749 😄

@flip1995
Copy link
Member

@camsteffen @Manishearth This broke Clippy in rust-lang/rust:

error: multiple workspace roots found in the same workspace:
  /home/pkrones/rust-lang/rust/src/tools/clippy
  /home/pkrones/rust-lang/rust
failed to run: /home/pkrones/rust-lang/rust/build/x86_64-unknown-linux-gnu/stage0/bin/cargo build --manifest-path /home/pkrones/rust-lang/rust/src/bootstrap/Cargo.toml
Build completed unsuccessfully in 0:01:01

@Manishearth
Copy link
Member

Argh, I had assumed this would happen but thought people had a reason why it wouldn't given all the discussion. Revert and self-r+ I guess?

@flip1995
Copy link
Member

I don't think we can do this. This is the fix, so that dogfood actually is run on Clippy. So we need a solution, not involving workspaces. I just removed the workspace in the Clippy update in rustc for now: rust-lang/rust#82514 This gives us 2 weeks to fix this.

bors added a commit that referenced this pull request Feb 28, 2021
Remove workspace and fix dogfood (again)

changelog: none

In response to #6733 (comment)
@camsteffen camsteffen deleted the workspace branch July 8, 2021 22:02
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants