-
Notifications
You must be signed in to change notification settings - Fork 204
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: Export conda explicit specification file from project #1873
Changes from 2 commits
1ff98c3
5a26aed
63479bb
bf436ee
c1a4e30
00007d4
272e8b3
c74c69a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,220 @@ | ||
use std::fs; | ||
use std::path::{Path, PathBuf}; | ||
|
||
use clap::Parser; | ||
|
||
use crate::cli::cli_config::PrefixUpdateConfig; | ||
use crate::cli::LockFileUsageArgs; | ||
use crate::lock_file::UpdateLockFileOptions; | ||
use crate::Project; | ||
use rattler_conda_types::{ExplicitEnvironmentEntry, ExplicitEnvironmentSpec, Platform}; | ||
use rattler_lock::{CondaPackage, Package, PackageHashes, PypiPackage, PypiPackageData, UrlOrPath}; | ||
|
||
#[derive(Debug, Parser)] | ||
#[clap(arg_required_else_help = false)] | ||
pub struct Args { | ||
/// Environment to render | ||
#[arg(short, long)] | ||
environment: Option<String>, | ||
|
||
/// The platform to render. Defaults to the current platform. | ||
#[arg(long)] | ||
pub platform: Option<Platform>, | ||
|
||
/// PyPI dependencies are not supported in the conda spec file. | ||
/// This flag allows creating the spec file even if PyPI dependencies are present. | ||
/// Alternatively see --write-pypi-requirements | ||
#[arg(long, default_value = "false")] | ||
ignore_pypi_errors: bool, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Couldnt we just rename this to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just want to clarify something, since the comment
leads me to believe that there might be some confusion. My understanding is that there are 3 conda formats in the wild currently:
What this PR contains is an implementation of the explicit specification file, so it requires both an environment and platform provided by the user, unless we want to dump a separate file for every combination found in the project. Additionally, since it can't contain pypi packages, a side file must be included or those packages have to be ignored. Making it its own format would require users to make similar choices about what they want in the I thought to make each format its own subcommand since they will have potentially independent flags, rather than a @baszalmstra -- I could change it to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right: ideally at least the docs/interactive
|
||
|
||
/// Write a requirements file containing all pypi dependencies | ||
#[arg(long, default_value = "false", conflicts_with = "ignore_pypi_errors")] | ||
write_pypi_requirements: bool, | ||
|
||
#[clap(flatten)] | ||
pub lock_file_usage: LockFileUsageArgs, | ||
|
||
#[clap(flatten)] | ||
pub prefix_update_config: PrefixUpdateConfig, | ||
} | ||
|
||
fn cwd() -> PathBuf { | ||
std::env::current_dir().expect("failed to obtain current working directory") | ||
} | ||
|
||
fn build_explicit_spec<'a>( | ||
platform: Platform, | ||
conda_packages: impl IntoIterator<Item = &'a CondaPackage>, | ||
) -> miette::Result<ExplicitEnvironmentSpec> { | ||
let mut packages = Vec::new(); | ||
|
||
for cp in conda_packages { | ||
let prec = cp.package_record(); | ||
let mut url = cp.url().to_owned(); | ||
let hash = prec.md5.ok_or(miette::miette!( | ||
"Package {} does not contain an md5 hash", | ||
prec.name.as_normalized() | ||
))?; | ||
|
||
url.set_fragment(Some(&format!("{:x}", hash))); | ||
|
||
packages.push(ExplicitEnvironmentEntry { | ||
url: url.to_owned(), | ||
}); | ||
} | ||
|
||
Ok(ExplicitEnvironmentSpec { | ||
platform: Some(platform), | ||
packages, | ||
}) | ||
} | ||
|
||
fn write_explicit_spec( | ||
target: impl AsRef<Path>, | ||
exp_env_spec: &ExplicitEnvironmentSpec, | ||
) -> miette::Result<()> { | ||
let mut environment = String::new(); | ||
environment.push_str("# Generated by `pixi project export`\n"); | ||
environment.push_str(exp_env_spec.to_spec_string().as_str()); | ||
|
||
fs::write(target, environment) | ||
.map_err(|e| miette::miette!("Could not write environment file: {}", e))?; | ||
baszalmstra marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Ok(()) | ||
} | ||
|
||
fn get_pypi_hash_str(package_data: &PypiPackageData) -> Option<String> { | ||
if let Some(hashes) = &package_data.hash { | ||
let h = match hashes { | ||
PackageHashes::Sha256(h) => format!("--hash=sha256:{:x}", h).to_string(), | ||
PackageHashes::Md5Sha256(_, h) => format!("--hash=sha256:{:x}", h).to_string(), | ||
PackageHashes::Md5(h) => format!("--hash=md5:{:x}", h).to_string(), | ||
}; | ||
Some(h) | ||
} else { | ||
None | ||
} | ||
} | ||
|
||
fn write_pypi_requirements( | ||
target: impl AsRef<Path>, | ||
packages: &[PypiPackage], | ||
) -> miette::Result<()> { | ||
let mut reqs = String::new(); | ||
|
||
for p in packages { | ||
// pip --verify-hashes does not accept hashes for local files | ||
let (s, include_hash) = match p.url() { | ||
UrlOrPath::Url(url) => (url.as_str(), true), | ||
UrlOrPath::Path(path) => ( | ||
path.as_os_str() | ||
.to_str() | ||
.unwrap_or_else(|| panic!("Could not convert {:?} to str", path)), | ||
false, | ||
), | ||
}; | ||
|
||
// remove "direct+ since not valid for pip urls" | ||
let s = s.trim_start_matches("direct+"); | ||
|
||
let hash = match (include_hash, get_pypi_hash_str(p.data().package)) { | ||
(true, Some(h)) => format!(" {}", h), | ||
(false, _) => "".to_string(), | ||
(_, None) => "".to_string(), | ||
}; | ||
|
||
if p.is_editable() { | ||
reqs.push_str(&format!("-e {}{}\n", s, hash)); | ||
} else { | ||
reqs.push_str(&format!("{}{}\n", s, hash)); | ||
} | ||
} | ||
|
||
fs::write(target, reqs) | ||
.map_err(|e| miette::miette!("Could not write requirements file: {}", e))?; | ||
|
||
Ok(()) | ||
} | ||
|
||
pub async fn execute(project: Project, args: Args) -> miette::Result<()> { | ||
let environment = project.environment_from_name_or_env_var(args.environment)?; | ||
// Load the platform | ||
let platform = args.platform.unwrap_or_else(|| environment.best_platform()); | ||
|
||
let lock_file = project | ||
.update_lock_file(UpdateLockFileOptions { | ||
lock_file_usage: args.prefix_update_config.lock_file_usage(), | ||
no_install: args.prefix_update_config.no_install, | ||
..UpdateLockFileOptions::default() | ||
}) | ||
.await? | ||
.lock_file; | ||
|
||
let env = lock_file | ||
.environment(environment.name().as_str()) | ||
.ok_or(miette::miette!( | ||
"unknown environment '{}' in {}", | ||
environment.name(), | ||
project | ||
.manifest_path() | ||
.to_str() | ||
.expect("expected to have a manifest_path") | ||
))?; | ||
|
||
let packages = env.packages(platform).ok_or(miette::miette!( | ||
"platform '{platform}' not found in {}", | ||
project | ||
.manifest_path() | ||
.to_str() | ||
.expect("expected to have a manifest_path"), | ||
))?; | ||
|
||
let mut conda_packages_from_lockfile: Vec<CondaPackage> = Vec::new(); | ||
let mut pypi_packages_from_lockfile: Vec<PypiPackage> = Vec::new(); | ||
|
||
for package in packages { | ||
match package { | ||
Package::Conda(p) => conda_packages_from_lockfile.push(p), | ||
Package::Pypi(pyp) => { | ||
if args.ignore_pypi_errors { | ||
tracing::warn!("ignoring PyPI package since PyPI packages are not supported"); | ||
} else if args.write_pypi_requirements { | ||
pypi_packages_from_lockfile.push(pyp); | ||
} else { | ||
miette::bail!( | ||
"PyPI packages are not supported. Specify `--ignore-pypi-errors` to ignore this error\ | ||
or `--write-pypi-requirements` to write pypi requirements to a separate requirements.txt file" | ||
); | ||
} | ||
} | ||
} | ||
} | ||
|
||
let ees = build_explicit_spec(platform, &conda_packages_from_lockfile)?; | ||
|
||
tracing::info!("Creating conda lock file"); | ||
let target = cwd() | ||
.join(format!( | ||
"conda-{}-{}.lock", | ||
platform, | ||
environment.name().as_str() | ||
)) | ||
.into_os_string(); | ||
|
||
write_explicit_spec(target, &ees)?; | ||
|
||
if args.write_pypi_requirements { | ||
tracing::info!("Creating conda lock file"); | ||
let pypi_target = cwd() | ||
.join(format!( | ||
"requirements-{}-{}.txt", | ||
platform, | ||
environment.name().as_str() | ||
)) | ||
.into_os_string(); | ||
|
||
write_pypi_requirements(pypi_target, &pypi_packages_from_lockfile)?; | ||
} | ||
|
||
Ok(()) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
use std::path::PathBuf; | ||
pub mod conda_explicit_spec; | ||
|
||
use crate::Project; | ||
use clap::Parser; | ||
|
||
/// Commands to export projects to other formats | ||
#[derive(Parser, Debug)] | ||
pub struct Args { | ||
/// The path to 'pixi.toml' or 'pyproject.toml' | ||
#[clap(long, global = true)] | ||
pub manifest_path: Option<PathBuf>, | ||
|
||
#[clap(subcommand)] | ||
pub command: Command, | ||
} | ||
|
||
#[derive(Parser, Debug)] | ||
pub enum Command { | ||
/// Export project environment to a conda explicit specification file | ||
#[clap(visible_alias = "ces")] | ||
CondaExplicitSpec(conda_explicit_spec::Args), | ||
} | ||
|
||
pub async fn execute(args: Args) -> miette::Result<()> { | ||
let project = Project::load_or_else_discover(args.manifest_path.as_deref())?; | ||
match args.command { | ||
Command::CondaExplicitSpec(args) => conda_explicit_spec::execute(project, args).await?, | ||
}; | ||
Ok(()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A high-value property of
conda-lock
is being able to get all the locks for a single env. Forpixi
, I'd probably also want multiple environments, by default all. This explosion is handled with the--filename-template
, where there are a few known tokens available. Alsoconstructor
really prefers.txt
as an extension, but who knows.Putting these together:
Might yield:
Or:
Would generate something more like:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to add a way to dump all env/platform combinations if that's what the pixi team thinks is the way to go. I'm not sure about
--filename-template
. I think that's non-trivial to do in rust, or at least I'm not sure how best to implement that. Indicatif does something similar with progress bar templating, so that could give some ideas. Would it be sufficient to have an option to dump all, but the file naming convention is not flexible? Currently it's justconda-{platform}-{env}.lock
andrequirements-{platform}-{env}.txt
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love that. Would love to see that either in this PR or a followup!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, the minimal thing for e.g.
miniforge
-style repos usingconstructor
would be:.txt
ending for the conda-lock so thatconstructor
knows what to do with it without invoking the solver again--output-folder
(which it would ensure exists)I would still want to check these in, as they are pretty much optimal inputs to
inputs/depends-on
and, by extension, CI caches, whilepixi.lock
is generally too-broad (e.g. don't rebuild the whole product just because you changed the test procedure).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bollwyvl Ok, so from what you're saying as well as the suggestions in #1873 (comment), we want to create an output directory that will contain the exported files. The linked comment just has it as a positional argument, while you're suggesting a flag. I'm not sure which would be preferred. Other questions/comments:
.txt
suffix since that's in the CEP standard. I was just basing it on the naming convention from the explicit render inconda-lock
which used.lock
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Positional, but defaults to
.
seems reasonable as......while I haven't kicked the tires, I imagine the runtime of this is probably so small it won't matter.
conda-lock
lets a user emit whatever they want, and doesn't generally talk about a specific named environment, so always needed something in anything but the most trivial case. Or, put differently, it never bothered me enough to PR to change it.