Skip to content

Commit

Permalink
refactor: project model using targets, features and environments (#616)
Browse files Browse the repository at this point in the history
First refactor to support multiple environments.

This PR refactors the internal project model and adapts the code to use
it. The concept of a `Feature` and a `Target` is introduced. At the
moment when parsing the project manifest a "default" feature is created
which is used throughout the code.

> [!NOTE] 
> to reviewers, this is a first PR of a few to come. I want to quickly
and iteratively land PRs. This is not the final result.

#### Feature

In the multi-env story
[`Feature`s](https://pixi.sh/design_proposals/multi_environment_proposal/#feature-environment-set-definitions)
describe a set of dependencies/tasks/etc. This concept was also
introduced in the project model. At the moment there is only one default
feature (described by all the keys at the root of the toml) but the
project model already supports multiple features which we can use in a
follow-up PR.

#### Target

A target encapsulates everything that you can find in for instance a
`[target.win-64]` table. The same datastructure is reused for elements
"at the root" e.g.

```toml
[dependencies]
```

is internally rewritten to

```toml
[target."default".dependencies]
```

where "default" has a special meaning.

`Targets` is a datastructure that encapsulates a collection of targets
and allows querying which targets apply in certain situations. E.g.

```toml
[dependencies]
..

[target.linux-64.dependencies]
...
```

`Targets` will hold two `Target`s, the default target (`[dependencies]`)
and the `linux-64` target. You can request all applicable targets for a
certain platform and it will return these two targets in order of most
specific first.

#### Environment

`Environment` is already part of the model (with a default one being
created) but it is not yet used throughout the rest of the code. A
followup PR will refactor the codebase to always use `Environment`s to
derive the tasks, dependencies and system-requirements currently
available.

#### Tests

The PR refactors a lot of snapshot tests. Since the project model is now
very different from the toml layout the debug snapshots
often didnt make a whole lot of sense. I changed most of the tests to
more closely verify the actual expected result of certain operations
instead of "just" outputting a snapshot. Hopefully this makes them less
prone to change with future iterations.
  • Loading branch information
baszalmstra authored Jan 6, 2024
1 parent dc5bab5 commit 625d475
Show file tree
Hide file tree
Showing 58 changed files with 2,964 additions and 3,670 deletions.
388 changes: 208 additions & 180 deletions Cargo.lock

Large diffs are not rendered by default.

20 changes: 10 additions & 10 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ regex = "1.10.2"
reqwest = { version = "0.11.22", default-features = false }
rip = { package = "rattler_installs_packages", version = "0.1.0", default-features = false }
serde = "1.0.193"
serde-untagged = "0.1.5"
serde_json = "1.0.108"
serde_spanned = "0.6.4"
serde_with = { version = "3.4.0", features = ["indexmap"] }
Expand Down Expand Up @@ -78,17 +79,16 @@ tokio = { version = "1.34.0", features = ["rt"] }
toml = "0.8.8"

[patch.crates-io]
#rattler = { git = "https://github.com/mamba-org/rattler", branch = "main" }
#rattler_conda_types = { git = "https://github.com/mamba-org/rattler", branch = "main" }
#rattler_digest = { git = "https://github.com/mamba-org/rattler", branch = "main" }
#rattler_lock = { git = "https://github.com/mamba-org/rattler", branch = "main" }
#rattler_networking = { git = "https://github.com/mamba-org/rattler", branch = "main" }
#rattler_repodata_gateway = { git = "https://github.com/mamba-org/rattler", branch = "main" }
#rattler_shell = { git = "https://github.com/mamba-org/rattler", branch = "main" }
#rattler_solve = { git = "https://github.com/mamba-org/rattler", branch = "main" }
#rattler_virtual_packages = { git = "https://github.com/mamba-org/rattler", branch = "main" }
rattler = { git = "https://github.com/mamba-org/rattler", branch = "main" }
rattler_conda_types = { git = "https://github.com/mamba-org/rattler", branch = "main" }
rattler_digest = { git = "https://github.com/mamba-org/rattler", branch = "main" }
rattler_lock = { git = "https://github.com/mamba-org/rattler", branch = "main" }
rattler_networking = { git = "https://github.com/mamba-org/rattler", branch = "main" }
rattler_repodata_gateway = { git = "https://github.com/mamba-org/rattler", branch = "main" }
rattler_shell = { git = "https://github.com/mamba-org/rattler", branch = "main" }
rattler_solve = { git = "https://github.com/mamba-org/rattler", branch = "main" }
rattler_virtual_packages = { git = "https://github.com/mamba-org/rattler", branch = "main" }
#rip = { package = "rattler_installs_packages", git = "https://github.com/prefix-dev/rattler_installs_packages", branch = "main" }

#deno_task_shell = { path = "../deno_task_shell" }

#rattler = { path = "../rattler/crates/rattler" }
Expand Down
14 changes: 6 additions & 8 deletions src/cli/add.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{
consts,
environment::{get_up_to_date_prefix, verify_prefix_location_unchanged, LockFileUsage},
project::{python::PyPiRequirement, DependencyType, Project, SpecType},
project::{manifest::PyPiRequirement, DependencyType, Project, SpecType},
};
use clap::Parser;
use indexmap::IndexMap;
Expand Down Expand Up @@ -207,12 +207,12 @@ pub async fn add_pypi_specs_to_project(
// TODO: Get best version
// Add the dependency to the project
if specs_platforms.is_empty() {
project.manifest.add_pypi_dependency(name, spec)?;
project.manifest.add_pypi_dependency(name, spec, None)?;
} else {
for platform in specs_platforms.iter() {
project
.manifest
.add_target_pypi_dependency(*platform, name.clone(), spec)?;
.add_pypi_dependency(name, spec, Some(*platform))?;
}
}
}
Expand Down Expand Up @@ -268,9 +268,7 @@ pub async fn add_conda_specs_to_project(
// SpecType::Build => project.build_dependencies(platform)?,
// SpecType::Run => project.dependencies(platform)?,
// };
let mut current_specs = project.dependencies(platform)?;
current_specs.extend(project.host_dependencies(platform)?);
current_specs.extend(project.build_dependencies(platform)?);
let current_specs = project.all_dependencies(platform);

// Solve the environment with the new specs added
let solved_versions = match determine_best_version(
Expand Down Expand Up @@ -312,12 +310,12 @@ pub async fn add_conda_specs_to_project(

// Add the dependency to the project
if specs_platforms.is_empty() {
project.manifest.add_dependency(&spec, spec_type)?;
project.manifest.add_dependency(&spec, spec_type, None)?;
} else {
for platform in specs_platforms.iter() {
project
.manifest
.add_target_dependency(*platform, &spec, spec_type)?;
.add_dependency(&spec, spec_type, Some(*platform))?;
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/cli/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ fn last_updated(path: impl Into<PathBuf>) -> miette::Result<String> {
/// Returns number of dependencies on current platform
fn dependency_count(project: &Project) -> miette::Result<u64> {
let dep_count = project
.all_dependencies(Platform::current())?
.all_dependencies(Platform::current())
.keys()
.cloned()
.fold(0, |acc, _| acc + 1);
Expand Down
30 changes: 13 additions & 17 deletions src/cli/remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
section_name
};

fn format_ok_message(pkg_name: &String, pkg_extras: &String, table_name: &String) -> String {
fn format_ok_message(pkg_name: &str, pkg_extras: &str, table_name: &str) -> String {
format!(
"Removed {} from [{}]",
console::style(format!("{pkg_name} {pkg_extras}")).bold(),
Expand All @@ -81,38 +81,34 @@ pub async fn execute(args: Args) -> miette::Result<()> {
if args.pypi {
let all_pkg_name = convert_pkg_name::<rip::types::PackageName>(&deps)?;
for dep in all_pkg_name.iter() {
let result = if let Some(p) = &args.platform {
project.manifest.remove_target_pypi_dependency(dep, p)?
} else {
project.manifest.remove_pypi_dependency(dep)?
};
let (name, req) = project
.manifest
.remove_pypi_dependency(dep, args.platform)?;
sucessful_output.push(format_ok_message(
&result.0.as_str().to_string(),
&result.1.to_string(),
name.as_str(),
&req.to_string(),
&table_name,
));
}
} else {
let all_pkg_name = convert_pkg_name::<rattler_conda_types::PackageName>(&deps)?;
for dep in all_pkg_name.iter() {
let result = if let Some(p) = &args.platform {
project
.manifest
.remove_target_dependency(dep, &spec_type, p)?
} else {
project.manifest.remove_dependency(dep, &spec_type)?
};
let (name, req) = project
.manifest
.remove_dependency(dep, spec_type, args.platform)?;
sucessful_output.push(format_ok_message(
&result.0,
&result.1.to_string(),
name.as_source(),
&req.to_string(),
&table_name,
));
}
};

project.save()?;
eprintln!("{}", sucessful_output.join("\n"));

// updating prefix after removing from toml
let _ = get_up_to_date_prefix(&project, LockFileUsage::Update, false, None).await?;

Ok(())
}
5 changes: 2 additions & 3 deletions src/cli/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ use crate::task::{
ExecutableTask, FailedToParseShellScript, InvalidWorkingDirectory, TraversalError,
};
use crate::{
environment::get_up_to_date_prefix, prefix::Prefix, progress::await_in_progress,
project::environment::get_metadata_env, Project,
environment::get_up_to_date_prefix, prefix::Prefix, progress::await_in_progress, Project,
};
use rattler_shell::{
activation::{ActivationVariables, Activator, PathModificationBehavior},
Expand Down Expand Up @@ -169,7 +168,7 @@ pub async fn get_task_env(
let activation_env = run_activation_async(project, prefix).await?;

// Get environment variables from the manifest
let manifest_env = get_metadata_env(project);
let manifest_env = project.get_metadata_env();

// Construct command environment by concatenating the environments
Ok(std::env::vars()
Expand Down
3 changes: 1 addition & 2 deletions src/cli/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use crate::unix::PtySession;
use crate::cli::LockFileUsageArgs;
use crate::environment::get_up_to_date_prefix;
use crate::environment::LockFileUsage;
use crate::project::environment::get_metadata_env;
#[cfg(target_family = "windows")]
use rattler_shell::shell::CmdExe;

Expand Down Expand Up @@ -209,7 +208,7 @@ pub async fn get_shell_env(
let activation_env = run_activation_async(project, prefix).await?;

// Get environment variables from the manifest
let manifest_env = get_metadata_env(project);
let manifest_env = project.get_metadata_env();

// Add the conda default env variable so that the existing tools know about the env.
let mut shell_env = HashMap::new();
Expand Down
44 changes: 43 additions & 1 deletion src/cli/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use clap::Parser;
use itertools::Itertools;
use rattler_conda_types::Platform;
use std::path::PathBuf;
use toml_edit::{Array, Item, Table, Value};

#[derive(Parser, Debug)]
pub enum Operation {
Expand Down Expand Up @@ -142,6 +143,7 @@ pub fn execute(args: Args) -> miette::Result<()> {
project
.manifest
.add_task(name, task.clone(), args.platform)?;
project.save()?;
eprintln!(
"{}Added task {}: {}",
console::style(console::Emoji("✔ ", "+")).green(),
Expand All @@ -155,7 +157,7 @@ pub fn execute(args: Args) -> miette::Result<()> {
if let Some(platform) = args.platform {
if !project
.manifest
.target_specific_tasks(platform)
.tasks(Some(platform))
.contains_key(name.as_str())
{
eprintln!(
Expand Down Expand Up @@ -195,6 +197,7 @@ pub fn execute(args: Args) -> miette::Result<()> {

for (name, platform) in to_remove {
project.manifest.remove_task(name, platform)?;
project.save()?;
eprintln!(
"{}Removed task {} ",
console::style(console::Emoji("✔ ", "+")).green(),
Expand All @@ -208,6 +211,7 @@ pub fn execute(args: Args) -> miette::Result<()> {
project
.manifest
.add_task(name, task.clone(), args.platform)?;
project.save()?;
eprintln!(
"{} Added alias {}: {}",
console::style("@").blue(),
Expand Down Expand Up @@ -239,3 +243,41 @@ pub fn execute(args: Args) -> miette::Result<()> {

Ok(())
}

impl From<Task> for Item {
fn from(value: Task) -> Self {
match value {
Task::Plain(str) => Item::Value(str.into()),
Task::Execute(process) => {
let mut table = Table::new().into_inline_table();
match process.cmd {
CmdArgs::Single(cmd_str) => {
table.insert("cmd", cmd_str.into());
}
CmdArgs::Multiple(cmd_strs) => {
table.insert("cmd", Value::Array(Array::from_iter(cmd_strs)));
}
}
if !process.depends_on.is_empty() {
table.insert(
"depends_on",
Value::Array(Array::from_iter(process.depends_on)),
);
}
if let Some(cwd) = process.cwd {
table.insert("cwd", cwd.to_string_lossy().to_string().into());
}
Item::Value(Value::InlineTable(table))
}
Task::Alias(alias) => {
let mut table = Table::new().into_inline_table();
table.insert(
"depends_on",
Value::Array(Array::from_iter(alias.depends_on)),
);
Item::Value(Value::InlineTable(table))
}
_ => Item::None,
}
}
}
2 changes: 1 addition & 1 deletion src/lock_file/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ async fn resolve_platform(
platform: Platform,
pb: ProgressBar,
) -> miette::Result<LockedPackagesBuilder> {
let dependencies = project.all_dependencies(platform)?;
let dependencies = project.all_dependencies(platform);
let match_specs = dependencies
.iter()
.map(|(name, constraint)| MatchSpec::from_nameless(constraint.clone(), Some(name.clone())))
Expand Down
29 changes: 4 additions & 25 deletions src/lock_file/satisfiability.rs
Original file line number Diff line number Diff line change
@@ -1,41 +1,20 @@
use super::package_identifier;
use crate::project::{DependencyKind, DependencyName};
use crate::{
lock_file::pypi::{determine_marker_environment, is_python_record},
Project,
};
use itertools::Itertools;
use miette::IntoDiagnostic;
use pep508_rs::Requirement;
use rattler_conda_types::{MatchSpec, PackageName, Platform, Version};
use rattler_conda_types::{MatchSpec, Platform, Version};
use rattler_lock::{CondaLock, LockedDependency, LockedDependencyKind};
use rip::types::NormalizedPackageName;
use std::{
collections::{HashMap, HashSet, VecDeque},
fmt::{Display, Formatter},
str::FromStr,
};

#[derive(Clone)]
enum DependencyKind {
Conda(MatchSpec),
PyPi(pep508_rs::Requirement),
}

impl Display for DependencyKind {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
DependencyKind::Conda(spec) => write!(f, "{}", spec),
DependencyKind::PyPi(req) => write!(f, "{}", req),
}
}
}

#[derive(Eq, PartialEq, Hash)]
enum DependencyName {
Conda(PackageName),
PyPi(NormalizedPackageName),
}

/// Returns true if the locked packages match the dependencies in the project.
pub fn lock_file_satisfies_project(
project: &Project,
Expand Down Expand Up @@ -66,15 +45,15 @@ pub fn lock_file_satisfies_project(
for platform in platforms.iter().cloned() {
// Check if all dependencies exist in the lock-file.
let conda_dependencies = project
.all_dependencies(platform)?
.all_dependencies(platform)
.into_iter()
.map(|(name, spec)| DependencyKind::Conda(MatchSpec::from_nameless(spec, Some(name))))
.collect::<Vec<_>>();

let mut pypi_dependencies = project
.pypi_dependencies(platform)
.into_iter()
.map(|(name, requirement)| requirement.as_pep508(name))
.map(|(name, requirement)| requirement.as_pep508(&name))
.map(DependencyKind::PyPi)
.peekable();

Expand Down
39 changes: 0 additions & 39 deletions src/project/environment.rs

This file was deleted.

Loading

0 comments on commit 625d475

Please sign in to comment.