Add very simple edition check to tidy. #63087

crlf0710 · 2019-07-28T17:20:28Z

rust-highfive · 2019-07-28T17:20:38Z

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

crlf0710 · 2019-07-28T17:28:39Z

Centril · 2019-07-28T18:08:41Z

Code looks good, I think; did you verify locally that e.g. removing edition = "2018" from a random crate results in a tidy error?

Also cc @Mark-Simulacrum.

Mark-Simulacrum

I would put this check into a separate file; this file is sort of vestigial (we don't generally use extern crate in 2018) and I think it's plausible we'll eventually just delete it, so it'd be good to have this check in a separate pass. cc #62036

src/tools/tidy/src/cargo.rs

Centril · 2019-07-28T18:29:20Z

r? @Mark-Simulacrum for better precision reviewership... ;)

crlf0710 · 2019-07-28T19:26:19Z

Did a local run, and found that this didn't work as expected. There's a bug in cargo.rs which excluded the whole src directory running checks.

So i moved out the code to a separate module. Got this result:

tidy error: L:\ThirdParty\rust\rust\src\doc\book\2018-edition\Cargo.toml doesn't have `edition = "2018"` on a separate line
tidy error: L:\ThirdParty\rust\rust\src\doc\book\ci\stable-check\Cargo.toml doesn't have `edition = "2018"` on a separate line
tidy error: L:\ThirdParty\rust\rust\src\doc\book\second-edition\Cargo.toml doesn't have `edition = "2018"` on a separate line
tidy error: L:\ThirdParty\rust\rust\src\doc\reference\stable-check\Cargo.toml doesn't have `edition = "2018"` on a separate line
tidy error: L:\ThirdParty\rust\rust\src\test\run-make\thumb-none-qemu\example\Cargo.toml doesn't have `edition = "2018"` on a separate line
tidy error: L:\ThirdParty\rust\rust\src\tools\rustc-std-workspace-alloc\Cargo.toml doesn't have `edition = "2018"` on a separate line
some tidy checks failed

Do we want to ignore or change some of them?

Mark-Simulacrum · 2019-07-28T19:29:28Z

The ones which are local, i.e., not in submodules, let's fix in this PR and ignore the others via a whitelist in tidy, we can fix them at a later point.

rust-highfive · 2019-07-28T19:48:19Z

The job x86_64-gnu-llvm-6.0 of your PR failed (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

2019-07-28T19:28:42.9673848Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-07-28T19:28:42.9879767Z ##[command]git config gc.auto 0
2019-07-28T19:28:42.9946565Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-07-28T19:28:42.9996474Z ##[command]git config --get-all http.proxy
2019-07-28T19:28:43.0142717Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/63087/merge:refs/remotes/pull/63087/merge
---
2019-07-28T19:29:16.5489786Z do so (now or later) by using -b with the checkout command again. Example:
2019-07-28T19:29:16.5489821Z 
2019-07-28T19:29:16.5490070Z   git checkout -b <new-branch-name>
2019-07-28T19:29:16.5490104Z 
2019-07-28T19:29:16.5490170Z HEAD is now at dda57795a Merge 7df07886c36f930be8ba3c970434bfbbacc06d9a into 023525dbda35748a10713471b948974b68a1c2cc
2019-07-28T19:29:16.5626633Z ##[section]Starting: Collect CPU-usage statistics in the background
2019-07-28T19:29:16.5630449Z ==============================================================================
2019-07-28T19:29:16.5630515Z Task         : Bash
2019-07-28T19:29:16.5630567Z Description  : Run a Bash script on macOS, Linux, or Windows
---
2019-07-28T19:35:21.3409978Z     Finished release [optimized] target(s) in 1m 27s
2019-07-28T19:35:21.3504385Z tidy check
2019-07-28T19:35:22.3034923Z * 577 error codes
2019-07-28T19:35:22.3035876Z * highest error code: E0732
2019-07-28T19:35:22.3062991Z tidy error: /checkout/src/tools/rustc-std-workspace-alloc/Cargo.toml doesn't have `edition = "2018"` on a separate line
2019-07-28T19:35:22.3241890Z tidy error: /checkout/src/test/run-make/thumb-none-qemu/example/Cargo.toml doesn't have `edition = "2018"` on a separate line
2019-07-28T19:35:22.3294076Z tidy error: /checkout/src/doc/book/2018-edition/Cargo.toml doesn't have `edition = "2018"` on a separate line
2019-07-28T19:35:22.3295234Z tidy error: /checkout/src/doc/book/ci/stable-check/Cargo.toml doesn't have `edition = "2018"` on a separate line
2019-07-28T19:35:22.3296006Z tidy error: /checkout/src/doc/book/second-edition/Cargo.toml doesn't have `edition = "2018"` on a separate line
2019-07-28T19:35:22.3303938Z tidy error: /checkout/src/doc/reference/stable-check/Cargo.toml doesn't have `edition = "2018"` on a separate line
2019-07-28T19:35:23.3171187Z some tidy checks failed
2019-07-28T19:35:23.3175898Z 
2019-07-28T19:35:23.3175898Z 
2019-07-28T19:35:23.3179400Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor"
2019-07-28T19:35:23.3179586Z 
2019-07-28T19:35:23.3179863Z 
2019-07-28T19:35:23.3194171Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
2019-07-28T19:35:23.3194274Z Build completed unsuccessfully in 0:01:31
2019-07-28T19:35:23.3194274Z Build completed unsuccessfully in 0:01:31
2019-07-28T19:35:24.6869602Z ##[error]Bash exited with code '1'.
2019-07-28T19:35:24.6911463Z ##[section]Starting: Checkout
2019-07-28T19:35:24.6913587Z ==============================================================================
2019-07-28T19:35:24.6913672Z Task         : Get sources
2019-07-28T19:35:24.6913727Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

src/test/run-make/thumb-none-qemu/example/src/main.rs

src/tools/rustc-std-workspace-alloc/lib.rs

petrochenkov · 2019-07-28T20:32:42Z

Is it possible to set the edition through rustbuild, or at least workspaces instead?

It's certainly possible to do at rustc level in rustbuild, but I'm not sure what will happen in that case if Cargo thinks the edition is 2015 even if 2018 is set through the wrapper.

Centril · 2019-07-28T20:35:32Z

Is it possible to set the edition through rustbuild, or at least workspaces instead?

Doesn't that mess with the ability to cargo check individual crates?

petrochenkov · 2019-07-28T20:52:28Z

@Centril
I don't know.
Is it a working scenario? What's an isolated crate without its workspace and other environment?

Centril · 2019-07-28T20:56:26Z

Is it a working scenario? What's an isolated crate without its workspace and other environment?

I've cargo checked some compiler crates in the past, but perhaps ./x.py check is always fast enough these days.

I'm also a bit "philosophically" inclined towards crates that are standalone and edition = "2018" doesn't seem like a huge burden... :)

Mark-Simulacrum · 2019-07-28T21:16:06Z

Cargo seems to not support setting the edition key in workspaces (rust-lang/cargo#5784) but I haven't actually tested. I think for the most part since cargo new adds the edition key by default these days it's presumably fairly low-effort for users to do this as well.

I'm currently using cargo check locally and it works well -- specifically, I run something like:

RUSTC_ERROR_METADATA_DST=build RUSTFLAGS="-Zforce-unstable-if-unmarked" CFG_COMPILER_HOST_TRIPLE=x86_64-unknown-linux-gnu RUSTC_BOOTSTRAP=1 cargo check --manifest-path $nearest_cargo_toml --color never --frozen --message-format=json -q in my vim to get compile errors for the current crate. This works for the most part -- and is in fact faster than x.py check, so long as the APIs being used in that crate are also present in the nightly sysroot I have, since the dependencies are loaded from there, not my (edited) tree.

However, even if I was to migrate to x.py check, I think it wouldn't really be viable to not list the edition key in Cargo.toml -- Cargo changes behavior on the new edition for some keys, and we want to opt-in to that behavior. It's nothing groundbreaking at this point, but it's good to be using the latest defaults.

Ideally here we'd probably fix the Cargo feature request but that needs buy-in from the Cargo team (and time).

src/test/run-make/thumb-none-qemu/example/Cargo.toml

crlf0710 · 2019-07-29T12:03:02Z

@Mark-Simulacrum ready for review again modulo the one comment by @petrochenkov above.

src/tools/tidy/src/edition.rs

Mark-Simulacrum

r=me modulo last nit

src/tools/tidy/src/edition.rs

crlf0710 · 2019-07-29T17:26:15Z

Ready~ @Mark-Simulacrum @petrochenkov @Centril

Mark-Simulacrum · 2019-07-29T17:29:18Z

Could you squash the changes into one commit? Feel free to approve after doing so @bors delegate+

bors · 2019-07-29T17:29:20Z

✌️ @crlf0710 can now approve this pull request

crlf0710 · 2019-07-29T18:00:33Z

@bors r+

bors · 2019-07-29T18:00:35Z

📌 Commit 870efe3 has been approved by crlf0710

Mark-Simulacrum · 2019-07-29T18:01:40Z

@bors r- r=Mark-Simulacrum rollup

bors · 2019-07-29T18:01:43Z

📌 Commit 870efe3 has been approved by Mark-Simulacrum

Add very simple edition check to tidy. Fixes rust-lang#58099.

@ghost

Rollup of 12 pull requests Successful merges: - #61965 (Remove mentions of removed `offset_to` method from `align_offset` docs) - #62928 (Syntax: Recover on `for ( $pat in $expr ) $block`) - #63000 (Impl Debug for Chars) - #63083 (Make generic parameters always use modern hygiene) - #63087 (Add very simple edition check to tidy.) - #63093 (Properly check the defining scope of existential types) - #63096 (Add tests for some `existential_type` ICEs) - #63099 (vxworks: Remove Linux-specific comments.) - #63106 (ci: Skip installing SWIG/xz on OSX ) - #63108 (Add links to None in Option doc) - #63109 (std: Fix a failing `fs` test on Windows) - #63111 (Add syntactic and semantic tests for rest patterns, i.e. `..`) Failed merges: r? @ghost

rust-highfive assigned nikomatsakis Jul 28, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 28, 2019

rust-highfive assigned Centril and unassigned nikomatsakis Jul 28, 2019

crlf0710 force-pushed the tidy_2018 branch from 2c6607d to a085b3f Compare July 28, 2019 17:40

Centril added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 28, 2019

Mark-Simulacrum reviewed Jul 28, 2019

View reviewed changes

src/tools/tidy/src/cargo.rs Outdated Show resolved Hide resolved

rust-highfive assigned Mark-Simulacrum and unassigned Centril Jul 28, 2019

crlf0710 force-pushed the tidy_2018 branch from a085b3f to 7df0788 Compare July 28, 2019 19:28

crlf0710 commented Jul 28, 2019

View reviewed changes

src/test/run-make/thumb-none-qemu/example/src/main.rs Show resolved Hide resolved

petrochenkov reviewed Jul 28, 2019

View reviewed changes

src/tools/rustc-std-workspace-alloc/lib.rs Outdated Show resolved Hide resolved

Mark-Simulacrum reviewed Jul 28, 2019

View reviewed changes

src/test/run-make/thumb-none-qemu/example/Cargo.toml Outdated Show resolved Hide resolved

crlf0710 force-pushed the tidy_2018 branch from f8ef3b2 to 18dbc91 Compare July 29, 2019 02:40

Mark-Simulacrum reviewed Jul 29, 2019

View reviewed changes

src/tools/tidy/src/edition.rs Outdated Show resolved Hide resolved

crlf0710 force-pushed the tidy_2018 branch from 0e49c60 to 7d2e8d7 Compare July 29, 2019 13:28

Mark-Simulacrum reviewed Jul 29, 2019

View reviewed changes

src/tools/tidy/src/edition.rs Outdated Show resolved Hide resolved

Add very simple edition check to tidy; and add missing edition = 2018s.

870efe3

crlf0710 force-pushed the tidy_2018 branch from f6e04c3 to 870efe3 Compare July 29, 2019 17:56

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 29, 2019

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 29, 2019

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 29, 2019

Centril added a commit to Centril/rust that referenced this pull request Jul 29, 2019

Rollup merge of rust-lang#63087 - crlf0710:tidy_2018, r=Mark-Simulacrum

2c0e220

Add very simple edition check to tidy. Fixes rust-lang#58099.

Centril mentioned this pull request Jul 29, 2019

Rollup of 9 pull requests #63112

Closed

Centril added a commit to Centril/rust that referenced this pull request Jul 29, 2019

Rollup merge of rust-lang#63087 - crlf0710:tidy_2018, r=Mark-Simulacrum

6edff11

Add very simple edition check to tidy. Fixes rust-lang#58099.

Centril mentioned this pull request Jul 29, 2019

Rollup of 13 pull requests #63119

Closed

Centril added a commit to Centril/rust that referenced this pull request Jul 30, 2019

Rollup merge of rust-lang#63087 - crlf0710:tidy_2018, r=Mark-Simulacrum

b62ddfe

Add very simple edition check to tidy. Fixes rust-lang#58099.

Centril mentioned this pull request Jul 30, 2019

Rollup of 12 pull requests #63120

Closed

Centril added a commit to Centril/rust that referenced this pull request Jul 30, 2019

Rollup merge of rust-lang#63087 - crlf0710:tidy_2018, r=Mark-Simulacrum

4413068

Add very simple edition check to tidy. Fixes rust-lang#58099.

Centril mentioned this pull request Jul 30, 2019

Rollup of 12 pull requests #63124

Merged

bors merged commit 870efe3 into rust-lang:master Jul 30, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add very simple edition check to tidy. #63087

Add very simple edition check to tidy. #63087

crlf0710 commented Jul 28, 2019 •

edited by Centril

Loading

rust-highfive commented Jul 28, 2019

crlf0710 commented Jul 28, 2019

Centril commented Jul 28, 2019

Mark-Simulacrum left a comment

Centril commented Jul 28, 2019

crlf0710 commented Jul 28, 2019

Mark-Simulacrum commented Jul 28, 2019

rust-highfive commented Jul 28, 2019

petrochenkov commented Jul 28, 2019

Centril commented Jul 28, 2019

petrochenkov commented Jul 28, 2019

Centril commented Jul 28, 2019

Mark-Simulacrum commented Jul 28, 2019

crlf0710 commented Jul 29, 2019

Mark-Simulacrum left a comment

crlf0710 commented Jul 29, 2019

Mark-Simulacrum commented Jul 29, 2019

bors commented Jul 29, 2019

crlf0710 commented Jul 29, 2019

bors commented Jul 29, 2019

Mark-Simulacrum commented Jul 29, 2019

bors commented Jul 29, 2019

Add very simple edition check to tidy. #63087

Add very simple edition check to tidy. #63087

Conversation

crlf0710 commented Jul 28, 2019 • edited by Centril Loading

rust-highfive commented Jul 28, 2019

crlf0710 commented Jul 28, 2019

Centril commented Jul 28, 2019

Mark-Simulacrum left a comment

Choose a reason for hiding this comment

Centril commented Jul 28, 2019

crlf0710 commented Jul 28, 2019

Mark-Simulacrum commented Jul 28, 2019

rust-highfive commented Jul 28, 2019

petrochenkov commented Jul 28, 2019

Centril commented Jul 28, 2019

petrochenkov commented Jul 28, 2019

Centril commented Jul 28, 2019

Mark-Simulacrum commented Jul 28, 2019

crlf0710 commented Jul 29, 2019

Mark-Simulacrum left a comment

Choose a reason for hiding this comment

crlf0710 commented Jul 29, 2019

Mark-Simulacrum commented Jul 29, 2019

bors commented Jul 29, 2019

crlf0710 commented Jul 29, 2019

bors commented Jul 29, 2019

Mark-Simulacrum commented Jul 29, 2019

bors commented Jul 29, 2019

crlf0710 commented Jul 28, 2019 •

edited by Centril

Loading