Upgrade to Rust 1.52.0-beta3 (pantsbuild#11940)

Rust upgrades have been blocked by a clippy bug which unfortunately missed the last release cycle. See pantsbuild#11631.

The relevant bugfix is available in the `1.52.0` beta series, and it seems better to temporarily be on beta than to wait 12+ weeks between upgrades.

Fixes pantsbuild#11631.

[ci skip-build-wheels]

.github/workflows/test-cron.yaml

@@ -56,7 +56,7 @@ jobs:
       uses: actions/cache@v2
       with:
         key: ${{ runner.os }}-rustup-${{ hashFiles('rust-toolchain') }}
-        path: '~/.rustup/toolchains/1.49.0-*
+        path: '~/.rustup/toolchains/beta-2021-04-07-*
 
           ~/.rustup/update-hashes
 
@@ -185,7 +185,7 @@ jobs:
       uses: actions/cache@v2
       with:
         key: ${{ runner.os }}-rustup-${{ hashFiles('rust-toolchain') }}
-        path: '~/.rustup/toolchains/1.49.0-*
+        path: '~/.rustup/toolchains/beta-2021-04-07-*
 
           ~/.rustup/update-hashes
 

.github/workflows/test.yaml

@@ -56,7 +56,7 @@ jobs:
       uses: actions/cache@v2
       with:
         key: ${{ runner.os }}-rustup-${{ hashFiles('rust-toolchain') }}
-        path: '~/.rustup/toolchains/1.49.0-*
+        path: '~/.rustup/toolchains/beta-2021-04-07-*
 
           ~/.rustup/update-hashes
 
@@ -184,7 +184,7 @@ jobs:
       uses: actions/cache@v2
       with:
         key: ${{ runner.os }}-rustup-${{ hashFiles('rust-toolchain') }}
-        path: '~/.rustup/toolchains/1.49.0-*
+        path: '~/.rustup/toolchains/beta-2021-04-07-*
 
           ~/.rustup/update-hashes
 
@@ -359,7 +359,7 @@ jobs:
       uses: actions/cache@v2
       with:
         key: ${{ runner.os }}-rustup-${{ hashFiles('rust-toolchain') }}
-        path: '~/.rustup/toolchains/1.49.0-*
+        path: '~/.rustup/toolchains/beta-2021-04-07-*
 
           ~/.rustup/update-hashes
 

rust-toolchain

@@ -1,5 +1,5 @@
 [toolchain]
-channel = "1.49.0"
+channel = "beta-2021-04-07"
 components = [
   "cargo",
   "clippy",

src/rust/engine/concrete_time/src/lib.rs

@@ -64,9 +64,9 @@ impl From<std::time::Duration> for Duration {
   }
 }
 
-impl Into<std::time::Duration> for Duration {
-  fn into(self) -> std::time::Duration {
-    std::time::Duration::new(self.secs, self.nanos)
+impl From<Duration> for std::time::Duration {
+  fn from(duration: Duration) -> std::time::Duration {
+    std::time::Duration::new(duration.secs, duration.nanos)
   }
 }
 

src/rust/engine/fs/brfs/src/main.rs

@@ -122,8 +122,8 @@ enum Node {
 
 #[derive(Clone, Copy, Debug)]
 pub enum BRFSEvent {
-  INIT,
-  DESTROY,
+  Init,
+  Destroy,
 }
 
 struct BuildResultFS {
@@ -413,14 +413,14 @@ impl BuildResultFS {
 //  ... created on demand and cached for the lifetime of the program.
 impl fuse::Filesystem for BuildResultFS {
   fn init(&mut self, _req: &fuse::Request) -> Result<(), libc::c_int> {
-    self.sender.send(BRFSEvent::INIT).map_err(|_| 1)
+    self.sender.send(BRFSEvent::Init).map_err(|_| 1)
   }
 
   fn destroy(&mut self, _req: &fuse::Request) {
     self
       .sender
-      .send(BRFSEvent::DESTROY)
-      .unwrap_or_else(|err| warn!("Failed to send {:?} event: {}", BRFSEvent::DESTROY, err))
+      .send(BRFSEvent::Destroy)
+      .unwrap_or_else(|err| warn!("Failed to send {:?} event: {}", BRFSEvent::Destroy, err))
   }
 
   // Used to answer stat calls
@@ -708,11 +708,9 @@ async fn main() {
   let mount_path = args.value_of("mount-path").unwrap();
   let store_path = args.value_of("local-store-path").unwrap();
 
-  let root_ca_certs = if let Some(path) = args.value_of("root-ca-cert-file") {
-    Some(std::fs::read(path).expect("Error reading root CA certs file"))
-  } else {
-    None
-  };
+  let root_ca_certs = args
+    .value_of("root-ca-cert-file")
+    .map(|path| std::fs::read(path).expect("Error reading root CA certs file"));
 
   let mut headers = BTreeMap::new();
   if let Some(oauth_path) = args.value_of("oauth-bearer-token-file") {
@@ -753,8 +751,8 @@ async fn main() {
 
   #[derive(Clone, Copy, Debug)]
   enum Sig {
-    INT,
-    TERM,
+    Int,
+    Term,
     Unmount,
   }
 
@@ -768,8 +766,8 @@ async fn main() {
     .map(move |_| Some(sig))
   }
 
-  let sigint = install_handler(SignalKind::interrupt, Sig::INT);
-  let sigterm = install_handler(SignalKind::terminate, Sig::TERM);
+  let sigint = install_handler(SignalKind::interrupt, Sig::Int);
+  let sigterm = install_handler(SignalKind::terminate, Sig::Term);
 
   match mount(mount_path, store, runtime.clone()) {
     Err(err) => {
@@ -781,8 +779,8 @@ async fn main() {
     }
     Ok((_, receiver)) => {
       match receiver.recv().unwrap() {
-        BRFSEvent::INIT => debug!("Store {} mounted at {}", store_path, mount_path),
-        BRFSEvent::DESTROY => {
+        BRFSEvent::Init => debug!("Store {} mounted at {}", store_path, mount_path),
+        BRFSEvent::Destroy => {
           warn!("Externally unmounted before we could mount.");
           return;
         }
@@ -792,8 +790,8 @@ async fn main() {
         // N.B.: In practice recv always errs and we exercise the or branch. It seems the sender
         // side thread always exits (which drops our BuildResultFS) before we get a chance to
         // complete the read.
-        match receiver.recv().unwrap_or(BRFSEvent::DESTROY) {
-          BRFSEvent::DESTROY => Some(Sig::Unmount),
+        match receiver.recv().unwrap_or(BRFSEvent::Destroy) {
+          BRFSEvent::Destroy => Some(Sig::Unmount),
           event => {
             warn!("Received unexpected event {:?}", event);
             None

src/rust/engine/fs/src/glob_matching.rs

@@ -17,7 +17,7 @@ use parking_lot::Mutex;
 
 use crate::{
   Dir, GitignoreStyleExcludes, GlobExpansionConjunction, Link, PathStat, Stat, StrictGlobMatching,
-  VFS,
+  Vfs,
 };
 
 lazy_static! {
@@ -350,7 +350,7 @@ impl PreparedPathGlobs {
 }
 
 #[async_trait]
-pub trait GlobMatching<E: Display + Send + Sync + 'static>: VFS<E> {
+pub trait GlobMatching<E: Display + Send + Sync + 'static>: Vfs<E> {
   ///
   /// Canonicalize the Link for the given Path to an underlying File or Dir. May result
   /// in None if the PathStat represents a broken Link.
@@ -380,14 +380,14 @@ pub trait GlobMatching<E: Display + Send + Sync + 'static>: VFS<E> {
   }
 }
 
-impl<E: Display + Send + Sync + 'static, T: VFS<E>> GlobMatching<E> for T {}
+impl<E: Display + Send + Sync + 'static, T: Vfs<E>> GlobMatching<E> for T {}
 
 // NB: This trait exists because `expand_single()` (and its return type) should be private, but
 // traits don't allow specifying private methods (and we don't want to use a top-level `fn` because
 // it's much more awkward than just specifying `&self`).
 // The methods of `GlobMatching` are forwarded to methods here.
 #[async_trait]
-trait GlobMatchingImplementation<E: Display + Send + Sync + 'static>: VFS<E> {
+trait GlobMatchingImplementation<E: Display + Send + Sync + 'static>: Vfs<E> {
   async fn directory_listing(
     &self,
     canonical_dir: Dir,
@@ -444,7 +444,7 @@ trait GlobMatchingImplementation<E: Display + Send + Sync + 'static>: VFS<E> {
     )
     .await?;
     // See the note above.
-    Ok(path_stats.into_iter().filter_map(|pso| pso).collect())
+    Ok(path_stats.into_iter().flatten().collect())
   }
 
   async fn expand_globs(
@@ -686,4 +686,4 @@ trait GlobMatchingImplementation<E: Display + Send + Sync + 'static>: VFS<E> {
   }
 }
 
-impl<E: Display + Send + Sync + 'static, T: VFS<E>> GlobMatchingImplementation<E> for T {}
+impl<E: Display + Send + Sync + 'static, T: Vfs<E>> GlobMatchingImplementation<E> for T {}

src/rust/engine/fs/src/lib.rs

@@ -662,7 +662,7 @@ impl PosixFS {
 }
 
 #[async_trait]
-impl VFS<io::Error> for Arc<PosixFS> {
+impl Vfs<io::Error> for Arc<PosixFS> {
   async fn read_link(&self, link: &Link) -> Result<PathBuf, io::Error> {
     PosixFS::read_link(self, link).await
   }
@@ -719,7 +719,7 @@ impl PathStatGetter<io::Error> for Arc<PosixFS> {
 /// A context for filesystem operations parameterized on an error type 'E'.
 ///
 #[async_trait]
-pub trait VFS<E: Send + Sync + 'static>: Clone + Send + Sync + 'static {
+pub trait Vfs<E: Send + Sync + 'static>: Clone + Send + Sync + 'static {
   async fn read_link(&self, link: &Link) -> Result<PathBuf, E>;
   async fn scandir(&self, dir: Dir) -> Result<Arc<DirectoryListing>, E>;
   fn is_ignored(&self, stat: &Stat) -> bool;

src/rust/engine/fs/src/posixfs_tests.rs

@@ -4,7 +4,7 @@ use testutil;
 use crate::{
   Dir, DirectoryListing, File, GitignoreStyleExcludes, GlobExpansionConjunction, GlobMatching,
   Link, PathGlobs, PathStat, PathStatGetter, PosixFS, Stat, StrictGlobMatching, SymlinkBehavior,
-  VFS,
+  Vfs,
 };
 
 use async_trait::async_trait;
@@ -467,7 +467,7 @@ async fn test_basic_gitignore_functionality() {
 }
 
 ///
-/// An in-memory implementation of VFS, useful for precisely reproducing glob matching behavior for
+/// An in-memory implementation of Vfs, useful for precisely reproducing glob matching behavior for
 /// a set of file paths.
 ///
 pub struct MemFS {
@@ -516,7 +516,7 @@ impl MemFS {
 }
 
 #[async_trait]
-impl VFS<String> for Arc<MemFS> {
+impl Vfs<String> for Arc<MemFS> {
   async fn read_link(&self, link: &Link) -> Result<PathBuf, String> {
     // The creation of a static filesystem does not allow for Links.
     Err(format!("{:?} does not exist within this filesystem.", link))

src/rust/engine/fs/store/src/lib.rs

@@ -819,8 +819,7 @@ impl Store {
   ) -> BoxFuture<'static, Result<HashMap<Digest, EntryType>, String>> {
     self
       .walk(digest, |_, _, digest, directory| {
-        let mut digest_types = Vec::new();
-        digest_types.push((digest, EntryType::Directory));
+        let mut digest_types = vec![(digest, EntryType::Directory)];
         for file in &directory.files {
           let file_digest = try_future!(require_digest(file.digest.as_ref()));
           digest_types.push((file_digest, EntryType::File));

src/rust/engine/fs/store/src/remote_tests.rs

@@ -67,7 +67,8 @@ async fn load_file_grpc_error() {
     .expect_err("Want error");
   assert!(
     error.contains("StubCAS is configured to always fail"),
-    format!("Bad error message, got: {}", error)
+    "Bad error message, got: {}",
+    error
   )
 }
 
@@ -83,7 +84,8 @@ async fn load_directory_grpc_error() {
   .expect_err("Want error");
   assert!(
     error.contains("StubCAS is configured to always fail"),
-    format!("Bad error message, got: {}", error)
+    "Bad error message, got: {}",
+    error
   )
 }
 
@@ -182,7 +184,9 @@ async fn write_file_multiple_chunks() {
   for size in write_message_sizes.iter() {
     assert!(
       size <= &(10 * 1024),
-      format!("Size {} should have been <= {}", size, 10 * 1024)
+      "Size {} should have been <= {}",
+      size,
+      10 * 1024
     );
   }
 }
@@ -216,7 +220,8 @@ async fn write_file_errors() {
     .expect_err("Want error");
   assert!(
     error.contains("StubCAS is configured to always fail"),
-    format!("Bad error message, got: {}", error)
+    "Bad error message, got: {}",
+    error
   );
 }
 
@@ -238,7 +243,8 @@ async fn write_connection_error() {
     .expect_err("Want error");
   assert!(
     error.contains("dns error: failed to lookup address information"),
-    format!("Bad error message, got: {}", error)
+    "Bad error message, got: {}",
+    error
   );
 }
 
@@ -290,7 +296,8 @@ async fn list_missing_digests_error() {
     .expect_err("Want error");
   assert!(
     error.contains("StubCAS is configured to always fail"),
-    format!("Bad error message, got: {}", error)
+    "Bad error message, got: {}",
+    error
   );
 }
 

src/rust/engine/fs/store/src/snapshot.rs

@@ -319,7 +319,7 @@ pub fn osstring_as_utf8(path: OsString) -> Result<String, String> {
 // StoreFileByDigest allows a File to be saved to an underlying Store, in such a way that it can be
 // looked up by the Digest produced by the store_by_digest method.
 // It is a separate trait so that caching implementations can be written which wrap the Store (used
-// to store the bytes) and VFS (used to read the files off disk if needed).
+// to store the bytes) and Vfs (used to read the files off disk if needed).
 pub trait StoreFileByDigest<Error> {
   fn store_by_digest(&self, file: File) -> future::BoxFuture<'static, Result<Digest, Error>>;
 }

src/rust/engine/fs/store/src/snapshot_ops.rs

@@ -397,7 +397,7 @@ impl IntermediateGlobbedFilesAndDirectories {
         let directory_node = cur_dir_directories.get(&directory_path).unwrap();
 
         // TODO(#9967): Figure out how to consume the existing glob matching logic that works on
-        // `VFS` instances!
+        // `Vfs` instances!
         match &path_glob {
           RestrictedPathGlob::Wildcard { wildcard } => {
             // NB: We interpret globs such that the *only* way to have a glob match the contents of

src/rust/engine/graph/src/lib.rs

@@ -54,7 +54,7 @@ use tokio::time::sleep;
 
 pub use crate::node::{EntryId, Node, NodeContext, NodeError, NodeVisualizer, Stats};
 
-type FNV = BuildHasherDefault<FnvHasher>;
+type Fnv = BuildHasherDefault<FnvHasher>;
 
 type PGraph<N> = DiGraph<Entry<N>, f32, u32>;
 
@@ -175,8 +175,7 @@ impl<N: Node> InnerGraph<N> {
       .expect("There should not be any negative edge weights");
 
     let mut next = dst;
-    let mut path = Vec::new();
-    path.push(next);
+    let mut path = vec![next];
     while let Some(current) = paths[next.index()] {
       path.push(current);
       if current == src {
@@ -312,7 +311,7 @@ impl<N: Node> InnerGraph<N> {
   ///
   fn invalidate_from_roots<P: Fn(&N) -> bool>(&mut self, predicate: P) -> InvalidationResult {
     // Collect all entries that will be cleared.
-    let root_ids: HashSet<_, FNV> = self
+    let root_ids: HashSet<_, Fnv> = self
       .nodes
       .iter()
       .filter_map(|(node, &entry_id)| {
@@ -925,7 +924,7 @@ where
   graph: &'a InnerGraph<N>,
   direction: Direction,
   deque: VecDeque<EntryId>,
-  walked: HashSet<EntryId, FNV>,
+  walked: HashSet<EntryId, Fnv>,
   stop_walking_predicate: F,
 }
 

src/rust/engine/grpc_util/src/lib.rs

@@ -63,7 +63,7 @@ pub fn create_tls_config(root_ca_certs: Option<Vec<u8>>) -> Result<ClientConfig,
   // Must set HTTP/2 as ALPN protocol otherwise cannot connect over TLS to gRPC servers.
   // Unfortunately, this is not a default value and, moreover, Tonic does not provide
   // any helper function to encapsulate this knowledge.
-  tls_config.set_protocols(&[Vec::from(&"h2"[..])]);
+  tls_config.set_protocols(&[Vec::from("h2")]);
 
   // Add the root store.
   match root_ca_certs {