From ba30fa95ce055c83fe24de1d1317e5e6b78153a6 Mon Sep 17 00:00:00 2001 From: Charles Stern <62192187+cisaacstern@users.noreply.github.com> Date: Fri, 4 Oct 2024 08:43:03 -0700 Subject: [PATCH 1/3] fix: return correct channel urls from `MatchSpec.channel.base_url` (#885) Co-authored-by: Bas Zalmstra --- py-rattler/Cargo.lock | 8 ++++---- py-rattler/rattler/channel/channel.py | 6 ++++++ py-rattler/rattler/match_spec/match_spec.py | 5 ++--- py-rattler/tests/unit/test_matchspec.py | 22 +++++++++++++++++++++ 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/py-rattler/Cargo.lock b/py-rattler/Cargo.lock index 6fcf5b3c9..24ea15987 100644 --- a/py-rattler/Cargo.lock +++ b/py-rattler/Cargo.lock @@ -2634,7 +2634,7 @@ dependencies = [ [[package]] name = "rattler" -version = "0.27.12" +version = "0.27.13" dependencies = [ "anyhow", "console", @@ -2747,7 +2747,7 @@ dependencies = [ [[package]] name = "rattler_index" -version = "0.19.29" +version = "0.19.30" dependencies = [ "fs-err", "rattler_conda_types", @@ -2850,7 +2850,7 @@ dependencies = [ [[package]] name = "rattler_repodata_gateway" -version = "0.21.14" +version = "0.21.15" dependencies = [ "anyhow", "async-compression", @@ -2918,7 +2918,7 @@ dependencies = [ [[package]] name = "rattler_solve" -version = "1.0.8" +version = "1.0.9" dependencies = [ "chrono", "futures", diff --git a/py-rattler/rattler/channel/channel.py b/py-rattler/rattler/channel/channel.py index 9ee4b1f19..c19530e05 100644 --- a/py-rattler/rattler/channel/channel.py +++ b/py-rattler/rattler/channel/channel.py @@ -25,6 +25,12 @@ def __init__(self, name: str, channel_configuration: Optional[ChannelConfig] = N self._channel = PyChannel(name, channel_configuration._channel_configuration) + @classmethod + def _from_py_channel(cls, py_channel: PyChannel) -> Channel: + channel = cls.__new__(cls) + channel._channel = py_channel + return channel + def to_lock_channel(self) -> LockChannel: """ Returns a new [`LockChannel`] from existing channel. diff --git a/py-rattler/rattler/match_spec/match_spec.py b/py-rattler/rattler/match_spec/match_spec.py index 806a4a94f..1d1bffaa4 100644 --- a/py-rattler/rattler/match_spec/match_spec.py +++ b/py-rattler/rattler/match_spec/match_spec.py @@ -141,9 +141,8 @@ def channel(self) -> Optional[Channel]: """ The channel of the package. """ - if (channel := self._match_spec.channel) is not None: - return Channel(channel.name) - return None + channel = self._match_spec.channel + return channel and Channel._from_py_channel(channel) @property def subdir(self) -> Optional[str]: diff --git a/py-rattler/tests/unit/test_matchspec.py b/py-rattler/tests/unit/test_matchspec.py index c91dbbe86..b4f9f37ee 100644 --- a/py-rattler/tests/unit/test_matchspec.py +++ b/py-rattler/tests/unit/test_matchspec.py @@ -13,3 +13,25 @@ def test_parse_channel_from_url() -> None: assert m.channel is not None assert m.channel.name == "conda-forge" assert m.channel.base_url == "https://conda.anaconda.org/conda-forge/" + + +def test_parse_channel_from_url_filesystem() -> None: + m = MatchSpec("file:///Users/rattler/channel0::python[version=3.9]") + assert m.channel is not None + assert m.channel.name == "channel0" + assert m.channel.base_url == "file:///Users/rattler/channel0/" + + +def test_parse_channel_from_url_localhost() -> None: + m = MatchSpec("http://localhost:8000/channel0::python[version=3.9]") + assert m.channel is not None + assert m.channel.name == "channel0" + assert m.channel.base_url == "http://localhost:8000/channel0/" + + +def test_parse_no_channel() -> None: + m = MatchSpec("python[version=3.9]") + assert m.channel is None + assert m.name is not None + assert m.name.normalized == "python" + assert m.version == "==3.9" From b3571c498dcdf615cc875efa2ea7811e929d093d Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Mon, 7 Oct 2024 10:00:59 +0200 Subject: [PATCH 2/3] add failing test for self-clobbering --- .../rattler/src/install/clobber_registry.rs | 93 ++++++++++++++++++- crates/rattler/src/install/test_utils.rs | 6 +- 2 files changed, 96 insertions(+), 3 deletions(-) diff --git a/crates/rattler/src/install/clobber_registry.rs b/crates/rattler/src/install/clobber_registry.rs index 3c9a62570..f571017b6 100644 --- a/crates/rattler/src/install/clobber_registry.rs +++ b/crates/rattler/src/install/clobber_registry.rs @@ -835,7 +835,7 @@ mod tests { .with_prefix_records(&prefix_records) .finish(); - execute_transaction( + let result = execute_transaction( transaction, target_prefix.path(), &reqwest_middleware::ClientWithMiddleware::from(reqwest::Client::new()), @@ -845,6 +845,8 @@ mod tests { ) .await; + println!("== RESULT: {:?}", result.clobbered_paths); + assert_check_files( target_prefix.path(), &[ @@ -867,6 +869,95 @@ mod tests { ); } + #[tokio::test] + async fn test_self_clobber_update() { + // Create a transaction + let repodata_record_1 = get_repodata_record( + get_test_data_dir().join("clobber/clobber-1-0.1.0-h4616a5c_0.tar.bz2"), + ); + + let transaction = transaction::Transaction:: { + operations: vec![TransactionOperation::Install(repodata_record_1.clone())], + python_info: None, + current_python_info: None, + platform: Platform::current(), + }; + + // execute transaction + let target_prefix = tempfile::tempdir().unwrap(); + + let packages_dir = tempfile::tempdir().unwrap(); + let cache = PackageCache::new(packages_dir.path()); + + execute_transaction( + transaction, + target_prefix.path(), + &reqwest_middleware::ClientWithMiddleware::from(reqwest::Client::new()), + &cache, + &InstallDriver::default(), + &InstallOptions::default(), + ) + .await; + + // check that the files are there + assert_check_files( + target_prefix.path(), + &[ + "clobber.txt", + "another-clobber.txt", + ], + ); + + let mut prefix_records = PrefixRecord::collect_from_prefix(target_prefix.path()).unwrap(); + prefix_records.sort_by(|a, b| { + a.repodata_record + .package_record + .name + .as_normalized() + .cmp(b.repodata_record.package_record.name.as_normalized()) + }); + + // Reinstall the same package + let transaction = transaction::Transaction:: { + operations: vec![TransactionOperation::Change { + old: prefix_records[0].clone(), + new: repodata_record_1, + }], + python_info: None, + current_python_info: None, + platform: Platform::current(), + }; + + + let install_driver = InstallDriver::builder() + .with_prefix_records(&prefix_records) + .finish(); + + install_driver.pre_process(&transaction, target_prefix.path()).unwrap(); + let dl_client = reqwest_middleware::ClientWithMiddleware::from(reqwest::Client::new()); + for op in &transaction.operations { + execute_operation( + target_prefix.path(), + &dl_client, + &cache, + &install_driver, + op.clone(), + &InstallOptions::default(), + ) + .await; + } + + // Check what files are in the prefix now (note that unclobbering wasn't run yet) + // But also, this is a reinstall so the files should just be overwritten. + assert_check_files( + target_prefix.path(), + &[ + "clobber.txt", + "another-clobber.txt", + ], + ); + } + #[tokio::test] async fn test_clobber_update_and_remove() { // Create a transaction diff --git a/crates/rattler/src/install/test_utils.rs b/crates/rattler/src/install/test_utils.rs index 2ef6510e6..a761e99dc 100644 --- a/crates/rattler/src/install/test_utils.rs +++ b/crates/rattler/src/install/test_utils.rs @@ -10,6 +10,8 @@ use crate::{ package_cache::PackageCache, }; +use super::driver::PostProcessResult; + /// Install a package into the environment and write a `conda-meta` file that /// contains information about how the file was linked. pub async fn install_package_to_environment( @@ -124,7 +126,7 @@ pub async fn execute_transaction( package_cache: &PackageCache, install_driver: &InstallDriver, install_options: &InstallOptions, -) { +) -> PostProcessResult { install_driver .pre_process(&transaction, target_prefix) .unwrap(); @@ -143,7 +145,7 @@ pub async fn execute_transaction( install_driver .post_process(&transaction, target_prefix) - .unwrap(); + .unwrap() } pub fn find_prefix_record<'a>( From 78c54d52bd7a34aea25b945d1f4afee2353c9b97 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Mon, 7 Oct 2024 10:02:10 +0200 Subject: [PATCH 3/3] fmt --- crates/rattler/src/install/clobber_registry.rs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/crates/rattler/src/install/clobber_registry.rs b/crates/rattler/src/install/clobber_registry.rs index f571017b6..6dd259b3e 100644 --- a/crates/rattler/src/install/clobber_registry.rs +++ b/crates/rattler/src/install/clobber_registry.rs @@ -902,10 +902,7 @@ mod tests { // check that the files are there assert_check_files( target_prefix.path(), - &[ - "clobber.txt", - "another-clobber.txt", - ], + &["clobber.txt", "another-clobber.txt"], ); let mut prefix_records = PrefixRecord::collect_from_prefix(target_prefix.path()).unwrap(); @@ -928,12 +925,13 @@ mod tests { platform: Platform::current(), }; - let install_driver = InstallDriver::builder() .with_prefix_records(&prefix_records) .finish(); - - install_driver.pre_process(&transaction, target_prefix.path()).unwrap(); + + install_driver + .pre_process(&transaction, target_prefix.path()) + .unwrap(); let dl_client = reqwest_middleware::ClientWithMiddleware::from(reqwest::Client::new()); for op in &transaction.operations { execute_operation( @@ -951,10 +949,7 @@ mod tests { // But also, this is a reinstall so the files should just be overwritten. assert_check_files( target_prefix.path(), - &[ - "clobber.txt", - "another-clobber.txt", - ], + &["clobber.txt", "another-clobber.txt"], ); }