Skip to content

Commit

Permalink
fix: don't allow installation of conflicting rocks
Browse files Browse the repository at this point in the history
  • Loading branch information
vhyrro committed Oct 28, 2024
1 parent c622c96 commit b49a493
Show file tree
Hide file tree
Showing 8 changed files with 170 additions and 48 deletions.
47 changes: 42 additions & 5 deletions rocks-bin/src/build.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
use eyre::eyre;
use indicatif::{MultiProgress, ProgressBar};
use inquire::Confirm;
use itertools::Itertools;
use std::path::PathBuf;

use clap::Args;
use eyre::Result;
use rocks_lib::{
build::BuildBehaviour,
config::{Config, DefaultFromConfig as _},
lockfile::LockConstraint::Unconstrained,
package::PackageName,
lockfile::{LockConstraint::Unconstrained, PinnedState},
package::{PackageName, PackageReq},
rockspec::Rockspec,
tree::Tree,
};
Expand All @@ -19,9 +21,14 @@ pub struct Build {

#[arg(long)]
pin: bool,

#[arg(long)]
force: bool,
}

pub async fn build(data: Build, config: Config) -> Result<()> {
let pin = PinnedState::from(data.pin);

let rockspec_path = data.rockspec_path.map_or_else(|| {
// Try to infer the rockspec the user meant.

Expand Down Expand Up @@ -64,6 +71,29 @@ pub async fn build(data: Build, config: Config) -> Result<()> {

let tree = Tree::new(config.tree().clone(), lua_version)?;

let build_behaviour = match tree.has_rock_and(
&PackageReq::new(
rockspec.package.to_string(),
Some(rockspec.version.to_string()),
)?,
|rock| pin == rock.pinned(),
) {
Some(_) if !data.force => {
if Confirm::new(&format!(
"Package {} already exists. Overwrite?",
rockspec.package,
))
.with_default(false)
.prompt()?
{
BuildBehaviour::Force
} else {
return Ok(());
}
}
_ => BuildBehaviour::from(data.force),
};

// Ensure all dependencies are installed first
let dependencies = rockspec
.dependencies
Expand All @@ -79,16 +109,23 @@ pub async fn build(data: Build, config: Config) -> Result<()> {
.filter(|req| tree.has_rock(req).is_none())
.enumerate()
{
rocks_lib::operations::install(&progress, dependency_req.clone(), data.pin.into(), &config)
.await?;
rocks_lib::operations::install(
&progress,
dependency_req.clone(),
pin,
build_behaviour,
&config,
)
.await?;
bar.set_position(index as u64);
}

rocks_lib::build::build(
&MultiProgress::new(),
rockspec,
data.pin.into(),
pin,
Unconstrained,
build_behaviour,
&config,
)
.await?;
Expand Down
41 changes: 37 additions & 4 deletions rocks-bin/src/install.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,54 @@
use eyre::Result;
use indicatif::MultiProgress;
use rocks_lib::{config::Config, package::PackageReq};
use inquire::Confirm;
use rocks_lib::{
build::BuildBehaviour,
config::{Config, LuaVersion},
lockfile::PinnedState,
package::PackageReq,
tree::Tree,
};

#[derive(clap::Args)]
pub struct Install {
package_req: PackageReq,

#[arg(long)]
pin: bool,

#[arg(long)]
force: bool,
}

pub async fn install(install_data: Install, config: Config) -> Result<()> {
pub async fn install(data: Install, config: Config) -> Result<()> {
let pin = PinnedState::from(data.pin);

let lua_version = LuaVersion::from(&config)?;
let tree = Tree::new(config.tree().clone(), lua_version)?;

let build_behaviour = match tree.has_rock_and(&data.package_req, |rock| pin == rock.pinned()) {
Some(_) if !data.force => {
if Confirm::new(&format!(
"Package {} already exists. Overwrite?",
data.package_req
))
.with_default(false)
.prompt()?
{
BuildBehaviour::Force
} else {
return Ok(());
}
}
_ => BuildBehaviour::from(data.force),
};

// TODO(vhyrro): If the tree doesn't exist then error out.
rocks_lib::operations::install(
&MultiProgress::new(),
install_data.package_req,
install_data.pin.into(),
data.package_req,
pin,
build_behaviour,
&config,
)
.await?;
Expand Down
74 changes: 48 additions & 26 deletions rocks-lib/src/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,22 @@ pub enum BuildError {
FetchSrcRockError(#[from] FetchSrcRockError),
}

#[derive(Copy, Clone, PartialEq, Eq)]
pub enum BuildBehaviour {
NoForce,
Force,
}

impl From<bool> for BuildBehaviour {
fn from(value: bool) -> Self {
if value {
Self::Force
} else {
Self::NoForce
}
}
}

async fn run_build(
progress: &MultiProgress,
rockspec: &Rockspec,
Expand Down Expand Up @@ -128,6 +144,7 @@ pub async fn build(
rockspec: Rockspec,
pinned: PinnedState,
constraint: LockConstraint,
behaviour: BuildBehaviour,
config: &Config,
) -> Result<LocalPackage, BuildError> {
let lua_version = rockspec.lua_version().or_default_from(config)?;
Expand Down Expand Up @@ -163,33 +180,38 @@ pub async fn build(
);
package.pinned = pinned;

let output_paths = tree.rock(&package)?;

let lua = LuaInstallation::new(&lua_version, config);

let build_dir = match &rock_source.unpack_dir {
Some(unpack_dir) => temp_dir.path().join(unpack_dir),
None => temp_dir.path().into(),
};

run_build(progress, &rockspec, &output_paths, &lua, config, &build_dir).await?;

install(progress, &rockspec, &tree, &output_paths, &lua, &build_dir).await?;

// Copy over all `copy_directories` to their respective paths
for directory in &rockspec.build.current_platform().copy_directories {
for file in walkdir::WalkDir::new(build_dir.join(directory))
.into_iter()
.flatten()
{
if file.file_type().is_file() {
let filepath = file.path();
let target = output_paths.etc.join(filepath);
std::fs::create_dir_all(target.parent().unwrap())?;
std::fs::copy(filepath, target)?;
match tree.lockfile()?.get(&package.id()) {
Some(package) if behaviour == BuildBehaviour::NoForce => Ok(package.clone()),
_ => {
let output_paths = tree.rock(&package)?;

let lua = LuaInstallation::new(&lua_version, config);

let build_dir = match &rock_source.unpack_dir {
Some(unpack_dir) => temp_dir.path().join(unpack_dir),
None => temp_dir.path().into(),
};

run_build(progress, &rockspec, &output_paths, &lua, config, &build_dir).await?;

install(progress, &rockspec, &tree, &output_paths, &lua, &build_dir).await?;

// Copy over all `copy_directories` to their respective paths
for directory in &rockspec.build.current_platform().copy_directories {
for file in walkdir::WalkDir::new(build_dir.join(directory))
.into_iter()
.flatten()
{
if file.file_type().is_file() {
let filepath = file.path();
let target = output_paths.etc.join(filepath);
std::fs::create_dir_all(target.parent().unwrap())?;
std::fs::copy(filepath, target)?;
}
}
}

Ok(package)
}
}

Ok(package)
}
2 changes: 1 addition & 1 deletion rocks-lib/src/lockfile/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ impl From<bool> for PinnedState {
}

impl PinnedState {
fn as_bool(&self) -> bool {
pub fn as_bool(&self) -> bool {
match self {
Self::Unpinned => false,
Self::Pinned => true,
Expand Down
19 changes: 14 additions & 5 deletions rocks-lib/src/operations/install.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::io;

use crate::{
build::BuildError,
build::{BuildBehaviour, BuildError},
config::{Config, DefaultFromConfig as _, LuaVersionUnset},
lockfile::{LocalPackage, LockConstraint, PinnedState},
package::{PackageName, PackageReq, PackageVersionReq},
Expand Down Expand Up @@ -31,12 +31,13 @@ pub async fn install(
progress: &MultiProgress,
package_req: PackageReq,
pin: PinnedState,
build_behaviour: BuildBehaviour,
config: &Config,
) -> Result<LocalPackage, InstallError> {
with_spinner(
progress,
format!("💻 Installing {}", package_req),
|| async { install_impl(progress, package_req, pin, config).await },
|| async { install_impl(progress, package_req, pin, build_behaviour, config).await },
)
.await
}
Expand All @@ -45,6 +46,7 @@ async fn install_impl(
progress: &MultiProgress,
package_req: PackageReq,
pin: PinnedState,
build_behaviour: BuildBehaviour,
config: &Config,
) -> Result<LocalPackage, InstallError> {
let rockspec = super::download_rockspec(progress, &package_req, config).await?;
Expand Down Expand Up @@ -76,14 +78,21 @@ async fn install_impl(
.filter(|req| tree.has_rock(req).is_none())
.enumerate()
{
let dependency =
crate::operations::install(progress, dependency_req.clone(), pin, config).await?;
let dependency = crate::operations::install(
progress,
dependency_req.clone(),
pin,
build_behaviour,
config,
)
.await?;

installed_dependencies.push(dependency);
bar.set_position(index as u64);
}

let package = crate::build::build(progress, rockspec, pin, constraint, config).await?;
let package =
crate::build::build(progress, rockspec, pin, constraint, build_behaviour, config).await?;

lockfile.add(&package);
for dependency in installed_dependencies {
Expand Down
12 changes: 12 additions & 0 deletions rocks-lib/src/operations/pin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ pub enum PinError {
pin_state: PinnedState,
rock: RemotePackage,
},
#[error("cannot change pin state of {rock}, since a second version of {rock} is already installed with `pin: {}`", .pin_state.as_bool())]
PinStateConflict {
pin_state: PinnedState,
rock: RemotePackage,
},
#[error(transparent)]
Io(#[from] io::Error),
#[error("failed to move old package: {0}")]
Expand Down Expand Up @@ -46,6 +51,13 @@ pub fn set_pinned_state(

package.pinned = pin;

if lockfile.get(&package.id()).is_some() {
return Err(PinError::PinStateConflict {
pin_state: package.pinned(),
rock: package.to_package(),
});
}

let new_root = tree.root_for(package);

std::fs::create_dir_all(&new_root)?;
Expand Down
19 changes: 13 additions & 6 deletions rocks-lib/src/operations/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use indicatif::MultiProgress;
use thiserror::Error;

use crate::{
build::BuildBehaviour,
config::Config,
lockfile::{LocalPackage, PinnedState},
manifest::ManifestMetadata,
Expand Down Expand Up @@ -46,12 +47,18 @@ pub async fn update(

if latest_version.is_some() && package.pinned() == PinnedState::Unpinned {
// Install the newest package.
install(progress, constraint, PinnedState::Unpinned, config)
.await
.map_err(|error| UpdateError::Install {
error,
package: package.to_package(),
})?;
install(
progress,
constraint,
PinnedState::Unpinned,
BuildBehaviour::NoForce,
config,
)
.await
.map_err(|error| UpdateError::Install {
error,
package: package.to_package(),
})?;

// Remove the old package
remove(progress, package.clone(), config)
Expand Down
4 changes: 3 additions & 1 deletion rocks-lib/tests/build.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use indicatif::MultiProgress;
use rocks_lib::{
build,
build::{self, BuildBehaviour::Force},
config::{ConfigBuilder, LuaVersion},
lockfile::{LockConstraint::Unconstrained, PinnedState::Unpinned},
rockspec::Rockspec,
Expand All @@ -27,6 +27,7 @@ async fn builtin_build() {
rockspec,
Unpinned,
Unconstrained,
Force,
&config,
)
.await
Expand Down Expand Up @@ -54,6 +55,7 @@ async fn make_build() {
rockspec,
Unpinned,
Unconstrained,
Force,
&config,
)
.await
Expand Down

0 comments on commit b49a493

Please sign in to comment.