From c2e1b1d5f360e362b592f852fcedc956aa73b5e3 Mon Sep 17 00:00:00 2001 From: Marek Kaput Date: Mon, 9 Oct 2023 10:41:53 +0200 Subject: [PATCH] Fix package op issues on Windows commit-id:75542114 --- scarb/src/ops/package.rs | 18 +++++++++++++----- scarb/tests/package.rs | 25 ++++++++++++++----------- utils/scarb-test-support/src/fsx.rs | 13 +++++++++++++ 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/scarb/src/ops/package.rs b/scarb/src/ops/package.rs index 40c834f0a..2ece347cb 100644 --- a/scarb/src/ops/package.rs +++ b/scarb/src/ops/package.rs @@ -147,6 +147,9 @@ fn list_one_impl( fn prepare_archive_recipe(pkg: &Package) -> Result { let mut recipe = source_files(pkg)?; + // Sort the recipe before any checks, to ensure generated errors are reproducible. + sort_recipe(&mut recipe); + check_filenames(&recipe)?; check_no_reserved_files(&recipe)?; @@ -171,11 +174,8 @@ fn prepare_archive_recipe(pkg: &Package) -> Result { contents: ArchiveFileContents::Generated(Box::new(|| Ok(VERSION.to_string().into_bytes()))), }); - // Sort archive files alphabetically, putting the version file first. - recipe.sort_unstable_by_key(|f| { - let priority = if f.path == VERSION_FILE_NAME { 0 } else { 1 }; - (priority, f.path.clone()) - }); + // Put generated files in right order within the recipe. + sort_recipe(&mut recipe); // Assert there are no duplicates. We make use of the fact, that recipe is now sorted. assert!( @@ -237,6 +237,14 @@ fn check_filenames(recipe: &ArchiveRecipe) -> Result<()> { Ok(()) } +/// Sort archive files alphabetically, putting the version file first. +fn sort_recipe(recipe: &mut ArchiveRecipe) { + recipe.sort_unstable_by_key(|f| { + let priority = if f.path == VERSION_FILE_NAME { 0 } else { 1 }; + (priority, f.path.clone()) + }); +} + fn normalize_manifest(pkg: Package) -> Result> { let mut buf = Vec::new(); diff --git a/scarb/tests/package.rs b/scarb/tests/package.rs index 60745ec85..cd3d0fb34 100644 --- a/scarb/tests/package.rs +++ b/scarb/tests/package.rs @@ -13,6 +13,7 @@ use itertools::Itertools; use test_case::test_case; use scarb_test_support::command::Scarb; +use scarb_test_support::fsx::unix_paths_to_os_lossy; use scarb_test_support::gitx; use scarb_test_support::project_builder::{Dep, DepBuilder, ProjectBuilder}; use scarb_test_support::workspace_builder::WorkspaceBuilder; @@ -224,13 +225,13 @@ fn list_simple() { .current_dir(&t) .assert() .success() - .stdout_eq(indoc! {r#" + .stdout_eq(unix_paths_to_os_lossy(indoc! {r#" VERSION Scarb.orig.toml Scarb.toml src/foo.cairo src/lib.cairo - "#}); + "#})); } #[test] @@ -254,7 +255,7 @@ fn list_workspace() { .current_dir(&t) .assert() .success() - .stdout_eq(indoc! {r#" + .stdout_eq(unix_paths_to_os_lossy(indoc! {r#" first: VERSION Scarb.orig.toml @@ -266,7 +267,7 @@ fn list_workspace() { Scarb.orig.toml Scarb.toml src/lib.cairo - "#}); + "#})); } #[test] @@ -536,12 +537,12 @@ fn list_ignore_nested() { .current_dir(&t) .assert() .success() - .stdout_eq(indoc! {r#" + .stdout_eq(unix_paths_to_os_lossy(indoc! {r#" VERSION Scarb.orig.toml Scarb.toml src/lib.cairo - "#}); + "#})); } // TODO(mkaput): Invalid readme/license path @@ -707,12 +708,12 @@ fn exclude_dot_files_and_directories_by_default() { .current_dir(&t) .assert() .success() - .stdout_eq(indoc! {r#" + .stdout_eq(unix_paths_to_os_lossy(indoc! {r#" VERSION Scarb.orig.toml Scarb.toml src/lib.cairo - "#}); + "#})); } #[test] @@ -776,13 +777,15 @@ fn ignore_file(ignore_path: &str, setup_git: bool, expect_ignore_to_work: bool) expected.push("src/lib.cairo"); expected.push(""); // Ensure there's trailing \n + let expected = unix_paths_to_os_lossy(&expected.join("\n")); + Scarb::quick_snapbox() .arg("package") .arg("--list") .current_dir(g.p) .assert() .success() - .stdout_eq(expected.join("\n")); + .stdout_eq(expected); } #[test] @@ -814,11 +817,11 @@ fn ignore_whitelist_pattern() { .current_dir(&t) .assert() .success() - .stdout_eq(indoc! {r#" + .stdout_eq(unix_paths_to_os_lossy(indoc! {r#" VERSION Scarb.orig.toml Scarb.toml noignore.txt src/lib.cairo - "#}); + "#})); } diff --git a/utils/scarb-test-support/src/fsx.rs b/utils/scarb-test-support/src/fsx.rs index feee93470..709d48836 100644 --- a/utils/scarb-test-support/src/fsx.rs +++ b/utils/scarb-test-support/src/fsx.rs @@ -1,5 +1,6 @@ use std::fs::File; use std::io::BufReader; +use std::path::MAIN_SEPARATOR_STR; use assert_fs::fixture::ChildPath; use assert_fs::TempDir; @@ -61,3 +62,15 @@ impl ChildPathEx for ChildPath { serde_json::from_reader(reader).unwrap() } } + +/// Convert all UNIX-style paths in a string to platform native. +/// +/// This method is doing dump pattern search & replace, it might replace unexpected parts of the +/// input string. Use with caution. +pub fn unix_paths_to_os_lossy(text: &str) -> String { + if cfg!(unix) { + text.to_string() + } else { + text.replace('/', MAIN_SEPARATOR_STR) + } +}