From 8f86be7483067c6f0a6f3933b0df19297e169d5e Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Sun, 13 Oct 2024 23:02:24 +0200 Subject: [PATCH 1/9] Unpack `.crate` created by Cargo if `--no-verify` is passed - Implementation closely follows the one used in Cargo as a verification step: https://github.com/rust-lang/cargo/blob/a4600184b8d6619ed0b5a0a19946dbbe97e1d739/src/cargo/ops/cargo_package.rs#L1110 - Add `flate2` dependency - Add `FileLockGuard.parent()` --- Cargo.lock | 13 +++++++++++++ Cargo.toml | 1 + scarb/Cargo.toml | 1 + scarb/src/flock.rs | 4 ++++ scarb/src/ops/package.rs | 31 ++++++++++++++++++++++++++++++- 5 files changed, 49 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 7f3de5f7d..2439b2b13 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1876,6 +1876,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5f54427cfd1c7829e2a139fcefea601bf088ebca651d2bf53ebc600eac295dae" dependencies = [ "crc32fast", + "libz-sys", "miniz_oxide", ] @@ -3474,6 +3475,17 @@ dependencies = [ "libc", ] +[[package]] +name = "libz-sys" +version = "1.1.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d2d16453e800a8cf6dd2fc3eb4bc99b786a9b90c663b8559a5b1a041bf89e472" +dependencies = [ + "cc", + "pkg-config", + "vcpkg", +] + [[package]] name = "linkme" version = "0.3.28" @@ -4614,6 +4626,7 @@ dependencies = [ "directories", "dunce", "expect-test", + "flate2", "fs4", "fs_extra", "futures", diff --git a/Cargo.toml b/Cargo.toml index 455ba8eb4..b90835437 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -72,6 +72,7 @@ dialoguer = "0.11.0" directories = "5" dunce = "1" expect-test = "1.5" +flate2 = { version = "1.0.30", default-features = false, features = ["zlib"] } fs4 = { version = "0.7", features = ["tokio"] } fs_extra = "1" futures = { version = "0.3", default-features = false, features = ["std", "async-await"] } diff --git a/scarb/Cargo.toml b/scarb/Cargo.toml index cb0cc9392..5f887e927 100644 --- a/scarb/Cargo.toml +++ b/scarb/Cargo.toml @@ -86,6 +86,7 @@ which.workspace = true windows-sys.workspace = true zstd.workspace = true cargo_metadata.workspace = true +flate2.workspace = true [target.'cfg(not(target_os = "linux"))'.dependencies] reqwest = { workspace = true, default-features = true } diff --git a/scarb/src/flock.rs b/scarb/src/flock.rs index 7cfa89482..1604b5a53 100644 --- a/scarb/src/flock.rs +++ b/scarb/src/flock.rs @@ -37,6 +37,10 @@ impl FileLockGuard { self.path.as_path() } + pub fn parent(&self) -> &Utf8Path { + self.path.parent().unwrap() + } + pub fn lock_kind(&self) -> FileLockKind { self.lock_kind } diff --git a/scarb/src/ops/package.rs b/scarb/src/ops/package.rs index 0d017327d..c626ab2af 100644 --- a/scarb/src/ops/package.rs +++ b/scarb/src/ops/package.rs @@ -1,10 +1,13 @@ use std::collections::BTreeMap; use std::fs::File; use std::io::{Seek, SeekFrom, Write}; +use std::ops::Deref; use anyhow::{bail, ensure, Context, Result}; use camino::Utf8PathBuf; +use flate2::read::GzDecoder; use indoc::{formatdoc, indoc, writedoc}; +use tar::Archive; use crate::core::registry::package_source_store::PackageSourceStore; use crate::sources::client::PackageRepository; @@ -20,7 +23,7 @@ use crate::core::publishing::manifest_normalization::prepare_manifest_for_publis use crate::core::publishing::source::list_source_files; use crate::core::{Config, Package, PackageId, PackageName, Target, TargetKind, Workspace}; use crate::flock::{FileLockGuard, Filesystem}; -use crate::internal::restricted_names; +use crate::internal::{fsx, restricted_names}; use crate::{ ops, CARGO_MANIFEST_FILE_NAME, DEFAULT_LICENSE_FILE_NAME, DEFAULT_README_FILE_NAME, MANIFEST_FILE_NAME, VCS_INFO_FILE_NAME, @@ -279,6 +282,32 @@ fn prepare_archive_recipe( )); } + // Unpack .crate file to make normalized Cargo.toml available. + if !opts.verify { + let crate_archive_name = format!("{}.crate", crate_archive_basename); + let tar = pkg.target_path(ws.config()).into_child("package").open_ro( + &crate_archive_name, + &crate_archive_name, + ws.config(), + )?; + + // The following implementation has been copied from the `Cargo` codebase with slight modifications only. + // The original implementation can be found here: + // https://github.com/rust-lang/cargo/blob/a4600184b8d6619ed0b5a0a19946dbbe97e1d739/src/cargo/ops/cargo_package.rs#L1110 + + tar.deref().seek(SeekFrom::Start(0))?; + let f = GzDecoder::new(tar.deref()); + let dst = tar.parent().join(&crate_archive_basename); + if dst.exists() { + fsx::remove_dir_all(&dst)?; + } + let mut archive = Archive::new(f); + // We don't need to set the Modified Time, as it's not relevant to verification + // and it errors on filesystems that don't support setting a modified timestamp + archive.set_preserve_mtime(false); + archive.unpack(dst.parent().unwrap())?; + } + // Add normalized Cargo.toml file. recipe.push(ArchiveFile { path: CARGO_MANIFEST_FILE_NAME.into(), From 06997add5ec81f2010c75657b38c9f844db837ef Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Sun, 13 Oct 2024 23:03:04 +0200 Subject: [PATCH 2/9] Add test --- scarb/tests/package.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/scarb/tests/package.rs b/scarb/tests/package.rs index 9e4c0d131..00d3cb279 100644 --- a/scarb/tests/package.rs +++ b/scarb/tests/package.rs @@ -1453,6 +1453,29 @@ fn package_without_verification() { "#}); } +#[test] +fn package_cairo_plugin_without_verification() { + let t = TempDir::new().unwrap(); + CairoPluginProjectBuilder::default().build(&t); + + Scarb::quick_snapbox() + .arg("package") + .arg("--no-verify") + .arg("--no-metadata") + .env("CARGO_TERM_QUIET", "true") + .current_dir(&t) + .assert() + .success() + .stdout_matches(indoc! {r#" + [..] Packaging some v1.0.0 [..] + [..]warn: package name or version differs between Cargo manifest and Scarb manifest + [..]Scarb manifest: `some-1.0.0`, Cargo manifest: `some-0.1.0` + [..]this might become an error in future Scarb releases + + [..] Packaged [..] + "#}); +} + #[test] fn package_without_publish_metadata() { let t = TempDir::new().unwrap(); From 9a92de0df232e292ff67782f7afd16eafc36d5a8 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Sun, 13 Oct 2024 23:33:48 +0200 Subject: [PATCH 3/9] Move unpacking logic to a separate function --- .../compiler/plugin/proc_macro/compilation.rs | 32 +++++++++++++++++++ scarb/src/ops/package.rs | 30 ++--------------- 2 files changed, 35 insertions(+), 27 deletions(-) diff --git a/scarb/src/compiler/plugin/proc_macro/compilation.rs b/scarb/src/compiler/plugin/proc_macro/compilation.rs index 451bdf3b8..0718d4e19 100644 --- a/scarb/src/compiler/plugin/proc_macro/compilation.rs +++ b/scarb/src/compiler/plugin/proc_macro/compilation.rs @@ -1,12 +1,14 @@ use crate::compiler::ProcMacroCompilationUnit; use crate::core::{Config, Package, Workspace}; use crate::flock::Filesystem; +use crate::internal::fsx; use crate::ops::PackageOpts; use crate::process::exec_piping; use crate::CARGO_MANIFEST_FILE_NAME; use anyhow::{anyhow, Context, Result}; use camino::Utf8PathBuf; use cargo_metadata::MetadataCommand; +use flate2::read::GzDecoder; use indoc::formatdoc; use libloading::library_filename; use ra_ap_toolchain::Tool; @@ -15,7 +17,10 @@ use serde::{Serialize, Serializer}; use serde_json::value::RawValue; use std::fmt::Display; use std::fs; +use std::io::{Seek, SeekFrom}; +use std::ops::Deref; use std::process::Command; +use tar::Archive; use tracing::trace_span; pub const PROC_MACRO_BUILD_PROFILE: &str = "release"; @@ -150,6 +155,33 @@ pub fn get_crate_archive_basename(package: &Package) -> Result { Ok(format!("{}-{}", package_name, package_version)) } +pub fn unpack_crate(package: &Package, config: &Config) -> Result<()> { + let crate_archive_basename = get_crate_archive_basename(package)?; + let crate_archive_name = format!("{}.crate", crate_archive_basename); + + let tar = + package + .target_path(config) + .into_child("package") + .open_ro(&crate_archive_name, &crate_archive_name, config)?; + + // The following implementation has been copied from the `Cargo` codebase with slight modifications only. + // The original implementation can be found here: + // https://github.com/rust-lang/cargo/blob/a4600184b8d6619ed0b5a0a19946dbbe97e1d739/src/cargo/ops/cargo_package.rs#L1110 + + tar.deref().seek(SeekFrom::Start(0))?; + let f = GzDecoder::new(tar.deref()); + let dst = tar.parent().join(&crate_archive_basename); + if dst.exists() { + fsx::remove_dir_all(&dst)?; + } + let mut archive = Archive::new(f); + archive.set_preserve_mtime(false); // Don't set modified time to avoid filesystem errors + archive.unpack(dst.parent().unwrap())?; + + Ok(()) +} + pub fn fetch_crate(package: &Package, ws: &Workspace<'_>) -> Result<()> { run_cargo(CargoAction::Fetch, package, ws) } diff --git a/scarb/src/ops/package.rs b/scarb/src/ops/package.rs index c626ab2af..a5e986856 100644 --- a/scarb/src/ops/package.rs +++ b/scarb/src/ops/package.rs @@ -1,13 +1,10 @@ use std::collections::BTreeMap; use std::fs::File; use std::io::{Seek, SeekFrom, Write}; -use std::ops::Deref; use anyhow::{bail, ensure, Context, Result}; use camino::Utf8PathBuf; -use flate2::read::GzDecoder; use indoc::{formatdoc, indoc, writedoc}; -use tar::Archive; use crate::core::registry::package_source_store::PackageSourceStore; use crate::sources::client::PackageRepository; @@ -17,13 +14,13 @@ use scarb_ui::{HumanBytes, HumanCount}; use serde::Serialize; use crate::compiler::plugin::proc_macro::compilation::{ - get_crate_archive_basename, package_crate, SharedLibraryProvider, + get_crate_archive_basename, package_crate, unpack_crate, SharedLibraryProvider, }; use crate::core::publishing::manifest_normalization::prepare_manifest_for_publish; use crate::core::publishing::source::list_source_files; use crate::core::{Config, Package, PackageId, PackageName, Target, TargetKind, Workspace}; use crate::flock::{FileLockGuard, Filesystem}; -use crate::internal::{fsx, restricted_names}; +use crate::internal::restricted_names; use crate::{ ops, CARGO_MANIFEST_FILE_NAME, DEFAULT_LICENSE_FILE_NAME, DEFAULT_README_FILE_NAME, MANIFEST_FILE_NAME, VCS_INFO_FILE_NAME, @@ -284,28 +281,7 @@ fn prepare_archive_recipe( // Unpack .crate file to make normalized Cargo.toml available. if !opts.verify { - let crate_archive_name = format!("{}.crate", crate_archive_basename); - let tar = pkg.target_path(ws.config()).into_child("package").open_ro( - &crate_archive_name, - &crate_archive_name, - ws.config(), - )?; - - // The following implementation has been copied from the `Cargo` codebase with slight modifications only. - // The original implementation can be found here: - // https://github.com/rust-lang/cargo/blob/a4600184b8d6619ed0b5a0a19946dbbe97e1d739/src/cargo/ops/cargo_package.rs#L1110 - - tar.deref().seek(SeekFrom::Start(0))?; - let f = GzDecoder::new(tar.deref()); - let dst = tar.parent().join(&crate_archive_basename); - if dst.exists() { - fsx::remove_dir_all(&dst)?; - } - let mut archive = Archive::new(f); - // We don't need to set the Modified Time, as it's not relevant to verification - // and it errors on filesystems that don't support setting a modified timestamp - archive.set_preserve_mtime(false); - archive.unpack(dst.parent().unwrap())?; + unpack_crate(pkg, ws.config())?; } // Add normalized Cargo.toml file. From 3aead3d20b302b81e935c7bfb6e30d015c948676 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Sun, 13 Oct 2024 23:34:20 +0200 Subject: [PATCH 4/9] Make cleaner comment --- scarb/src/ops/package.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scarb/src/ops/package.rs b/scarb/src/ops/package.rs index a5e986856..3e523ba18 100644 --- a/scarb/src/ops/package.rs +++ b/scarb/src/ops/package.rs @@ -279,7 +279,7 @@ fn prepare_archive_recipe( )); } - // Unpack .crate file to make normalized Cargo.toml available. + // Unpack .crate to make normalized Cargo.toml available. if !opts.verify { unpack_crate(pkg, ws.config())?; } From c52ea823ee71d28f8f25b38ec7cd2a0262811036 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Sun, 13 Oct 2024 23:37:09 +0200 Subject: [PATCH 5/9] Format --- scarb/src/compiler/plugin/proc_macro/compilation.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scarb/src/compiler/plugin/proc_macro/compilation.rs b/scarb/src/compiler/plugin/proc_macro/compilation.rs index 0718d4e19..2f2c30544 100644 --- a/scarb/src/compiler/plugin/proc_macro/compilation.rs +++ b/scarb/src/compiler/plugin/proc_macro/compilation.rs @@ -159,11 +159,11 @@ pub fn unpack_crate(package: &Package, config: &Config) -> Result<()> { let crate_archive_basename = get_crate_archive_basename(package)?; let crate_archive_name = format!("{}.crate", crate_archive_basename); - let tar = - package - .target_path(config) - .into_child("package") - .open_ro(&crate_archive_name, &crate_archive_name, config)?; + let tar = package.target_path(config).into_child("package").open_ro( + &crate_archive_name, + &crate_archive_name, + config, + )?; // The following implementation has been copied from the `Cargo` codebase with slight modifications only. // The original implementation can be found here: From 54c04b8e87201c4f2e38caa038ad5be0ada73485 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Sun, 13 Oct 2024 23:41:15 +0200 Subject: [PATCH 6/9] Simplify variable naming --- scarb/src/compiler/plugin/proc_macro/compilation.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scarb/src/compiler/plugin/proc_macro/compilation.rs b/scarb/src/compiler/plugin/proc_macro/compilation.rs index 2f2c30544..9591a43e3 100644 --- a/scarb/src/compiler/plugin/proc_macro/compilation.rs +++ b/scarb/src/compiler/plugin/proc_macro/compilation.rs @@ -156,12 +156,12 @@ pub fn get_crate_archive_basename(package: &Package) -> Result { } pub fn unpack_crate(package: &Package, config: &Config) -> Result<()> { - let crate_archive_basename = get_crate_archive_basename(package)?; - let crate_archive_name = format!("{}.crate", crate_archive_basename); + let archive_basename = get_crate_archive_basename(package)?; + let archive_name = format!("{}.crate", archive_basename); let tar = package.target_path(config).into_child("package").open_ro( - &crate_archive_name, - &crate_archive_name, + &archive_name, + &archive_name, config, )?; @@ -171,7 +171,7 @@ pub fn unpack_crate(package: &Package, config: &Config) -> Result<()> { tar.deref().seek(SeekFrom::Start(0))?; let f = GzDecoder::new(tar.deref()); - let dst = tar.parent().join(&crate_archive_basename); + let dst = tar.parent().join(&archive_basename); if dst.exists() { fsx::remove_dir_all(&dst)?; } From 8cca25395a07860b5a453c80d5b5a2e5dd2ee206 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Tue, 15 Oct 2024 12:51:09 +0200 Subject: [PATCH 7/9] Always unpack crate; Always run `cargo package` with `--no-verify` --- scarb/src/compiler/plugin/proc_macro/compilation.rs | 1 + scarb/src/ops/package.rs | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/scarb/src/compiler/plugin/proc_macro/compilation.rs b/scarb/src/compiler/plugin/proc_macro/compilation.rs index 9591a43e3..5a409c85f 100644 --- a/scarb/src/compiler/plugin/proc_macro/compilation.rs +++ b/scarb/src/compiler/plugin/proc_macro/compilation.rs @@ -265,6 +265,7 @@ impl<'c> From> for Command { CargoAction::Package(ref opts) => { cmd.arg("--target-dir"); cmd.arg(args.target_dir); + cmd.arg("--no-verify"); if !opts.check_metadata { cmd.arg("--no-metadata"); } diff --git a/scarb/src/ops/package.rs b/scarb/src/ops/package.rs index 3e523ba18..7164ad134 100644 --- a/scarb/src/ops/package.rs +++ b/scarb/src/ops/package.rs @@ -280,9 +280,7 @@ fn prepare_archive_recipe( } // Unpack .crate to make normalized Cargo.toml available. - if !opts.verify { - unpack_crate(pkg, ws.config())?; - } + unpack_crate(pkg, ws.config())?; // Add normalized Cargo.toml file. recipe.push(ArchiveFile { From 79d60fe30be226a8873caa23b99051b6c1c9444f Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Tue, 15 Oct 2024 17:07:08 +0200 Subject: [PATCH 8/9] Open archive with exclusive lock --- .../compiler/plugin/proc_macro/compilation.rs | 9 +++---- scarb/src/flock.rs | 25 +++++++++++++++++++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/scarb/src/compiler/plugin/proc_macro/compilation.rs b/scarb/src/compiler/plugin/proc_macro/compilation.rs index 5a409c85f..ce187b44a 100644 --- a/scarb/src/compiler/plugin/proc_macro/compilation.rs +++ b/scarb/src/compiler/plugin/proc_macro/compilation.rs @@ -159,11 +159,10 @@ pub fn unpack_crate(package: &Package, config: &Config) -> Result<()> { let archive_basename = get_crate_archive_basename(package)?; let archive_name = format!("{}.crate", archive_basename); - let tar = package.target_path(config).into_child("package").open_ro( - &archive_name, - &archive_name, - config, - )?; + let tar = package + .target_path(config) + .into_child("package") + .open_ro_exclusive(&archive_name, &archive_name, config)?; // The following implementation has been copied from the `Cargo` codebase with slight modifications only. // The original implementation can be found here: diff --git a/scarb/src/flock.rs b/scarb/src/flock.rs index 1604b5a53..266568f22 100644 --- a/scarb/src/flock.rs +++ b/scarb/src/flock.rs @@ -297,6 +297,31 @@ impl Filesystem { ) } + /// Opens exclusive access to a [`File`], returning the locked version of it. + /// + /// This function will fail if `path` doesn't already exist, but if it does then it will + /// acquire an exclusive lock on `path`. + /// If the process must block waiting for the lock, the `description` annotated with _blocking_ + /// status message is printed to [`Config::ui`]. + /// + /// The returned file can be accessed to look at the path and also has read + /// access to the underlying file. + /// Any writes to the file will return an error. + pub fn open_ro_exclusive( + &self, + path: impl AsRef, + description: &str, + config: &Config, + ) -> Result { + self.open( + path.as_ref(), + OpenOptions::new().read(true), + FileLockKind::Exclusive, + description, + config, + ) + } + fn open( &self, path: &Utf8Path, From b1f2359e248096fd47c8a770116f47e8456f3496 Mon Sep 17 00:00:00 2001 From: Maksim Zdobnikau Date: Tue, 15 Oct 2024 17:15:34 +0200 Subject: [PATCH 9/9] Don't unwrap in `FileLockGuard.parent()` --- scarb/src/compiler/plugin/proc_macro/compilation.rs | 2 +- scarb/src/flock.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/scarb/src/compiler/plugin/proc_macro/compilation.rs b/scarb/src/compiler/plugin/proc_macro/compilation.rs index ce187b44a..487e56afe 100644 --- a/scarb/src/compiler/plugin/proc_macro/compilation.rs +++ b/scarb/src/compiler/plugin/proc_macro/compilation.rs @@ -170,7 +170,7 @@ pub fn unpack_crate(package: &Package, config: &Config) -> Result<()> { tar.deref().seek(SeekFrom::Start(0))?; let f = GzDecoder::new(tar.deref()); - let dst = tar.parent().join(&archive_basename); + let dst = tar.parent().unwrap().join(&archive_basename); if dst.exists() { fsx::remove_dir_all(&dst)?; } diff --git a/scarb/src/flock.rs b/scarb/src/flock.rs index 266568f22..5f071f487 100644 --- a/scarb/src/flock.rs +++ b/scarb/src/flock.rs @@ -37,8 +37,8 @@ impl FileLockGuard { self.path.as_path() } - pub fn parent(&self) -> &Utf8Path { - self.path.parent().unwrap() + pub fn parent(&self) -> Option<&Utf8Path> { + self.path.parent() } pub fn lock_kind(&self) -> FileLockKind {