Merge branch 'main' into data

bazelbuild · Jul 10, 2021 · 9852caa · 9852caa
2 parents 06782b3 + ed64716
commit 9852caa
Show file tree

Hide file tree

Showing 26 changed files with 402 additions and 197 deletions.
diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml
@@ -17,13 +17,15 @@ tasks:
       - "//test/versioned_dylib:versioned_dylib_test"
     build_flags:
       - "--config=rustfmt"
+      - "--config=clippy"
   ubuntu2004:
     name: "Minimum Supported Version"
     bazel: "3.5.0"
     build_targets: *default_linux_targets
     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
@@ -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
@@ -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<P: AsRef<Path>>(rustc: &P) -> Result<BTreeMap<String, Str
             if value.starts_with('"') && value.ends_with('"') && value.len() >= 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
@@ -94,7 +94,7 @@ impl BuildScriptOutput {
     }
 
     /// Converts a [BufReader] into a vector of [BuildScriptOutput] enums.
-    fn from_reader<T: Read>(mut reader: BufReader<T>) -> Vec<BuildScriptOutput> {
+    fn outputs_from_reader<T: Read>(mut reader: BufReader<T>) -> Vec<BuildScriptOutput> {
         let mut result = Vec::<BuildScriptOutput>::new();
         let mut line = String::new();
         while reader.read_line(&mut line).expect("Cannot read line") != 0 {
@@ -107,20 +107,23 @@ 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<BuildScriptOutput>, Output), Output> {
+    pub fn outputs_from_command(
+        cmd: &mut Command,
+    ) -> Result<(Vec<BuildScriptOutput>, 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)
         }
     }
 
     /// Convert a vector of [BuildScriptOutput] into a list of environment variables.
-    pub fn to_env(v: &Vec<BuildScriptOutput>, 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<BuildScriptOutput>, 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<BuildScriptOutput>, 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.