From e6ee935c11dc783ac22f75264c49d1a31d993211 Mon Sep 17 00:00:00 2001 From: daimond113 Date: Sun, 9 Mar 2025 16:00:40 +0100 Subject: [PATCH] feat: show available targets when none fit This will help prevent user confusion with targets. Instead of a cryptic "no matching versions available" error, it'll now list the available targets (if the source decides to output them, so if the specifier has a target option) --- CHANGELOG.md | 1 + src/cli/commands/add.rs | 27 +++++++++---- src/cli/commands/execute.rs | 75 ++++++++++++------------------------ src/cli/commands/init.rs | 3 +- src/cli/commands/outdated.rs | 1 + src/cli/commands/publish.rs | 1 + src/resolver.rs | 19 ++++++++- src/source/git/mod.rs | 8 +++- src/source/mod.rs | 24 ++++++++---- src/source/path/mod.rs | 3 +- src/source/pesde/mod.rs | 27 +++++++++++-- src/source/traits.rs | 3 ++ src/source/wally/manifest.rs | 13 ++++++- src/source/wally/mod.rs | 22 ++++------- src/source/workspace/mod.rs | 14 +++++-- 15 files changed, 148 insertions(+), 93 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 813ab0b..9021a04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Binary linkers are now done in Rust to simplify their implementation and cross-runtime portability by @daimond113 +- Show available targets in add and install commands if the specifier hasn't matched any by @daimond113 ## [0.6.0] - 2025-02-22 ### Added diff --git a/src/cli/commands/add.rs b/src/cli/commands/add.rs index 82fe468..f393b8d 100644 --- a/src/cli/commands/add.rs +++ b/src/cli/commands/add.rs @@ -143,22 +143,35 @@ impl AddCommand { .await .context("failed to refresh package source")?; - let Some(version_id) = source + let (_, mut versions, suggestions) = source .resolve( &specifier, &ResolveOptions { project: project.clone(), target: manifest.target.kind(), refreshed_sources, + loose_target: false, }, ) .await - .context("failed to resolve package")? - .1 - .pop_last() - .map(|(v_id, _)| v_id) - else { - anyhow::bail!("no versions found for package"); + .context("failed to resolve package")?; + + let Some((version_id, _)) = versions.pop_last() else { + anyhow::bail!( + "no matching versions found for package{}", + if suggestions.is_empty() { + "".into() + } else { + format!( + ". available targets: {}", + suggestions + .into_iter() + .map(|t| t.to_string()) + .collect::>() + .join(", ") + ) + } + ); }; let project_target = manifest.target.kind(); diff --git a/src/cli/commands/execute.rs b/src/cli/commands/execute.rs index 5c49a75..030de72 100644 --- a/src/cli/commands/execute.rs +++ b/src/cli/commands/execute.rs @@ -82,56 +82,28 @@ impl ExecuteCommand { .context("failed to refresh source")?; let version_req = self.package.1.unwrap_or(VersionReq::STAR); - let Some((id, pkg_ref)) = ('finder: { - let specifier = PesdeDependencySpecifier { - name: self.package.0.clone(), - version: version_req.clone(), - index: DEFAULT_INDEX_NAME.into(), - target: None, - }; - - if let Some((v_id, pkg_ref)) = source - .resolve( - &specifier, - &ResolveOptions { - project: project.clone(), - target: TargetKind::Lune, - refreshed_sources: refreshed_sources.clone(), - }, - ) - .await - .context("failed to resolve package")? - .1 - .pop_last() - { - break 'finder Some(( - PackageId::new(PackageNames::Pesde(self.package.0.clone()), v_id), - pkg_ref, - )); - } - - source - .resolve( - &specifier, - &ResolveOptions { - project: project.clone(), - target: TargetKind::Luau, - refreshed_sources: refreshed_sources.clone(), - }, - ) - .await - .context("failed to resolve package")? - .1 - .pop_last() - .map(|(v_id, pkg_ref)| { - ( - PackageId::new(PackageNames::Pesde(self.package.0.clone()), v_id), - pkg_ref, - ) - }) - }) else { + let Some((v_id, pkg_ref)) = source + .resolve( + &PesdeDependencySpecifier { + name: self.package.0.clone(), + version: version_req.clone(), + index: DEFAULT_INDEX_NAME.into(), + target: None, + }, + &ResolveOptions { + project: project.clone(), + target: TargetKind::Luau, + refreshed_sources: refreshed_sources.clone(), + loose_target: true, + }, + ) + .await + .context("failed to resolve package")? + .1 + .pop_last() + else { anyhow::bail!( - "no Lune or Luau package could be found for {}@{version_req}", + "no compatible package could be found for {}@{version_req}", self.package.0, ); }; @@ -151,7 +123,10 @@ impl ExecuteCommand { project.auth_config().clone(), ); - let id = Arc::new(id); + let id = Arc::new(PackageId::new( + PackageNames::Pesde(self.package.0.clone()), + v_id, + )); let fs = source .download( diff --git a/src/cli/commands/init.rs b/src/cli/commands/init.rs index 18b9c89..eda3571 100644 --- a/src/cli/commands/init.rs +++ b/src/cli/commands/init.rs @@ -204,8 +204,9 @@ impl InitCommand { }, &ResolveOptions { project: project.clone(), - target: TargetKind::Lune, + target: TargetKind::Luau, refreshed_sources, + loose_target: true, }, ) .await diff --git a/src/cli/commands/outdated.rs b/src/cli/commands/outdated.rs index dc0f95c..3a6e78c 100644 --- a/src/cli/commands/outdated.rs +++ b/src/cli/commands/outdated.rs @@ -92,6 +92,7 @@ impl OutdatedCommand { project: project.clone(), target: manifest_target_kind, refreshed_sources: refreshed_sources.clone(), + loose_target: false, }, ) .await diff --git a/src/cli/commands/publish.rs b/src/cli/commands/publish.rs index b6c24a4..fdb3a6a 100644 --- a/src/cli/commands/publish.rs +++ b/src/cli/commands/publish.rs @@ -465,6 +465,7 @@ info: otherwise, the file was deemed unnecessary, if you don't understand why, p project: project.clone(), target: target_kind, refreshed_sources: refreshed_sources.clone(), + loose_target: false, }, ) .await diff --git a/src/resolver.rs b/src/resolver.rs index 4de51ae..7d60b14 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -234,11 +234,12 @@ impl Project { .await .map_err(|e| Box::new(e.into()))?; - let (name, resolved) = source + let (name, resolved, suggestions) = source .resolve(&specifier, &ResolveOptions { project: self.clone(), target, refreshed_sources: refreshed_sources.clone(), + loose_target: false, }) .await .map_err(|e| Box::new(e.into()))?; @@ -251,7 +252,21 @@ impl Project { .or_else(|| resolved.last_key_value().map(|(ver, _)| PackageId::new(name, ver.clone()))) else { return Err(Box::new(errors::DependencyGraphError::NoMatchingVersion( - format!("{specifier} ({target})"), + format!( + "{specifier} {target}{}", + if suggestions.is_empty() { + "".into() + } else { + format!( + " available targets: {}", + suggestions + .into_iter() + .map(|t| t.to_string()) + .collect::>() + .join(", ") + ) + } + ), ))); }; diff --git a/src/source/git/mod.rs b/src/source/git/mod.rs index 9b92dc8..315a3f7 100644 --- a/src/source/git/mod.rs +++ b/src/source/git/mod.rs @@ -20,7 +20,12 @@ use crate::{ use fs_err::tokio as fs; use gix::{bstr::BStr, traverse::tree::Recorder, ObjectId, Url}; use relative_path::RelativePathBuf; -use std::{collections::BTreeMap, fmt::Debug, hash::Hash, path::PathBuf}; +use std::{ + collections::{BTreeMap, BTreeSet}, + fmt::Debug, + hash::Hash, + path::PathBuf, +}; use tokio::task::{spawn_blocking, JoinSet}; use tracing::instrument; @@ -322,6 +327,7 @@ impl PackageSource for GitPackageSource { dependencies, }, )]), + BTreeSet::new(), )) } diff --git a/src/source/mod.rs b/src/source/mod.rs index 6c3df03..f021bf2 100644 --- a/src/source/mod.rs +++ b/src/source/mod.rs @@ -1,5 +1,5 @@ use crate::{ - manifest::target::Target, + manifest::target::{Target, TargetKind}, names::PackageNames, reporters::DownloadProgressReporter, source::{ @@ -7,7 +7,10 @@ use crate::{ traits::*, }, }; -use std::{collections::BTreeMap, fmt::Debug}; +use std::{ + collections::{BTreeMap, BTreeSet}, + fmt::Debug, +}; /// Packages' filesystems pub mod fs; @@ -43,7 +46,7 @@ pub const ADDITIONAL_FORBIDDEN_FILES: &[&str] = &["default.project.json"]; pub const IGNORED_DIRS: &[&str] = &[".git"]; /// The result of resolving a package -pub type ResolveResult = (PackageNames, BTreeMap); +pub type ResolveResult = (PackageNames, BTreeMap, BTreeSet); /// All possible package sources #[derive(Debug, Eq, PartialEq, Hash, Clone)] @@ -98,13 +101,14 @@ impl PackageSource for PackageSources { (PackageSources::Pesde(source), DependencySpecifiers::Pesde(specifier)) => source .resolve(specifier, options) .await - .map(|(name, results)| { + .map(|(name, results, suggestions)| { ( name, results .into_iter() .map(|(version, pkg_ref)| (version, PackageRefs::Pesde(pkg_ref))) .collect(), + suggestions, ) }) .map_err(Into::into), @@ -113,13 +117,14 @@ impl PackageSource for PackageSources { (PackageSources::Wally(source), DependencySpecifiers::Wally(specifier)) => source .resolve(specifier, options) .await - .map(|(name, results)| { + .map(|(name, results, suggestions)| { ( name, results .into_iter() .map(|(version, pkg_ref)| (version, PackageRefs::Wally(pkg_ref))) .collect(), + suggestions, ) }) .map_err(Into::into), @@ -127,13 +132,14 @@ impl PackageSource for PackageSources { (PackageSources::Git(source), DependencySpecifiers::Git(specifier)) => source .resolve(specifier, options) .await - .map(|(name, results)| { + .map(|(name, results, suggestions)| { ( name, results .into_iter() .map(|(version, pkg_ref)| (version, PackageRefs::Git(pkg_ref))) .collect(), + suggestions, ) }) .map_err(Into::into), @@ -142,7 +148,7 @@ impl PackageSource for PackageSources { source .resolve(specifier, options) .await - .map(|(name, results)| { + .map(|(name, results, suggestions)| { ( name, results @@ -151,6 +157,7 @@ impl PackageSource for PackageSources { (version, PackageRefs::Workspace(pkg_ref)) }) .collect(), + suggestions, ) }) .map_err(Into::into) @@ -159,13 +166,14 @@ impl PackageSource for PackageSources { (PackageSources::Path(source), DependencySpecifiers::Path(specifier)) => source .resolve(specifier, options) .await - .map(|(name, results)| { + .map(|(name, results, suggestions)| { ( name, results .into_iter() .map(|(version, pkg_ref)| (version, PackageRefs::Path(pkg_ref))) .collect(), + suggestions, ) }) .map_err(Into::into), diff --git a/src/source/path/mod.rs b/src/source/path/mod.rs index c98e556..1436b88 100644 --- a/src/source/path/mod.rs +++ b/src/source/path/mod.rs @@ -14,7 +14,7 @@ use crate::{ Project, }; use futures::TryStreamExt as _; -use std::collections::{BTreeMap, HashMap}; +use std::collections::{BTreeMap, BTreeSet, HashMap}; use tracing::instrument; /// The path package reference @@ -122,6 +122,7 @@ impl PackageSource for PathPackageSource { VersionId::new(manifest.version, manifest.target.kind()), pkg_ref, )]), + BTreeSet::new(), )) } diff --git a/src/source/pesde/mod.rs b/src/source/pesde/mod.rs index 85e8ef2..d3d5f1c 100644 --- a/src/source/pesde/mod.rs +++ b/src/source/pesde/mod.rs @@ -14,7 +14,10 @@ use specifier::PesdeDependencySpecifier; use crate::{ engine::EngineKind, - manifest::{target::Target, Alias, DependencyType}, + manifest::{ + target::{Target, TargetKind}, + Alias, DependencyType, + }, names::{PackageName, PackageNames}, reporters::{response_to_async_read, DownloadProgressReporter}, source::{ @@ -147,6 +150,7 @@ impl PackageSource for PesdePackageSource { let ResolveOptions { project, target: project_target, + loose_target, .. } = options; @@ -158,14 +162,28 @@ impl PackageSource for PesdePackageSource { tracing::debug!("{} has {} possible entries", specifier.name, entries.len()); + let suggestions = entries + .iter() + .filter(|(_, entry)| !entry.yanked) + .filter(|(v_id, _)| version_matches(&specifier.version, v_id.version())) + .map(|(v_id, _)| v_id.target()) + .collect(); + + let specifier_target = specifier.target.unwrap_or(*project_target); + Ok(( PackageNames::Pesde(specifier.name.clone()), entries .into_iter() .filter(|(_, entry)| !entry.yanked) - .filter(|(VersionId(version, target), _)| { - version_matches(&specifier.version, version) - && specifier.target.unwrap_or(*project_target) == *target + .filter(|(v_id, _)| version_matches(&specifier.version, v_id.version())) + .filter(|(v_id, _)| { + // we want anything which might contain bins, scripts (so not Roblox) + if *loose_target && specifier_target == TargetKind::Luau { + !matches!(v_id.target(), TargetKind::Roblox | TargetKind::RobloxServer) + } else { + specifier_target == v_id.target() + } }) .map(|(id, entry)| { ( @@ -177,6 +195,7 @@ impl PackageSource for PesdePackageSource { ) }) .collect(), + suggestions, )) } diff --git a/src/source/traits.rs b/src/source/traits.rs index 4449576..5a09957 100644 --- a/src/source/traits.rs +++ b/src/source/traits.rs @@ -44,6 +44,9 @@ pub struct ResolveOptions { pub target: TargetKind, /// The sources that have been refreshed pub refreshed_sources: RefreshedSources, + /// Whether to find any compatible target instead of a strict equal. Each source defines its + /// own loose rules. + pub loose_target: bool, } /// Options for downloading a package diff --git a/src/source/wally/manifest.rs b/src/source/wally/manifest.rs index 79dc66f..d945a0d 100644 --- a/src/source/wally/manifest.rs +++ b/src/source/wally/manifest.rs @@ -1,7 +1,7 @@ use std::collections::BTreeMap; use crate::{ - manifest::{errors, Alias, DependencyType}, + manifest::{errors, target::TargetKind, Alias, DependencyType}, names::wally::WallyPackageName, source::{specifiers::DependencySpecifiers, wally::specifier::WallyDependencySpecifier}, }; @@ -9,7 +9,7 @@ use semver::{Version, VersionReq}; use serde::{Deserialize, Deserializer}; use tracing::instrument; -#[derive(Deserialize, Clone, Debug)] +#[derive(Deserialize, Copy, Clone, Debug)] #[serde(rename_all = "lowercase")] pub enum Realm { #[serde(alias = "dev")] @@ -17,6 +17,15 @@ pub enum Realm { Server, } +impl Realm { + pub fn to_target(self) -> TargetKind { + match self { + Realm::Shared => TargetKind::Roblox, + Realm::Server => TargetKind::RobloxServer, + } + } +} + #[derive(Deserialize, Clone, Debug)] #[serde(rename_all = "kebab-case")] pub struct WallyPackage { diff --git a/src/source/wally/mod.rs b/src/source/wally/mod.rs index 2eb65dd..54fa8c4 100644 --- a/src/source/wally/mod.rs +++ b/src/source/wally/mod.rs @@ -1,5 +1,5 @@ use crate::{ - manifest::target::{Target, TargetKind}, + manifest::target::Target, names::{wally::WallyPackageName, PackageNames}, reporters::{response_to_async_read, DownloadProgressReporter}, source::{ @@ -9,11 +9,7 @@ use crate::{ traits::{ DownloadOptions, GetTargetOptions, PackageSource, RefreshOptions, ResolveOptions, }, - wally::{ - compat_util::get_target, - manifest::{Realm, WallyManifest}, - pkg_ref::WallyPackageRef, - }, + wally::{compat_util::get_target, manifest::WallyManifest, pkg_ref::WallyPackageRef}, PackageSources, ResolveResult, IGNORED_DIRS, IGNORED_FILES, }, util::hash, @@ -24,7 +20,10 @@ use gix::Url; use relative_path::RelativePathBuf; use reqwest::header::AUTHORIZATION; use serde::Deserialize; -use std::{collections::BTreeMap, path::PathBuf}; +use std::{ + collections::{BTreeMap, BTreeSet}, + path::PathBuf, +}; use tokio::{io::AsyncReadExt as _, pin, task::spawn_blocking}; use tokio_util::compat::FuturesAsyncReadCompatExt as _; use tracing::instrument; @@ -198,13 +197,7 @@ impl PackageSource for WallyPackageSource { })?; Ok(( - VersionId( - manifest.package.version, - match manifest.package.realm { - Realm::Server => TargetKind::RobloxServer, - Realm::Shared => TargetKind::Roblox, - }, - ), + VersionId(manifest.package.version, manifest.package.realm.to_target()), WallyPackageRef { index_url: index_url.clone(), dependencies, @@ -212,6 +205,7 @@ impl PackageSource for WallyPackageSource { )) }) .collect::>()?, + BTreeSet::new(), )) } diff --git a/src/source/workspace/mod.rs b/src/source/workspace/mod.rs index c96f54a..76f112f 100644 --- a/src/source/workspace/mod.rs +++ b/src/source/workspace/mod.rs @@ -14,7 +14,7 @@ use crate::{ }; use futures::StreamExt as _; use relative_path::RelativePathBuf; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use tokio::pin; use tracing::instrument; @@ -47,6 +47,8 @@ impl PackageSource for WorkspacePackageSource { .. } = options; + let mut suggestions = BTreeSet::new(); + let (path, manifest) = 'finder: { let target = specifier.target.unwrap_or(*project_target); @@ -54,8 +56,13 @@ impl PackageSource for WorkspacePackageSource { pin!(members); while let Some((path, manifest)) = members.next().await.transpose()? { - if manifest.name == specifier.name && manifest.target.kind() == target { - break 'finder (path, manifest); + let member_target = manifest.target.kind(); + if manifest.name == specifier.name { + suggestions.insert(member_target); + + if member_target == target { + break 'finder (path, manifest); + } } } @@ -121,6 +128,7 @@ impl PackageSource for WorkspacePackageSource { VersionId::new(manifest.version, manifest_target_kind), pkg_ref, )]), + suggestions, )) }