fix: download engines eagerly

Previously, to download an engine pesde would require its linker to be executed. This caused issues if called concurrently, such as with scripts & required the user to manually execute the engine before usage. This change makes it so it is downloaded in the install step before any scripts which solves the issue.
2025-05-04 10:33:47 +01:00 · 2025-04-26 19:31:05 +02:00 · 2025-04-26 19:31:05 +02:00 · 398763a171
commit 398763a171
parent 63e43ff283
12 changed files with 240 additions and 179 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 - Add dev only installs in #32 by @Stefanuk12

+### Fixed
+- Download engines in install step rather than lazily by @daimond113
+
 ### Performance
 - Remove unnecessary `Arc`s from codebase by @daimond113

--- a/Cargo.lock
+++ b/Cargo.lock
@ -259,6 +259,12 @@ dependencies = [
 "memchr",
 ]

+[[package]]
+name = "aliasable"
+version = "0.1.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "250f629c0161ad8107cf89319e990051fae62832fd343083bea452d93e2205fd"
+
 [[package]]
 name = "alloc-no-stdlib"
 version = "2.0.4"
@ -773,7 +779,7 @@ version = "4.5.32"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "09176aae279615badda0765c0c0b3f6ed53f4709118af73cf4655d85d1530cd7"
 dependencies = [
- "heck",
+ "heck 0.5.0",
 "proc-macro2",
 "quote",
 "syn 2.0.100",
@ -2421,6 +2427,12 @@ dependencies = [
 "foldhash",
 ]

+[[package]]
+name = "heck"
+version = "0.4.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "95505c38b4572b2d910cecb0281560f54b440a19336cbbcb27bf6ce6adc6f5a8"
+
 [[package]]
 name = "heck"
 version = "0.5.0"
@ -3448,6 +3460,30 @@ dependencies = [
 "windows-sys 0.52.0",
 ]

+[[package]]
+name = "ouroboros"
+version = "0.18.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "1e0f050db9c44b97a94723127e6be766ac5c340c48f2c4bb3ffa11713744be59"
+dependencies = [
+ "aliasable",
+ "ouroboros_macro",
+ "static_assertions",
+]
+
+[[package]]
+name = "ouroboros_macro"
+version = "0.18.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "3c7028bdd3d43083f6d8d4d5187680d0d3560d54df4cc9d752005268b41e64d0"
+dependencies = [
+ "heck 0.4.1",
+ "proc-macro2",
+ "proc-macro2-diagnostics",
+ "quote",
+ "syn 2.0.100",
+]
+
 [[package]]
 name = "overload"
 version = "0.1.1"
@ -3531,6 +3567,7 @@ dependencies = [
 "jiff",
 "keyring",
 "open",
+ "ouroboros",
 "paste",
 "pathdiff",
 "relative-path",
@ -3726,6 +3763,19 @@ dependencies = [
 "unicode-ident",
 ]

+[[package]]
+name = "proc-macro2-diagnostics"
+version = "0.10.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "af066a9c399a26e020ada66a034357a868728e72cd426f3adcd35f80d88d88c8"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn 2.0.100",
+ "version_check",
+ "yansi",
+]
+
 [[package]]
 name = "prodash"
 version = "29.0.2"
@ -5921,6 +5971,12 @@ dependencies = [
 "windows-sys 0.59.0",
 ]

+[[package]]
+name = "yansi"
+version = "1.0.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "cfe53a6657fd280eaa890a3bc59152892ffa3e30101319d168b781ed6529b049"
+
 [[package]]
 name = "yoke"
 version = "0.7.5"
--- a/Cargo.toml
+++ b/Cargo.toml
@ -69,6 +69,7 @@ wax = { version = "0.6.0", default-features = false }
 fs-err = { version = "3.1.0", features = ["tokio"] }
 urlencoding = "2.1.3"
 async_zip = { version = "0.0.17", features = ["tokio", "deflate", "deflate64", "tokio-fs"] }
+ouroboros = "0.18.5"

 # TODO: remove this when gitoxide adds support for: committing, pushing, adding
 git2 = { version = "0.20.1", optional = true }
@ -189,7 +190,6 @@ needless_collect = "deny"
 needless_bitwise_bool = "deny"
 mut_mut = "deny"
 must_use_candidate = "warn"
-mem_forget = "deny"
 maybe_infinite_iter = "deny"
 match_wildcard_for_single_variants = "deny"
 match_bool = "warn"
--- a/docs/src/content/docs/guides/engines.mdx
+++ b/docs/src/content/docs/guides/engines.mdx
@ -20,16 +20,7 @@ lune = "^0.8.9"
 ```

 After you add the engines to your manifest run `pesde install` to set up the
-necessary files in pesde's bin directory. Then, you should execute the engine to
-ensure it is properly downloaded & setup.
-
-```sh
-lune
-```
-
-This is only required if you're installing the version for the first time, or if
-this is a requirement which none of your local installations of the engine
-fulfill. After doing so you can start using the engine as normal.
+necessary files in pesde's bin directory.

 <Aside type="note">
 	You can also use engines outside projects. They will run the latest version installed locally when
--- a/src/cli/commands/init.rs
+++ b/src/cli/commands/init.rs
@ -270,8 +270,8 @@ impl InitCommand {
 				}
 			} else {
 				println!(
-                    "{ERROR_PREFIX}: no scripts package configured, this can cause issues with Roblox compatibility"
-                );
+					"{ERROR_PREFIX}: no scripts package configured, this can cause issues with Roblox compatibility"
+				);
 				if !inquire::prompt_confirmation("initialize regardless?").unwrap() {
 					return Ok(());
 				}
--- a/src/cli/commands/self_upgrade.rs
+++ b/src/cli/commands/self_upgrade.rs
@ -1,6 +1,7 @@
 use crate::{
 	cli::{
 		config::read_config,
+		reporters::run_with_reporter,
 		style::{ADDED_STYLE, CLI_STYLE, REMOVED_STYLE},
 		version::{
 			current_version, find_latest_version, get_or_download_engine, replace_pesde_bin_exe,
@ -51,12 +52,23 @@ impl SelfUpgradeCommand {
 			return Ok(());
 		}

-		let path = get_or_download_engine(
-			&reqwest,
-			EngineKind::Pesde,
-			VersionReq::parse(&format!("={latest_version}")).unwrap(),
-		)
-		.await?;
+		let path = run_with_reporter(|_, root_progress, reporter| async {
+			let root_progress = root_progress;
+
+			root_progress.reset();
+			root_progress.set_message("download");
+
+			get_or_download_engine(
+				&reqwest,
+				EngineKind::Pesde,
+				VersionReq::parse(&format!("={latest_version}")).unwrap(),
+				reporter,
+			)
+			.await
+		})
+		.await?
+		.0;
+
 		replace_pesde_bin_exe(&path).await?;

 		println!("upgraded to version {display_latest_version}!");
--- a/src/cli/install.rs
+++ b/src/cli/install.rs
@ -1,7 +1,7 @@
 use super::files::make_executable;
 use crate::cli::{
 	bin_dir, dep_type_to_key,
-	reporters::{self, CliReporter},
+	reporters::{self, run_with_reporter, CliReporter},
 	resolve_overrides, run_on_workspace_members,
 	style::{ADDED_STYLE, REMOVED_STYLE, WARN_PREFIX},
 	up_to_date_lockfile,
@ -181,6 +181,50 @@ pub async fn install(
 		}
 	};

+	let progress_prefix = format!("{} {}: ", manifest.name, manifest.target);
+
+	#[cfg(feature = "version-management")]
+	let resolved_engine_versions = run_with_reporter(|_, root_progress, reporter| async {
+		let root_progress = root_progress;
+		let reporter = reporter;
+
+		root_progress.set_prefix(progress_prefix.clone());
+		root_progress.reset();
+		root_progress.set_message("update engines");
+
+		let mut tasks = manifest
+			.engines
+			.iter()
+			.map(|(engine, req)| {
+				let engine = *engine;
+				let req = req.clone();
+				let reqwest = reqwest.clone();
+				let reporter = reporter.clone();
+
+				async move {
+					let version = crate::cli::version::get_or_download_engine(
+						&reqwest, engine, req, reporter,
+					)
+					.await?
+					.1;
+					crate::cli::version::make_linker_if_needed(engine).await?;
+
+					Ok::<_, anyhow::Error>((engine, version))
+				}
+			})
+			.collect::<JoinSet<_>>();
+
+		let mut resolved_engine_versions = HashMap::new();
+
+		while let Some(task) = tasks.join_next().await {
+			let (engine, version) = task.unwrap()?;
+			resolved_engine_versions.insert(engine, version);
+		}
+
+		Ok::<_, anyhow::Error>(resolved_engine_versions)
+	})
+	.await?;
+
 	let overrides = resolve_overrides(&manifest)?;

 	let (new_lockfile, old_graph) =
@ -188,23 +232,7 @@ pub async fn install(
 			let multi = multi;
 			let root_progress = root_progress;

-			root_progress.set_prefix(format!("{} {}: ", manifest.name, manifest.target));
-			#[cfg(feature = "version-management")]
-			{
-				root_progress.reset();
-				root_progress.set_message("update engine linkers");
-
-				let mut tasks = manifest
-					.engines
-					.keys()
-					.map(|engine| crate::cli::version::make_linker_if_needed(*engine))
-					.collect::<JoinSet<_>>();
-
-				while let Some(task) = tasks.join_next().await {
-					task.unwrap()?;
-				}
-			}
-
+			root_progress.set_prefix(progress_prefix);
 			root_progress.reset();
 			root_progress.set_message("resolve");

@ -294,29 +322,6 @@ pub async fn install(

 				#[cfg(feature = "version-management")]
 				{
-					let mut tasks = manifest
-						.engines
-						.into_iter()
-						.map(|(engine, req)| async move {
-							Ok::<_, anyhow::Error>(
-								crate::cli::version::get_installed_versions(engine)
-									.await?
-									.into_iter()
-									.filter(|version| version_matches(&req, version))
-									.next_back()
-									.map(|version| (engine, version)),
-							)
-						})
-						.collect::<JoinSet<_>>();
-
-					let mut resolved_engine_versions = HashMap::new();
-					while let Some(task) = tasks.join_next().await {
-						let Some((engine, version)) = task.unwrap()? else {
-							continue;
-						};
-						resolved_engine_versions.insert(engine, version);
-					}
-
 					let manifest_target_kind = manifest.target.kind();
 					let mut tasks = downloaded_graph.iter()
 						.map(|(id, node)| {
--- a/src/cli/version.rs
+++ b/src/cli/version.rs
@ -4,7 +4,6 @@ use crate::{
 		config::{read_config, write_config, CliConfig},
 		files::make_executable,
 		home_dir,
-		reporters::run_with_reporter,
 		style::{ADDED_STYLE, CLI_STYLE, REMOVED_STYLE, URL_STYLE},
 	},
 	util::no_build_metadata,
@ -21,7 +20,7 @@ use pesde::{
 		},
 		EngineKind,
 	},
-	reporters::DownloadsReporter as _,
+	reporters::DownloadsReporter,
 	version_matches,
 };
 use semver::{Version, VersionReq};
@ -29,6 +28,7 @@ use std::{
 	collections::BTreeSet,
 	env::current_exe,
 	path::{Path, PathBuf},
+	sync::Arc,
 };
 use tracing::instrument;

@ -118,7 +118,7 @@ pub async fn check_for_updates(reqwest: &reqwest::Client) -> anyhow::Result<()>
 const ENGINES_DIR: &str = "engines";

 #[instrument(level = "trace")]
-pub async fn get_installed_versions(engine: EngineKind) -> anyhow::Result<BTreeSet<Version>> {
+async fn get_installed_versions(engine: EngineKind) -> anyhow::Result<BTreeSet<Version>> {
 	let source = engine.source();
 	let path = home_dir()?.join(ENGINES_DIR).join(source.directory());
 	let mut installed_versions = BTreeSet::new();
@ -144,12 +144,13 @@ pub async fn get_installed_versions(engine: EngineKind) -> anyhow::Result<BTreeS
 	Ok(installed_versions)
 }

-#[instrument(skip(reqwest), level = "trace")]
+#[instrument(skip(reqwest, reporter), level = "trace")]
 pub async fn get_or_download_engine(
 	reqwest: &reqwest::Client,
 	engine: EngineKind,
 	req: VersionReq,
-) -> anyhow::Result<PathBuf> {
+	reporter: Arc<impl DownloadsReporter>,
+) -> anyhow::Result<(PathBuf, Version)> {
 	let source = engine.source();
 	let path = home_dir()?.join(ENGINES_DIR).join(source.directory());

@ -160,77 +161,70 @@ pub async fn get_or_download_engine(
 		.filter(|v| version_matches(&req, v))
 		.next_back();
 	if let Some(version) = max_matching {
-		return Ok(path
-			.join(version.to_string())
-			.join(source.expected_file_name())
-			.with_extension(std::env::consts::EXE_EXTENSION));
+		return Ok((
+			path.join(version.to_string())
+				.join(source.expected_file_name())
+				.with_extension(std::env::consts::EXE_EXTENSION),
+			version.clone(),
+		));
 	}

-	run_with_reporter(|_, root_progress, reporter| async {
-		let root_progress = root_progress;
-		let reporter = reporter;
-
-		root_progress.set_message("resolve version");
-		let mut versions = source
-			.resolve(
-				&req,
-				&ResolveOptions {
-					reqwest: reqwest.clone(),
-				},
-			)
-			.await
-			.context("failed to resolve versions")?;
-		let (version, engine_ref) = versions.pop_last().context("no matching versions found")?;
-
-		root_progress.set_message("download");
-
-		let reporter = reporter.report_download(format!("{engine} v{version}"));
-
-		let archive = source
-			.download(
-				&engine_ref,
-				&DownloadOptions {
-					reqwest: reqwest.clone(),
-					reporter: reporter.into(),
-					version: version.clone(),
-				},
-			)
-			.await
-			.context("failed to download engine")?;
-
-		let path = path.join(version.to_string());
-		fs::create_dir_all(&path)
-			.await
-			.context("failed to create engine container folder")?;
-		let path = path
-			.join(source.expected_file_name())
-			.with_extension(std::env::consts::EXE_EXTENSION);
-
-		let mut file = fs::File::create(&path)
-			.await
-			.context("failed to create new file")?;
-
-		tokio::io::copy(
-			&mut archive
-				.find_executable(source.expected_file_name())
-				.await
-				.context("failed to find executable")?,
-			&mut file,
+	let mut versions = source
+		.resolve(
+			&req,
+			&ResolveOptions {
+				reqwest: reqwest.clone(),
+			},
 		)
 		.await
-		.context("failed to write to file")?;
+		.context("failed to resolve versions")?;
+	let (version, engine_ref) = versions.pop_last().context("no matching versions found")?;

-		make_executable(&path)
+	let reporter = reporter.report_download(format!("{engine} v{}", no_build_metadata(&version)));
+
+	let archive = source
+		.download(
+			&engine_ref,
+			&DownloadOptions {
+				reqwest: reqwest.clone(),
+				reporter: reporter.into(),
+				version: version.clone(),
+			},
+		)
+		.await
+		.context("failed to download engine")?;
+
+	let path = path.join(version.to_string());
+	fs::create_dir_all(&path)
+		.await
+		.context("failed to create engine container folder")?;
+	let path = path
+		.join(source.expected_file_name())
+		.with_extension(std::env::consts::EXE_EXTENSION);
+
+	let mut file = fs::File::create(&path)
+		.await
+		.context("failed to create new file")?;
+
+	tokio::io::copy(
+		&mut archive
+			.find_executable(source.expected_file_name())
 			.await
-			.context("failed to make downloaded version executable")?;
-
-		if engine != EngineKind::Pesde {
-			make_linker_if_needed(engine).await?;
-		}
-
-		Ok::<_, anyhow::Error>(path)
-	})
+			.context("failed to find executable")?,
+		&mut file,
+	)
 	.await
+	.context("failed to write to file")?;
+
+	make_executable(&path)
+		.await
+		.context("failed to make downloaded version executable")?;
+
+	if engine != EngineKind::Pesde {
+		make_linker_if_needed(engine).await?;
+	}
+
+	Ok((path, version))
 }

 #[instrument(level = "trace")]
--- a/src/download_and_link.rs
+++ b/src/download_and_link.rs
@ -187,7 +187,11 @@ impl Clone for DownloadAndLinkOptions {

 impl Project {
 	/// Downloads a graph of dependencies and links them in the correct order
-	#[instrument(skip_all, fields(install_dependencies = debug(options.install_dependencies_mode)), level = "debug")]
+	#[instrument(
+		skip_all,
+		fields(install_dependencies = debug(options.install_dependencies_mode)),
+		level = "debug"
+	)]
 	pub async fn download_and_link<Reporter, Hooks>(
 		&self,
 		graph: &DependencyGraph,
--- a/src/engine/source/archive.rs
+++ b/src/engine/source/archive.rs
@ -1,7 +1,7 @@
 use futures::StreamExt as _;
+use ouroboros::self_referencing;
 use std::{
 	collections::BTreeSet,
-	mem::ManuallyDrop,
 	path::{Path, PathBuf},
 	pin::Pin,
 	str::FromStr,
@ -11,7 +11,7 @@ use tokio::{
 	io::{AsyncBufRead, AsyncRead, AsyncReadExt as _, ReadBuf},
 	pin,
 };
-use tokio_util::compat::{Compat, FuturesAsyncReadCompatExt as _};
+use tokio_util::compat::Compat;

 /// The kind of encoding used for the archive
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@ -66,49 +66,36 @@ enum TarReader {
 	Plain(ArchiveReader),
 }

-// TODO: try to see if we can avoid the unsafe blocks
-
 impl AsyncRead for TarReader {
 	fn poll_read(
 		self: Pin<&mut Self>,
 		cx: &mut Context<'_>,
 		buf: &mut ReadBuf<'_>,
 	) -> Poll<std::io::Result<()>> {
-		unsafe {
-			match self.get_unchecked_mut() {
-				Self::Gzip(r) => Pin::new_unchecked(r).poll_read(cx, buf),
-				Self::Plain(r) => Pin::new_unchecked(r).poll_read(cx, buf),
-			}
+		match Pin::into_inner(self) {
+			Self::Gzip(r) => Pin::new(r).poll_read(cx, buf),
+			Self::Plain(r) => Pin::new(r).poll_read(cx, buf),
 		}
 	}
 }

+#[self_referencing]
+struct ZipArchiveEntry {
+	archive: async_zip::tokio::read::seek::ZipFileReader<std::io::Cursor<Vec<u8>>>,
+	#[borrows(mut archive)]
+	#[not_covariant]
+	reader: Compat<
+		async_zip::tokio::read::ZipEntryReader<
+			'this,
+			std::io::Cursor<Vec<u8>>,
+			async_zip::base::read::WithoutEntry,
+		>,
+	>,
+}
+
 enum ArchiveEntryInner {
 	Tar(Box<tokio_tar::Entry<tokio_tar::Archive<TarReader>>>),
-	Zip {
-		archive: *mut async_zip::tokio::read::seek::ZipFileReader<std::io::Cursor<Vec<u8>>>,
-		reader: ManuallyDrop<
-			Compat<
-				async_zip::tokio::read::ZipEntryReader<
-					'static,
-					std::io::Cursor<Vec<u8>>,
-					async_zip::base::read::WithoutEntry,
-				>,
-			>,
-		>,
-	},
-}
-
-impl Drop for ArchiveEntryInner {
-	fn drop(&mut self) {
-		match self {
-			Self::Tar(_) => {}
-			Self::Zip { archive, reader } => unsafe {
-				ManuallyDrop::drop(reader);
-				drop(Box::from_raw(*archive));
-			},
-		}
-	}
+	Zip(ZipArchiveEntry),
 }

 /// An entry in an archive. Usually the executable
@ -120,12 +107,10 @@ impl AsyncRead for ArchiveEntry {
 		cx: &mut Context<'_>,
 		buf: &mut ReadBuf<'_>,
 	) -> Poll<std::io::Result<()>> {
-		unsafe {
-			match &mut self.get_unchecked_mut().0 {
-				ArchiveEntryInner::Tar(r) => Pin::new_unchecked(r).poll_read(cx, buf),
-				ArchiveEntryInner::Zip { reader, .. } => {
-					Pin::new_unchecked(&mut **reader).poll_read(cx, buf)
-				}
+		match &mut Pin::into_inner(self).0 {
+			ArchiveEntryInner::Tar(r) => Pin::new(r).poll_read(cx, buf),
+			ArchiveEntryInner::Zip(z) => {
+				z.with_reader_mut(|reader| Pin::new(reader).poll_read(cx, buf))
 			}
 		}
 	}
@ -269,12 +254,21 @@ impl Archive {

 					let path: &Path = entry.filename().as_str()?.as_ref();
 					if candidate.path == path {
-						let ptr = Box::into_raw(Box::new(archive));
-						let reader = (unsafe { &mut *ptr }).reader_without_entry(i).await?;
-						return Ok(ArchiveEntry(ArchiveEntryInner::Zip {
-							archive: ptr,
-							reader: ManuallyDrop::new(reader.compat()),
-						}));
+						let entry = ZipArchiveEntryAsyncSendTryBuilder {
+							archive,
+							reader_builder: |archive| {
+								Box::pin(async move {
+									archive
+										.reader_without_entry(i)
+										.await
+										.map(tokio_util::compat::FuturesAsyncReadCompatExt::compat)
+								})
+							},
+						}
+						.try_build()
+						.await?;
+
+						return Ok(ArchiveEntry(ArchiveEntryInner::Zip(entry)));
 					}
 				}
 			}
--- a/src/linking/incremental.rs
+++ b/src/linking/incremental.rs
@ -183,8 +183,8 @@ impl Project {
 			.iter()
 			.flat_map(|(name, versions)| {
 				versions
-					.iter()
-					.map(|(v_id, _)| crate::source::ids::PackageId::new(name.clone(), v_id.clone()))
+					.keys()
+					.map(|v_id| crate::source::ids::PackageId::new(name.clone(), v_id.clone()))
 			})
 			.filter_map(|id| graph.get(&id).map(|node| (id, node)))
 			.map(|(id, node)| {
--- a/src/main.rs
+++ b/src/main.rs
@ -357,7 +357,9 @@ async fn run() -> anyhow::Result<()> {
 		}

 		let exe_path =
-			get_or_download_engine(&reqwest, engine, req.unwrap_or(VersionReq::STAR)).await?;
+			get_or_download_engine(&reqwest, engine, req.unwrap_or(VersionReq::STAR), ().into())
+				.await?
+				.0;
 		if exe_path == current_exe {
 			anyhow::bail!("engine linker executed by itself")
 		}