From 42c9dfa840f957b722cf8689a0c3691fc8e2c6e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20D=C3=BCrr?= <102963075+cd-work@users.noreply.github.com> Date: Fri, 15 Sep 2023 20:10:48 +0000 Subject: [PATCH] Fix cargo lockfile generation (#1233) When generating a lockfile for a Cargo manifest, it is possible that the lockfile will be placed in a parent directory rather than the manifest's directory. Since this wasn't accounted for, lockfile generation would fail and the lockfile would not get cleaned up. To allow tracking lockfiles generated in directories above the manifest, the `locate-project` subcommand of cargo is used now to locate the place where the lockfile will be put. --- CHANGELOG.md | 3 +++ Cargo.lock | 7 +++--- lockfile_generator/Cargo.toml | 1 + lockfile_generator/src/bundler.rs | 11 +++++---- lockfile_generator/src/cargo.rs | 37 +++++++++++++++++++++++++++---- lockfile_generator/src/dotnet.rs | 11 +++++---- lockfile_generator/src/go.rs | 11 +++++---- lockfile_generator/src/gradle.rs | 11 +++++---- lockfile_generator/src/lib.rs | 23 +++++++++++++------ lockfile_generator/src/maven.rs | 16 ++++++++----- lockfile_generator/src/npm.rs | 19 +++++++++++----- lockfile_generator/src/pip.rs | 4 ++-- lockfile_generator/src/pipenv.rs | 11 +++++---- lockfile_generator/src/pnpm.rs | 11 +++++---- lockfile_generator/src/poetry.rs | 11 +++++---- lockfile_generator/src/yarn.rs | 9 +++++--- 16 files changed, 137 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f1b27185..8ac0b5f14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [Unreleased] +### Fixed +- Cargo manifest generation for workspaces + ## [5.7.1] - 2023-09-08 ### Fixed diff --git a/Cargo.lock b/Cargo.lock index d5a8fa41c..490916367 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -535,15 +535,15 @@ checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" [[package]] name = "chrono" -version = "0.4.30" +version = "0.4.26" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "defd4e7873dbddba6c7c91e199c7fcb946abc4a6a4ac3195400bcfb01b5de877" +checksum = "ec837a71355b28f6556dbd569b37b3f363091c0bd4b2e735674521b4c5fd9bc5" dependencies = [ "android-tzdata", "iana-time-zone", "num-traits", "serde", - "windows-targets 0.48.5", + "winapi", ] [[package]] @@ -3001,6 +3001,7 @@ dependencies = [ name = "lockfile_generator" version = "0.1.0" dependencies = [ + "anyhow", "serde", "serde_json", ] diff --git a/lockfile_generator/Cargo.toml b/lockfile_generator/Cargo.toml index 3490aa5d4..5ade1a098 100644 --- a/lockfile_generator/Cargo.toml +++ b/lockfile_generator/Cargo.toml @@ -9,3 +9,4 @@ rust-version = "1.68.0" [dependencies] serde = { version = "1.0.163", features = ["derive"] } serde_json = "1.0.96" +anyhow = "1.0.75" diff --git a/lockfile_generator/src/bundler.rs b/lockfile_generator/src/bundler.rs index 8d87a3506..1e9dde769 100644 --- a/lockfile_generator/src/bundler.rs +++ b/lockfile_generator/src/bundler.rs @@ -1,15 +1,18 @@ //! Ruby bundler ecosystem. -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::Command; -use crate::Generator; +use crate::{Error, Generator, Result}; pub struct Bundler; impl Generator for Bundler { - fn lockfile_name(&self) -> &'static str { - "Gemfile.lock" + fn lockfile_path(&self, manifest_path: &Path) -> Result { + let project_path = manifest_path + .parent() + .ok_or_else(|| Error::InvalidManifest(manifest_path.to_path_buf()))?; + Ok(project_path.join("Gemfile.lock")) } fn command(&self, _manifest_path: &Path) -> Command { diff --git a/lockfile_generator/src/cargo.rs b/lockfile_generator/src/cargo.rs index 7a7795249..30f53f631 100644 --- a/lockfile_generator/src/cargo.rs +++ b/lockfile_generator/src/cargo.rs @@ -1,15 +1,38 @@ //! Rust cargo ecosystem. -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::Command; -use crate::Generator; +use serde::Deserialize; + +use crate::{Error, Generator, Result}; pub struct Cargo; impl Generator for Cargo { - fn lockfile_name(&self) -> &'static str { - "Cargo.lock" + fn lockfile_path(&self, manifest_path: &Path) -> Result { + let manifest_arg = format!("--manifest-path={}", manifest_path.display()); + let output = Command::new("cargo") + .args(["locate-project", &manifest_arg, "--workspace"]) + .output()?; + + // Ensure command was successful. + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(Error::NonZeroExit(output.status.code(), stderr.into())); + } + + // Parse metadata output. + let stdout = String::from_utf8(output.stdout).map_err(Error::InvalidUtf8)?; + let project_location: ProjectLocation = serde_json::from_str(&stdout)?; + + // Go from root manifest to root project. + let workspace_root = match project_location.root.parent() { + Some(workspace_root) => workspace_root, + None => return Err(Error::InvalidManifest(project_location.root.clone())), + }; + + Ok(workspace_root.join("Cargo.lock")) } fn command(&self, _manifest_path: &Path) -> Command { @@ -22,3 +45,9 @@ impl Generator for Cargo { "Cargo" } } + +/// Output of `cargo locate-project`. +#[derive(Deserialize)] +struct ProjectLocation { + root: PathBuf, +} diff --git a/lockfile_generator/src/dotnet.rs b/lockfile_generator/src/dotnet.rs index 67d3e5aee..f495bc889 100644 --- a/lockfile_generator/src/dotnet.rs +++ b/lockfile_generator/src/dotnet.rs @@ -1,15 +1,18 @@ //! C# .NET ecosystem. -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::Command; -use crate::Generator; +use crate::{Error, Generator, Result}; pub struct Dotnet; impl Generator for Dotnet { - fn lockfile_name(&self) -> &'static str { - "packages.lock.json" + fn lockfile_path(&self, manifest_path: &Path) -> Result { + let project_path = manifest_path + .parent() + .ok_or_else(|| Error::InvalidManifest(manifest_path.to_path_buf()))?; + Ok(project_path.join("packages.lock.json")) } fn command(&self, manifest_path: &Path) -> Command { diff --git a/lockfile_generator/src/go.rs b/lockfile_generator/src/go.rs index 6c69f7985..f5798a426 100644 --- a/lockfile_generator/src/go.rs +++ b/lockfile_generator/src/go.rs @@ -1,15 +1,18 @@ //! Go ecosystem. -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::Command; -use crate::Generator; +use crate::{Error, Generator, Result}; pub struct Go; impl Generator for Go { - fn lockfile_name(&self) -> &'static str { - "go.sum" + fn lockfile_path(&self, manifest_path: &Path) -> Result { + let project_path = manifest_path + .parent() + .ok_or_else(|| Error::InvalidManifest(manifest_path.to_path_buf()))?; + Ok(project_path.join("go.sum")) } fn command(&self, _manifest_path: &Path) -> Command { diff --git a/lockfile_generator/src/gradle.rs b/lockfile_generator/src/gradle.rs index 3b0f9f70e..7f65d7863 100644 --- a/lockfile_generator/src/gradle.rs +++ b/lockfile_generator/src/gradle.rs @@ -1,15 +1,18 @@ //! Java gradle ecosystem. -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::Command; -use crate::Generator; +use crate::{Error, Generator, Result}; pub struct Gradle; impl Generator for Gradle { - fn lockfile_name(&self) -> &'static str { - "gradle.lockfile" + fn lockfile_path(&self, manifest_path: &Path) -> Result { + let project_path = manifest_path + .parent() + .ok_or_else(|| Error::InvalidManifest(manifest_path.to_path_buf()))?; + Ok(project_path.join("gradle.lockfile")) } fn command(&self, _manifest_path: &Path) -> Command { diff --git a/lockfile_generator/src/lib.rs b/lockfile_generator/src/lib.rs index d98bb5339..98dfad681 100644 --- a/lockfile_generator/src/lib.rs +++ b/lockfile_generator/src/lib.rs @@ -23,8 +23,8 @@ pub mod yarn; /// Lockfile generation. pub trait Generator { - /// Lockfile file name. - fn lockfile_name(&self) -> &'static str; + /// Lockfile path. + fn lockfile_path(&self, manifest_path: &Path) -> Result; /// Command for generating the lockfile. fn command(&self, manifest_path: &Path) -> Command; @@ -36,8 +36,8 @@ pub trait Generator { /// /// These files are temporarily renamed during lockfile generation to ensure /// the correct lockfile is updated. - fn conflicting_files(&self) -> Vec<&'static str> { - vec![self.lockfile_name()] + fn conflicting_files(&self, manifest_path: &Path) -> Result> { + Ok(vec![self.lockfile_path(manifest_path)?]) } /// Verify that all the prerequisites for lockfile generation are met. @@ -59,9 +59,9 @@ pub trait Generator { // Move files which interfere with lockfile generation. let _relocators = self - .conflicting_files() + .conflicting_files(&canonicalized)? .drain(..) - .map(|file| FileRelocator::new(project_path.join(file))) + .map(FileRelocator::new) .collect::>>()?; // Generate lockfile at the target location. @@ -84,7 +84,7 @@ pub trait Generator { } // Ensure lockfile was created. - let lockfile_path = project_path.join(self.lockfile_name()); + let lockfile_path = self.lockfile_path(&canonicalized)?; if !lockfile_path.exists() { return Err(Error::NoLockfileGenerated); } @@ -145,6 +145,7 @@ pub enum Error { InvalidManifest(PathBuf), PipReportVersionMismatch(&'static str, String), UnsupportedCommandVersion(&'static str, &'static str, String), + Anyhow(anyhow::Error), NoLockfileGenerated, } @@ -155,6 +156,7 @@ impl StdError for Error { Self::Json(err) => Some(err), Self::Io(err) => Some(err), Self::ProcessCreation(_, _, err) => Some(err), + Self::Anyhow(err) => err.source(), _ => None, } } @@ -183,6 +185,7 @@ impl Display for Error { write!(f, "unsupported {command:?} version {received:?}, expected {expected:?}") }, Self::NoLockfileGenerated => write!(f, "no lockfile was generated"), + Self::Anyhow(err) => write!(f, "{err}"), } } } @@ -204,3 +207,9 @@ impl From for Error { Self::Json(err) } } + +impl From for Error { + fn from(err: anyhow::Error) -> Self { + Self::Anyhow(err) + } +} diff --git a/lockfile_generator/src/maven.rs b/lockfile_generator/src/maven.rs index 3ee04ba71..5ef733a50 100644 --- a/lockfile_generator/src/maven.rs +++ b/lockfile_generator/src/maven.rs @@ -1,20 +1,24 @@ //! Java maven ecosystem. -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::Command; -use crate::Generator; +use crate::{Error, Generator, Result}; pub struct Maven; impl Generator for Maven { - fn lockfile_name(&self) -> &'static str { - "effective-pom.xml" + fn lockfile_path(&self, manifest_path: &Path) -> Result { + let project_path = manifest_path + .parent() + .ok_or_else(|| Error::InvalidManifest(manifest_path.to_path_buf()))?; + Ok(project_path.join("effective-pom.xml")) } - fn command(&self, _manifest_path: &Path) -> Command { + fn command(&self, manifest_path: &Path) -> Command { + let lockfile_path = self.lockfile_path(manifest_path).unwrap(); let mut command = Command::new("mvn"); - command.args(["help:effective-pom", &format!("-Doutput={}", self.lockfile_name())]); + command.args(["help:effective-pom", &format!("-Doutput={}", lockfile_path.display())]); command } diff --git a/lockfile_generator/src/npm.rs b/lockfile_generator/src/npm.rs index 117950ed4..2c5cfd0e9 100644 --- a/lockfile_generator/src/npm.rs +++ b/lockfile_generator/src/npm.rs @@ -1,19 +1,26 @@ //! JavaScript npm ecosystem. -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::Command; -use crate::Generator; +use crate::{Error, Generator, Result}; pub struct Npm; impl Generator for Npm { - fn lockfile_name(&self) -> &'static str { - "package-lock.json" + fn lockfile_path(&self, manifest_path: &Path) -> Result { + let project_path = manifest_path + .parent() + .ok_or_else(|| Error::InvalidManifest(manifest_path.to_path_buf()))?; + Ok(project_path.join("package-lock.json")) } - fn conflicting_files(&self) -> Vec<&'static str> { - vec![self.lockfile_name(), "npm-shrinkwrap.json", "yarn.lock"] + fn conflicting_files(&self, manifest_path: &Path) -> Result> { + Ok(vec![ + self.lockfile_path(manifest_path)?, + PathBuf::from("npm-shrinkwrap.json"), + PathBuf::from("yarn.lock"), + ]) } fn command(&self, _manifest_path: &Path) -> Command { diff --git a/lockfile_generator/src/pip.rs b/lockfile_generator/src/pip.rs index 1dc576cbc..d26ff8f5b 100644 --- a/lockfile_generator/src/pip.rs +++ b/lockfile_generator/src/pip.rs @@ -2,7 +2,7 @@ use std::fmt::Write; use std::fs; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use serde::Deserialize; @@ -12,7 +12,7 @@ use crate::{Error, Generator, Result}; pub struct Pip; impl Generator for Pip { - fn lockfile_name(&self) -> &'static str { + fn lockfile_path(&self, _manifest_path: &Path) -> Result { // NOTE: Pip's `generate_lockfile` will never write to disk. unreachable!() } diff --git a/lockfile_generator/src/pipenv.rs b/lockfile_generator/src/pipenv.rs index 21e19282b..891555a91 100644 --- a/lockfile_generator/src/pipenv.rs +++ b/lockfile_generator/src/pipenv.rs @@ -1,15 +1,18 @@ //! Python pipenv ecosystem. -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::Command; -use crate::Generator; +use crate::{Error, Generator, Result}; pub struct Pipenv; impl Generator for Pipenv { - fn lockfile_name(&self) -> &'static str { - "Pipfile.lock" + fn lockfile_path(&self, manifest_path: &Path) -> Result { + let project_path = manifest_path + .parent() + .ok_or_else(|| Error::InvalidManifest(manifest_path.to_path_buf()))?; + Ok(project_path.join("Pipfile.lock")) } fn command(&self, _manifest_path: &Path) -> Command { diff --git a/lockfile_generator/src/pnpm.rs b/lockfile_generator/src/pnpm.rs index cd811f624..43d0ce35e 100644 --- a/lockfile_generator/src/pnpm.rs +++ b/lockfile_generator/src/pnpm.rs @@ -1,15 +1,18 @@ //! JavaScript pnpm ecosystem. -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::Command; -use crate::Generator; +use crate::{Error, Generator, Result}; pub struct Pnpm; impl Generator for Pnpm { - fn lockfile_name(&self) -> &'static str { - "pnpm-lock.yaml" + fn lockfile_path(&self, manifest_path: &Path) -> Result { + let project_path = manifest_path + .parent() + .ok_or_else(|| Error::InvalidManifest(manifest_path.to_path_buf()))?; + Ok(project_path.join("pnpm-lock.yaml")) } fn command(&self, _manifest_path: &Path) -> Command { diff --git a/lockfile_generator/src/poetry.rs b/lockfile_generator/src/poetry.rs index f4700ecb8..ca0efd9a6 100644 --- a/lockfile_generator/src/poetry.rs +++ b/lockfile_generator/src/poetry.rs @@ -1,15 +1,18 @@ //! Python poetry ecosystem. -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::Command; -use crate::Generator; +use crate::{Error, Generator, Result}; pub struct Poetry; impl Generator for Poetry { - fn lockfile_name(&self) -> &'static str { - "poetry.lock" + fn lockfile_path(&self, manifest_path: &Path) -> Result { + let project_path = manifest_path + .parent() + .ok_or_else(|| Error::InvalidManifest(manifest_path.to_path_buf()))?; + Ok(project_path.join("poetry.lock")) } fn command(&self, _manifest_path: &Path) -> Command { diff --git a/lockfile_generator/src/yarn.rs b/lockfile_generator/src/yarn.rs index f4a57314c..c971d1e5a 100644 --- a/lockfile_generator/src/yarn.rs +++ b/lockfile_generator/src/yarn.rs @@ -1,7 +1,7 @@ //! JavaScript yarn ecosystem. use std::fs; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::Command; use crate::{Error, Generator, Result}; @@ -9,8 +9,11 @@ use crate::{Error, Generator, Result}; pub struct Yarn; impl Generator for Yarn { - fn lockfile_name(&self) -> &'static str { - "yarn.lock" + fn lockfile_path(&self, manifest_path: &Path) -> Result { + let project_path = manifest_path + .parent() + .ok_or_else(|| Error::InvalidManifest(manifest_path.to_path_buf()))?; + Ok(project_path.join("yarn.lock")) } fn command(&self, _manifest_path: &Path) -> Command {