Auto merge of #82208 - jyn514:rustfmt-subtree, r=Mark-Simulacrum

Convert rustfmt from a submodule to a subtree

r? `@calebcartwright` cc `@Manishearth` `@Mark-Simulacrum`

The motivation is that submodule updates cause rustfmt to not be available on nightly a lot; most recently it was unavailable for over 10 days, causing the beta release to be delayed. Additionally this is much less work on the part of the rustfmt maintainers to keep the rustfmt compiling, since now people making breaking changes will be responsible for fixing them.

I kept the rustfmt git history so it looks like there are thousands of commits. The important commits are https://github.com/rust-lang/rust/compare/851dee3af9404bf399c3c4ffefe5105edb3debad~..pull/82208/head. This adds about 10 MB of git history, which is not terribly much compared to the 702 MB that already exist.

- Add `src/tools/rustfmt` to `x.py check`
- Fix CRLF issues with rustfmt tests (see commit for details)
- Use `rustc_private` instead of crates.io dependencies

  This was already switched upstream and would have landed in the next submodule bump anyway. This just updates Cargo.lock for rust-lang/rust.

- Add `yansi-term` to the list of allowed dependencies.

  This is a false positive - rustc doesn't actually use it, only rustfmt, but because it's activated by the cargo feature of a dependency, tidy gets confused. It's fairly innocuous in any case, it's used for color printing.
  This would have happened in the next submodule bump.

- Remove rustfmt from the list of toolstate tools.
- Give a hard error if testing or building rustfmt fails.
-  Update log to 0.4.14

   This avoids a warning about semicolons in macros; see the commit for details.

- Don't add tools to the sysroot when they finish building.

  This is the only change that could be considered a regression - this avoids a "colliding StableCrateId" error due to a bug in resolve (#56935). The regression is that this rebuilds dependencies more often than strictly necessary. See the commit for details.

Fixes #85226 (permanently). Closes #82385. Helps with #70651. Helps with #80639.

.gitattributes

@@ -16,3 +16,11 @@ config.toml.example linguist-language=TOML
 *.ico binary
 *.woff binary
 *.woff2 binary
+
+# Needed as part of converting rustfmt to a subtree, can hopefully be removed later.
+src/tools/rustfmt/tests/source/issue-3494/crlf.rs -text
+src/tools/rustfmt/tests/source/comment_crlf_newline.rs -text
+src/tools/rustfmt/tests/source/configs/enum_discrim_align_threshold/40.rs -text
+src/tools/rustfmt/tests/target/issue-3494/crlf.rs -text
+src/tools/rustfmt/tests/target/comment_crlf_newline.rs -text
+src/tools/rustfmt/tests/target/configs/enum_discrim_align_threshold/40.rs -text

.gitmodules

@@ -16,9 +16,6 @@
 [submodule "src/tools/rls"]
 	path = src/tools/rls
 	url = https://github.com/rust-lang/rls.git
-[submodule "src/tools/rustfmt"]
-	path = src/tools/rustfmt
-	url = https://github.com/rust-lang/rustfmt.git
 [submodule "src/tools/miri"]
 	path = src/tools/miri
 	url = https://github.com/rust-lang/miri.git

Cargo.lock

@@ -1985,11 +1985,11 @@ dependencies = [
 
 [[package]]
 name = "log"
-version = "0.4.11"
+version = "0.4.14"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "4fabed175da42fed1fa0746b0ea71f412aa9d35e76e95e59b192c64b9dc2bf8b"
+checksum = "51b9bbe6c47d51fc3e1a9b945965946b4c44142ab8792c50835a980d362c2710"
 dependencies = [
- "cfg-if 0.1.10",
+ "cfg-if 1.0.0",
 ]
 
 [[package]]

src/bootstrap/builder.rs

@@ -379,6 +379,7 @@ impl<'a> Builder<'a> {
                 check::Clippy,
                 check::Miri,
                 check::Rls,
+                check::Rustfmt,
                 check::Bootstrap
             ),
             Kind::Test => describe!(

src/bootstrap/check.rs

@@ -342,10 +342,6 @@ macro_rules! tool_check_step {
                     true,
                 );
 
-                let libdir = builder.sysroot_libdir(compiler, target);
-                let hostdir = builder.sysroot_libdir(compiler, compiler.host);
-                add_to_sysroot(&builder, &libdir, &hostdir, &stamp(builder, compiler, target));
-
                 /// Cargo's output path in a given stage, compiled by a particular
                 /// compiler for the specified target.
                 fn stamp(
@@ -363,13 +359,14 @@ macro_rules! tool_check_step {
 }
 
 tool_check_step!(Rustdoc, "src/tools/rustdoc", "src/librustdoc", SourceType::InTree);
-// Clippy is a hybrid. It is an external tool, but uses a git subtree instead
+// Clippy and Rustfmt are hybrids. They are external tools, but use a git subtree instead
 // of a submodule. Since the SourceType only drives the deny-warnings
 // behavior, treat it as in-tree so that any new warnings in clippy will be
 // rejected.
 tool_check_step!(Clippy, "src/tools/clippy", SourceType::InTree);
 tool_check_step!(Miri, "src/tools/miri", SourceType::Submodule);
 tool_check_step!(Rls, "src/tools/rls", SourceType::Submodule);
+tool_check_step!(Rustfmt, "src/tools/rustfmt", SourceType::InTree);
 
 tool_check_step!(Bootstrap, "src/bootstrap", SourceType::InTree, false);
 

src/bootstrap/test.rs

@@ -319,15 +319,9 @@ impl Step for Rustfmt {
         let host = self.host;
         let compiler = builder.compiler(stage, host);
 
-        let build_result = builder.ensure(tool::Rustfmt {
-            compiler,
-            target: self.host,
-            extra_features: Vec::new(),
-        });
-        if build_result.is_none() {
-            eprintln!("failed to test rustfmt: could not build");
-            return;
-        }
+        builder
+            .ensure(tool::Rustfmt { compiler, target: self.host, extra_features: Vec::new() })
+            .expect("in-tree tool");
 
         let mut cargo = tool::prepare_tool_cargo(
             builder,
@@ -346,9 +340,7 @@ impl Step for Rustfmt {
 
         cargo.add_rustc_lib_path(builder, compiler);
 
-        if try_run(builder, &mut cargo.into()) {
-            builder.save_toolstate("rustfmt", ToolState::TestPass);
-        }
+        builder.run(&mut cargo.into());
     }
 }
 

src/bootstrap/tool.rs

@@ -726,7 +726,7 @@ macro_rules! tool_extended {
 // Note: tools need to be also added to `Builder::get_step_descriptions` in `builder.rs`
 // to make `./x.py build <tool>` work.
 tool_extended!((self, builder),
-    Cargofmt, rustfmt, "src/tools/rustfmt", "cargo-fmt", stable=true, {};
+    Cargofmt, rustfmt, "src/tools/rustfmt", "cargo-fmt", stable=true, in_tree=true, {};
     CargoClippy, clippy, "src/tools/clippy", "cargo-clippy", stable=true, in_tree=true, {};
     Clippy, clippy, "src/tools/clippy", "clippy-driver", stable=true, in_tree=true, {};
     Miri, miri, "src/tools/miri", "miri", stable=false, {};
@@ -740,7 +740,7 @@ tool_extended!((self, builder),
         self.extra_features.push("clippy".to_owned());
     };
     RustDemangler, rust_demangler, "src/tools/rust-demangler", "rust-demangler", stable=false, in_tree=true, {};
-    Rustfmt, rustfmt, "src/tools/rustfmt", "rustfmt", stable=true, {};
+    Rustfmt, rustfmt, "src/tools/rustfmt", "rustfmt", stable=true, in_tree=true, {};
     RustAnalyzer, rust_analyzer, "src/tools/rust-analyzer/crates/rust-analyzer", "rust-analyzer", stable=false, {};
 );
 

src/bootstrap/toolstate.rs

@@ -77,7 +77,6 @@ static STABLE_TOOLS: &[(&str, &str)] = &[
     ("rust-by-example", "src/doc/rust-by-example"),
     ("edition-guide", "src/doc/edition-guide"),
     ("rls", "src/tools/rls"),
-    ("rustfmt", "src/tools/rustfmt"),
 ];
 
 // These tools are permitted to not build on the beta/stable channels.
@@ -278,10 +277,9 @@ impl Builder<'_> {
         if self.config.dry_run {
             return;
         }
-        // Toolstate isn't tracked for clippy, but since most tools do, we avoid
-        // checking in all the places we could save toolstate and just do so
-        // here.
-        if tool == "clippy-driver" {
+        // Toolstate isn't tracked for clippy or rustfmt, but since most tools do, we avoid checking
+        // in all the places we could save toolstate and just do so here.
+        if tool == "clippy-driver" || tool == "rustfmt" {
             return;
         }
         if let Some(ref path) = self.config.save_toolstates {

src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh

@@ -15,7 +15,6 @@ python3 "$X_PY" test --stage 2 --no-fail-fast \
     src/doc/embedded-book \
     src/doc/edition-guide \
     src/tools/rls \
-    src/tools/rustfmt \
     src/tools/miri \
 
 set -e
@@ -24,3 +23,4 @@ set -e
 cat /tmp/toolstate/toolstates.json
 python3 "$X_PY" test --stage 2 check-tools
 python3 "$X_PY" test --stage 2 src/tools/clippy
+python3 "$X_PY" test --stage 2 src/tools/rustfmt

src/ci/scripts/should-skip-this.sh

@@ -10,15 +10,23 @@ source "$(cd "$(dirname "$0")" && pwd)/../shared.sh"
 
 if [[ -z "${CI_ONLY_WHEN_SUBMODULES_CHANGED+x}" ]]; then
     echo "Executing the job since there is no skip rule in effect"
-elif git diff HEAD^ | grep --quiet "^index .* 160000"; then
+    exit 0
+fi
+
+git fetch "https://github.com/$GITHUB_REPOSITORY" "$GITHUB_BASE_REF"
+BASE_COMMIT="$(git merge-base FETCH_HEAD HEAD)"
+
+echo "Searching for toolstate changes between $BASE_COMMIT and $(git rev-parse HEAD)"
+
+if git diff "$BASE_COMMIT" | grep --quiet "^index .* 160000"; then
     # Submodules pseudo-files inside git have the 160000 permissions, so when
     # those files are present in the diff a submodule was updated.
     echo "Executing the job since submodules are updated"
-elif git diff --name-only HEAD^ | grep --quiet src/tools/clippy; then
+elif ! git diff --quiet "$BASE_COMMIT" -- src/tools/clippy src/tools/rustfmt; then
     # There is not an easy blanket search for subtrees. For now, manually list
-    # clippy.
-    echo "Executing the job since clippy subtree was updated"
+    # the subtrees.
+    echo "Executing the job since clippy or rustfmt subtree was updated"
 else
-    echo "Not executing this job since no submodules were updated"
+    echo "Not executing this job since no submodules nor subtrees were updated"
     ciCommandSetEnv SKIP_JOB 1
 fi

src/tools/rustfmt/.editorconfig

@@ -0,0 +1,26 @@
+root = true
+
+[*]
+charset = utf-8
+end_of_line = lf
+indent_size = 2
+indent_style = space
+trim_trailing_whitespace = true
+insert_final_newline = true
+
+[*.md]
+trim_trailing_whitespace = false
+
+[*.rs]
+indent_size = 4
+
+[tests/**/*.rs]
+charset = utf-8
+end_of_line = unset
+indent_size = unset
+indent_style = unset
+trim_trailing_whitespace = unset
+insert_final_newline = unset
+
+[appveyor.yml]
+end_of_line = unset

src/tools/rustfmt/.gitattributes

@@ -0,0 +1,7 @@
+* text=auto eol=lf
+tests/source/issue-3494/crlf.rs -text
+tests/source/comment_crlf_newline.rs -text
+tests/source/configs/enum_discrim_align_threshold/40.rs -text
+tests/target/issue-3494/crlf.rs -text
+tests/target/comment_crlf_newline.rs -text
+tests/target/configs/enum_discrim_align_threshold/40.rs -text

src/tools/rustfmt/.github/workflows/integration.yml

@@ -0,0 +1,85 @@
+name: integration
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+
+jobs:
+  integration-tests:
+    runs-on: ubuntu-latest
+    name: ${{ matrix.integration }}
+    strategy:
+      # https://help.github.com/en/actions/getting-started-with-github-actions/about-github-actions#usage-limits
+      # There's a limit of 60 concurrent jobs across all repos in the rust-lang organization.
+      # In order to prevent overusing too much of that 60 limit, we throttle the
+      # number of rustfmt jobs that will run concurrently.
+      max-parallel: 4
+      fail-fast: false
+      matrix:
+        integration: [
+          bitflags,
+          error-chain,
+          log,
+          mdbook,
+          packed_simd,
+          rust-semverver,
+          tempdir,
+          futures-rs,
+          rust-clippy,
+          failure,
+        ]
+        include:
+          # Allowed Failures
+          # Actions doesn't yet support explicitly marking matrix legs as allowed failures
+          # https://github.community/t5/GitHub-Actions/continue-on-error-allow-failure-UI-indication/td-p/37033
+          # https://github.community/t5/GitHub-Actions/Why-a-matrix-step-will-be-canceled-if-another-one-failed/td-p/30920
+          # Instead, leverage `continue-on-error`
+          # https://help.github.com/en/actions/automating-your-workflow-with-github-actions/workflow-syntax-for-github-actions#jobsjob_idstepscontinue-on-error
+          #
+          # Failing due to breaking changes in rustfmt 2.0 where empty
+          # match blocks have trailing commas removed
+          # https://github.com/rust-lang/rustfmt/pull/4226
+          - integration: chalk
+            allow-failure: true
+          - integration: crater
+            allow-failure: true
+          - integration: glob
+            allow-failure: true
+          - integration: stdsimd
+            allow-failure: true
+          # Using old rustfmt configuration option
+          - integration: rand
+            allow-failure: true
+          # Keep this as an allowed failure as it's fragile to breaking changes of rustc.
+          - integration: rust-clippy
+            allow-failure: true
+          # Using old rustfmt configuration option
+          - integration: packed_simd
+            allow-failure: true
+          # calebcartwright (2019-12-24)
+          # Keeping this as an allowed failure since it was flagged as such in the TravisCI config, even though
+          # it appears to have been passing for quite some time.
+          # Original comment was: temporal build failure due to breaking changes in the nightly compiler
+          - integration: rust-semverver
+            allow-failure: true
+          # Can be moved back to include section after https://github.com/rust-lang-nursery/failure/pull/298 is merged
+          - integration: failure
+            allow-failure: true
+
+    steps:
+    - name: checkout
+      uses: actions/checkout@v2
+
+      # Run build
+    - name: install rustup
+      run: |
+        curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs > rustup-init.sh
+        sh rustup-init.sh -y --default-toolchain none
+
+    - name: run integration tests
+      env:
+        INTEGRATION: ${{ matrix.integration }}
+        TARGET: x86_64-unknown-linux-gnu
+      run: ./ci/integration.sh
+      continue-on-error: ${{ matrix.allow-failure == true }}

src/tools/rustfmt/.github/workflows/linux.yml

@@ -0,0 +1,42 @@
+name: linux
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    name: (${{ matrix.target }}, nightly)
+    strategy:
+      # https://help.github.com/en/actions/getting-started-with-github-actions/about-github-actions#usage-limits
+      # There's a limit of 60 concurrent jobs across all repos in the rust-lang organization.
+      # In order to prevent overusing too much of that 60 limit, we throttle the
+      # number of rustfmt jobs that will run concurrently.
+      max-parallel: 1
+      fail-fast: false
+      matrix:
+        target: [
+          x86_64-unknown-linux-gnu,
+        ]
+
+    steps:
+    - name: checkout
+      uses: actions/checkout@v2
+
+      # Run build
+    - name: install rustup
+      run: |
+        curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs > rustup-init.sh
+        sh rustup-init.sh -y --default-toolchain none
+        rustup target add ${{ matrix.target }}
+
+    - name: build
+      run: |
+        rustc -Vv
+        cargo -V
+        cargo build
+
+    - name: test
+      run: cargo test

src/tools/rustfmt/.github/workflows/mac.yml

@@ -0,0 +1,39 @@
+name: mac
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+
+jobs:
+  test:
+    # https://help.github.com/en/actions/automating-your-workflow-with-github-actions/virtual-environments-for-github-hosted-runners#supported-runners-and-hardware-resources
+    # macOS Catalina 10.15
+    runs-on: macos-latest
+    name: (${{ matrix.target }}, nightly)
+    strategy:
+      fail-fast: false
+      matrix:
+        target: [
+          x86_64-apple-darwin,
+        ]
+
+    steps:
+    - name: checkout
+      uses: actions/checkout@v2
+
+      # Run build
+    - name: install rustup
+      run: |
+        curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs > rustup-init.sh
+        sh rustup-init.sh -y --default-toolchain none
+        rustup target add ${{ matrix.target }}
+
+    - name: build
+      run: |
+        rustc -Vv
+        cargo -V
+        cargo build
+
+    - name: test
+      run: cargo test