Skip to content

Commit

Permalink
fix: incorrectly saving absolute base as path component (prefix-dev#2501
Browse files Browse the repository at this point in the history
)

Co-authored-by: Ruben Arts <[email protected]>
  • Loading branch information
tdejager and ruben-arts authored Nov 20, 2024
1 parent 24dd763 commit 903284a
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 5 deletions.
11 changes: 8 additions & 3 deletions src/lock_file/resolve/pypi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -584,13 +584,18 @@ fn get_url_or_path(
// This happens when it is relative to the non-standard index
// location on disk.
FileLocation::RelativeUrl(base, relative) => {
// base is a file:// url, but we need to convert it to a path
// This is the same logic as the `AbsoluteUrl` case
// basically but we just make an absolute path first
let absolute = PathBuf::from_str(base)
.map_err(|_| GetUrlOrPathError::ExpectedPath(base.clone()))?;
let base = Url::from_str(base)
.map_err(|_| GetUrlOrPathError::InvalidBaseUrl(base.clone()))?;
let base = base
.to_file_path()
.map_err(|_| GetUrlOrPathError::ExpectedPath(base.to_string()))?;

let relative = PathBuf::from_str(relative)
.map_err(|_| GetUrlOrPathError::ExpectedPath(relative.clone()))?;
let absolute = absolute.join(relative);
let absolute = base.join(relative);

let relative = absolute.strip_prefix(abs_project_root);
let path = match relative {
Expand Down
23 changes: 22 additions & 1 deletion tests/integration_rust/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use pixi_consts::consts;
use pixi_manifest::{EnvironmentName, FeatureName};
use pixi_progress::global_multi_progress;
use rattler_conda_types::{MatchSpec, ParseStrictness::Lenient, Platform};
use rattler_lock::{LockFile, Package};
use rattler_lock::{LockFile, Package, UrlOrPath};
use tempfile::TempDir;
use thiserror::Error;

Expand Down Expand Up @@ -106,6 +106,13 @@ pub trait LockFileExt {
platform: Platform,
package: &str,
) -> Option<String>;

fn get_pypi_package_url(
&self,
environment: &str,
platform: Platform,
package: &str,
) -> Option<UrlOrPath>;
}

impl LockFileExt for LockFile {
Expand Down Expand Up @@ -184,6 +191,20 @@ impl LockFileExt for LockFile {
})
.map(|p| p.version().to_string())
}

fn get_pypi_package_url(
&self,
environment: &str,
platform: Platform,
package: &str,
) -> Option<UrlOrPath> {
self.environment(environment)
.and_then(|env| {
env.packages(platform)
.and_then(|mut packages| packages.find(|p| p.name() == package))
})
.map(|p| p.url_or_path().into_owned())
}
}

impl PixiControl {
Expand Down
83 changes: 82 additions & 1 deletion tests/integration_rust/pypi_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,91 @@ use crate::common::{LockFileExt, PixiControl};
use rattler_conda_types::Platform;
use url::Url;

#[tokio::test]
#[cfg_attr(not(feature = "slow_integration_tests"), ignore)]
async fn test_flat_links_based_index_returns_path() {
let pypi_indexes = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/data/pypi-indexes");
let pixi = PixiControl::from_manifest(&format!(
r#"
[project]
name = "pypi-extra-index-url"
platforms = ["{platform}"]
channels = ["conda-forge"]
[dependencies]
python = "~=3.12.0"
[pypi-dependencies]
foo = "*"
[pypi-options]
find-links = [{{ path = "{pypi_indexes}/multiple-indexes-a/flat"}}]"#,
platform = Platform::current(),
pypi_indexes = pypi_indexes.display().to_string().replace("\\", "/"),
));
let lock_file = pixi.unwrap().update_lock_file().await.unwrap();

// This assertion is specifically to test that if we have a url-based *local* index
// we will get a path back to the index and the corresponding file
assert_eq!(
lock_file
.get_pypi_package_url("default", Platform::current(), "foo")
.unwrap()
.as_path()
.unwrap(),
pypi_indexes
.join("multiple-indexes-a")
.join("flat")
.join("foo-1.0.0-py2.py3-none-any.whl")
);
}

#[tokio::test]
#[cfg_attr(not(feature = "slow_integration_tests"), ignore)]
async fn test_file_based_index_returns_path() {
let pypi_indexes = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/data/pypi-indexes");
let pypi_indexes_url = Url::from_directory_path(pypi_indexes.clone()).unwrap();
let pixi = PixiControl::from_manifest(&format!(
r#"
[project]
name = "pypi-extra-index-url"
platforms = ["{platform}"]
channels = ["conda-forge"]
[dependencies]
python = "~=3.12.0"
[pypi-dependencies]
foo = "*"
[pypi-options]
extra-index-urls = [
"{pypi_indexes}multiple-indexes-a/index"
]"#,
platform = Platform::current(),
pypi_indexes = pypi_indexes_url,
));
let lock_file = pixi.unwrap().update_lock_file().await.unwrap();

// This assertion is specifically to test that if we have a url-based *local* index
// we will get a path back to the index and the corresponding file
assert_eq!(
lock_file
.get_pypi_package_url("default", Platform::current(), "foo")
.unwrap()
.as_path()
.unwrap(),
pypi_indexes
.join("multiple-indexes-a/index/foo")
.join("foo-1.0.0-py2.py3-none-any.whl")
);
}

#[tokio::test]
#[cfg_attr(not(feature = "slow_integration_tests"), ignore)]
async fn test_index_strategy() {
let pypi_indexes = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/data/pypi-indexes");
let pypi_indexes_url = Url::from_directory_path(pypi_indexes).unwrap();
let pypi_indexes_url = Url::from_directory_path(pypi_indexes.clone()).unwrap();

let pixi = PixiControl::from_manifest(&format!(
r#"
Expand Down Expand Up @@ -69,6 +149,7 @@ async fn test_index_strategy() {
),
Some("1.0.0".into())
);

assert_eq!(
lock_file.get_pypi_package_version(
"unsafe-first-match-constrained",
Expand Down

0 comments on commit 903284a

Please sign in to comment.