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: Persistant shell manifests #1080

Merged
merged 11 commits into from
Apr 5, 2024
1 change: 1 addition & 0 deletions src/activation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ impl Project {
version.to_string()
}),
),
(String::from("PIXI_IN_SHELL"), String::from("1")),
]);

if let Ok(exe_path) = std::env::current_exe() {
Expand Down
1 change: 1 addition & 0 deletions src/cli/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
)
}

Project::warn_on_discovered_from_env(args.manifest_path.as_deref());
Ok(())
}

Expand Down
4 changes: 4 additions & 0 deletions src/cli/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -379,9 +379,13 @@ pub async fn execute(args: Args) -> miette::Result<()> {

if args.json {
println!("{}", serde_json::to_string_pretty(&info).into_diagnostic()?);

Project::warn_on_discovered_from_env(args.manifest_path.as_deref());
Ok(())
} else {
println!("{}", info);

Project::warn_on_discovered_from_env(args.manifest_path.as_deref());
Ok(())
}
}
5 changes: 2 additions & 3 deletions src/cli/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ pub struct Args {
pub async fn execute(args: Args) -> miette::Result<()> {
let project =
Project::load_or_else_discover(args.manifest_path.as_deref())?.with_cli_config(args.config);
let environment_name = args
.environment
.map_or_else(|| EnvironmentName::Default, EnvironmentName::Named);
let environment_name = EnvironmentName::from_arg_or_env_var(args.environment);
let environment = project
.environment(&environment_name)
.ok_or_else(|| miette::miette!("unknown environment '{environment_name}'"))?;
Expand All @@ -47,5 +45,6 @@ pub async fn execute(args: Args) -> miette::Result<()> {
console::style(console::Emoji("✔ ", "")).green(),
project.root().display()
);
Project::warn_on_discovered_from_env(args.manifest_path.as_deref());
Ok(())
}
6 changes: 3 additions & 3 deletions src/cli/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,7 @@ where

pub async fn execute(args: Args) -> miette::Result<()> {
let project = Project::load_or_else_discover(args.manifest_path.as_deref())?;
let environment_name = args
.environment
.map_or_else(|| EnvironmentName::Default, EnvironmentName::Named);
let environment_name = EnvironmentName::from_arg_or_env_var(args.environment);
let environment = project
.environment(&environment_name)
.ok_or_else(|| miette::miette!("unknown environment '{environment_name}'"))?;
Expand Down Expand Up @@ -199,6 +197,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
"{}No packages found.",
console::style(console::Emoji("✘ ", "")).red(),
);
Project::warn_on_discovered_from_env(args.manifest_path.as_deref());
return Ok(());
}

Expand All @@ -211,6 +210,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
print_packages_as_table(&packages_to_output).expect("an io error occurred");
}

Project::warn_on_discovered_from_env(args.manifest_path.as_deref());
Ok(())
}

Expand Down
1 change: 1 addition & 0 deletions src/cli/remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,5 +134,6 @@ pub async fn execute(args: Args) -> miette::Result<()> {
)
.await?;

Project::warn_on_discovered_from_env(args.manifest_path.as_deref());
Ok(())
}
1 change: 1 addition & 0 deletions src/cli/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
.into_diagnostic()?;
}

Project::warn_on_discovered_from_env(args.manifest_path.as_deref());
Ok(())
}

Expand Down
1 change: 1 addition & 0 deletions src/cli/search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
search_exact_package(package_name, repo_data, stdout).await?;
}

Project::warn_on_discovered_from_env(args.manifest_path.as_deref());
Ok(())
}

Expand Down
4 changes: 1 addition & 3 deletions src/cli/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,7 @@ async fn start_nu_shell(
pub async fn execute(args: Args) -> miette::Result<()> {
let project =
Project::load_or_else_discover(args.manifest_path.as_deref())?.with_cli_config(args.config);
let environment_name = args
.environment
.map_or_else(|| EnvironmentName::Default, EnvironmentName::Named);
let environment_name = EnvironmentName::from_arg_or_env_var(args.environment);
let environment = project
.environment(&environment_name)
.ok_or_else(|| miette::miette!("unknown environment '{environment_name}'"))?;
Expand Down
4 changes: 2 additions & 2 deletions src/cli/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use itertools::Itertools;
use miette::miette;
use rattler_conda_types::Platform;
use std::path::PathBuf;
use std::str::FromStr;
use toml_edit::{Array, Item, Table, Value};

#[derive(Parser, Debug)]
Expand Down Expand Up @@ -253,7 +252,7 @@ pub fn execute(args: Args) -> miette::Result<()> {
);
}
Operation::List(args) => {
let env = EnvironmentName::from_str(args.environment.as_deref().unwrap_or("default"))?;
let env = EnvironmentName::from_arg_or_env_var(args.environment);
let tasks = project
.environment(&env)
.ok_or(miette!("Environment `{}` not found in project", env))?
Expand All @@ -280,6 +279,7 @@ pub fn execute(args: Args) -> miette::Result<()> {
}
};

Project::warn_on_discovered_from_env(args.manifest_path.as_deref());
Ok(())
}

Expand Down
5 changes: 2 additions & 3 deletions src/cli/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,7 @@ static UTF8_SYMBOLS: Symbols = Symbols {

pub async fn execute(args: Args) -> miette::Result<()> {
let project = Project::load_or_else_discover(args.manifest_path.as_deref())?;
let environment_name = args
.environment
.map_or_else(|| EnvironmentName::Default, EnvironmentName::Named);
let environment_name = EnvironmentName::from_arg_or_env_var(args.environment);
let environment = project
.environment(&environment_name)
.ok_or_else(|| miette::miette!("unknown environment '{environment_name}'"))?;
Expand All @@ -99,6 +97,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
} else {
print_dependency_tree(&dep_map, &direct_deps, &args.regex)?;
}
Project::warn_on_discovered_from_env(args.manifest_path.as_deref());
Ok(())
}

Expand Down
16 changes: 16 additions & 0 deletions src/project/manifest/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,22 @@ impl EnvironmentName {
pub fn fancy_display(&self) -> console::StyledObject<&str> {
console::style(self.as_str()).magenta()
}

/// Tries to read the environment name from an argument, then it will try
/// to read from an environment variable, otherwise it will fall back to default
pub fn from_arg_or_env_var(arg_name: Option<String>) -> Self {
if let Some(arg_name) = arg_name {
return EnvironmentName::Named(arg_name);
} else if std::env::var("PIXI_IN_SHELL").is_ok() {
if let Ok(env_var_name) = std::env::var("PIXI_ENVIRONMENT_NAME") {
if env_var_name == consts::DEFAULT_ENVIRONMENT_NAME {
return EnvironmentName::Default;
}
return EnvironmentName::Named(env_var_name);
}
}
EnvironmentName::Default
}
}

impl Borrow<str> for EnvironmentName {
Expand Down
32 changes: 32 additions & 0 deletions src/project/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,20 @@ impl Project {
PYPROJECT_MANIFEST
),
};
abkfenris marked this conversation as resolved.
Show resolved Hide resolved

if std::env::var("PIXI_IN_SHELL").is_ok() {
if let Ok(env_manifest_path) = std::env::var("PIXI_PROJECT_MANIFEST") {
if env_manifest_path.as_str() != project_toml.to_str().unwrap() {
tracing::warn!(
"Using manifest {} from `PIXI_PROJECT_MANIFEST` rather than local {}",
env_manifest_path,
project_toml.to_string_lossy()
);
return Self::load(Path::new(env_manifest_path.as_str()));
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I would change this to only display a warning if the discovered manifest is not equal to the manifest from the environment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what this should do. Are you finding an instance when it's warning more than it should?

Warning when local doesn't match the manifest from environment variable

pixi on  manifest-in-shell is 📦 v0.17.1 via 🦀 v1.76.0 took 5s
❯
pixi shell --manifest-path /Users/akerney/GMRI/NERACOOS/NERACOOS_ERDDAP_K8S/pixi.toml
 . "/var/folders/n6/hl07fcc55ys7mbybt01gb8r80000gn/T/pixi_env_LkI.sh"

pixi on  manifest-in-shell is 📦 v0.17.1 via 🦀 v1.76.0
❯  . "/var/folders/n6/hl07fcc55ys7mbybt01gb8r80000gn/T/pixi_env_LkI.sh"
(sea-eagle)
pixi on  manifest-in-shell is 📦 v0.17.1 via 🦀 v1.76.0 via 🅒 sea-eagle
❯ pixi info
 WARN pixi::project: Using manifest /Users/akerney/GMRI/NERACOOS/NERACOOS_ERDDAP_K8S/pixi.toml from `PIXI_PROJECT_MANIFEST` rather than local /Users/akerney/Geek/pixi/pixi.toml
      Pixi version: 0.17.1
          Platform: osx-arm64
  Virtual packages: __unix=0=0
                  : __osx=14.4.1=0
                  : __archspec=1=arm64
         Cache dir: /Users/akerney/Library/Caches/rattler/cache
      Auth storage: /Users/akerney/.rattler/credentials.json

Project
------------
           Version: 0.1.0
     Manifest file: /Users/akerney/GMRI/NERACOOS/NERACOOS_ERDDAP_K8S/pixi.toml
  Config locations:
      Last updated: 28-03-2024 10:18:19

Environments
------------
       Environment: default
          Features: default
          Channels: conda-forge
  Dependency count: 28
      Dependencies: python, cftime, dagster-postgres, dagster-k8s, dagster, dask, fsspec, ioos_qc, pandas, pydantic, pystac, python-frontmatter, yaml, requests, sentry-sdk, shapely, xarray, netcdf4, pint, ipykernel, matplotlib, erddapy, uvicorn, xpublish-edr, fastapi, xpublish, xpublish-opendap, uxarray
  Target platforms: osx-arm64
             Tasks: install-kernel


 WARN pixi::project: Used manifest /Users/akerney/GMRI/NERACOOS/NERACOOS_ERDDAP_K8S/pixi.toml from `PIXI_PROJECT_MANIFEST` rather than local /Users/akerney/Geek/pixi/pixi.toml
(sea-eagle)

Quiet when they are the same.

pixi on  manifest-in-shell [+] is 📦 v0.17.1 via 🦀 v1.76.0 took 5s
❯
pixi shell
 . "/var/folders/n6/hl07fcc55ys7mbybt01gb8r80000gn/T/pixi_env_2hk.sh"

pixi on  manifest-in-shell is 📦 v0.17.1 via 🦀 v1.76.0
❯  . "/var/folders/n6/hl07fcc55ys7mbybt01gb8r80000gn/T/pixi_env_2hk.sh"
(pixi)
pixi on  manifest-in-shell is 📦 v0.17.1 via 🦀 v1.76.0 via 🅒 pixi
❯ pixi info
      Pixi version: 0.17.1
          Platform: osx-arm64
  Virtual packages: __unix=0=0
                  : __osx=14.4.1=0
                  : __archspec=1=arm64
         Cache dir: /Users/akerney/Library/Caches/rattler/cache
      Auth storage: /Users/akerney/.rattler/credentials.json

Project
------------
     Manifest file: /Users/akerney/Geek/pixi/pixi.toml
  Config locations:
      Last updated: 28-03-2024 09:26:37

Environments
------------
       Environment: default
          Features: default
          Channels: conda-forge
  Dependency count: 7
      Dependencies: pre-commit, rust, openssl, pkg-config, git, cffconvert, tbump
  Target platforms: linux-64, win-64, osx-64, osx-arm64
             Tasks: install, build, lint, test, test-all, bump

       Environment: docs
          Features: docs, default
          Channels: conda-forge
  Dependency count: 13
      Dependencies: mkdocs, mkdocs-material, mdx_truly_sane_lists, pillow, cairosvg, mike, pre-commit, rust, openssl, pkg-config, git, cffconvert, tbump
  Target platforms: osx-arm64, win-64, osx-64, linux-64
             Tasks: install, docs, deploy-dev, deploy-latest, test-all, build, lint, bump, test, build-docs

       Environment: schema
          Features: schema, default
          Channels: conda-forge
  Dependency count: 12
      Dependencies: python, pytest, jsonschema, pydantic, pyyaml, pre-commit, rust, openssl, pkg-config, git, cffconvert, tbump
  Target platforms: osx-64, win-64, linux-64, osx-arm64
             Tasks: bump, test-all, build, generate-schema, install, test, lint, test-schema


Self::load(&project_toml)
}

Expand Down Expand Up @@ -210,6 +224,24 @@ impl Project {
Ok(project)
}

/// Warns if Pixi is using a manifest from an environment variable rather than a discovered version
pub fn warn_on_discovered_from_env(manifest_path: Option<&Path>) {
if manifest_path.is_none() && std::env::var("PIXI_IN_SHELL").is_ok() {
let discover_path = find_project_manifest();
let env_path = std::env::var("PIXI_PROJECT_MANIFEST");

if let (Some(discover_path), Ok(env_path)) = (discover_path, env_path) {
if env_path.as_str() != discover_path.to_str().unwrap() {
tracing::warn!(
"Used manifest {} from `PIXI_PROJECT_MANIFEST` rather than local {}",
env_path,
discover_path.to_string_lossy()
);
}
}
}
}
baszalmstra marked this conversation as resolved.
Show resolved Hide resolved

pub fn with_cli_config<C>(mut self, config: C) -> Self
where
C: Into<Config>,
Expand Down
Loading