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

Enable incremental rustfmt adoption #65939

Merged
merged 7 commits into from
Dec 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ dependencies = [
"cmake",
"filetime",
"getopts",
"ignore",
"lazy_static 1.3.0",
"libc",
"num_cpus",
Expand Down Expand Up @@ -1525,9 +1526,9 @@ checksum = "c3360c7b59e5ffa2653671fb74b4741a5d343c03f331c0a4aeda42b5c2b0ec7d"

[[package]]
name = "ignore"
version = "0.4.7"
version = "0.4.10"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8dc57fa12805f367736a38541ac1a9fc6a52812a0ca959b1d4d4b640a89eb002"
checksum = "0ec16832258409d571aaef8273f3c3cc5b060d784e159d1a0f3b0017308f84a7"
dependencies = [
"crossbeam-channel",
"globset",
Expand Down
83 changes: 83 additions & 0 deletions rustfmt.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,86 @@
# be picked up automatically).
version = "Two"
use_small_heuristics = "Max"

# by default we ignore everything in the repository
# tidy only checks files which are not ignored, each entry follows gitignore style
ignore = [
# remove directories below, or opt out their subdirectories, as they are formatted
"src/bootstrap/",
"src/build_helper/",
"src/liballoc/",
"src/libarena/",
"src/libcore/",
"src/libfmt_macros/",
"src/libgraphviz/",
"src/libpanic_abort/",
"src/libpanic_unwind/",
"src/libproc_macro/",
"src/libprofiler_builtins/",
"src/librustc/",
"src/librustc_apfloat/",
"src/librustc_asan/",
"src/librustc_codegen_llvm/",
"src/librustc_codegen_ssa/",
"src/librustc_codegen_utils/",
"src/librustc_data_structures/",
"src/librustc_driver/",
"src/librustc_errors/",
"src/librustc_feature/",
"src/librustc_incremental/",
"src/librustc_index/",
"src/librustc_interface/",
"src/librustc_lexer/",
"src/librustc_lint/",
"src/librustc_llvm/",
"src/librustc_lsan/",
"src/librustc_macros/",
"src/librustc_metadata/",
"src/librustc_mir/",
"src/librustc_msan/",
"src/librustc_parse/",
"src/librustc_passes/",
"src/librustc_plugin/",
"src/librustc_plugin_impl/",
"src/librustc_privacy/",
"src/librustc_resolve/",
"src/librustc_save_analysis/",
"src/librustc_session/",
"src/librustc_target/",
"src/librustc_traits/",
"src/librustc_tsan/",
"src/librustc_typeck/",
"src/librustdoc/",
"src/libserialize/",
"src/libstd/",
"src/libsyntax/",
"src/libsyntax_expand/",
"src/libsyntax_ext/",
"src/libsyntax_pos/",
"src/libterm/",
"src/libtest/",
"src/libunwind/",
"src/rtstartup/",
"src/rustc/",
"src/rustllvm/",
"src/test/",
"src/tools/",
"src/etc",

# do not format submodules
"src/doc/book",
"src/doc/edition-guide",
"src/doc/embedded-book",
"src/doc/nomicon",
"src/doc/reference",
"src/doc/rust-by-example",
"src/doc/rustc-guide",
"src/llvm-project",
"src/stdarch",
"src/tools/cargo",
"src/tools/clippy",
"src/tools/miri",
"src/tools/rls",
"src/tools/rust-installer",
"src/tools/rustfmt",
]
1 change: 1 addition & 0 deletions src/bootstrap/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ serde_json = "1.0.2"
toml = "0.5"
lazy_static = "1.3.0"
time = "0.1"
ignore = "0.4.10"

[dev-dependencies]
pretty_assertions = "0.5"
45 changes: 42 additions & 3 deletions src/bootstrap/bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,7 @@ def __init__(self):
self.date = ''
self._download_url = ''
self.rustc_channel = ''
self.rustfmt_channel = ''
self.build = ''
self.build_dir = os.path.join(os.getcwd(), "build")
self.clean = False
Expand All @@ -344,6 +345,7 @@ def download_stage0(self):
"""
rustc_channel = self.rustc_channel
cargo_channel = self.cargo_channel
rustfmt_channel = self.rustfmt_channel

def support_xz():
try:
Expand Down Expand Up @@ -393,13 +395,29 @@ def support_xz():
with output(self.cargo_stamp()) as cargo_stamp:
cargo_stamp.write(self.date)

def _download_stage0_helper(self, filename, pattern, tarball_suffix):
if self.rustfmt() and self.rustfmt().startswith(self.bin_root()) and (
not os.path.exists(self.rustfmt())
or self.program_out_of_date(self.rustfmt_stamp())
):
if rustfmt_channel:
anp marked this conversation as resolved.
Show resolved Hide resolved
tarball_suffix = '.tar.xz' if support_xz() else '.tar.gz'
[channel, date] = rustfmt_channel.split('-', 1)
filename = "rustfmt-{}-{}{}".format(channel, self.build, tarball_suffix)
self._download_stage0_helper(filename, "rustfmt-preview", tarball_suffix, date)
self.fix_executable("{}/bin/rustfmt".format(self.bin_root()))
self.fix_executable("{}/bin/cargo-fmt".format(self.bin_root()))
with output(self.rustfmt_stamp()) as rustfmt_stamp:
rustfmt_stamp.write(self.date)

def _download_stage0_helper(self, filename, pattern, tarball_suffix, date=None):
if date is None:
date = self.date
cache_dst = os.path.join(self.build_dir, "cache")
rustc_cache = os.path.join(cache_dst, self.date)
rustc_cache = os.path.join(cache_dst, date)
if not os.path.exists(rustc_cache):
os.makedirs(rustc_cache)

url = "{}/dist/{}".format(self._download_url, self.date)
url = "{}/dist/{}".format(self._download_url, date)
tarball = os.path.join(rustc_cache, filename)
if not os.path.exists(tarball):
get("{}/{}".format(url, filename), tarball, verbose=self.verbose)
Expand Down Expand Up @@ -493,6 +511,16 @@ def cargo_stamp(self):
"""
return os.path.join(self.bin_root(), '.cargo-stamp')

def rustfmt_stamp(self):
"""Return the path for .rustfmt-stamp

>>> rb = RustBuild()
>>> rb.build_dir = "build"
>>> rb.rustfmt_stamp() == os.path.join("build", "stage0", ".rustfmt-stamp")
True
"""
return os.path.join(self.bin_root(), '.rustfmt-stamp')

def program_out_of_date(self, stamp_path):
"""Check if the given program stamp is out of date"""
if not os.path.exists(stamp_path) or self.clean:
Expand Down Expand Up @@ -565,6 +593,12 @@ def rustc(self):
"""Return config path for rustc"""
return self.program_config('rustc')

def rustfmt(self):
"""Return config path for rustfmt"""
if not self.rustfmt_channel:
return None
return self.program_config('rustfmt')
anp marked this conversation as resolved.
Show resolved Hide resolved

def program_config(self, program):
"""Return config path for the given program

Expand Down Expand Up @@ -868,6 +902,9 @@ def bootstrap(help_triggered):
build.rustc_channel = data['rustc']
build.cargo_channel = data['cargo']

if "rustfmt" in data:
build.rustfmt_channel = data['rustfmt']

if 'dev' in data:
build.set_dev_environment()
else:
Expand Down Expand Up @@ -895,6 +932,8 @@ def bootstrap(help_triggered):
env["RUSTC_BOOTSTRAP"] = '1'
env["CARGO"] = build.cargo()
env["RUSTC"] = build.rustc()
if build.rustfmt():
env["RUSTFMT"] = build.rustfmt()
run(args, env=env, verbose=build.verbose)


Expand Down
4 changes: 2 additions & 2 deletions src/bootstrap/bootstrap_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ def setUp(self):
os.mkdir(os.path.join(self.rust_root, "src"))
with open(os.path.join(self.rust_root, "src",
"stage0.txt"), "w") as stage0:
stage0.write("#ignore\n\ndate: 2017-06-15\nrustc: beta\ncargo: beta")
stage0.write("#ignore\n\ndate: 2017-06-15\nrustc: beta\ncargo: beta\nrustfmt: beta")

def tearDown(self):
rmtree(self.rust_root)

def test_stage0_data(self):
"""Extract data from stage0.txt"""
expected = {"date": "2017-06-15", "rustc": "beta", "cargo": "beta"}
expected = {"date": "2017-06-15", "rustc": "beta", "cargo": "beta", "rustfmt": "beta"}
data = bootstrap.stage0_data(self.rust_root)
self.assertDictEqual(data, expected)

Expand Down
5 changes: 3 additions & 2 deletions src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ pub enum Kind {
Check,
Clippy,
Fix,
Format,
Test,
Bench,
Dist,
Expand Down Expand Up @@ -353,7 +354,7 @@ impl<'a> Builder<'a> {
tool::Miri,
native::Lld
),
Kind::Check | Kind::Clippy | Kind::Fix => describe!(
Kind::Check | Kind::Clippy | Kind::Fix | Kind::Format => describe!(
check::Std,
check::Rustc,
check::Rustdoc
Expand Down Expand Up @@ -514,7 +515,7 @@ impl<'a> Builder<'a> {
Subcommand::Bench { ref paths, .. } => (Kind::Bench, &paths[..]),
Subcommand::Dist { ref paths } => (Kind::Dist, &paths[..]),
Subcommand::Install { ref paths } => (Kind::Install, &paths[..]),
Subcommand::Clean { .. } => panic!(),
Subcommand::Format { .. } | Subcommand::Clean { .. } => panic!(),
};

let builder = Builder {
Expand Down
11 changes: 9 additions & 2 deletions src/bootstrap/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

use std::collections::{HashMap, HashSet};
use std::env;
use std::ffi::OsString;
use std::fs;
use std::path::{Path, PathBuf};
use std::process;
Expand Down Expand Up @@ -149,6 +150,7 @@ pub struct Config {
// These are either the stage0 downloaded binaries or the locally installed ones.
pub initial_cargo: PathBuf,
pub initial_rustc: PathBuf,
pub initial_rustfmt: Option<PathBuf>,
pub out: PathBuf,
}

Expand Down Expand Up @@ -348,12 +350,16 @@ struct TomlTarget {
impl Config {
fn path_from_python(var_key: &str) -> PathBuf {
match env::var_os(var_key) {
// Do not trust paths from Python and normalize them slightly (#49785).
Some(var_val) => Path::new(&var_val).components().collect(),
Some(var_val) => Self::normalize_python_path(var_val),
_ => panic!("expected '{}' to be set", var_key),
}
}

/// Normalizes paths from Python slightly. We don't trust paths from Python (#49785).
fn normalize_python_path(path: OsString) -> PathBuf {
Path::new(&path).components().collect()
}

pub fn default_opts() -> Config {
let mut config = Config::default();
config.llvm_optimize = true;
Expand All @@ -380,6 +386,7 @@ impl Config {

config.initial_rustc = Config::path_from_python("RUSTC");
config.initial_cargo = Config::path_from_python("CARGO");
config.initial_rustfmt = env::var_os("RUSTFMT").map(Config::normalize_python_path);

config
}
Expand Down
26 changes: 25 additions & 1 deletion src/bootstrap/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ pub enum Subcommand {
Fix {
paths: Vec<PathBuf>,
},
Format {
check: bool,
},
Doc {
paths: Vec<PathBuf>,
},
Expand Down Expand Up @@ -102,6 +105,7 @@ Subcommands:
check Compile either the compiler or libraries, using cargo check
clippy Run clippy
fix Run cargo fix
fmt Run rustfmt
test Build and run some test suites
bench Build and run some benchmarks
doc Build documentation
Expand Down Expand Up @@ -160,6 +164,7 @@ To learn more about a subcommand, run `./x.py <subcommand> -h`"
|| (s == "check")
|| (s == "clippy")
|| (s == "fix")
|| (s == "fmt")
|| (s == "test")
|| (s == "bench")
|| (s == "doc")
Expand Down Expand Up @@ -222,6 +227,9 @@ To learn more about a subcommand, run `./x.py <subcommand> -h`"
"clean" => {
opts.optflag("", "all", "clean all build artifacts");
}
"fmt" => {
opts.optflag("", "check", "check formatting instead of applying.");
}
_ => {}
};

Expand Down Expand Up @@ -323,6 +331,17 @@ Arguments:
./x.py fix src/libcore src/libproc_macro",
);
}
"fmt" => {
subcommand_help.push_str(
"\n
Arguments:
This subcommand optionally accepts a `--check` flag which succeeds if formatting is correct and
fails if it is not. For example:

./x.py fmt
./x.py fmt --check",
);
}
"test" => {
subcommand_help.push_str(
"\n
Expand Down Expand Up @@ -388,7 +407,7 @@ Arguments:

let maybe_rules_help = Builder::get_help(&build, subcommand.as_str());
extra_help.push_str(maybe_rules_help.unwrap_or_default().as_str());
} else if subcommand.as_str() != "clean" {
} else if !(subcommand.as_str() == "clean" || subcommand.as_str() == "fmt") {
extra_help.push_str(
format!(
"Run `./x.py {} -h -v` to see a list of available paths.",
Expand Down Expand Up @@ -439,6 +458,11 @@ Arguments:
all: matches.opt_present("all"),
}
}
"fmt" => {
Subcommand::Format {
check: matches.opt_present("check"),
}
}
"dist" => Subcommand::Dist { paths },
"install" => Subcommand::Install { paths },
_ => {
Expand Down
Loading