Skip to content

Commit

Permalink
Fix cargo lockfile generation (#1233)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
cd-work authored Sep 15, 2023
1 parent d488884 commit 42c9dfa
Show file tree
Hide file tree
Showing 16 changed files with 137 additions and 59 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions lockfile_generator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
11 changes: 7 additions & 4 deletions lockfile_generator/src/bundler.rs
Original file line number Diff line number Diff line change
@@ -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<PathBuf> {
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 {
Expand Down
37 changes: 33 additions & 4 deletions lockfile_generator/src/cargo.rs
Original file line number Diff line number Diff line change
@@ -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<PathBuf> {
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 {
Expand All @@ -22,3 +45,9 @@ impl Generator for Cargo {
"Cargo"
}
}

/// Output of `cargo locate-project`.
#[derive(Deserialize)]
struct ProjectLocation {
root: PathBuf,
}
11 changes: 7 additions & 4 deletions lockfile_generator/src/dotnet.rs
Original file line number Diff line number Diff line change
@@ -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<PathBuf> {
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 {
Expand Down
11 changes: 7 additions & 4 deletions lockfile_generator/src/go.rs
Original file line number Diff line number Diff line change
@@ -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<PathBuf> {
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 {
Expand Down
11 changes: 7 additions & 4 deletions lockfile_generator/src/gradle.rs
Original file line number Diff line number Diff line change
@@ -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<PathBuf> {
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 {
Expand Down
23 changes: 16 additions & 7 deletions lockfile_generator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<PathBuf>;

/// Command for generating the lockfile.
fn command(&self, manifest_path: &Path) -> Command;
Expand All @@ -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<Vec<PathBuf>> {
Ok(vec![self.lockfile_path(manifest_path)?])
}

/// Verify that all the prerequisites for lockfile generation are met.
Expand All @@ -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::<Result<Vec<_>>>()?;

// Generate lockfile at the target location.
Expand All @@ -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);
}
Expand Down Expand Up @@ -145,6 +145,7 @@ pub enum Error {
InvalidManifest(PathBuf),
PipReportVersionMismatch(&'static str, String),
UnsupportedCommandVersion(&'static str, &'static str, String),
Anyhow(anyhow::Error),
NoLockfileGenerated,
}

Expand All @@ -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,
}
}
Expand Down Expand Up @@ -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}"),
}
}
}
Expand All @@ -204,3 +207,9 @@ impl From<JsonError> for Error {
Self::Json(err)
}
}

impl From<anyhow::Error> for Error {
fn from(err: anyhow::Error) -> Self {
Self::Anyhow(err)
}
}
16 changes: 10 additions & 6 deletions lockfile_generator/src/maven.rs
Original file line number Diff line number Diff line change
@@ -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<PathBuf> {
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
}

Expand Down
19 changes: 13 additions & 6 deletions lockfile_generator/src/npm.rs
Original file line number Diff line number Diff line change
@@ -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<PathBuf> {
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<Vec<PathBuf>> {
Ok(vec![
self.lockfile_path(manifest_path)?,
PathBuf::from("npm-shrinkwrap.json"),
PathBuf::from("yarn.lock"),
])
}

fn command(&self, _manifest_path: &Path) -> Command {
Expand Down
4 changes: 2 additions & 2 deletions lockfile_generator/src/pip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<PathBuf> {
// NOTE: Pip's `generate_lockfile` will never write to disk.
unreachable!()
}
Expand Down
11 changes: 7 additions & 4 deletions lockfile_generator/src/pipenv.rs
Original file line number Diff line number Diff line change
@@ -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<PathBuf> {
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 {
Expand Down
11 changes: 7 additions & 4 deletions lockfile_generator/src/pnpm.rs
Original file line number Diff line number Diff line change
@@ -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<PathBuf> {
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 {
Expand Down
Loading

0 comments on commit 42c9dfa

Please sign in to comment.