fix: fixed misc bugs with ubi+pipx backends

jdx · May 13, 2024 · c4bc03a · c4bc03a
1 parent 13f16cb
commit c4bc03a
Show file tree

Hide file tree

Showing 12 changed files with 128 additions and 102 deletions.
diff --git a/e2e/assert.sh b/e2e/assert.sh
@@ -13,6 +13,7 @@ fail() {
 
 quiet_assert_succeed() {
   local status=0
+  debug "$ $1"
   bash -c "$1" || status=$?
   if [[ $status -ne 0 ]]; then
     fail "[$1] command failed with status $status"
@@ -26,6 +27,7 @@ assert_succeed() {
 }
 
 assert_fail() {
+  debug "$ $1"
   if ! bash -c "$1" 2>&1; then
     ok "[$1] expected failure"
   else
@@ -45,6 +47,7 @@ assert() {
 
 assert_not() {
   local actual
+  debug "$ $1"
   actual="$(bash -c "$1" || true)"
   if [[ $actual != "$2" ]]; then
     ok "[$1] output is different from '$2'"

diff --git a/e2e/cli/test_hook_env b/e2e/cli/test_hook_env
@@ -12,6 +12,7 @@ mise shell dummy@1.0.0
 eval "$(mise hook-env)"
 assert_contains "dummy" "1.0.0"
 
-export MISE_DUMMY_VERSION=v1.1.0
+# TODO: make "v" prefixes optional
+export MISE_DUMMY_VERSION=1.1.0
 eval "$(mise hook-env)"
 assert_contains "dummy" "1.1.0"
diff --git a/e2e/run_test b/e2e/run_test
@@ -31,7 +31,7 @@ setup_isolated_env() {
 
 create_isolated_env() {
   # Create the required diretories
-  mkdir --parents "$TEST_HOME/bin" "$TEST_WORKDIR" "$TEST_TMPDIR" "$MISE_SYSTEM_DIR" "$MISE_DATA_DIR" "$MISE_STATE_DIR" "$MISE_CACHE_DIR" "$MISE_CONFIG_DIR"
+  mkdir -p "$TEST_HOME/bin" "$TEST_WORKDIR" "$TEST_TMPDIR" "$MISE_SYSTEM_DIR" "$MISE_DATA_DIR" "$MISE_STATE_DIR" "$MISE_CACHE_DIR" "$MISE_CONFIG_DIR"
 
   # The dummy plugin is required for some tests
   mkdir -p "$MISE_DATA_DIR/plugins"
@@ -43,7 +43,7 @@ remove_isolated_env() {
 }
 
 within_isolated_env() {
-  env \
+  _env \
     --ignore-environment \
     --chdir="$TEST_WORKDIR" \
     - \
@@ -101,4 +101,12 @@ run_test() {
   return "$status"
 }
 
+_env() {
+  if type genv >/dev/null 2>&1; then
+    genv "$@"
+  else
+    env "$@"
+  fi
+}
+
 as_group "E2E $TEST" run_test
diff --git a/e2e/style.sh b/e2e/style.sh
@@ -10,6 +10,9 @@ if [[ -n ${GITHUB_ACTION:-} ]]; then
   }
   err() { type=error annotate "$*"; }
   warn() { type=warning annotate "$*"; }
+  notice() { type=notice annotate "$*"; }
+  # debug() { type=debug annotate "$*"; }
+  debug() { echo $'\e[90m'"$*"$'\e[0m' >&2; }
   start_group() { echo "::group::$*" >&2; }
   end_group() { echo ::endgroup:: >&2; }
 
@@ -21,14 +24,18 @@ elif [[ -t 2 ]]; then
   ok() { echo $'\e[92m'"$*"$'\e[0m' >&2; }
   err() { echo $'\e[91m'"${title:+$title: }$*"$'\e[0m' >&2; }
   warn() { echo $'\e[93m'"${title:+$title: }$*"$'\e[0m' >&2; }
+  notice() { echo $'\e[94m'"$*"$'\e[0m' >&2; }
+  debug() { echo $'\e[90m'"$*"$'\e[0m' >&2; }
   start_group() { echo $'\e[1m'">>> $*"$'\e[0m' >&2; }
   end_group() { echo >&2; }
 
 else
   # No styling
   ok() { echo "SUCCESS: $*" >&2; }
   err() { echo "ERROR: ${title:+$title: }$*" >&2; }
-  warn() { echo "wARNING: ${title:+$title: }$*" >&2; }
+  warn() { echo "WARNING: ${title:+$title: }$*" >&2; }
+  notice() { echo "NOTICE: $*" >&2; }
+  debug() { echo "DEBUG: $*" >&2; }
   start_group() { echo ">>> $*" >&2; }
   end_group() { echo >&2; }
 fi

diff --git a/src/config/mod.rs b/src/config/mod.rs
@@ -95,6 +95,13 @@ impl Config {
 
         Ok(config)
     }
+    pub fn env_maybe(&self) -> Option<IndexMap<String, String>> {
+        self.env_with_sources.get().map(|env| {
+            env.iter()
+                .map(|(k, (v, _))| (k.clone(), v.clone()))
+                .collect()
+        })
+    }
     pub fn env(&self) -> eyre::Result<IndexMap<String, String>> {
         Ok(self
             .env_with_sources()?
@@ -603,15 +610,16 @@ impl Debug for Config {
                 &tasks.values().map(|t| t.to_string()).collect_vec(),
             );
         }
-        if let Ok(env) = self.env() {
+        if let Some(env) = self.env_maybe() {
             if !env.is_empty() {
                 s.field("Env", &env);
                 // s.field("Env Sources", &self.env_sources);
             }
         }
-        let path_dirs = self.path_dirs().cloned().unwrap_or_default();
-        if !path_dirs.is_empty() {
-            s.field("Path Dirs", &path_dirs);
+        if let Some(env_results) = self.env.get() {
+            if !env_results.env_files.is_empty() {
+                s.field("Path Dirs", &env_results.env_paths);
+            }
         }
         if !self.aliases.is_empty() {
             s.field("Aliases", &self.aliases);

diff --git a/src/config/snapshots/mise__config__tests__load.snap b/src/config/snapshots/mise__config__tests__load.snap
@@ -8,9 +8,6 @@ Config {
         "~/.test-tool-versions",
         "~/config/config.toml",
     ],
-    Env: {
-        "TEST_ENV_VAR": "test-123",
-    },
     Aliases: {
         ForgeArg("tiny"): {
             "my/alias": "3.0",

diff --git a/src/forge/mod.rs b/src/forge/mod.rs
@@ -456,7 +456,7 @@ pub trait Forge: Debug + Send + Sync {
 fn fuzzy_match_filter(versions: Vec<String>, query: &str) -> eyre::Result<Vec<String>> {
     let mut query = query;
     if query == "latest" {
-        query = "[0-9].*";
+        query = "v?[0-9].*";
     }
     let query_regex = Regex::new(&format!("^{}([-.].+)?$", query))?;
     let versions = versions
@@ -475,11 +475,10 @@ fn fuzzy_match_filter(versions: Vec<String>, query: &str) -> eyre::Result<Vec<St
 }
 
 fn find_match_in_list(list: &[String], query: &str) -> Option<String> {
-    let v = match list.contains(&query.to_string()) {
+    match list.contains(&query.to_string()) {
         true => Some(query.to_string()),
         false => list.last().map(|s| s.to_string()),
-    };
-    v
+    }
 }
 
 fn rmdir(dir: &Path, pr: &dyn SingleReport) -> eyre::Result<()> {

diff --git a/src/forge/pipx.rs b/src/forge/pipx.rs
@@ -1,10 +1,12 @@
 use std::fmt::Debug;
+use std::str::FromStr;
 
 use crate::cache::CacheManager;
 use crate::cli::args::ForgeArg;
 use crate::cmd::CmdLineRunner;
 use crate::config::{Config, Settings};
 use crate::forge::{Forge, ForgeType};
+use crate::github;
 use crate::install_context::InstallContext;
 use crate::toolset::ToolVersionRequest;
 
@@ -34,7 +36,26 @@ impl Forge for PIPXForge {
      */
     fn _list_remote_versions(&self) -> eyre::Result<Vec<String>> {
         self.remote_version_cache
-            .get_or_try_init(|| Ok(vec!["latest".to_string()]))
+            .get_or_try_init(|| match self.name().parse()? {
+                PipxRequest::Pypi(package) => {
+                    let url = format!("https://pypi.org/pypi/{}/json", package);
+                    let raw = crate::http::HTTP_FETCH.get_text(url)?;
+                    let data: serde_json::Value = serde_json::from_str(&raw)?;
+                    let versions = data["releases"]
+                        .as_object()
+                        .ok_or_else(|| eyre::eyre!("Invalid pypi response"))?
+                        .keys()
+                        .map(|k| k.to_string())
+                        .collect();
+                    Ok(versions)
+                }
+                PipxRequest::Git(url) if url.starts_with("https://github.com/") => {
+                    let repo = url.strip_prefix("https://github.com/").unwrap();
+                    let data = github::list_releases(repo)?;
+                    Ok(data.into_iter().map(|r| r.tag_name).collect())
+                }
+                PipxRequest::Git { .. } => Ok(vec!["latest".to_string()]),
+            })
             .cloned()
     }
 
@@ -48,11 +69,14 @@ impl Forge for PIPXForge {
         let config = Config::try_get()?;
         let settings = Settings::get();
         settings.ensure_experimental("pipx backend")?;
-        let project_name = transform_project_name(ctx, self.name());
+        let pipx_request = self
+            .name()
+            .parse::<PipxRequest>()?
+            .pipx_request(&ctx.tv.version);
 
         CmdLineRunner::new("pipx")
             .arg("install")
-            .arg(project_name)
+            .arg(pipx_request)
             .with_pr(ctx.pr.as_ref())
             .env("PIPX_HOME", ctx.tv.install_path())
             .env("PIPX_BIN_DIR", ctx.tv.install_path().join("bin"))
@@ -79,29 +103,40 @@ impl PIPXForge {
     }
 }
 
-/*
- * Supports the following formats
- * - git+https://github.com/psf/black.git@24.2.0 => github longhand and version
- * - psf/black@24.2.0 => github shorthand and version
- * - black@24.2.0 => pypi shorthand and version
- * - black => pypi shorthand and latest
- */
-fn transform_project_name(ctx: &InstallContext, name: &str) -> String {
-    let parts: Vec<&str> = name.split('/').collect();
-    match (
-        name,
-        name.starts_with("git+http"),
-        parts.len(),
-        ctx.tv.version.as_str(),
-    ) {
-        (_, false, 2, "latest") => format!("git+https://github.com/{}.git", name),
-        (_, false, 2, v) => format!("git+https://github.com/{}.git@{}", name, v),
-        (_, true, _, "latest") => name.to_string(),
-        (_, true, _, v) => format!("{}@{}", name, v),
-        (".", false, _, _) => name.to_string(),
-        (_, false, _, "latest") => name.to_string(),
-        // Treat this as a pypi package@version and translate the version syntax
-        (_, false, 1, v) => format!("{}=={}", name, v),
-        _ => name.to_string(),
+enum PipxRequest {
+    /// git+https://github.com/psf/black.git@24.2.0
+    /// psf/black@24.2.0
+    Git(String),
+    /// black@24.2.0
+    Pypi(String),
+}
+
+impl PipxRequest {
+    fn pipx_request(&self, v: &str) -> String {
+        if v == "latest" {
+            match self {
+                PipxRequest::Git(url) => format!("git+{url}.git"),
+                PipxRequest::Pypi(package) => package.to_string(),
+            }
+        } else {
+            match self {
+                PipxRequest::Git(url) => format!("git+{}.git@{}", url, v),
+                PipxRequest::Pypi(package) => format!("{}=={}", package, v),
+            }
+        }
+    }
+}
+
+impl FromStr for PipxRequest {
+    type Err = eyre::Error;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        if let Some(cap) = regex!(r"(git\+)(.*)(\.git)").captures(s) {
+            Ok(PipxRequest::Git(cap.get(2).unwrap().as_str().to_string()))
+        } else if s.contains('/') {
+            Ok(PipxRequest::Git(format!("https://github.com/{s}")))
+        } else {
+            Ok(PipxRequest::Pypi(s.to_string()))
+        }
     }
 }