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

Remove workspace and fix dogfood (again) #6802

Merged
merged 5 commits into from
Feb 28, 2021
Merged

Conversation

camsteffen
Copy link
Contributor

changelog: none

In response to #6733 (comment)

@rust-highfive
Copy link

r? @llogiq

(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 Feb 26, 2021
@llogiq
Copy link
Contributor

llogiq commented Feb 28, 2021

@bors r+

@bors
Copy link
Collaborator

bors commented Feb 28, 2021

📌 Commit d71ed26 has been approved by llogiq

@bors
Copy link
Collaborator

bors commented Feb 28, 2021

⌛ Testing commit d71ed26 with merge abd2c7e...

@bors
Copy link
Collaborator

bors commented Feb 28, 2021

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing abd2c7e to master...

@bors bors merged commit abd2c7e into rust-lang:master Feb 28, 2021
@matthiaskrgr
Copy link
Member

Uff, reverting this broke lintcheck again :(
Maybe we have to keep workspace.exclude = target/lintcheck/crates ?

@camsteffen
Copy link
Contributor Author

Hmm how can that be?

@matthiaskrgr
Copy link
Member

I'm not really sure yet.
It seems to work fine on the master branch.
On my WIP branch I did a rebase and now I get panics inside clippy_dev::clippy_project_root()
But if I revert the rebase, everything seems to work fine 🤔

panic!("error: Can't determine root of project. Please run inside a Clippy working dir.");


Downloading and extracting anyhow 1.0.38 from https://crates.io/api/v1/crates/anyhow/1.0.38/download
1/22 4% Linting anyhow 1.0.38
thread 'main' panicked at 'error: Can't determine root of project. Please run inside a Clippy working dir.', src/lib.rs:361:5
stack backtrace:
   0:     0x563f0d778950 - std::backtrace_rs::backtrace::libunwind::trace::h9d49145f95eb5894
                               at /rustc/a8486b64b0c87dabd045453b6c81500015d122d6/library/std/src/../../backtrace/src/backtrace/libunwind.rs:90:5
   1:     0x563f0d778950 - std::backtrace_rs::backtrace::trace_unsynchronized::hab1d020365bb6864
                               at /rustc/a8486b64b0c87dabd045453b6c81500015d122d6/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x563f0d778950 - std::sys_common::backtrace::_print_fmt::h7659588431e304bd
                               at /rustc/a8486b64b0c87dabd045453b6c81500015d122d6/library/std/src/sys_common/backtrace.rs:67:5
   3:     0x563f0d778950 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h09f4a9e3befae3c7
                               at /rustc/a8486b64b0c87dabd045453b6c81500015d122d6/library/std/src/sys_common/backtrace.rs:46:22
   4:     0x563f0d79e2ac - core::fmt::write::hf3fdfde304b9a088
                               at /rustc/a8486b64b0c87dabd045453b6c81500015d122d6/library/core/src/fmt/mod.rs:1092:17
   5:     0x563f0d76f862 - std::io::Write::write_fmt::h1cb850689c7116f0
                               at /rustc/a8486b64b0c87dabd045453b6c81500015d122d6/library/std/src/io/mod.rs:1568:15
   6:     0x563f0d77afc5 - std::sys_common::backtrace::_print::hdbccd5aa093ba544
                               at /rustc/a8486b64b0c87dabd045453b6c81500015d122d6/library/std/src/sys_common/backtrace.rs:49:5
   7:     0x563f0d77afc5 - std::sys_common::backtrace::print::hc639c4f320222558
                               at /rustc/a8486b64b0c87dabd045453b6c81500015d122d6/library/std/src/sys_common/backtrace.rs:36:9
   8:     0x563f0d77afc5 - std::panicking::default_hook::{{closure}}::hdb012dd7a485bb5d
                               at /rustc/a8486b64b0c87dabd045453b6c81500015d122d6/library/std/src/panicking.rs:208:50
   9:     0x563f0d77aa73 - std::panicking::default_hook::h75facbce77b6ba91
                               at /rustc/a8486b64b0c87dabd045453b6c81500015d122d6/library/std/src/panicking.rs:225:9
  10:     0x563f0d77b761 - std::panicking::rust_panic_with_hook::hbcaa5de2cb5e22d5
                               at /rustc/a8486b64b0c87dabd045453b6c81500015d122d6/library/std/src/panicking.rs:591:17
  11:     0x563f0d2a98fe - std::panicking::begin_panic::{{closure}}::h14879ca0384134c2
                               at /home/matthias/.rustup/toolchains/nightly-2021-02-25-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:520:9
  12:     0x563f0d2b7599 - std::sys_common::backtrace::__rust_end_short_backtrace::h4c74be0b0e6286b3
                               at /home/matthias/.rustup/toolchains/nightly-2021-02-25-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:141:18
  13:     0x563f0d2a982b - std::panicking::begin_panic::h6875e2ff6a4df448
                               at /home/matthias/.rustup/toolchains/nightly-2021-02-25-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:519:12
  14:     0x563f0cfc039a - clippy_dev::clippy_project_root::h67938ed3b4092f51
                               at /home/matthias/vcs/github/rust-clippy/clippy_dev/src/lib.rs:361:5
  15:     0x563f0d02b9dc - clippy_dev::lintcheck::Crate::run_clippy_lints::hda23d37712e3e67f
                               at /home/matthias/vcs/github/rust-clippy/clippy_dev/src/lintcheck.rs:255:33
  16:     0x563f0d0578e1 - clippy_dev::lintcheck::run::{{closure}}::h765727597147f879
                               at /home/matthias/vcs/github/rust-clippy/clippy_dev/src/lintcheck.rs:630:30
  17:     0x563f0d05474f - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &mut F>::call_once::h82dfd79707418f19
                               at /home/matthias/.rustup/toolchains/nightly-2021-02-25-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:280:13
  18:     0x563f0cfb3fa5 - core::option::Option<T>::map::h0e762c19ebd0d03f
                               at /home/matthias/.rustup/toolchains/nightly-2021-02-25-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/option.rs:487:29
  19:     0x563f0d040883 - <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::next::h16104d73915367fc
                               at /home/matthias/.rustup/toolchains/nightly-2021-02-25-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/iter/adapters/map.rs:100:9
  20:     0x563f0cfe99db - <core::iter::adapters::fuse::Fuse<I> as core::iter::adapters::fuse::FuseImpl<I>>::next::hb53d182b063431ca
                               at /home/matthias/.rustup/toolchains/nightly-2021-02-25-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/iter/adapters/fuse.rs:420:9
  21:     0x563f0cfe94a7 - <core::iter::adapters::fuse::Fuse<I> as core::iter::traits::iterator::Iterator>::next::hcda27b9533947387
                               at /home/matthias/.rustup/toolchains/nightly-2021-02-25-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/iter/adapters/fuse.rs:67:9
  22:     0x563f0cfbb61d - <core::iter::adapters::flatten::FlattenCompat<I,U> as core::iter::traits::iterator::Iterator>::next::h33e9f6193aa5cf58
                               at /home/matthias/.rustup/toolchains/nightly-2021-02-25-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/iter/adapters/flatten.rs:267:19
  23:     0x563f0cfba527 - <core::iter::adapters::flatten::Flatten<I> as core::iter::traits::iterator::Iterator>::next::h21e1b8a3d32890f1
                               at /home/matthias/.rustup/toolchains/nightly-2021-02-25-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/iter/adapters/flatten.rs:168:9
  24:     0x563f0cfc5ef2 - <alloc::vec::Vec<T> as alloc::vec::spec_from_iter_nested::SpecFromIterNested<T,I>>::from_iter::h90496f19050e3ac1
                               at /home/matthias/.rustup/toolchains/nightly-2021-02-25-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/vec/spec_from_iter_nested.rs:23:32
  25:     0x563f0cfd535e - <alloc::vec::Vec<T> as alloc::vec::spec_from_iter::SpecFromIter<T,I>>::from_iter::h561a5360cf545974
                               at /home/matthias/.rustup/toolchains/nightly-2021-02-25-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/vec/spec_from_iter.rs:36:9
  26:     0x563f0cfd23bf - <alloc::vec::Vec<T> as core::iter::traits::collect::FromIterator<T>>::from_iter::hd4fc3aa926b3cb6f
                               at /home/matthias/.rustup/toolchains/nightly-2021-02-25-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/vec/mod.rs:2302:9
  27:     0x563f0cfbe52e - core::iter::traits::iterator::Iterator::collect::ha5a55068e7678703
                               at /home/matthias/.rustup/toolchains/nightly-2021-02-25-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/iter/traits/iterator.rs:1763:9
  28:     0x563f0d02e86d - clippy_dev::lintcheck::run::h849b241ac46bcaf3
                               at /home/matthias/vcs/github/rust-clippy/clippy_dev/src/lintcheck.rs:627:13
  29:     0x563f0cf9ce11 - clippy_dev::main::hcc2d2ffd082b0c76
                               at /home/matthias/vcs/github/rust-clippy/clippy_dev/src/main.rs:18:13
  30:     0x563f0cf9be6b - core::ops::function::FnOnce::call_once::heea7d10f712a1f77
                               at /home/matthias/.rustup/toolchains/nightly-2021-02-25-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
  31:     0x563f0cf9b41e - std::sys_common::backtrace::__rust_begin_short_backtrace::hd67addc4007e04fd
                               at /home/matthias/.rustup/toolchains/nightly-2021-02-25-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:125:18
  32:     0x563f0cf9c411 - std::rt::lang_start::{{closure}}::hd5f7602c6c1c82d8
                               at /home/matthias/.rustup/toolchains/nightly-2021-02-25-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:66:18
  33:     0x563f0d77bc6a - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::h31ee16c075ae2d62
                               at /rustc/a8486b64b0c87dabd045453b6c81500015d122d6/library/core/src/ops/function.rs:259:13
  34:     0x563f0d77bc6a - std::panicking::try::do_call::h3993344bf97066b6
                               at /rustc/a8486b64b0c87dabd045453b6c81500015d122d6/library/std/src/panicking.rs:379:40
  35:     0x563f0d77bc6a - std::panicking::try::h09e523df7c9c2d28
                               at /rustc/a8486b64b0c87dabd045453b6c81500015d122d6/library/std/src/panicking.rs:343:19
  36:     0x563f0d77bc6a - std::panic::catch_unwind::h9d4ddb10ebb815ff
                               at /rustc/a8486b64b0c87dabd045453b6c81500015d122d6/library/std/src/panic.rs:431:14
  37:     0x563f0d77bc6a - std::rt::lang_start_internal::hc92e27a69d75de2a
                               at /rustc/a8486b64b0c87dabd045453b6c81500015d122d6/library/std/src/rt.rs:51:25
  38:     0x563f0cf9c3e7 - std::rt::lang_start::h60e9e8d0068e6acc
                               at /home/matthias/.rustup/toolchains/nightly-2021-02-25-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:65:5
  39:     0x563f0cf9eaca - main
  40:     0x7f3c26a6bb25 - __libc_start_main
  41:     0x563f0cf9b02e - _start
  42:                0x0 - <unknown>

@matthiaskrgr
Copy link
Member

I've fixed the issue by redoing one of my commits but I still have no idea what caused this 🤷 😄

@flip1995
Copy link
Member

flip1995 commented Mar 2, 2021

With this change, the dogfood test doesn't actually run. 🤔

@flip1995
Copy link
Member

flip1995 commented Mar 2, 2021

I found the reason. If something is a (path) dep and not a workspace member, cargo will run rustc on that (path) dep instead of the RUSTC_WORKSPACE_WRAPPER. This can be seen in this verbose output:

$ cd clippy_workspace_tests/subcrate
$ echo "$(head -n -2 ../Cargo.toml)" > ../Cargo.toml  # remove `workspace` from crate
$ ../../target/debug/cargo-clippy clippy -v
    Checking path_dep v0.1.0 (/home/pkrones/rust-lang/rust-clippy/clippy_workspace_tests/path_dep)
!!!  Running `rustc --crate-name path_dep /home/pkrones/rust-lang/rust-clippy/clippy_workspace_tests/path_dep/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type lib --emit=dep-info,metadata -C embed-bitcode=no -C debuginfo=2 -C metadata=bdbed90518da5aa3 -C extra-filename=-bdbed90518da5aa3 --out-dir /home/pkrones/rust-lang/rust-clippy/clippy_workspace_tests/subcrate/target/debug/deps -C incremental=/home/pkrones/rust-lang/rust-clippy/clippy_workspace_tests/subcrate/target/debug/incremental -L dependency=/home/pkrones/rust-lang/rust-clippy/clippy_workspace_tests/subcrate/target/debug/deps -Zunstable-options`
    Checking subcrate v0.1.0 (/home/pkrones/rust-lang/rust-clippy/clippy_workspace_tests/subcrate)
     Running `/home/pkrones/rust-lang/rust-clippy/target/debug/clippy-driver rustc --crate-name subcrate src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type lib --emit=dep-info,metadata -C embed-bitcode=no -C debuginfo=2 -C metadata=4bb424f3a7cef4f6 -C extra-filename=-4bb424f3a7cef4f6 --out-dir /home/pkrones/rust-lang/rust-clippy/clippy_workspace_tests/subcrate/target/debug/deps -C incremental=/home/pkrones/rust-lang/rust-clippy/clippy_workspace_tests/subcrate/target/debug/incremental -L dependency=/home/pkrones/rust-lang/rust-clippy/clippy_workspace_tests/subcrate/target/debug/deps --extern path_dep=/home/pkrones/rust-lang/rust-clippy/clippy_workspace_tests/subcrate/target/debug/deps/libpath_dep-bdbed90518da5aa3.rmeta -Zunstable-options`
    Finished dev [unoptimized + debuginfo] target(s) in 0.37s

And with workspace:

$ git checkout ../Cargo.toml
$ cargo clean
$ ../../target/debug/cargo-clippy clippy -v
    Checking path_dep v0.1.0 (/home/pkrones/rust-lang/rust-clippy/clippy_workspace_tests/path_dep)
!!!  Running `/home/pkrones/rust-lang/rust-clippy/target/debug/clippy-driver rustc --crate-name path_dep path_dep/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type lib --emit=dep-info,metadata -C embed-bitcode=no -C debuginfo=2 -C metadata=758172c486bda354 -C extra-filename=-758172c486bda354 --out-dir /home/pkrones/rust-lang/rust-clippy/clippy_workspace_tests/target/debug/deps -C incremental=/home/pkrones/rust-lang/rust-clippy/clippy_workspace_tests/target/debug/incremental -L dependency=/home/pkrones/rust-lang/rust-clippy/clippy_workspace_tests/target/debug/deps -Zunstable-options`
    Checking subcrate v0.1.0 (/home/pkrones/rust-lang/rust-clippy/clippy_workspace_tests/subcrate)
     Running `/home/pkrones/rust-lang/rust-clippy/target/debug/clippy-driver rustc --crate-name subcrate subcrate/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type lib --emit=dep-info,metadata -C embed-bitcode=no -C debuginfo=2 -C metadata=75da0d1bce2f8558 -C extra-filename=-75da0d1bce2f8558 --out-dir /home/pkrones/rust-lang/rust-clippy/clippy_workspace_tests/target/debug/deps -C incremental=/home/pkrones/rust-lang/rust-clippy/clippy_workspace_tests/target/debug/incremental -L dependency=/home/pkrones/rust-lang/rust-clippy/clippy_workspace_tests/target/debug/deps --extern path_dep=/home/pkrones/rust-lang/rust-clippy/clippy_workspace_tests/target/debug/deps/libpath_dep-758172c486bda354.rmeta -Zunstable-options`
    Finished dev [unoptimized + debuginfo] target(s) in 0.31s

@ehuss Is this intended behavior?

If this is intended, I guess this part of Clippy is really useless:

rust-clippy/src/driver.rs

Lines 294 to 306 in 3cd6ca0

// We enable Clippy if one of the following conditions is met
// - IF Clippy is run on its test suite OR
// - IF Clippy is run on the main crate, not on deps (`!cap_lints_allow`) THEN
// - IF `--no-deps` is not set (`!no_deps`) OR
// - IF `--no-deps` is set and Clippy is run on the specified primary package
let clippy_tests_set = env::var("__CLIPPY_INTERNAL_TESTS").map_or(false, |val| val == "true");
let cap_lints_allow = arg_value(&orig_args, "--cap-lints", |val| val == "allow").is_some();
let in_primary_package = env::var("CARGO_PRIMARY_PACKAGE").is_ok();
let clippy_enabled = clippy_tests_set || (!cap_lints_allow && (!no_deps || in_primary_package));
if clippy_enabled {
args.extend(clippy_args);
}

@camsteffen
Copy link
Contributor Author

Where are you seeing the dogfood test not run?

@flip1995
Copy link
Member

flip1995 commented Mar 2, 2021

If I run cargo clippy inside clippy_lints on the current master branch, I get multiple lint warnings. But dogfood succeeds. So I guess it doesn't really run.

@ehuss
Copy link
Contributor

ehuss commented Mar 2, 2021

@ehuss Is this intended behavior?

Yes. There is a subtle distinction that path dependencies in a [workspace] are members of the workspace, but without [workspace] they are not.

@camsteffen
Copy link
Contributor Author

Ugh. I thought I had it working...

@flip1995
Copy link
Member

flip1995 commented Mar 2, 2021

I have a fix, but after fixing 5 or 6 errors in Clippy, now the lintcheck tool triggers 30 Clippy lints (cough @matthiaskrgr cough). 😄

I will fix them and open a PR tomorrow.

@camsteffen
Copy link
Contributor Author

Ok cool. It seems like the dogfood "subprojects" test can become "test all the things"?

@matthiaskrgr
Copy link
Member

, now the lintcheck tool triggers 30 Clippy lints

oh no.. I'll have a look 😅

@flip1995
Copy link
Member

flip1995 commented Mar 2, 2021

Ok cool. It seems like the dogfood "subprojects" test can become "test all the things"?

Yep, that's exactly my fix.

@flip1995 flip1995 mentioned this pull request Mar 5, 2021
bors added a commit that referenced this pull request Mar 5, 2021
Dogfood and CI fixes

The CI fix is practically #6829 rebased and squashed into one commit

Dogfood fix is a follow up of #6802

r? `@matthiaskrgr` for lintcheck changes

(best reviewed with whitespace changes hidden)

changelog: none
bors added a commit that referenced this pull request Mar 5, 2021
Dogfood and CI fixes

The CI fix is practically #6829 rebased and squashed into one commit

Dogfood fix is a follow up of #6802

r? `@matthiaskrgr` for lintcheck changes

(best reviewed with whitespace changes hidden)

changelog: none
bors added a commit that referenced this pull request Mar 5, 2021
Dogfood and CI fixes

The CI fix is practically #6829 rebased and squashed into one commit

Dogfood fix is a follow up of #6802

r? `@matthiaskrgr` for lintcheck changes

(best reviewed with whitespace changes hidden)

changelog: none
@camsteffen camsteffen deleted the dogfood-fix branch July 8, 2021 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

7 participants