Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Enable global expose with nested paths #2362

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions crates/pixi_utils/src/executable_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ mod tests {
#[case::python3_config("python3-config", "python3-config")]
#[case::x2to3("2to3", "2to3")]
#[case::x2to3312("2to3-3.12", "2to3-3.12")]
#[case::nested_executable("subdir/executable.sh", "subdir/executable")]
fn test_strip_executable_unix(#[case] path: &str, #[case] expected: &str) {
let path = Path::new(path);
let result = strip_unix_executable_extension(path.to_string_lossy().to_string());
Expand All @@ -100,6 +101,7 @@ mod tests {
#[case::python3_config("python3-config", "python3-config")]
#[case::x2to3("2to3", "2to3")]
#[case::x2to3312("2to3-3.12", "2to3-3.12")]
#[case::nested_executable("subdir\\executable.exe", "subdir\\executable")]
fn test_strip_executable_windows(#[case] path: &str, #[case] expected: &str) {
let path = Path::new(path);
let result = strip_windows_executable_extension(path.to_string_lossy().to_string());
Expand All @@ -114,6 +116,7 @@ mod tests {
#[case::package010("package0.1.0", "package0.1.0")]
#[case::x2to3("2to3", "2to3")]
#[case::x2to3312("2to3-3.12", "2to3-3.12")]
#[case::nested_executable("subdir/executable", "subdir/executable")]
fn test_strip_executable_extension(#[case] path: &str, #[case] expected: &str) {
let result = strip_executable_extension(path.into());
assert_eq!(result, expected);
Expand Down
19 changes: 19 additions & 0 deletions docs/features/global_tools.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,25 @@ exposed = { ansible = "ansible" } # (1)!

1. The `ansible` binary is exposed even though it is installed by a dependency of `ansible`, the `ansible-core` package.

It's also possible to expose an executable which is located in a nested directory.
For example dotnet.exe executable is located in a dotnet folder,
to expose dotnet.exe you must specify its relative path :

```
pixi global install dotnet --expose dotnet=dotnet\dotnet.exe
```
you can also omit the extension
```
pixi global install dotnet --expose dotnet=dotnet\dotnet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this example on Linux but that package doesn't seem to work. Do you understand why the dotnet setup is using a non bin/xx setup for it's binaries?

Copy link
Contributor Author

@bahugo bahugo Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no Idea.
I had never tried to install dotnet on linux but you're right, it's installed in .pixi/envs/dotnet/lib/dotnet/dotnet
For some reason everything is painful with windows tools...
Maybe a more cross platform or generic tool would be better for the documentation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I contacted @dhirschfeld as he is a pixi user and the maintainer of dotnet maybe he has some smart things to say about this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about smart - the TL;DR is that it's just a binary repackage of upstream and that seems to be the way it's packaged by Microsoft.

The package sets a number of env vars in an activation script, including the PATH. I'd like to port setting/unsetting the env vars to the JSON format (assuming that works with pixi?) but haven't gotten around to it.

```
will create the following entry in the manifest:
```toml
[envs.dotnet]
channels = ["conda-forge"]
dependencies = { dotnet = "*" }
exposed = { dotnet = 'dotnet\dotnet' }
```

### Dependencies
Dependencies are the **Conda** packages that will be installed into your environment. For example, running:
```
Expand Down
70 changes: 42 additions & 28 deletions src/global/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -877,51 +877,65 @@ mod tests {
ExposedName::from_str("test").unwrap(),
"test".to_string(),
));
exposed.insert(Mapping::new(
ExposedName::from_str("nested_test").unwrap(),
Path::new("other_dir")
.join("nested_test")
.to_str()
.unwrap()
.to_string(),
));
let (to_remove, to_add) = get_expose_scripts_sync_status(&bin_dir, &env_dir, &exposed)
.await
.unwrap();
assert!(to_remove.is_empty());
assert_eq!(to_add.len(), 1);
assert_eq!(to_add.len(), 2);

// Add a script to the bin directory
let script_path = if cfg!(windows) {
bin_dir.path().join("test.bat")
} else {
bin_dir.path().join("test")
};
let script_names = ["test", "nested_test"];

#[cfg(windows)]
{
let script = format!(
r#"
for script_name in script_names {
let script_path = bin_dir.path().join(format!("{}.bat", script_name));
let script = format!(
r#"
@"{}" %*
"#,
env_dir
.path()
.join("bin")
.join("test.exe")
.to_string_lossy()
);
tokio_fs::write(&script_path, script).await.unwrap();
env_dir
.path()
.join("bin")
.join(format!("{}.exe", script_name))
.to_string_lossy()
);
tokio_fs::write(&script_path, script).await.unwrap();
}
}
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;

let script = format!(
r#"#!/bin/sh
for script_name in script_names {
let script_path = bin_dir.path().join(script_name);
let script = format!(
r#"#!/bin/sh
"{}" "$@"
"#,
env_dir.path().join("bin").join("test").to_string_lossy()
);
tokio_fs::write(&script_path, script).await.unwrap();
// Set the file permissions to make it executable
let metadata = tokio_fs::metadata(&script_path).await.unwrap();
let mut permissions = metadata.permissions();
permissions.set_mode(0o755); // rwxr-xr-x
tokio_fs::set_permissions(&script_path, permissions)
.await
.unwrap();
env_dir
.path()
.join("bin")
.join(script_name)
.to_string_lossy()
);
tokio_fs::write(&script_path, script).await.unwrap();
// Set the file permissions to make it executable
let metadata = tokio_fs::metadata(&script_path).await.unwrap();
let mut permissions = metadata.permissions();
permissions.set_mode(0o755); // rwxr-xr-x
tokio_fs::set_permissions(&script_path, permissions)
.await
.unwrap();
}
};

let (to_remove, to_add) = get_expose_scripts_sync_status(&bin_dir, &env_dir, &exposed)
Expand All @@ -935,7 +949,7 @@ mod tests {
get_expose_scripts_sync_status(&bin_dir, &env_dir, &IndexSet::new())
.await
.unwrap();
assert_eq!(to_remove.len(), 1);
assert_eq!(to_remove.len(), 2);
assert!(to_add.is_empty());
}
}
4 changes: 2 additions & 2 deletions src/global/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,13 @@ pub fn format_exposed(exposed: &IndexSet<Mapping>, last: bool) -> Option<String>

fn format_mapping(mapping: &Mapping) -> String {
let exp = mapping.exposed_name().to_string();
if exp == mapping.executable_name() {
if exp == mapping.executable_relname() {
console::style(exp).yellow().to_string()
} else {
format!(
"{} -> {}",
console::style(exp).yellow(),
console::style(mapping.executable_name()).yellow()
console::style(mapping.executable_relname()).yellow()
)
}
}
Expand Down
46 changes: 20 additions & 26 deletions src/global/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@ pub(crate) mod project;

pub(crate) use common::{BinDir, EnvChanges, EnvDir, EnvRoot, EnvState, StateChange, StateChanges};
pub(crate) use install::extract_executable_from_script;
use pixi_utils::executable_from_path;
pub(crate) use project::{EnvironmentName, ExposedName, Mapping, Project};

use crate::prefix::Prefix;
use crate::prefix::{Executable, Prefix};
use rattler_conda_types::PrefixRecord;
use std::path::{Path, PathBuf};

Expand All @@ -22,33 +23,26 @@ fn find_executables(prefix: &Prefix, prefix_package: &PrefixRecord) -> Vec<PathB
.collect()
}

fn is_executable(prefix: &Prefix, relative_path: &Path) -> bool {
// Check if the file is in a known executable directory.
let binary_folders = if cfg!(windows) {
&([
"",
"Library/mingw-w64/bin/",
"Library/usr/bin/",
"Library/bin/",
"Scripts/",
"bin/",
][..])
} else {
&(["bin"][..])
};

let parent_folder = match relative_path.parent() {
Some(dir) => dir,
None => return false,
};

if !binary_folders
/// Processes prefix records (that you can get by using `find_installed_packages`)
/// to filter and collect executable files.
pub fn find_executables_for_many_records(
prefix: &Prefix,
prefix_packages: &[PrefixRecord],
) -> Vec<Executable> {
let executables = prefix_packages
.iter()
.any(|bin_path| Path::new(bin_path) == parent_folder)
{
return false;
}
.flat_map(|record| {
record
.files
.iter()
.filter(|relative_path| is_executable(prefix, relative_path))
.map(|path| Executable::new(executable_from_path(path), path.clone()))
})
.collect();
executables
}

fn is_executable(prefix: &Prefix, relative_path: &Path) -> bool {
// Check if the file is executable
let absolute_path = prefix.root().join(relative_path);
is_executable::is_executable(absolute_path)
Expand Down
73 changes: 56 additions & 17 deletions src/global/project/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::global::project::ParsedEnvironment;
use pixi_config::Config;
use pixi_manifest::{PrioritizedChannel, TomlManifest};
use pixi_spec::PixiSpec;
use pixi_utils::strip_executable_extension;
use rattler_conda_types::{ChannelConfig, MatchSpec, NamedChannelOrUrl, PackageName, Platform};
use serde::{Deserialize, Serialize};
use toml_edit::{DocumentMut, Item};
Expand Down Expand Up @@ -349,7 +350,7 @@ impl Manifest {
self.document.insert_into_inline_table(
&format!("envs.{env_name}.exposed"),
&mapping.exposed_name.to_string(),
toml_edit::Value::from(mapping.executable_name.clone()),
toml_edit::Value::from(mapping.executable_relname.clone()),
)?;

tracing::debug!("Added exposed mapping {mapping} to toml document");
Expand Down Expand Up @@ -431,40 +432,48 @@ impl Manifest {
#[derive(Debug, Clone, Serialize, Deserialize, Hash, PartialEq, Eq)]
pub struct Mapping {
exposed_name: ExposedName,
executable_name: String,
executable_relname: String,
ruben-arts marked this conversation as resolved.
Show resolved Hide resolved
}

impl Mapping {
pub fn new(exposed_name: ExposedName, executable_name: String) -> Self {
pub fn new(exposed_name: ExposedName, executable_relname: String) -> Self {
Self {
exposed_name,
executable_name,
executable_relname: strip_executable_extension(executable_relname),
}
}

pub fn exposed_name(&self) -> &ExposedName {
&self.exposed_name
}

pub fn executable_relname(&self) -> &str {
&self.executable_relname
}
pub fn executable_name(&self) -> &str {
&self.executable_name
if let Some(executable_file_name) = Path::new(&self.executable_relname).file_name() {
return executable_file_name
.to_str()
.unwrap_or(&self.executable_relname);
};
&self.executable_relname
ruben-arts marked this conversation as resolved.
Show resolved Hide resolved
}
}

impl fmt::Display for Mapping {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}={}", self.exposed_name, self.executable_name)
write!(f, "{}={}", self.exposed_name, self.executable_relname)
}
}

impl FromStr for Mapping {
type Err = miette::Error;

fn from_str(input: &str) -> Result<Self, Self::Err> {
// If we can't parse exposed_name=executable_name, assume input=input
let (exposed_name, executable_name) = input.split_once('=').unwrap_or((input, input));
// If we can't parse exposed_name=executable_relname, assume input=input
let (exposed_name, executable_relname) = input.split_once('=').unwrap_or((input, input));
let exposed_name = ExposedName::from_str(exposed_name)?;
Ok(Mapping::new(exposed_name, executable_name.to_string()))
Ok(Mapping::new(exposed_name, executable_relname.to_string()))
}
}

Expand Down Expand Up @@ -505,6 +514,36 @@ mod tests {

use super::*;

#[test]
fn test_mapping_executable_names() {
let exposed_name = ExposedName::from_str("test_exposed").unwrap();
let executable_name = "test_executable".to_string();
let mapping = Mapping::new(exposed_name.clone(), executable_name);
assert_eq!("test_executable", mapping.executable_name());
assert_eq!("test_executable", mapping.executable_relname());

let executable_name = "nested/test_executable".to_string();
let mapping = Mapping::new(exposed_name.clone(), executable_name);
assert_eq!("test_executable", mapping.executable_name());
assert_eq!("nested/test_executable", mapping.executable_relname());

let executable_name: String;
let expected_exe_relname: &str;
#[cfg(windows)]
{
executable_name = "nested\\test_executable.exe".to_string();
expected_exe_relname = "nested\\test_executable";
}
#[cfg(unix)]
{
executable_name = "nested/test_executable.sh".to_string();
expected_exe_relname = "nested/test_executable";
}
let mapping = Mapping::new(exposed_name.clone(), executable_name);
assert_eq!("test_executable", mapping.executable_name());
assert_eq!(expected_exe_relname, mapping.executable_relname());
}

#[test]
fn test_add_exposed_mapping_new_env() {
let mut manifest = Manifest::default();
Expand Down Expand Up @@ -540,24 +579,24 @@ mod tests {
.iter()
.find(|map| map.exposed_name() == &exposed_name)
.unwrap()
.executable_name();
.executable_relname();
assert_eq!(expected_value, actual_value)
}

#[test]
fn test_add_exposed_mapping_existing_env() {
let mut manifest = Manifest::default();
let exposed_name1 = ExposedName::from_str("test_exposed1").unwrap();
let executable_name1 = "test_executable1".to_string();
let mapping1 = Mapping::new(exposed_name1.clone(), executable_name1);
let executable_relname1 = "test_executable1".to_string();
let mapping1 = Mapping::new(exposed_name1.clone(), executable_relname1);
let env_name = EnvironmentName::from_str("test-env").unwrap();
manifest.add_environment(&env_name, None).unwrap();

manifest.add_exposed_mapping(&env_name, &mapping1).unwrap();

let exposed_name2 = ExposedName::from_str("test_exposed2").unwrap();
let executable_name2 = "test_executable2".to_string();
let mapping2 = Mapping::new(exposed_name2.clone(), executable_name2);
let executable_relname2 = "nested/test_executable2".to_string();
let mapping2 = Mapping::new(exposed_name2.clone(), executable_relname2);
let result = manifest.add_exposed_mapping(&env_name, &mapping2);
assert!(result.is_ok());

Expand All @@ -583,11 +622,11 @@ mod tests {
.iter()
.find(|map| map.exposed_name() == &exposed_name1)
.unwrap()
.executable_name();
.executable_relname();
assert_eq!(expected_value1, actual_value1);

// Check document for executable2
let expected_value2 = "test_executable2";
let expected_value2 = "nested/test_executable2";
let actual_value2 = manifest
.document
.get_or_insert_nested_table(&format!("envs.{env_name}.exposed"))
Expand All @@ -608,7 +647,7 @@ mod tests {
.iter()
.find(|map| map.exposed_name() == &exposed_name2)
.unwrap()
.executable_name();
.executable_relname();
assert_eq!(expected_value2, actual_value2);
}

Expand Down
Loading
Loading