Auto merge of #405 - pietroalbini:update-lockfile, r=pietroalbini

Regenerate the lockfile if needed

This PR automatically regenerates the lockfile if an existing `Cargo.lock` was present and that's not valid anymore. This should remove a bunch of the errors during a run.

config.toml

@@ -36,7 +36,6 @@ build-log-max-lines = 10000
 # - skip (bool): ignore this crate/repo
 # - skip-tests (bool): don't run tests in this crate/repo
 # - quiet (bool): don't kill after two minutes without output
-# - update-lockfile (bool): update the lockfile even if the crate has one
 # - broken (bool): treat a Crater error on this crate/repo as a build
 # failure (typically the crate is broken in an
 # unusual way and we want to indicate the failure
@@ -6504,7 +6503,6 @@ zyre-sys = { skip = true } #automatic
 "Kayren/wardice" = { skip-tests = true } #automatic
 "Kbrew/risp" = { skip = true } #automatic
 "Keats/kickstart" = { skip-tests = true } #automatic
-"KeenS/cargo-pack-docker" = { update-lockfile = true } # outdated lockfile
 "KeenS/rayon-playground" = { skip = true } #automatic
 "KeenS/rockscached" = { skip = true } #automatic
 "KeenS/webml" = { skip = true } #automatic
@@ -8235,7 +8233,6 @@ zyre-sys = { skip = true } #automatic
 "caolan/tamawiki" = { skip = true } #automatic
 "captainbland/exothermic" = { skip = true } #automatic
 "carbidegames/scheming-sun" = { skip = true } #automatic
-"cardoe/cargo-bitbake" = { update-lockfile = true } # outdated lockfile
 "carllerche/minidb" = { skip = true } #automatic
 "carllerche/tokio-examples" = { skip = true } #automatic
 "carlosgj94/VanitityAddresses" = { skip = true } #automatic
@@ -8496,7 +8493,6 @@ zyre-sys = { skip = true } #automatic
 "creativcoder/avro-sys" = { skip = true } #automatic
 "creativcoder/graphin" = { skip = true } #automatic
 "creativcoder/rust_calls_cpp" = { skip = true } #automatic
-"creators/cli.sudoku.rs" = { update-lockfile = true } # outdated lockfile
 "creekpld/hello-gtk" = { skip = true } #automatic
 "cretz/rust-qt_cef_poc" = { skip = true } #automatic
 "criloz/rust-pet" = { skip = true } #automatic
@@ -8777,7 +8773,6 @@ zyre-sys = { skip = true } #automatic
 "djallen89/beams" = { skip = true } #automatic
 "djcsdy/gfx-dx11-empty-vertex-buffer" = { skip = true } #automatic
 "djgood/rust_os" = { skip = true } #automatic
-"djmcgill/form" = { update-lockfile = true } # outdated lockfile
 "djtal/readrust-cli" = { skip = true } #automatic
 "djtorel/rl-tut-rust" = { skip = true } #automatic
 "djudd/human-name" = { skip = true } #automatic
@@ -9223,7 +9218,6 @@ zyre-sys = { skip = true } #automatic
 "freiguy1/prodev" = { skip = true } #automatic
 "freiguy1/rust-roller" = { skip = true } #automatic
 "freshtonic/selecta-rs" = { skip = true } #automatic
-"frewsxcv/alert-after" = { update-lockfile = true } # outdated lockfile
 "frewsxcv/rgis" = { skip = true } #automatic
 "frewsxcv/rust-crates-index-graph" = { skip = true } #automatic
 "frgomes/mkvm" = { skip = true } #automatic
@@ -11057,7 +11051,6 @@ zyre-sys = { skip = true } #automatic
 "mlsteele/bittles" = { skip = true } #automatic
 "mlsteele/gdaxstat" = { skip = true } #automatic
 "mlsteele/pastiche" = { skip = true } #automatic
-"mmacedoeu/csvtoredis.rs" = { update-lockfile = true } # outdated lockfile
 "mmacedoeu/swapi" = { skip = true } #automatic
 "mmacedoeu/toyrpc.rs" = { skip = true } #automatic
 "mmacedoeu/waldo-mmacedoeu" = { skip = true } #automatic
@@ -12230,7 +12223,6 @@ zyre-sys = { skip = true } #automatic
 "rustit-be/hue_persistence" = { skip = true } #automatic
 "rustit-be/madparts-rs" = { skip = true } #automatic
 "rustmx/workshop-rocket-meetup008" = { skip = true } #automatic
-"rustoscript/js.rs-parser" = { update-lockfile = true } # outdated lockfile
 "rustox/futures-example" = { skip = true } #automatic
 "rustox/grpc-calculator-example" = { skip = true } #automatic
 "rusts/team" = { skip = true } #automatic

local-crates/outdated-lockfile/Cargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "outdated-lockfile"
+version = "0.1.0"
+authors = ["Pietro Albini <pietro@pietroalbini.org>"]
+edition = "2018"
+
+[dependencies]

local-crates/outdated-lockfile/src/main.rs

@@ -0,0 +1,3 @@
+fn main() {
+ println!("Hello, world!");
+}

src/actions/experiments/create.rs

@@ -207,7 +207,6 @@ mod tests {
  skip: true,
  skip_tests: false,
  quiet: false,
- update_lockfile: false,
  broken: false,
  },
  );

src/actions/experiments/edit.rs

@@ -264,7 +264,6 @@ mod tests {
  skip: true,
  skip_tests: false,
  quiet: false,
- update_lockfile: false,
  broken: false,
  },
  );

src/config.rs

@@ -30,8 +30,6 @@ pub struct CrateConfig {
  #[serde(default = "default_false")]
  pub quiet: bool,
  #[serde(default = "default_false")]
- pub update_lockfile: bool,
- #[serde(default = "default_false")]
  pub broken: bool,
 }
 
@@ -123,12 +121,6 @@ impl Config {
  self.crate_config(c).map(|c| c.quiet).unwrap_or(false)
  }
 
- pub fn should_update_lockfile(&self, c: &Crate) -> bool {
- self.crate_config(c)
- .map(|c| c.update_lockfile)
- .unwrap_or(false)
- }
-
  pub fn is_broken(&self, c: &Crate) -> bool {
  self.crate_config(c).map(|c| c.broken).unwrap_or(false)
  }

src/crates/mod.rs

@@ -18,14 +18,6 @@ pub enum Crate {
 }
 
 impl Crate {
- pub(crate) fn is_repo(&self) -> bool {
- match *self {
- Crate::Registry(_) => false,
- Crate::GitHub(_) => true,
- Crate::Local(_) => false,
- }
- }
-
  pub(crate) fn id(&self) -> String {
  match *self {
  Crate::Registry(ref details) => format!("reg/{}/{}", details.name, details.version),

src/report/mod.rs

@@ -634,7 +634,6 @@ mod tests {
  skip: true,
  skip_tests: false,
  quiet: false,
- update_lockfile: false,
  broken: false,
  },
  );

src/run.rs

@@ -32,7 +32,7 @@ pub(crate) enum Binary {
 pub(crate) trait Runnable {
  fn binary(&self) -> Binary;
 
- fn prepare_command(&self, cmd: RunCommand) -> RunCommand {
+ fn prepare_command<'pl>(&self, cmd: RunCommand<'pl>) -> RunCommand<'pl> {
  cmd
  }
 }
@@ -54,28 +54,30 @@ impl<'a, R: Runnable> Runnable for &'a R {
  Runnable::binary(*self)
  }
 
- fn prepare_command(&self, cmd: RunCommand) -> RunCommand {
+ fn prepare_command<'pl>(&self, cmd: RunCommand<'pl>) -> RunCommand<'pl> {
  Runnable::prepare_command(*self, cmd)
  }
 }
 
-pub(crate) struct RunCommand {
+pub(crate) struct RunCommand<'pl> {
  binary: Binary,
  args: Vec<OsString>,
  env: Vec<(OsString, OsString)>,
+ process_lines: Option<&'pl mut dyn FnMut(&str)>,
  cd: Option<PathBuf>,
  quiet: bool,
  enable_timeout: bool,
  local_rustup: bool,
  hide_output: bool,
 }
 
-impl RunCommand {
+impl<'pl> RunCommand<'pl> {
  pub(crate) fn new<R: Runnable>(runnable: R) -> Self {
  runnable.prepare_command(RunCommand {
  binary: runnable.binary(),
  args: Vec::new(),
  env: Vec::new(),
+ process_lines: None,
  cd: None,
  quiet: false,
  enable_timeout: true,
@@ -98,6 +100,11 @@ impl RunCommand {
  self
  }
 
+ pub(crate) fn process_lines(mut self, f: &'pl mut dyn FnMut(&str)) -> Self {
+ self.process_lines = Some(f);
+ self
+ }
+
  pub(crate) fn cd<P: AsRef<Path>>(mut self, path: P) -> Self {
  self.cd = Some(path.as_ref().to_path_buf());
  self
@@ -123,7 +130,7 @@ impl RunCommand {
  self
  }
 
- pub(crate) fn sandboxed(self, docker_env: &DockerEnv) -> SandboxedCommand {
+ pub(crate) fn sandboxed<'a>(self, docker_env: &'a DockerEnv) -> SandboxedCommand<'a, 'pl> {
  SandboxedCommand::new(self, docker_env)
  }
 
@@ -175,6 +182,7 @@ impl RunCommand {
  info!("running `{}`", cmdstr);
  let out = log_command(
  cmd,
+ self.process_lines,
  capture,
  self.quiet,
  self.enable_timeout,
@@ -193,13 +201,13 @@ impl RunCommand {
  }
 }
 
-pub(crate) struct SandboxedCommand<'a> {
- command: RunCommand,
+pub(crate) struct SandboxedCommand<'a, 'pl> {
+ command: RunCommand<'pl>,
  container: ContainerBuilder<'a>,
 }
 
-impl<'a> SandboxedCommand<'a> {
- fn new(command: RunCommand, docker_env: &'a DockerEnv) -> Self {
+impl<'a, 'pl> SandboxedCommand<'a, 'pl> {
+ fn new(command: RunCommand<'pl>, docker_env: &'a DockerEnv) -> Self {
  let container = ContainerBuilder::new(docker_env)
  .env("USER_ID", native::current_user().to_string())
  .enable_networking(false);
@@ -303,6 +311,7 @@ const HEARTBEAT_TIMEOUT_SECS: u64 = 60 * 5;
 
 fn log_command(
  mut cmd: Command,
+ mut process_lines: Option<&mut dyn FnMut(&str)>,
  capture: bool,
  quiet: bool,
  enable_timeout: bool,
@@ -368,6 +377,9 @@ fn log_command(
  .fold(
  (Vec::new(), Vec::new()),
  move |mut res, (kind, line)| -> Fallible<_> {
+ if let Some(f) = &mut process_lines {
+ f(&line);
+ }
  if capture {
  match kind {
  OutputKind::Stdout => res.0.push(line),

src/runner/prepare.rs

@@ -1,4 +1,3 @@
-use crate::config::Config;
 use crate::crates::Crate;
 use crate::dirs::crate_source_dir;
 use crate::experiments::Experiment;
@@ -14,18 +13,13 @@ use std::path::PathBuf;
 pub(super) struct PrepareCrate<'a, DB: WriteResults + 'a> {
  experiment: &'a Experiment,
  krate: &'a Crate,
- config: &'a Config,
  db: &'a DB,
  source_dirs: Vec<(&'a Toolchain, PathBuf)>,
+ lockfile_captured: bool,
 }
 
 impl<'a, DB: WriteResults + 'a> PrepareCrate<'a, DB> {
- pub(super) fn new(
- experiment: &'a Experiment,
- krate: &'a Crate,
- config: &'a Config,
- db: &'a DB,
- ) -> Self {
+ pub(super) fn new(experiment: &'a Experiment, krate: &'a Crate, db: &'a DB) -> Self {
  let source_dirs = experiment
  .toolchains
  .iter()
@@ -35,21 +29,21 @@ impl<'a, DB: WriteResults + 'a> PrepareCrate<'a, DB> {
  PrepareCrate {
  experiment,
  krate,
- config,
  db,
  source_dirs,
+ lockfile_captured: false,
  }
  }
 
- pub(super) fn prepare(&self) -> Fallible<()> {
+ pub(super) fn prepare(&mut self) -> Fallible<()> {
  self.krate.fetch()?;
  for (_, source_dir) in &self.source_dirs {
  self.krate.copy_to(source_dir)?;
  }
  self.capture_sha()?;
  self.validate_manifest()?;
  self.frob_toml()?;
- self.capture_lockfile()?;
+ self.capture_lockfile(false)?;
  self.fetch_deps()?;
  Ok(())
  }
@@ -121,13 +115,13 @@ impl<'a, DB: WriteResults + 'a> PrepareCrate<'a, DB> {
  Ok(())
  }
 
- fn capture_lockfile(&self) -> Fallible<()> {
+ fn capture_lockfile(&mut self, force: bool) -> Fallible<()> {
  for (toolchain, source_dir) in &self.source_dirs {
- if !self.config.should_update_lockfile(&self.krate)
- && self.krate.is_repo()
- && source_dir.join("Cargo.lock").exists()
- {
- info!("crate {} has a lockfile. skipping", self.krate);
+ if !force && source_dir.join("Cargo.lock").exists() {
+ info!(
+  "crate {} already has a lockfile, it will not be regenerated",
+  self.krate
+ );
  return Ok(());
  }
 
@@ -140,16 +134,36 @@ impl<'a, DB: WriteResults + 'a> PrepareCrate<'a, DB> {
  ])
  .cd(source_dir)
  .run()?;
+ self.lockfile_captured = true;
  }
  Ok(())
  }
 
- fn fetch_deps(&self) -> Fallible<()> {
+ fn fetch_deps(&mut self) -> Fallible<()> {
  for (toolchain, source_dir) in &self.source_dirs {
- RunCommand::new(CARGO.toolchain(toolchain))
+ let mut outdated_lockfile = false;
+ let res = RunCommand::new(CARGO.toolchain(toolchain))
  .args(&["fetch", "--locked", "--manifest-path", "Cargo.toml"])
  .cd(source_dir)
- .run()?;
+ .process_lines(&mut |line| {
+ if line.ends_with(
+ "Cargo.lock needs to be updated but --locked was passed to prevent this",
+ ) {
+ outdated_lockfile = true;
+ }
+ })
+ .run();
+ match res {
+ Ok(_) => {}
+ Err(_) if outdated_lockfile && !self.lockfile_captured => {
+ info!("the lockfile is outdated, regenerating it");
+ // Force-update the lockfile and recursively call this function to fetch
+ // dependencies again.
+ self.capture_lockfile(true)?;
+ return self.fetch_deps();
+ }
+ err => return err,
+ }
  }
  Ok(())
  }

src/runner/tasks.rs

@@ -181,7 +181,7 @@ impl Task {
  .prepare_logs
  .insert(self.krate.clone(), storage.clone());
  logs::capture(&storage, || {
- let prepare = PrepareCrate::new(ex, &self.krate, config, db);
+ let mut prepare = PrepareCrate::new(ex, &self.krate, db);
  prepare.prepare()
  })?;
  }

src/tools/binary_crates.rs

@@ -18,7 +18,7 @@ impl Runnable for BinaryCrate {
  }
  }
 
- fn prepare_command(&self, mut cmd: RunCommand) -> RunCommand {
+ fn prepare_command<'pl>(&self, mut cmd: RunCommand<'pl>) -> RunCommand<'pl> {
  if let Some(subcommand) = self.cargo_subcommand {
  cmd = cmd.args(&[subcommand]);
  }

src/tools/rustup.rs

@@ -19,7 +19,7 @@ impl Runnable for Rustup {
  Binary::InstalledByCrater("rustup".into())
  }
 
- fn prepare_command(&self, cmd: RunCommand) -> RunCommand {
+ fn prepare_command<'pl>(&self, cmd: RunCommand<'pl>) -> RunCommand<'pl> {
  cmd.local_rustup(true)
  }
 }
@@ -112,7 +112,7 @@ impl<'a> Runnable for Cargo<'a> {
  Binary::InstalledByCrater("cargo".into())
  }
 
- fn prepare_command(&self, mut cmd: RunCommand) -> RunCommand {
+ fn prepare_command<'pl>(&self, mut cmd: RunCommand<'pl>) -> RunCommand<'pl> {
  if self.unstable_features {
  cmd = cmd.env("__CARGO_TEST_CHANNEL_OVERRIDE_DO_NOT_USE_THIS", "nightly");
  }