diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index a3a8ea0fd6..96fb068dc6 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -17,6 +17,7 @@ tasks: - "//test/versioned_dylib:versioned_dylib_test" build_flags: - "--config=rustfmt" + - "--config=clippy" ubuntu2004: name: "Minimum Supported Version" bazel: "3.5.0" @@ -24,6 +25,7 @@ tasks: test_targets: *default_linux_targets build_flags: - "--config=rustfmt" + - "--config=clippy" macos: osx_targets: &osx_targets - "--" # Allows negative patterns; hack for https://github.com/bazelbuild/continuous-integration/pull/245 @@ -35,6 +37,7 @@ tasks: test_targets: *osx_targets build_flags: - "--config=rustfmt" + - "--config=clippy" rbe_ubuntu1604: test_targets: - "--" # Allows negative patterns; hack for https://github.com/bazelbuild/continuous-integration/pull/245 @@ -51,10 +54,12 @@ tasks: - "-@examples//ffi/rust_calling_c:matrix_dylib_test" build_flags: - "--config=rustfmt" + - "--config=clippy" windows: build_flags: - "--enable_runfiles" # this is not enabled by default on windows and is necessary for the cargo build scripts - "--config=rustfmt" + - "--config=clippy" windows_targets: &windows_targets - "--" # Allows negative patterns; hack for https://github.com/bazelbuild/continuous-integration/pull/245 - "..." @@ -81,6 +86,7 @@ tasks: - //... build_flags: - "--config=rustfmt" + - "--config=clippy" docs_linux: name: Docs platform: ubuntu1804 @@ -89,20 +95,11 @@ tasks: - //... run_targets: - "//:test_docs" - clippy_examples: - name: Clippy on Examples - platform: ubuntu1804 - working_directory: examples - build_flags: - - "--aspects=@rules_rust//rust:defs.bzl%rust_clippy_aspect" - - "--output_groups=clippy_checks" - build_targets: - - //... clippy_failure: name: Negative Clippy Tests platform: ubuntu1804 - shell_commands: - - ./test/clippy/clippy_failure_test.sh + run_targets: + - "//test/clippy:clippy_failure_test" rustfmt_failure: name: Negative Rustfmt Tests platform: ubuntu2004 @@ -112,6 +109,8 @@ tasks: name: Ubuntu 20.04 with Clang platform: ubuntu2004 build_flags: + - "--config=rustfmt" + - "--config=clippy" - "--repo_env=CC=clang" # TODO(hlopko): Make this work (some tests were failing) # - "--linkopt=-fuse-ld=lld" @@ -127,6 +126,9 @@ tasks: - "//..." test_targets: - "//..." + build_flags: + - "--config=rustfmt" + - "--config=clippy" crate_universe_rbe_ubuntu1604: name: Crate Universe Examples platform: rbe_ubuntu1604 @@ -139,6 +141,7 @@ tasks: - "//..." build_flags: - "--config=rustfmt" + - "--config=clippy" crate_universe_examples_macos: name: Crate Universe Examples platform: macos @@ -151,6 +154,7 @@ tasks: - "//..." build_flags: - "--config=rustfmt" + - "--config=clippy" crate_universe_examples_windows: name: Crate Universe Examples platform: windows @@ -160,6 +164,7 @@ tasks: build_flags: - "--enable_runfiles" # this is not enabled by default on windows and is necessary for the cargo build scripts - "--config=rustfmt" + - "--config=clippy" crate_universe_windows_targets: &crate_universe_windows_targets - "//..." # TODO: There are windows specific build issues in the generated diff --git a/.bazelrc b/.bazelrc index db9695ba08..c54ffa8be3 100644 --- a/.bazelrc +++ b/.bazelrc @@ -4,3 +4,7 @@ # Enable rustfmt for all targets in the workspace build:rustfmt --aspects=//rust:defs.bzl%rustfmt_aspect build:rustfmt --output_groups=+rustfmt_checks + +# Enable clippy for all targets in the workspace +build:clippy --aspects=//rust:defs.bzl%rust_clippy_aspect +build:clippy --output_groups=+clippy_checks diff --git a/cargo/cargo_build_script_runner/bin.rs b/cargo/cargo_build_script_runner/bin.rs index 4eda6b41b0..b42683e300 100644 --- a/cargo/cargo_build_script_runner/bin.rs +++ b/cargo/cargo_build_script_runner/bin.rs @@ -50,10 +50,8 @@ fn run_buildrs() -> Result<(), String> { let out_dir_abs = exec_root.join(&out_dir); // For some reason Google's RBE does not create the output directory, force create it. - create_dir_all(&out_dir_abs).expect(&format!( - "Failed to make output directory: {:?}", - out_dir_abs - )); + create_dir_all(&out_dir_abs) + .unwrap_or_else(|_| panic!("Failed to make output directory: {:?}", out_dir_abs)); let target_env_vars = get_target_env_vars(&rustc_env).expect("Error getting target env vars from rustc"); @@ -119,47 +117,52 @@ fn run_buildrs() -> Result<(), String> { } } - let (buildrs_outputs, process_output) = - BuildScriptOutput::from_command(&mut command).map_err(|process_output| { - format!( - "Build script process failed{}\n--stdout:\n{}\n--stderr:\n{}", - if let Some(exit_code) = process_output.status.code() { - format!(" with exit code {}", exit_code) - } else { - String::new() - }, - String::from_utf8(process_output.stdout) - .expect("Failed to parse stdout of child process"), - String::from_utf8(process_output.stderr) - .expect("Failed to parse stdout of child process"), - ) - })?; + let (buildrs_outputs, process_output) = BuildScriptOutput::outputs_from_command(&mut command) + .map_err(|process_output| { + format!( + "Build script process failed{}\n--stdout:\n{}\n--stderr:\n{}", + if let Some(exit_code) = process_output.status.code() { + format!(" with exit code {}", exit_code) + } else { + String::new() + }, + String::from_utf8(process_output.stdout) + .expect("Failed to parse stdout of child process"), + String::from_utf8(process_output.stderr) + .expect("Failed to parse stdout of child process"), + ) + })?; write( &env_file, - BuildScriptOutput::to_env(&buildrs_outputs, &exec_root.to_string_lossy()).as_bytes(), + BuildScriptOutput::outputs_to_env(&buildrs_outputs, &exec_root.to_string_lossy()) + .as_bytes(), ) - .expect(&format!("Unable to write file {:?}", env_file)); + .unwrap_or_else(|_| panic!("Unable to write file {:?}", env_file)); write( &output_dep_env_path, - BuildScriptOutput::to_dep_env(&buildrs_outputs, &crate_links, &exec_root.to_string_lossy()) - .as_bytes(), + BuildScriptOutput::outputs_to_dep_env( + &buildrs_outputs, + &crate_links, + &exec_root.to_string_lossy(), + ) + .as_bytes(), ) - .expect(&format!("Unable to write file {:?}", output_dep_env_path)); + .unwrap_or_else(|_| panic!("Unable to write file {:?}", output_dep_env_path)); write(&stdout_path, process_output.stdout) - .expect(&format!("Unable to write file {:?}", stdout_path)); + .unwrap_or_else(|_| panic!("Unable to write file {:?}", stdout_path)); write(&stderr_path, process_output.stderr) - .expect(&format!("Unable to write file {:?}", stderr_path)); + .unwrap_or_else(|_| panic!("Unable to write file {:?}", stderr_path)); let CompileAndLinkFlags { compile_flags, link_flags, - } = BuildScriptOutput::to_flags(&buildrs_outputs, &exec_root.to_string_lossy()); + } = BuildScriptOutput::outputs_to_flags(&buildrs_outputs, &exec_root.to_string_lossy()); write(&compile_flags_file, compile_flags.as_bytes()) - .expect(&format!("Unable to write file {:?}", compile_flags_file)); + .unwrap_or_else(|_| panic!("Unable to write file {:?}", compile_flags_file)); write(&link_flags_file, link_flags.as_bytes()) - .expect(&format!("Unable to write file {:?}", link_flags_file)); + .unwrap_or_else(|_| panic!("Unable to write file {:?}", link_flags_file)); Ok(()) } @@ -243,7 +246,7 @@ fn get_target_env_vars>(rustc: &P) -> Result= 2 { values .entry(key) - .or_insert(vec![]) + .or_insert_with(Vec::new) .push(value[1..(value.len() - 1)].to_owned()); } } diff --git a/cargo/cargo_build_script_runner/lib.rs b/cargo/cargo_build_script_runner/lib.rs index 273c0f4578..e2d1b7033d 100644 --- a/cargo/cargo_build_script_runner/lib.rs +++ b/cargo/cargo_build_script_runner/lib.rs @@ -94,7 +94,7 @@ impl BuildScriptOutput { } /// Converts a [BufReader] into a vector of [BuildScriptOutput] enums. - fn from_reader(mut reader: BufReader) -> Vec { + fn outputs_from_reader(mut reader: BufReader) -> Vec { let mut result = Vec::::new(); let mut line = String::new(); while reader.read_line(&mut line).expect("Cannot read line") != 0 { @@ -107,11 +107,13 @@ impl BuildScriptOutput { } /// Take a [Command], execute it and converts its input into a vector of [BuildScriptOutput] - pub fn from_command(cmd: &mut Command) -> Result<(Vec, Output), Output> { + pub fn outputs_from_command( + cmd: &mut Command, + ) -> Result<(Vec, Output), Output> { let child_output = cmd.output().expect("Unable to start binary"); if child_output.status.success() { let reader = BufReader::new(child_output.stdout.as_slice()); - let output = Self::from_reader(reader); + let output = Self::outputs_from_reader(reader); Ok((output, child_output)) } else { Err(child_output) @@ -119,8 +121,9 @@ impl BuildScriptOutput { } /// Convert a vector of [BuildScriptOutput] into a list of environment variables. - pub fn to_env(v: &Vec, exec_root: &str) -> String { - v.iter() + pub fn outputs_to_env(outputs: &[BuildScriptOutput], exec_root: &str) -> String { + outputs + .iter() .filter_map(|x| { if let BuildScriptOutput::Env(env) = x { Some(Self::escape_for_serializing(Self::redact_exec_root( @@ -135,9 +138,14 @@ impl BuildScriptOutput { } /// Convert a vector of [BuildScriptOutput] into a list of dependencies environment variables. - pub fn to_dep_env(v: &Vec, crate_links: &str, exec_root: &str) -> String { + pub fn outputs_to_dep_env( + outputs: &[BuildScriptOutput], + crate_links: &str, + exec_root: &str, + ) -> String { let prefix = format!("DEP_{}_", crate_links.replace("-", "_").to_uppercase()); - v.iter() + outputs + .iter() .filter_map(|x| { if let BuildScriptOutput::DepEnv(env) = x { Some(format!( @@ -154,11 +162,11 @@ impl BuildScriptOutput { } /// Convert a vector of [BuildScriptOutput] into a flagfile. - pub fn to_flags(v: &Vec, exec_root: &str) -> CompileAndLinkFlags { + pub fn outputs_to_flags(outputs: &[BuildScriptOutput], exec_root: &str) -> CompileAndLinkFlags { let mut compile_flags = Vec::new(); let mut link_flags = Vec::new(); - for flag in v { + for flag in outputs { match flag { BuildScriptOutput::Cfg(e) => compile_flags.push(format!("--cfg={}", e)), BuildScriptOutput::Flags(e) => compile_flags.push(e.to_owned()), @@ -214,7 +222,7 @@ cargo:rustc-env=SOME_PATH=/some/absolute/path/beep ", ); let reader = BufReader::new(buff); - let result = BuildScriptOutput::from_reader(reader); + let result = BuildScriptOutput::outputs_from_reader(reader); assert_eq!(result.len(), 10); assert_eq!(result[0], BuildScriptOutput::LinkLib("sdfsdf".to_owned())); assert_eq!(result[1], BuildScriptOutput::Env("FOO=BAR".to_owned())); @@ -242,15 +250,15 @@ cargo:rustc-env=SOME_PATH=/some/absolute/path/beep ); assert_eq!( - BuildScriptOutput::to_dep_env(&result, "ssh2", "/some/absolute/path"), + BuildScriptOutput::outputs_to_dep_env(&result, "ssh2", "/some/absolute/path"), "DEP_SSH2_VERSION=123\nDEP_SSH2_VERSION_NUMBER=1010107f\nDEP_SSH2_INCLUDE_PATH=${pwd}/include".to_owned() ); assert_eq!( - BuildScriptOutput::to_env(&result, "/some/absolute/path"), + BuildScriptOutput::outputs_to_env(&result, "/some/absolute/path"), "FOO=BAR\nBAR=FOO\nSOME_PATH=${pwd}/beep".to_owned() ); assert_eq!( - BuildScriptOutput::to_flags(&result, "/some/absolute/path"), + BuildScriptOutput::outputs_to_flags(&result, "/some/absolute/path"), CompileAndLinkFlags { // -Lblah was output as a rustc-flags, so even though it probably _should_ be a link // flag, we don't treat it like one. diff --git a/crate_universe/README.md b/crate_universe/README.md index 91efffc5b0..f3bc54caa2 100644 --- a/crate_universe/README.md +++ b/crate_universe/README.md @@ -1,11 +1,5 @@ # Crate Universe -## Experimental! - -**Note**: `crate_universe` is experimental, and may have breaking API changes at any time. These instructions may also change without notice. - -## What is it? - Crate Universe is akin to a modified version of cargo-raze that can be run as part of a Bazel build. Instead of generating BUILD files in advance with cargo-raze, the BUILD files can be created automatically at build time. It can @@ -13,102 +7,6 @@ expose crates gathered from Cargo.toml files like cargo-raze does, and also supports declaring crates directly in BUILD files, for cases where compatibility with Cargo is not required. -## Workspace installation - -To avoid having to build a Rust binary, pre-made releases are made -available. - -1. Find the most up-to-date `crate_universe` release at https://github.com/bazelbuild/rules_rust/releases. -2. Copy and paste the text into a file like `crate_universe_defaults.bzl` in your repo. -3. Add something like the following to your `WORKSPACE.bazel` file: - -```python -load("//3rdparty/rules_rust:crate_universe_defaults.bzl", "DEFAULT_URL_TEMPLATE", "DEFAULT_SHA256_CHECKSUMS") - -load("@rules_rust//crate_universe:defs.bzl", "crate", "crate_universe") - -crate_universe( - name = "crates", - cargo_toml_files = [ - "//some_crate:Cargo.toml", - "//some_other:Cargo.toml", - ], - resolver_download_url_template = DEFAULT_URL_TEMPLATE, - resolver_sha256s = DEFAULT_SHA256_CHECKSUMS, - # leave unset for default multi-platform support - supported_targets = [ - "x86_64-apple-darwin", - "x86_64-unknown-linux-gnu", - ], - # [package.metadata.raze.xxx] lines in Cargo.toml files are ignored; - # the overrides need to be declared in the repo rule instead. - overrides = { - "example-sys": crate.override( - extra_build_script_env_vars = {"PATH": "/usr/bin"}, - ), - }, - # to use a lockfile, uncomment the following line, - # create an empty file in the location, and then build - # with REPIN=1 bazel build ... - #lockfile = "//:crate_universe.lock", -) - -load("@crates//:defs.bzl", "pinned_rust_install") - -pinned_rust_install() -``` - -In the above example, two separate Cargo.toml files have been -provided. Multiple Cargo.toml can be provided, and crate_universe -will combine them together to ensure each crate uses a common version. - -This is similar to a Cargo workspace, and can be used with an existing -workspace. But some things to note: - -- the top level workspace Cargo.toml, if one exists, should not be - included in cargo_toml_files, as it does not list any dependencies by itself -- presently any existing Cargo.lock file is ignored, as crate_universe does its - own resolution and locking. You can uncomment the lockfile line above to - enable a separate lockfile for crate_universe; if left disabled, versions will - float, like if you were to remove the Cargo.lock file each time you updated a - Cargo.toml file in a Cargo workspace. Currently the lockfile is in a custom - format; in the future, the Cargo.lock file may be used instead. - -## Build file usage - -With the crates declared in the workspace, they can now be referenced in BUILD -files. For example: - -```python -load("@crates//:defs.bzl", "build_crates_from", "crates_from") - -cargo_build_script( - name = "build", - srcs = ["build.rs"], - deps = build_crates_from("//some_crate:Cargo.toml"), -) - -rust_library( - name = "some_crate", - srcs = [ - "lib.rs", - ], - deps = crates_from("//some_crate:Cargo.toml") + [":build"] -) -``` - -If you prefer, you can also list out each crate individually, eg: - -```python -load("@crates//:defs.bzl", "crate") - -rust_library( - name = "some_crate", - srcs = [ - "lib.rs", - ], - deps = [crate("serde"), crate("log"), ":build"] -) -``` +**Note**: `crate_universe` is experimental, and may have breaking API changes at any time. These instructions may also change without notice. -See [some more examples](../examples/crate_universe) and the [API docs](https://bazelbuild.github.io/rules_rust/crate_universe.html) for more info. +More information can be found in the [rules_rust documentation](https://bazelbuild.github.io/rules_rust/crate_universe.html). diff --git a/docs/BUILD.bazel b/docs/BUILD.bazel index 7ae3166eb8..51e704d498 100644 --- a/docs/BUILD.bazel +++ b/docs/BUILD.bazel @@ -39,6 +39,7 @@ PAGES = dict([ ), page( name = "crate_universe", + header_template = ":crate_universe.vm", symbols = [ "crate_universe", "crate", diff --git a/docs/crate_universe.md b/docs/crate_universe.md index e1e5a203f5..cb636f8ebe 100644 --- a/docs/crate_universe.md +++ b/docs/crate_universe.md @@ -4,6 +4,121 @@ * [crate_universe](#crate_universe) * [crate](#crate) + +## Experimental! + +**Note**: `crate_universe` is experimental, and may have breaking API changes at any time. These instructions may also change without notice. + +## What is it? + +Crate Universe is akin to a modified version of cargo-raze that can be run as +part of a Bazel build. Instead of generating BUILD files in advance with +cargo-raze, the BUILD files can be created automatically at build time. It can +expose crates gathered from Cargo.toml files like cargo-raze does, and also +supports declaring crates directly in BUILD files, for cases where compatibility +with Cargo is not required. + +## Workspace installation + +To avoid having to build a Rust binary, pre-made releases are made +available. + +1. Find the most up-to-date `crate_universe` release at https://github.com/bazelbuild/rules_rust/releases. +2. Copy and paste the text into a file like `crate_universe_defaults.bzl` in your repo. +3. Add something like the following to your `WORKSPACE.bazel` file: + +```python +load("//3rdparty/rules_rust:crate_universe_defaults.bzl", "DEFAULT_URL_TEMPLATE", "DEFAULT_SHA256_CHECKSUMS") + +load("@rules_rust//crate_universe:defs.bzl", "crate", "crate_universe") + +crate_universe( + name = "crates", + cargo_toml_files = [ + "//some_crate:Cargo.toml", + "//some_other:Cargo.toml", + ], + resolver_download_url_template = DEFAULT_URL_TEMPLATE, + resolver_sha256s = DEFAULT_SHA256_CHECKSUMS, + # leave unset for default multi-platform support + supported_targets = [ + "x86_64-apple-darwin", + "x86_64-unknown-linux-gnu", + ], + # [package.metadata.raze.xxx] lines in Cargo.toml files are ignored; + # the overrides need to be declared in the repo rule instead. + overrides = { + "example-sys": crate.override( + extra_build_script_env_vars = {"PATH": "/usr/bin"}, + ), + }, + # to use a lockfile, uncomment the following line, + # create an empty file in the location, and then build + # with REPIN=1 bazel build ... + #lockfile = "//:crate_universe.lock", +) + +load("@crates//:defs.bzl", "pinned_rust_install") + +pinned_rust_install() +``` + +In the above example, two separate Cargo.toml files have been +provided. Multiple Cargo.toml can be provided, and crate_universe +will combine them together to ensure each crate uses a common version. + +This is similar to a Cargo workspace, and can be used with an existing +workspace. But some things to note: + +- the top level workspace Cargo.toml, if one exists, should not be + included in cargo_toml_files, as it does not list any dependencies by itself +- presently any existing Cargo.lock file is ignored, as crate_universe does its + own resolution and locking. You can uncomment the lockfile line above to + enable a separate lockfile for crate_universe; if left disabled, versions will + float, like if you were to remove the Cargo.lock file each time you updated a + Cargo.toml file in a Cargo workspace. Currently the lockfile is in a custom + format; in the future, the Cargo.lock file may be used instead. + +## Build file usage + +With the crates declared in the workspace, they can now be referenced in BUILD +files. For example: + +```python +load("@crates//:defs.bzl", "build_crates_from", "crates_from") + +cargo_build_script( + name = "build", + srcs = ["build.rs"], + deps = build_crates_from("//some_crate:Cargo.toml"), +) + +rust_library( + name = "some_crate", + srcs = [ + "lib.rs", + ], + deps = crates_from("//some_crate:Cargo.toml") + [":build"] +) +``` + +If you prefer, you can also list out each crate individually, eg: + +```python +load("@crates//:defs.bzl", "crate") + +rust_library( + name = "some_crate", + srcs = [ + "lib.rs", + ], + deps = [crate("serde"), crate("log"), ":build"] +) +``` + +See [some more examples](../examples/crate_universe) and the documentation below for more info. + + ## crate_universe diff --git a/docs/crate_universe.vm b/docs/crate_universe.vm new file mode 100644 index 0000000000..b1be6aa120 --- /dev/null +++ b/docs/crate_universe.vm @@ -0,0 +1,114 @@ +#[[ +## Experimental! + +**Note**: `crate_universe` is experimental, and may have breaking API changes at any time. These instructions may also change without notice. + +## What is it? + +Crate Universe is akin to a modified version of cargo-raze that can be run as +part of a Bazel build. Instead of generating BUILD files in advance with +cargo-raze, the BUILD files can be created automatically at build time. It can +expose crates gathered from Cargo.toml files like cargo-raze does, and also +supports declaring crates directly in BUILD files, for cases where compatibility +with Cargo is not required. + +## Workspace installation + +To avoid having to build a Rust binary, pre-made releases are made +available. + +1. Find the most up-to-date `crate_universe` release at https://github.com/bazelbuild/rules_rust/releases. +2. Copy and paste the text into a file like `crate_universe_defaults.bzl` in your repo. +3. Add something like the following to your `WORKSPACE.bazel` file: + +```python +load("//3rdparty/rules_rust:crate_universe_defaults.bzl", "DEFAULT_URL_TEMPLATE", "DEFAULT_SHA256_CHECKSUMS") + +load("@rules_rust//crate_universe:defs.bzl", "crate", "crate_universe") + +crate_universe( + name = "crates", + cargo_toml_files = [ + "//some_crate:Cargo.toml", + "//some_other:Cargo.toml", + ], + resolver_download_url_template = DEFAULT_URL_TEMPLATE, + resolver_sha256s = DEFAULT_SHA256_CHECKSUMS, + # leave unset for default multi-platform support + supported_targets = [ + "x86_64-apple-darwin", + "x86_64-unknown-linux-gnu", + ], + # [package.metadata.raze.xxx] lines in Cargo.toml files are ignored; + # the overrides need to be declared in the repo rule instead. + overrides = { + "example-sys": crate.override( + extra_build_script_env_vars = {"PATH": "/usr/bin"}, + ), + }, + # to use a lockfile, uncomment the following line, + # create an empty file in the location, and then build + # with REPIN=1 bazel build ... + #lockfile = "//:crate_universe.lock", +) + +load("@crates//:defs.bzl", "pinned_rust_install") + +pinned_rust_install() +``` + +In the above example, two separate Cargo.toml files have been +provided. Multiple Cargo.toml can be provided, and crate_universe +will combine them together to ensure each crate uses a common version. + +This is similar to a Cargo workspace, and can be used with an existing +workspace. But some things to note: + +- the top level workspace Cargo.toml, if one exists, should not be + included in cargo_toml_files, as it does not list any dependencies by itself +- presently any existing Cargo.lock file is ignored, as crate_universe does its + own resolution and locking. You can uncomment the lockfile line above to + enable a separate lockfile for crate_universe; if left disabled, versions will + float, like if you were to remove the Cargo.lock file each time you updated a + Cargo.toml file in a Cargo workspace. Currently the lockfile is in a custom + format; in the future, the Cargo.lock file may be used instead. + +## Build file usage + +With the crates declared in the workspace, they can now be referenced in BUILD +files. For example: + +```python +load("@crates//:defs.bzl", "build_crates_from", "crates_from") + +cargo_build_script( + name = "build", + srcs = ["build.rs"], + deps = build_crates_from("//some_crate:Cargo.toml"), +) + +rust_library( + name = "some_crate", + srcs = [ + "lib.rs", + ], + deps = crates_from("//some_crate:Cargo.toml") + [":build"] +) +``` + +If you prefer, you can also list out each crate individually, eg: + +```python +load("@crates//:defs.bzl", "crate") + +rust_library( + name = "some_crate", + srcs = [ + "lib.rs", + ], + deps = [crate("serde"), crate("log"), ":build"] +) +``` + +See [some more examples](../examples/crate_universe) and the documentation below for more info. +]]# diff --git a/docs/rust_clippy.md b/docs/rust_clippy.md index 8646a5c978..16addd8d37 100644 --- a/docs/rust_clippy.md +++ b/docs/rust_clippy.md @@ -27,6 +27,8 @@ build --output_groups=+clippy_checks This will enable clippy on all [Rust targets](./defs.md). +Note that targets tagged with `noclippy` will not perform clippy checks + ## rust_clippy diff --git a/docs/rust_clippy.vm b/docs/rust_clippy.vm index e059e8d136..d444a04543 100644 --- a/docs/rust_clippy.vm +++ b/docs/rust_clippy.vm @@ -20,3 +20,5 @@ build --output_groups=+clippy_checks ``` This will enable clippy on all [Rust targets](./defs.md). + +Note that targets tagged with `noclippy` will not perform clippy checks diff --git a/examples/.bazelrc b/examples/.bazelrc index a61cbe3c11..7dcdbcf2bb 100644 --- a/examples/.bazelrc +++ b/examples/.bazelrc @@ -4,3 +4,7 @@ # Enable rustfmt for all targets in the workspace build:rustfmt --aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect build:rustfmt --output_groups=+rustfmt_checks + +# Enable clippy for all targets in the workspace +build:clippy --aspects=@rules_rust//rust:defs.bzl%rust_clippy_aspect +build:clippy --output_groups=+clippy_checks diff --git a/examples/crate_universe/.bazelrc b/examples/crate_universe/.bazelrc index a61cbe3c11..7dcdbcf2bb 100644 --- a/examples/crate_universe/.bazelrc +++ b/examples/crate_universe/.bazelrc @@ -4,3 +4,7 @@ # Enable rustfmt for all targets in the workspace build:rustfmt --aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect build:rustfmt --output_groups=+rustfmt_checks + +# Enable clippy for all targets in the workspace +build:clippy --aspects=@rules_rust//rust:defs.bzl%rust_clippy_aspect +build:clippy --output_groups=+clippy_checks diff --git a/proto/optional_output_wrapper.rs b/proto/optional_output_wrapper.rs index 60ff1cd580..59eba1b944 100644 --- a/proto/optional_output_wrapper.rs +++ b/proto/optional_output_wrapper.rs @@ -26,7 +26,7 @@ fn ensure() -> Result> { let optional_outputs = args().take(index).collect::>(); let exe = args().nth(index + 1).ok_or("no exe")?; let exe_args = args().skip(index + 2).collect::>(); - if exe_args.len() < 1 { + if exe_args.is_empty() { return Err("no exe args".into()); } match Command::new(exe).args(exe_args).status()?.code() { diff --git a/rust/private/clippy.bzl b/rust/private/clippy.bzl index 913accd39e..1cccc87800 100644 --- a/rust/private/clippy.bzl +++ b/rust/private/clippy.bzl @@ -23,11 +23,12 @@ load( ) load("//rust/private:utils.bzl", "determine_output_hash", "find_cc_toolchain", "find_toolchain") -def _get_clippy_ready_crate_info(target): +def _get_clippy_ready_crate_info(target, aspect_ctx): """Check that a target is suitable for clippy and extract the `CrateInfo` provider from it. Args: target (Target): The target the aspect is running on. + aspect_ctx (ctx, optional): The aspect's context object. Returns: CrateInfo, optional: A `CrateInfo` provider if clippy should be run or `None`. @@ -37,6 +38,10 @@ def _get_clippy_ready_crate_info(target): if target.label.workspace_root.startswith("external"): return None + # Targets annotated with `noclippy` will not be formatted + if aspect_ctx and "noclippy" in aspect_ctx.rule.attr.tags: + return None + # Obviously ignore any targets that don't contain `CrateInfo` if rust_common.crate_info not in target: return None @@ -44,7 +49,7 @@ def _get_clippy_ready_crate_info(target): return target[rust_common.crate_info] def _clippy_aspect_impl(target, ctx): - crate_info = _get_clippy_ready_crate_info(target) + crate_info = _get_clippy_ready_crate_info(target, ctx) if not crate_info: return [] @@ -189,7 +194,8 @@ $ bazel build --aspects=@rules_rust//rust:defs.bzl%rust_clippy_aspect \ ) def _rust_clippy_rule_impl(ctx): - files = depset([], transitive = [dep[OutputGroupInfo].clippy_checks for dep in ctx.attr.deps]) + clippy_ready_targets = [dep for dep in ctx.attr.deps if "clippy_checks" in dir(dep[OutputGroupInfo])] + files = depset([], transitive = [dep[OutputGroupInfo].clippy_checks for dep in clippy_ready_targets]) return [DefaultInfo(files = files)] rust_clippy = rule( diff --git a/test/chained_direct_deps/mod3.rs b/test/chained_direct_deps/mod3.rs index 1719ae98a3..2f31e6dbc8 100644 --- a/test/chained_direct_deps/mod3.rs +++ b/test/chained_direct_deps/mod3.rs @@ -25,7 +25,7 @@ pub fn greet_default() { /// ); /// ``` pub fn am_i_the_world(me: &str) -> bool { - return me == mod1::world(); + me == mod1::world() } #[cfg(test)] diff --git a/test/clippy/BUILD.bazel b/test/clippy/BUILD.bazel index 29e6e3050d..c93d849f8a 100644 --- a/test/clippy/BUILD.bazel +++ b/test/clippy/BUILD.bazel @@ -1,5 +1,5 @@ load( - "//rust:rust.bzl", + "@rules_rust//rust:rust.bzl", "rust_binary", "rust_clippy", "rust_library", @@ -50,18 +50,21 @@ rust_binary( name = "bad_binary", srcs = ["bad_src/main.rs"], edition = "2018", + tags = ["noclippy"], ) rust_library( name = "bad_library", srcs = ["bad_src/lib.rs"], edition = "2018", + tags = ["noclippy"], ) rust_test( name = "bad_test", srcs = ["bad_src/lib.rs"], edition = "2018", + tags = ["noclippy"], ) # Clippy analysis of failing targets. @@ -84,3 +87,8 @@ rust_clippy( tags = ["manual"], deps = [":bad_test"], ) + +sh_binary( + name = "clippy_failure_test", + srcs = ["clippy_failure_test.sh"], +) diff --git a/test/clippy/clippy_failure_test.sh b/test/clippy/clippy_failure_test.sh index b6c00028db..79dae7b5b4 100755 --- a/test/clippy/clippy_failure_test.sh +++ b/test/clippy/clippy_failure_test.sh @@ -7,6 +7,13 @@ set -euo pipefail +if [[ -z "${BUILD_WORKSPACE_DIRECTORY:-}" ]]; then + echo "This script should be run under Bazel" + exit 1 +fi + +cd "${BUILD_WORKSPACE_DIRECTORY}" + # Executes a bazel build command and handles the return value, exiting # upon seeing an error. # @@ -31,6 +38,31 @@ function test_all() { local -r BUILD_FAILED=1 local -r TEST_FAIL=3 + temp_dir="$(mktemp -d -t ci-XXXXXXXXXX)" + new_workspace="${temp_dir}/rules_rust_test_clippy" + + mkdir -p "${new_workspace}/test/clippy" && \ + cp -r test/clippy/* "${new_workspace}/test/clippy/" && \ + cat << EOF > "${new_workspace}/WORKSPACE.bazel" +workspace(name = "rules_rust_test_clippy") +local_repository( + name = "rules_rust", + path = "${BUILD_WORKSPACE_DIRECTORY}", +) +load("@rules_rust//rust:repositories.bzl", "rust_repositories") +rust_repositories() +EOF + + # Drop the 'noclippy' tags + if [ "$(uname)" == "Darwin" ]; then + SEDOPTS=(-i '' -e) + else + SEDOPTS=(-i) + fi + sed ${SEDOPTS[@]} 's/"noclippy"//' "${new_workspace}/test/clippy/BUILD.bazel" + + pushd "${new_workspace}" + check_build_result $BUILD_OK ok_binary_clippy check_build_result $BUILD_OK ok_library_clippy check_build_result $BUILD_OK ok_test_clippy diff --git a/test/renamed_deps/mod3.rs b/test/renamed_deps/mod3.rs index 4942bf2086..4a424827b4 100644 --- a/test/renamed_deps/mod3.rs +++ b/test/renamed_deps/mod3.rs @@ -25,7 +25,7 @@ pub fn greet_default() { /// ); /// ``` pub fn am_i_the_world(me: &str) -> bool { - return me == alias_a::world(); + me == alias_a::world() } #[cfg(test)] diff --git a/test/rust_analyzer/aspect_traversal_test/rust_project_json_test.rs b/test/rust_analyzer/aspect_traversal_test/rust_project_json_test.rs index a2cb8f9309..a297a1449b 100644 --- a/test/rust_analyzer/aspect_traversal_test/rust_project_json_test.rs +++ b/test/rust_analyzer/aspect_traversal_test/rust_project_json_test.rs @@ -9,7 +9,7 @@ mod tests { r.rlocation("rules_rust/test/rust_analyzer/aspect_traversal_test/rust-project.json"); let content = std::fs::read_to_string(&rust_project_path) - .expect(&format!("couldn't open {:?}", &rust_project_path)); + .unwrap_or_else(|_| panic!("couldn't open {:?}", &rust_project_path)); for dep in &[ "lib_dep", diff --git a/test/rust_analyzer/merging_crates_test/rust_project_json_test.rs b/test/rust_analyzer/merging_crates_test/rust_project_json_test.rs index 88d0ce884b..417f72f25b 100644 --- a/test/rust_analyzer/merging_crates_test/rust_project_json_test.rs +++ b/test/rust_analyzer/merging_crates_test/rust_project_json_test.rs @@ -9,7 +9,7 @@ mod tests { r.rlocation("rules_rust/test/rust_analyzer/merging_crates_test/rust-project.json"); let content = std::fs::read_to_string(&rust_project_path) - .expect(&format!("couldn't open {:?}", &rust_project_path)); + .unwrap_or_else(|_| panic!("couldn't open {:?}", &rust_project_path)); assert!( content.contains(r#""root_module":"test/rust_analyzer/merging_crates_test/mylib.rs","deps":[{"crate":0,"name":"lib_dep"},{"crate":2,"name":"extra_test_dep"}]"#), diff --git a/test/rust_analyzer/merging_crates_test_reversed/rust_project_json_test.rs b/test/rust_analyzer/merging_crates_test_reversed/rust_project_json_test.rs index 3e8600e8ba..f77c2a6ef5 100644 --- a/test/rust_analyzer/merging_crates_test_reversed/rust_project_json_test.rs +++ b/test/rust_analyzer/merging_crates_test_reversed/rust_project_json_test.rs @@ -10,7 +10,7 @@ mod tests { ); let content = std::fs::read_to_string(&rust_project_path) - .expect(&format!("couldn't open {:?}", &rust_project_path)); + .unwrap_or_else(|_| panic!("couldn't open {:?}", &rust_project_path)); assert!( content.contains(r#""root_module":"test/rust_analyzer/merging_crates_test_reversed/mylib.rs","deps":[{"crate":0,"name":"lib_dep"},{"crate":1,"name":"extra_test_dep"}]"#), diff --git a/tools/runfiles/runfiles.rs b/tools/runfiles/runfiles.rs index 6486c2e0e3..7b6fc86582 100644 --- a/tools/runfiles/runfiles.rs +++ b/tools/runfiles/runfiles.rs @@ -97,10 +97,9 @@ impl Runfiles { Mode::DirectoryBased(runfiles_dir) => runfiles_dir.join(path), Mode::ManifestBased(path_mapping) => path_mapping .get(path) - .expect(&format!( - "Path {} not found among runfiles.", - path.to_string_lossy() - )) + .unwrap_or_else(|| { + panic!("Path {} not found among runfiles.", path.to_string_lossy()) + }) .clone(), } } @@ -109,10 +108,11 @@ impl Runfiles { /// Returns the .runfiles directory for the currently executing binary. pub fn find_runfiles_dir() -> io::Result { assert_ne!( - std::env::var_os("RUNFILES_MANIFEST_ONLY").unwrap_or(OsString::from("0")), + std::env::var_os("RUNFILES_MANIFEST_ONLY").unwrap_or_else(|| OsString::from("0")), "1" ); - let exec_path = std::env::args().nth(0).expect("arg 0 was not set"); + // Consume the first argument (argv[0]) + let exec_path = std::env::args().next().expect("arg 0 was not set"); let mut binary_path = PathBuf::from(&exec_path); loop { diff --git a/tools/rustfmt/srcs/lib.rs b/tools/rustfmt/srcs/lib.rs index 35ccf78a94..ad2e86a5d6 100644 --- a/tools/rustfmt/srcs/lib.rs +++ b/tools/rustfmt/srcs/lib.rs @@ -3,7 +3,7 @@ use std::fs; use std::path::{Path, PathBuf}; /// The expected extension of rustfmt manifest files generated by `rustfmt_aspect`. -pub const RUSTFMT_MANIFEST_EXTENSION: &'static str = "rustfmt"; +pub const RUSTFMT_MANIFEST_EXTENSION: &str = "rustfmt"; /// Generate an absolute path to a file without resolving symlinks fn absolutify_existing>(path: &T) -> std::io::Result { @@ -50,13 +50,11 @@ pub struct RustfmtManifest { /// Parse rustfmt flags from a manifest generated by builds using `rustfmt_aspect`. pub fn parse_rustfmt_manifest(manifest: &Path) -> RustfmtManifest { - let content = fs::read_to_string(manifest).expect(&format!( - "Failed to read rustfmt manifest: {}", - manifest.display() - )); + let content = fs::read_to_string(manifest) + .unwrap_or_else(|_| panic!("Failed to read rustfmt manifest: {}", manifest.display())); let mut lines: Vec = content - .split("\n") + .split('\n') .into_iter() .filter(|s| !s.is_empty()) .map(|s| s.to_owned()) @@ -70,7 +68,7 @@ pub fn parse_rustfmt_manifest(manifest: &Path) -> RustfmtManifest { .expect("The edition should be a numeric value. eg `2018`."); RustfmtManifest { - edition: edition, + edition, sources: lines, } } diff --git a/tools/rustfmt/srcs/test_main.rs b/tools/rustfmt/srcs/test_main.rs index d1520d7a0f..87c145fa2a 100644 --- a/tools/rustfmt/srcs/test_main.rs +++ b/tools/rustfmt/srcs/test_main.rs @@ -2,9 +2,6 @@ use std::fs; use std::path::{Path, PathBuf}; use std::process::Command; -use runfiles; -use rustfmt_lib; - fn main() { // Gather all and environment settings let options = parse_args(); diff --git a/util/label/label.rs b/util/label/label.rs index 6b441c58c8..e25770d5db 100644 --- a/util/label/label.rs +++ b/util/label/label.rs @@ -8,7 +8,7 @@ use label_error::LabelError; /// /// TODO: validate . and .. in target name /// TODO: validate used characters in target name -pub fn analyze<'s>(input: &'s str) -> Result> { +pub fn analyze(input: &'_ str) -> Result> { let label = input; let (input, repository_name) = consume_repository_name(input, label)?; let (input, package_name) = consume_package_name(input, label)?; @@ -40,7 +40,7 @@ impl<'s> Label<'s> { pub fn packages(&self) -> Vec<&'s str> { match self.package_name { - Some(name) => name.split("/").collect(), + Some(name) => name.split('/').collect(), None => vec![], } } @@ -57,13 +57,13 @@ fn consume_repository_name<'s>( input: &'s str, label: &'s str, ) -> Result<(&'s str, Option<&'s str>)> { - if !input.starts_with("@") { + if !input.starts_with('@') { return Ok((input, None)); } let slash_pos = input .find("//") - .ok_or(err(label, "labels with repository must contain //."))?; + .ok_or_else(|| err(label, "labels with repository must contain //."))?; let repository_name = &input[1..slash_pos]; if repository_name.is_empty() { return Ok((&input[1..], None)); @@ -93,7 +93,7 @@ fn consume_repository_name<'s>( } fn consume_package_name<'s>(input: &'s str, label: &'s str) -> Result<(&'s str, Option<&'s str>)> { - let colon_pos = input.find(":"); + let colon_pos = input.find(':'); let start_pos; let mut is_absolute = false; if input.starts_with("//") { @@ -143,7 +143,7 @@ fn consume_package_name<'s>(input: &'s str, label: &'s str) -> Result<(&'s str, a-z, 0-9, '/', '-', '.', ' ', '$', '(', ')' and '_'.", ))); } - if package_name.ends_with("/") { + if package_name.ends_with('/') { return Err(LabelError(err( label, "package names may not end with '/'.", @@ -154,7 +154,7 @@ fn consume_package_name<'s>(input: &'s str, label: &'s str) -> Result<(&'s str, // This label doesn't contain the target name, we have to use // last segment of the package name as target name. return Ok(( - match package_name.rfind("/") { + match package_name.rfind('/') { Some(pos) => &package_name[pos..], None => package_name, }, @@ -169,15 +169,15 @@ fn consume_name<'s>(input: &'s str, label: &'s str) -> Result<&'s str> { if input.is_empty() { return Err(LabelError(err(label, "empty target name."))); } - let name = if input.starts_with(":") { - &input[1..] + let name = if let Some(stripped) = input.strip_prefix(':') { + stripped } else { input }; if name.is_empty() { return Err(LabelError(err(label, "empty target name."))); } - if name.starts_with("/") { + if name.starts_with('/') { return Err(LabelError(err( label, "target names may not start with '/'.", diff --git a/util/launcher/launcher_main.rs b/util/launcher/launcher_main.rs index 02ffbf2d7d..006d051911 100644 --- a/util/launcher/launcher_main.rs +++ b/util/launcher/launcher_main.rs @@ -9,7 +9,7 @@ use std::vec::Vec; use std::os::unix::process::CommandExt; /// This string must match the one found in `_create_test_launcher` -const LAUNCHFILES_ENV_PATH: &'static str = ".launchfiles/env"; +const LAUNCHFILES_ENV_PATH: &str = ".launchfiles/env"; /// Load environment variables from a uniquly formatted fn environ() -> BTreeMap { @@ -17,8 +17,11 @@ fn environ() -> BTreeMap { let mut key: Option = None; + // Consume the first argument (argv[0]) + let exe_path = std::env::args().next().expect("arg 0 was not set"); + // Load the environment file into a map - let env_path = std::env::args().nth(0).expect("arg 0 was not set") + LAUNCHFILES_ENV_PATH; + let env_path = exe_path + LAUNCHFILES_ENV_PATH; let file = File::open(env_path).expect("Failed to load the environment file"); // Variables will have the `${pwd}` variable replaced which is rendered by @@ -47,7 +50,8 @@ fn environ() -> BTreeMap { /// Locate the executable based on the name of the launcher executable fn executable() -> PathBuf { - let mut exec_path = std::env::args().nth(0).expect("arg 0 was not set"); + // Consume the first argument (argv[0]) + let mut exec_path = std::env::args().next().expect("arg 0 was not set"); let stem_index = exec_path .rfind(".launcher") .expect("This executable should always contain `.launcher`");