Skip to content

Commit

Permalink
refactor: make the channel in the matchspec struct an actual channel (#…
Browse files Browse the repository at this point in the history 
…401)

This should really help make installation more correct. 

I got the issue that `conda-forge::python` and `conda::python` got the
same result after installation.
So I changed the type of the channel to be a channel from the start. 

---------

Co-authored-by: Bas Zalmstra <zalmstra.bas@gmail.com>
  • Loading branch information
@ruben-arts @baszalmstra
ruben-arts and baszalmstra authored Nov 14, 2023
1 parent e75bbe2 commit f839abd
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 24 deletions.
2 changes: 1 addition & 1 deletion crates/rattler_conda_types/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ itertools = "0.11.0"
lazy-regex = "3.0.2"
nom = "7.1.3"
regex = "1.9.6"
serde = { version = "1.0.188", features = ["derive"] }
serde = { version = "1.0.188", features = ["derive", "rc"] }
serde_json = "1.0.107"
serde_yaml = "0.9.25"
serde_with = { version = "3.3.0", features = ["indexmap_2"] }
Expand Down
47 changes: 38 additions & 9 deletions crates/rattler_conda_types/src/match_spec/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
use crate::{build_spec::BuildNumberSpec, PackageName, PackageRecord, VersionSpec};
use rattler_digest::{serde::SerializableHash, Md5Hash, Sha256Hash};
use serde::{Deserialize, Serialize};
use serde::{Deserialize, Deserializer, Serialize};
use serde_with::{serde_as, skip_serializing_none, DisplayFromStr};
use std::fmt::{Debug, Display, Formatter};
use std::hash::Hash;
use std::sync::Arc;

use crate::Channel;
use crate::ChannelConfig;

pub mod matcher;
pub mod parse;
Expand Down Expand Up @@ -64,8 +68,9 @@ use matcher::StringMatcher;
/// # Examples:
///
/// ```rust
/// use rattler_conda_types::{MatchSpec, VersionSpec, StringMatcher, PackageName};
/// use rattler_conda_types::{MatchSpec, VersionSpec, StringMatcher, PackageName, Channel};
/// use std::str::FromStr;
/// use std::sync::Arc;
///
/// let spec = MatchSpec::from_str("foo 1.0 py27_0").unwrap();
/// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo")));
Expand All @@ -80,18 +85,18 @@ use matcher::StringMatcher;
/// let spec = MatchSpec::from_str(r#"conda-forge::foo[version="1.0.*"]"#).unwrap();
/// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo")));
/// assert_eq!(spec.version, Some(VersionSpec::from_str("1.0.*").unwrap()));
/// assert_eq!(spec.channel, Some("conda-forge".to_string()));
/// assert_eq!(spec.channel, Some(Channel::from_str("conda-forge", &Default::default()).map(|channel| Arc::new(channel)).unwrap()));
///
/// let spec = MatchSpec::from_str("conda-forge/linux-64::foo>=1.0").unwrap();
/// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo")));
/// assert_eq!(spec.version, Some(VersionSpec::from_str(">=1.0").unwrap()));
/// assert_eq!(spec.channel, Some("conda-forge".to_string()));
/// assert_eq!(spec.channel, Some(Channel::from_str("conda-forge", &Default::default()).map(|channel| Arc::new(channel)).unwrap()));
/// assert_eq!(spec.subdir, Some("linux-64".to_string()));
///
/// let spec = MatchSpec::from_str("*/linux-64::foo>=1.0").unwrap();
/// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo")));
/// assert_eq!(spec.version, Some(VersionSpec::from_str(">=1.0").unwrap()));
/// assert_eq!(spec.channel, Some("*".to_string()));
/// assert_eq!(spec.channel, Some(Channel::from_str("*", &Default::default()).map(|channel| Arc::new(channel)).unwrap()));
/// assert_eq!(spec.subdir, Some("linux-64".to_string()));
///
/// let spec = MatchSpec::from_str(r#"foo[build="py2*"]"#).unwrap();
Expand Down Expand Up @@ -125,7 +130,7 @@ pub struct MatchSpec {
/// Match the specific filename of the package
pub file_name: Option<String>,
/// The channel of the package
pub channel: Option<String>,
pub channel: Option<Arc<Channel>>,
/// The subdir of the channel
pub subdir: Option<String>,
/// The namespace of the package (currently not used)
Expand All @@ -141,8 +146,10 @@ pub struct MatchSpec {
impl Display for MatchSpec {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
if let Some(channel) = &self.channel {
// TODO: namespace
write!(f, "{}", channel)?;
if let Some(name) = &channel.name {
// TODO: namespace
write!(f, "{}", name)?;
}
}

if let Some(subdir) = &self.subdir {
Expand Down Expand Up @@ -264,7 +271,8 @@ pub struct NamelessMatchSpec {
/// Match the specific filename of the package
pub file_name: Option<String>,
/// The channel of the package
pub channel: Option<String>,
#[serde(deserialize_with = "deserialize_channel")]
pub channel: Option<Arc<Channel>>,
/// The subdir of the channel
pub subdir: Option<String>,
/// The namespace of the package (currently not used)
Expand Down Expand Up @@ -371,6 +379,27 @@ impl MatchSpec {
}
}

/// Deserialize channel from string
/// TODO: This should be refactored so that the front ends are the one setting the channel config,
/// and rattler only takes care of the url.
fn deserialize_channel<'de, D>(deserializer: D) -> Result<Option<Arc<Channel>>, D::Error>
where
D: Deserializer<'de>,
{
let s: Option<String> = Option::deserialize(deserializer)?;

match s {
Some(str_val) => {
let config = ChannelConfig::default();

Channel::from_str(str_val, &config)
.map(|channel| Some(Arc::new(channel)))
.map_err(serde::de::Error::custom)
}
None => Ok(None),
}
}

#[cfg(test)]
mod tests {
use std::str::FromStr;
Expand Down
39 changes: 32 additions & 7 deletions crates/rattler_conda_types/src/match_spec/parse.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ use crate::package::ArchiveType;
use crate::version_spec::version_tree::{recognize_constraint, recognize_version};
use crate::version_spec::{is_start_of_version_constraint, ParseVersionSpecError};
use crate::{
InvalidPackageNameError, NamelessMatchSpec, PackageName, ParseChannelError, VersionSpec,
Channel, InvalidPackageNameError, NamelessMatchSpec, PackageName, ParseChannelError,
VersionSpec,
};
use nom::branch::alt;
use nom::bytes::complete::{tag, take_till1, take_until, take_while, take_while1};
Expand Down Expand Up @@ -397,10 +398,12 @@ fn parse(input: &str) -> Result<MatchSpec, ParseMatchSpecError> {

if let Some(channel_str) = channel_str {
if let Some((channel, subdir)) = channel_str.rsplit_once('/') {
nameless_match_spec.channel = Some(channel.to_string());
nameless_match_spec.channel =
Some(Channel::from_str(channel, &Default::default())?.into());
nameless_match_spec.subdir = Some(subdir.to_string());
} else {
nameless_match_spec.channel = Some(channel_str.to_string());
nameless_match_spec.channel =
Some(Channel::from_str(channel_str, &Default::default())?.into());
}
}

Expand Down Expand Up @@ -468,12 +471,13 @@ mod tests {
use serde::Serialize;
use std::collections::BTreeMap;
use std::str::FromStr;
use std::sync::Arc;

use super::{
split_version_and_build, strip_brackets, BracketVec, MatchSpec, ParseMatchSpecError,
};
use crate::match_spec::parse::parse_bracket_list;
use crate::{BuildNumberSpec, NamelessMatchSpec, VersionSpec};
use crate::{BuildNumberSpec, Channel, NamelessMatchSpec, VersionSpec};
use smallvec::smallvec;

#[test]
Expand Down Expand Up @@ -573,18 +577,39 @@ mod tests {
let spec = MatchSpec::from_str("conda-forge::foo[version=\"1.0.*\"]").unwrap();
assert_eq!(spec.name, Some("foo".parse().unwrap()));
assert_eq!(spec.version, Some(VersionSpec::from_str("1.0.*").unwrap()));
assert_eq!(spec.channel, Some("conda-forge".to_string()));
assert_eq!(
spec.channel,
Some(
Channel::from_str("conda-forge", &Default::default())
.map(|channel| Arc::new(channel))
.unwrap()
)
);

let spec = MatchSpec::from_str("conda-forge::foo[version=1.0.*]").unwrap();
assert_eq!(spec.name, Some("foo".parse().unwrap()));
assert_eq!(spec.version, Some(VersionSpec::from_str("1.0.*").unwrap()));
assert_eq!(spec.channel, Some("conda-forge".to_string()));
assert_eq!(
spec.channel,
Some(
Channel::from_str("conda-forge", &Default::default())
.map(|channel| Arc::new(channel))
.unwrap()
)
);

let spec =
MatchSpec::from_str(r#"conda-forge::foo[version=1.0.*, build_number=">6"]"#).unwrap();
assert_eq!(spec.name, Some("foo".parse().unwrap()));
assert_eq!(spec.version, Some(VersionSpec::from_str("1.0.*").unwrap()));
assert_eq!(spec.channel, Some("conda-forge".to_string()));
assert_eq!(
spec.channel,
Some(
Channel::from_str("conda-forge", &Default::default())
.map(|channel| Arc::new(channel))
.unwrap()
)
);
assert_eq!(
spec.build_number,
Some(BuildNumberSpec::from_str(">6").unwrap())
Expand Down
18 changes: 11 additions & 7 deletions crates/rattler_solve/src/resolvo/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,15 +259,19 @@ impl<'a> CondaDependencyProvider<'a> {
}) {
// Check if the spec has a channel, and compare it to the repodata channel
if let Some(spec_channel) = &spec.channel {
if !&record.channel.contains(spec_channel) {
if record.channel != spec_channel.base_url.to_string() {
tracing::debug!("Ignoring {} from {} because it was not requested from that channel.", &record.package_record.name.as_normalized(), &record.channel);
// Add record to the excluded with reason of being in the non requested channel.
candidates.excluded.push((
solvable_id,
pool.intern_string(format!(
"candidate not in requested channel: '{spec_channel}'"
)),
));
let message = format!(
"candidate not in requested channel: '{}'",
spec_channel
.name
.clone()
.unwrap_or(spec_channel.base_url.to_string())
);
candidates
.excluded
.push((solvable_id, pool.intern_string(message)));
continue;
}
}
Expand Down

0 comments on commit f839abd

Please sign in to comment.