From b49a493e91d889f243bc4ef6a06fe93c379124dc Mon Sep 17 00:00:00 2001 From: Vhyrro Date: Wed, 23 Oct 2024 17:39:41 +0200 Subject: [PATCH 1/2] fix: don't allow installation of conflicting rocks --- rocks-bin/src/build.rs | 47 ++++++++++++++++-- rocks-bin/src/install.rs | 41 ++++++++++++++-- rocks-lib/src/build/mod.rs | 74 +++++++++++++++++++---------- rocks-lib/src/lockfile/mod.rs | 2 +- rocks-lib/src/operations/install.rs | 19 ++++++-- rocks-lib/src/operations/pin.rs | 12 +++++ rocks-lib/src/operations/update.rs | 19 +++++--- rocks-lib/tests/build.rs | 4 +- 8 files changed, 170 insertions(+), 48 deletions(-) diff --git a/rocks-bin/src/build.rs b/rocks-bin/src/build.rs index e21b0f19..fdfc8fa0 100644 --- a/rocks-bin/src/build.rs +++ b/rocks-bin/src/build.rs @@ -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, }; @@ -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. @@ -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 @@ -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?; diff --git a/rocks-bin/src/install.rs b/rocks-bin/src/install.rs index 797ec363..63512b69 100644 --- a/rocks-bin/src/install.rs +++ b/rocks-bin/src/install.rs @@ -1,6 +1,13 @@ 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 { @@ -8,14 +15,40 @@ pub struct Install { #[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?; diff --git a/rocks-lib/src/build/mod.rs b/rocks-lib/src/build/mod.rs index 92540ed9..aae33aac 100644 --- a/rocks-lib/src/build/mod.rs +++ b/rocks-lib/src/build/mod.rs @@ -38,6 +38,22 @@ pub enum BuildError { FetchSrcRockError(#[from] FetchSrcRockError), } +#[derive(Copy, Clone, PartialEq, Eq)] +pub enum BuildBehaviour { + NoForce, + Force, +} + +impl From for BuildBehaviour { + fn from(value: bool) -> Self { + if value { + Self::Force + } else { + Self::NoForce + } + } +} + async fn run_build( progress: &MultiProgress, rockspec: &Rockspec, @@ -128,6 +144,7 @@ pub async fn build( rockspec: Rockspec, pinned: PinnedState, constraint: LockConstraint, + behaviour: BuildBehaviour, config: &Config, ) -> Result { let lua_version = rockspec.lua_version().or_default_from(config)?; @@ -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) } diff --git a/rocks-lib/src/lockfile/mod.rs b/rocks-lib/src/lockfile/mod.rs index ae557b29..dd07b9a3 100644 --- a/rocks-lib/src/lockfile/mod.rs +++ b/rocks-lib/src/lockfile/mod.rs @@ -25,7 +25,7 @@ impl From for PinnedState { } impl PinnedState { - fn as_bool(&self) -> bool { + pub fn as_bool(&self) -> bool { match self { Self::Unpinned => false, Self::Pinned => true, diff --git a/rocks-lib/src/operations/install.rs b/rocks-lib/src/operations/install.rs index e206f1cd..05087d8f 100644 --- a/rocks-lib/src/operations/install.rs +++ b/rocks-lib/src/operations/install.rs @@ -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}, @@ -31,12 +31,13 @@ pub async fn install( progress: &MultiProgress, package_req: PackageReq, pin: PinnedState, + build_behaviour: BuildBehaviour, config: &Config, ) -> Result { 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 } @@ -45,6 +46,7 @@ async fn install_impl( progress: &MultiProgress, package_req: PackageReq, pin: PinnedState, + build_behaviour: BuildBehaviour, config: &Config, ) -> Result { let rockspec = super::download_rockspec(progress, &package_req, config).await?; @@ -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 { diff --git a/rocks-lib/src/operations/pin.rs b/rocks-lib/src/operations/pin.rs index 78885cbe..5813f723 100644 --- a/rocks-lib/src/operations/pin.rs +++ b/rocks-lib/src/operations/pin.rs @@ -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}")] @@ -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)?; diff --git a/rocks-lib/src/operations/update.rs b/rocks-lib/src/operations/update.rs index 6f3e8578..0fb0131d 100644 --- a/rocks-lib/src/operations/update.rs +++ b/rocks-lib/src/operations/update.rs @@ -2,6 +2,7 @@ use indicatif::MultiProgress; use thiserror::Error; use crate::{ + build::BuildBehaviour, config::Config, lockfile::{LocalPackage, PinnedState}, manifest::ManifestMetadata, @@ -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) diff --git a/rocks-lib/tests/build.rs b/rocks-lib/tests/build.rs index 08496edf..cc478bff 100644 --- a/rocks-lib/tests/build.rs +++ b/rocks-lib/tests/build.rs @@ -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, @@ -27,6 +27,7 @@ async fn builtin_build() { rockspec, Unpinned, Unconstrained, + Force, &config, ) .await @@ -54,6 +55,7 @@ async fn make_build() { rockspec, Unpinned, Unconstrained, + Force, &config, ) .await From 717a4d5d54ee61b1a5c72b9ad8ad439021e2acc7 Mon Sep 17 00:00:00 2001 From: Vhyrro Date: Wed, 23 Oct 2024 18:04:48 +0200 Subject: [PATCH 2/2] feat: `rocks add` --- rocks-bin/src/add.rs | 13 ++++++++++ rocks-bin/src/main.rs | 5 +++- rocks-lib/src/project/mod.rs | 40 ++++++++++++++++++++++++++++++ rocks-lib/src/rockspec/platform.rs | 2 +- 4 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 rocks-bin/src/add.rs diff --git a/rocks-bin/src/add.rs b/rocks-bin/src/add.rs new file mode 100644 index 00000000..a3b1604a --- /dev/null +++ b/rocks-bin/src/add.rs @@ -0,0 +1,13 @@ +use clap::Args; +use eyre::{bail, OptionExt, Result}; +use rocks_lib::{config::Config, project::Project}; + +#[derive(Args)] +pub struct Add {} + +pub fn add(data: Add, config: Config) -> Result<()> { + let mut project = Project::current()?.ok_or_eyre("Unable to add dependency - current directory does not belong to a Lua project. Run `rocks new` to create one.")?; + project.rockspec_mut().dependencies.push("what the dog doing".to_string()); + + Ok(()) +} diff --git a/rocks-bin/src/main.rs b/rocks-bin/src/main.rs index 125036cc..fb1b923d 100644 --- a/rocks-bin/src/main.rs +++ b/rocks-bin/src/main.rs @@ -1,6 +1,7 @@ use crate::project::write_new::NewProject; use std::{path::PathBuf, time::Duration}; +use add::Add; use build::Build; use clap::{Parser, Subcommand}; use debug::Debug; @@ -18,6 +19,7 @@ use rocks_lib::{ use search::Search; use update::Update; +mod add; mod build; mod debug; mod download; @@ -100,7 +102,7 @@ struct Cli { #[derive(Subcommand)] enum Commands { /// Add a dependency to the current project. - Add, + Add(Add), /// Build/compile a rock. Build(Build), /// Query information about Rocks's configuration. @@ -208,6 +210,7 @@ async fn main() { Commands::Info(info_data) => info::info(info_data, config).await.unwrap(), Commands::Pin(pin_data) => pin::set_pinned_state(pin_data, config, Pinned).unwrap(), Commands::Unpin(pin_data) => pin::set_pinned_state(pin_data, config, Unpinned).unwrap(), + Commands::Add(add_data) => add::add(add_data, config).unwrap(), _ => unimplemented!(), } } diff --git a/rocks-lib/src/project/mod.rs b/rocks-lib/src/project/mod.rs index 84e40f88..9707fce1 100644 --- a/rocks-lib/src/project/mod.rs +++ b/rocks-lib/src/project/mod.rs @@ -1,4 +1,6 @@ use lets_find_up::{find_up_with, FindUpKind, FindUpOptions}; +use mlua::{Lua, LuaSerdeExt}; +use serde::{Deserialize, Serialize}; use std::{ io, path::{Path, PathBuf}, @@ -24,6 +26,27 @@ pub struct Project { root: PathBuf, /// The parsed rockspec. rockspec: Rockspec, + /// A partial rockspec representation for write operations + partial_rockspec: PartialRockspec, +} + +#[derive(Debug, Serialize)] +pub struct PartialRockspec { + pub dependencies: Vec, +} + +impl PartialRockspec { + // TODO: Don't propagate different error types + pub fn new(rockspec_content: &str) -> mlua::Result { + let lua = Lua::new(); + lua.load(dbg!(rockspec_content)).exec()?; + + let globals = lua.globals(); + + Ok(PartialRockspec { + dependencies: globals.get("dependencies")?, + }) + } } impl Project { @@ -41,6 +64,7 @@ impl Project { )? { Some(path) => { let rockspec_content = std::fs::read_to_string(&path)?; + let partial_rockspec = PartialRockspec::new(&rockspec_content).unwrap(); let rockspec = Rockspec::new(&rockspec_content)?; let root = path.parent().unwrap(); @@ -50,11 +74,23 @@ impl Project { Ok(Some(Project { root: root.to_path_buf(), rockspec, + partial_rockspec, })) } None => Ok(None), } } + + pub fn flush(&mut self) { + let lua = Lua::new(); + std::fs::write(self.root().join("project.rockspec"), lua.to_value(&self.partial_rockspec).unwrap().to_string().unwrap()).unwrap(); + } +} + +impl Drop for Project { + fn drop(&mut self) { + let _ = self.flush(); + } } impl Project { @@ -66,6 +102,10 @@ impl Project { &self.rockspec } + pub fn rockspec_mut(&mut self) -> &mut PartialRockspec { + &mut self.partial_rockspec + } + pub fn tree(&self, lua_version: LuaVersion) -> io::Result { Tree::new(self.root.clone(), lua_version) } diff --git a/rocks-lib/src/rockspec/platform.rs b/rocks-lib/src/rockspec/platform.rs index a822cf57..27d8c319 100644 --- a/rocks-lib/src/rockspec/platform.rs +++ b/rocks-lib/src/rockspec/platform.rs @@ -7,7 +7,7 @@ use thiserror::Error; use serde::{ de::{self, DeserializeOwned}, - Deserialize, Deserializer, + Deserialize, Deserializer, Serialize, }; use serde_enum_str::{Deserialize_enum_str, Serialize_enum_str};