From 205989d0986f73d29da26bd2b75c3d794514376e Mon Sep 17 00:00:00 2001 From: Mateusz Kowalski Date: Wed, 27 Nov 2024 16:57:53 +0100 Subject: [PATCH 01/14] wip --- scarb/src/bin/scarb/commands/package.rs | 3 +- scarb/src/bin/scarb/commands/publish.rs | 2 +- scarb/src/consts.rs | 29 +++++++++++++ scarb/src/lib.rs | 1 + scarb/src/ops/package.rs | 58 +++++++++++++++++++++++-- scarb/src/ops/publish.rs | 11 ++++- 6 files changed, 97 insertions(+), 7 deletions(-) create mode 100644 scarb/src/consts.rs diff --git a/scarb/src/bin/scarb/commands/package.rs b/scarb/src/bin/scarb/commands/package.rs index 06d633628..ad78e5c75 100644 --- a/scarb/src/bin/scarb/commands/package.rs +++ b/scarb/src/bin/scarb/commands/package.rs @@ -14,6 +14,7 @@ use crate::args::PackageArgs; #[tracing::instrument(skip_all, level = "info")] pub fn run(args: PackageArgs, config: &Config) -> Result<()> { + println!("test"); let ws = ops::read_workspace(config.manifest_path(), config)?; let packages = args .packages_filter @@ -38,7 +39,7 @@ pub fn run(args: PackageArgs, config: &Config) -> Result<()> { let result = ops::package_list(&packages, &opts, &ws)?; ws.config().ui().print(ListMessage(result)); } else { - ops::package(&packages, &opts, &ws)?; + ops::package(&packages, &opts, &ws, &args.args)?; } Ok(()) diff --git a/scarb/src/bin/scarb/commands/publish.rs b/scarb/src/bin/scarb/commands/publish.rs index 73a786fd7..aaac9e4ec 100644 --- a/scarb/src/bin/scarb/commands/publish.rs +++ b/scarb/src/bin/scarb/commands/publish.rs @@ -30,5 +30,5 @@ pub fn run(args: PublishArgs, config: &Config) -> Result<()> { }, }; - ops::publish(package.id, &ops, &ws) + ops::publish(package.id, &ops, &ws, &args.args) } diff --git a/scarb/src/consts.rs b/scarb/src/consts.rs new file mode 100644 index 000000000..1318cea0c --- /dev/null +++ b/scarb/src/consts.rs @@ -0,0 +1,29 @@ +pub enum SupportedPlatform { + Aarch64AppleDarwin, + Aarch64UnknownLinuxGnu, + X8664AppleDarwin, + X8664PcWindowsMsvc, + X8664UnknownLinuxGnu, +} + +impl SupportedPlatform { + pub fn as_str(&self) -> &'static str { + match self { + Self::Aarch64AppleDarwin => "aarch64-apple-darwin", + Self::Aarch64UnknownLinuxGnu => "aarch64-unknown-linux-gnu", + Self::X8664AppleDarwin => "x86_64-apple-darwin", + Self::X8664PcWindowsMsvc => "x86_64-pc-windows-msvc", + Self::X8664UnknownLinuxGnu => "x86_64-unknown-linux-gnu", + } + } + + pub fn variants() -> &'static [SupportedPlatform; 5] { + &[ + Self::Aarch64AppleDarwin, + Self::Aarch64UnknownLinuxGnu, + Self::X8664AppleDarwin, + Self::X8664PcWindowsMsvc, + Self::X8664UnknownLinuxGnu, + ] + } +} diff --git a/scarb/src/lib.rs b/scarb/src/lib.rs index 673632428..f6c047170 100644 --- a/scarb/src/lib.rs +++ b/scarb/src/lib.rs @@ -13,6 +13,7 @@ use std::sync::LazyLock; pub use subcommands::EXTERNAL_CMD_PREFIX; pub mod compiler; +pub mod consts; pub mod core; pub mod flock; mod internal; diff --git a/scarb/src/ops/package.rs b/scarb/src/ops/package.rs index c01ceda23..c332bc2dc 100644 --- a/scarb/src/ops/package.rs +++ b/scarb/src/ops/package.rs @@ -1,11 +1,15 @@ +use core::str; use std::collections::BTreeMap; +use std::ffi::OsString; use std::fs::File; use std::io::{Seek, SeekFrom, Write}; +use std::process::Command; use anyhow::{bail, ensure, Context, Result}; use camino::Utf8PathBuf; use indoc::{formatdoc, indoc, writedoc}; +use crate::consts::SupportedPlatform; use crate::core::registry::package_source_store::PackageSourceStore; use crate::sources::client::PackageRepository; @@ -18,7 +22,9 @@ use crate::compiler::plugin::proc_macro::compilation::{ }; 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::core::{ + Config, Package, PackageId, PackageName, ScriptDefinition, Target, TargetKind, Workspace, +}; use crate::flock::{FileLockGuard, Filesystem}; use crate::internal::restricted_names; use crate::{ @@ -76,12 +82,13 @@ pub fn package( packages: &[PackageId], opts: &PackageOpts, ws: &Workspace<'_>, + args: &[OsString], ) -> Result> { before_package(ws)?; packages .iter() - .map(|pkg| package_one_impl(*pkg, opts, ws)) + .map(|pkg| package_one_impl(*pkg, opts, ws, args)) .collect() } @@ -89,8 +96,9 @@ pub fn package_one( package_id: PackageId, opts: &PackageOpts, ws: &Workspace<'_>, + args: &[OsString], ) -> Result { - package(&[package_id], opts, ws).map(|mut v| v.pop().unwrap()) + package(&[package_id], opts, ws, args).map(|mut v| v.pop().unwrap()) } #[tracing::instrument(level = "debug", skip(opts, ws))] @@ -155,6 +163,7 @@ fn package_one_impl( pkg_id: PackageId, opts: &PackageOpts, ws: &Workspace<'_>, + args: &[OsString], ) -> Result { let pkg = ws.fetch_package(&pkg_id)?; @@ -169,6 +178,13 @@ fn package_one_impl( let recipe = prepare_archive_recipe(pkg, opts, ws)?; let num_files = recipe.len(); + // wawel37: tutaj zaczynamy jazde + if let Some(script_definition) = pkg.manifest.scripts.get("package") { + if pkg.is_cairo_plugin() { + package_cairo_plugin(pkg, ws, args, script_definition); + } + } + // Package up and test a temporary tarball and only move it to the final location if it actually // passes all verification checks. Any previously existing tarball can be assumed as corrupt // or invalid, so we can overwrite it if it exists. @@ -606,3 +622,39 @@ fn check_metadata(pkg: &Package, config: &Config) -> Result<()> { Ok(()) } + +#[tracing::instrument(level = "trace", skip_all)] +fn package_cairo_plugin( + package: &Package, + ws: &Workspace<'_>, + args: &[OsString], + script_definition: &ScriptDefinition, +) { + package.manifest.targets + let target_dir = ws.target_dir().child("cairo-plugin"); + + + // Install standard library for each supported targets. + // If the std is already installed, cargo will not download it again if the std is up to date. + for platform in SupportedPlatform::variants() { + let output = Command::new("rustup").args(["target", "add", platform.as_str()]).output(); + + match output { + Ok(output) => { + if !output.status.success() { + ws.config().ui().error(format!("failed to install standard library for target {}:\n{:?}", platform.as_str(), str::from_utf8(&output.stderr).unwrap_or("failed to decode stderr as UTF-8"))); + } + }, + Err(e) => { + ws.config().ui().error(format!("failed to install standard library for target {}:\n{}", platform.as_str(), e)); + } + } + + // Build targets + + + } + +} + + diff --git a/scarb/src/ops/publish.rs b/scarb/src/ops/publish.rs index 007525b50..ec980c87d 100644 --- a/scarb/src/ops/publish.rs +++ b/scarb/src/ops/publish.rs @@ -1,3 +1,5 @@ +use std::ffi::OsString; + use anyhow::{ensure, Context, Result}; use indoc::formatdoc; use url::Url; @@ -17,7 +19,12 @@ pub struct PublishOpts { } #[tracing::instrument(level = "debug", skip(opts, ws))] -pub fn publish(package_id: PackageId, opts: &PublishOpts, ws: &Workspace<'_>) -> Result<()> { +pub fn publish( + package_id: PackageId, + opts: &PublishOpts, + ws: &Workspace<'_>, + args: &[OsString], +) -> Result<()> { let package = ws.fetch_package(&package_id)?.clone(); ensure!( package.is_publishable(), @@ -43,7 +50,7 @@ pub fn publish(package_id: PackageId, opts: &PublishOpts, ws: &Workspace<'_>) -> "publishing packages is not supported by registry: {source_id}" ); - let tarball = ops::package_one(package_id, &opts.package_opts, ws)?; + let tarball = ops::package_one(package_id, &opts.package_opts, ws, args)?; let dest_package_id = package_id.with_source_id(source_id); From 89299faddc1d5b7acc6456c61955d5394def2a23 Mon Sep 17 00:00:00 2001 From: Mateusz Kowalski Date: Thu, 28 Nov 2024 15:07:44 +0100 Subject: [PATCH 02/14] Execute package script after packaging with cargo --- scarb/src/ops/package.rs | 4 ++-- scarb/src/ops/scripts.rs | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/scarb/src/ops/package.rs b/scarb/src/ops/package.rs index 1700bda4c..42863c182 100644 --- a/scarb/src/ops/package.rs +++ b/scarb/src/ops/package.rs @@ -2,6 +2,7 @@ use core::str; use std::collections::BTreeMap; use std::fs::File; use std::io::{Seek, SeekFrom, Write}; +use std::process::Command; use anyhow::{bail, ensure, Context, Result}; use camino::Utf8PathBuf; @@ -169,6 +170,7 @@ fn package_one_impl( let recipe = prepare_archive_recipe(pkg, opts, ws)?; let num_files = recipe.len(); + let target_dir = ws.target_dir().child("package"); if let Some(script_definition) = pkg.manifest.scripts.get("package") { if pkg.is_cairo_plugin() { @@ -184,8 +186,6 @@ fn package_one_impl( // passes all verification checks. Any previously existing tarball can be assumed as corrupt // or invalid, so we can overwrite it if it exists. let filename = pkg_id.tarball_name(); - let target_dir = ws.target_dir().child("package"); - // println!("{:?}", recipe); let mut dst = target_dir.create_rw(format!(".{filename}"), "package scratch space", ws.config())?; diff --git a/scarb/src/ops/scripts.rs b/scarb/src/ops/scripts.rs index 8c60cd417..875f0b4a9 100644 --- a/scarb/src/ops/scripts.rs +++ b/scarb/src/ops/scripts.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use std::env; use std::ffi::OsString; -use std::process::ExitCode; +use std::process::{exit, ExitCode}; use std::rc::Rc; use anyhow::Result; @@ -56,6 +56,8 @@ pub fn execute_script( custom_commands, )); + println!("exit code: {:?}", exit_code); + if exit_code != 0 { let exit_code: ExitCode = u8::try_from(exit_code) .map(Into::into) From e6dc3a91c942b2d3a7a83b2febd85c93c1690b62 Mon Sep 17 00:00:00 2001 From: Mateusz Kowalski Date: Thu, 28 Nov 2024 15:09:49 +0100 Subject: [PATCH 03/14] Fix clippy and cleanup --- scarb/src/bin/scarb/commands/package.rs | 1 - scarb/src/consts.rs | 29 ------------------------- scarb/src/lib.rs | 1 - scarb/src/ops/package.rs | 1 - scarb/src/ops/scripts.rs | 4 +--- 5 files changed, 1 insertion(+), 35 deletions(-) delete mode 100644 scarb/src/consts.rs diff --git a/scarb/src/bin/scarb/commands/package.rs b/scarb/src/bin/scarb/commands/package.rs index 382489c1e..06d633628 100644 --- a/scarb/src/bin/scarb/commands/package.rs +++ b/scarb/src/bin/scarb/commands/package.rs @@ -14,7 +14,6 @@ use crate::args::PackageArgs; #[tracing::instrument(skip_all, level = "info")] pub fn run(args: PackageArgs, config: &Config) -> Result<()> { - println!("test"); let ws = ops::read_workspace(config.manifest_path(), config)?; let packages = args .packages_filter diff --git a/scarb/src/consts.rs b/scarb/src/consts.rs deleted file mode 100644 index 1318cea0c..000000000 --- a/scarb/src/consts.rs +++ /dev/null @@ -1,29 +0,0 @@ -pub enum SupportedPlatform { - Aarch64AppleDarwin, - Aarch64UnknownLinuxGnu, - X8664AppleDarwin, - X8664PcWindowsMsvc, - X8664UnknownLinuxGnu, -} - -impl SupportedPlatform { - pub fn as_str(&self) -> &'static str { - match self { - Self::Aarch64AppleDarwin => "aarch64-apple-darwin", - Self::Aarch64UnknownLinuxGnu => "aarch64-unknown-linux-gnu", - Self::X8664AppleDarwin => "x86_64-apple-darwin", - Self::X8664PcWindowsMsvc => "x86_64-pc-windows-msvc", - Self::X8664UnknownLinuxGnu => "x86_64-unknown-linux-gnu", - } - } - - pub fn variants() -> &'static [SupportedPlatform; 5] { - &[ - Self::Aarch64AppleDarwin, - Self::Aarch64UnknownLinuxGnu, - Self::X8664AppleDarwin, - Self::X8664PcWindowsMsvc, - Self::X8664UnknownLinuxGnu, - ] - } -} diff --git a/scarb/src/lib.rs b/scarb/src/lib.rs index f6c047170..673632428 100644 --- a/scarb/src/lib.rs +++ b/scarb/src/lib.rs @@ -13,7 +13,6 @@ use std::sync::LazyLock; pub use subcommands::EXTERNAL_CMD_PREFIX; pub mod compiler; -pub mod consts; pub mod core; pub mod flock; mod internal; diff --git a/scarb/src/ops/package.rs b/scarb/src/ops/package.rs index 42863c182..682c53860 100644 --- a/scarb/src/ops/package.rs +++ b/scarb/src/ops/package.rs @@ -2,7 +2,6 @@ use core::str; use std::collections::BTreeMap; use std::fs::File; use std::io::{Seek, SeekFrom, Write}; -use std::process::Command; use anyhow::{bail, ensure, Context, Result}; use camino::Utf8PathBuf; diff --git a/scarb/src/ops/scripts.rs b/scarb/src/ops/scripts.rs index 875f0b4a9..8c60cd417 100644 --- a/scarb/src/ops/scripts.rs +++ b/scarb/src/ops/scripts.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use std::env; use std::ffi::OsString; -use std::process::{exit, ExitCode}; +use std::process::ExitCode; use std::rc::Rc; use anyhow::Result; @@ -56,8 +56,6 @@ pub fn execute_script( custom_commands, )); - println!("exit code: {:?}", exit_code); - if exit_code != 0 { let exit_code: ExitCode = u8::try_from(exit_code) .map(Into::into) From aa1ca15a22ad47b4e601233b908c0eb9843a0568 Mon Sep 17 00:00:00 2001 From: Mateusz Kowalski Date: Fri, 29 Nov 2024 13:34:27 +0100 Subject: [PATCH 04/14] Get rid of cairo plugin check and don't create target directory for package script --- scarb/src/ops/package.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/scarb/src/ops/package.rs b/scarb/src/ops/package.rs index 682c53860..0a8ad3dcc 100644 --- a/scarb/src/ops/package.rs +++ b/scarb/src/ops/package.rs @@ -172,13 +172,10 @@ fn package_one_impl( let target_dir = ws.target_dir().child("package"); if let Some(script_definition) = pkg.manifest.scripts.get("package") { - if pkg.is_cairo_plugin() { - ws.target_dir() - .child("scarb") - .child("cairo-plugin") - .path_existent()?; - ops::execute_script(script_definition, &[], ws, pkg.root(), None)?; - } + ws.config() + .ui() + .print(Status::new("Packaging", &pkg_id.to_string())); + ops::execute_script(script_definition, &[], ws, pkg.root(), None)?; } // Package up and test a temporary tarball and only move it to the final location if it actually From 61cca2e93564fb770416a03baf1563a900b3bbf1 Mon Sep 17 00:00:00 2001 From: Mateusz Kowalski Date: Fri, 29 Nov 2024 13:50:39 +0100 Subject: [PATCH 05/14] Display status when running the package script --- scarb/src/ops/package.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/scarb/src/ops/package.rs b/scarb/src/ops/package.rs index 0a8ad3dcc..4eb639d3c 100644 --- a/scarb/src/ops/package.rs +++ b/scarb/src/ops/package.rs @@ -172,9 +172,10 @@ fn package_one_impl( let target_dir = ws.target_dir().child("package"); if let Some(script_definition) = pkg.manifest.scripts.get("package") { - ws.config() - .ui() - .print(Status::new("Packaging", &pkg_id.to_string())); + ws.config().ui().print(Status::new( + "Running package script with package", + &pkg_id.to_string(), + )); ops::execute_script(script_definition, &[], ws, pkg.root(), None)?; } From c28fa318134ac15d54005f98bfe0dd65f1f10d21 Mon Sep 17 00:00:00 2001 From: Mateusz Kowalski Date: Fri, 29 Nov 2024 15:03:06 +0100 Subject: [PATCH 06/14] Run script from cwd as target_dir --- scarb/src/ops/package.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/scarb/src/ops/package.rs b/scarb/src/ops/package.rs index 4eb639d3c..be5f3147b 100644 --- a/scarb/src/ops/package.rs +++ b/scarb/src/ops/package.rs @@ -19,7 +19,9 @@ use crate::compiler::plugin::proc_macro::compilation::{ }; 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::core::{ + Config, Package, PackageId, PackageName, ScriptDefinition, Target, TargetKind, Workspace, +}; use crate::flock::{FileLockGuard, Filesystem}; use crate::internal::restricted_names; use crate::{ @@ -176,7 +178,18 @@ fn package_one_impl( "Running package script with package", &pkg_id.to_string(), )); - ops::execute_script(script_definition, &[], ws, pkg.root(), None)?; + let absolute_path_script_definition = ScriptDefinition::new( + pkg.root() + .join(format!("{}", script_definition)) + .to_string(), + ); + ops::execute_script( + &absolute_path_script_definition, + &[], + ws, + target_dir.path_existent()?, + None, + )?; } // Package up and test a temporary tarball and only move it to the final location if it actually From 4a68d19b3c16466560ed0f4a5552a22057018d01 Mon Sep 17 00:00:00 2001 From: Mateusz Kowalski Date: Fri, 29 Nov 2024 16:53:10 +0100 Subject: [PATCH 07/14] Add package script test --- scarb/src/ops/package.rs | 8 +++++- scarb/tests/package.rs | 56 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/scarb/src/ops/package.rs b/scarb/src/ops/package.rs index be5f3147b..63588c6a5 100644 --- a/scarb/src/ops/package.rs +++ b/scarb/src/ops/package.rs @@ -2,8 +2,9 @@ use core::str; use std::collections::BTreeMap; use std::fs::File; use std::io::{Seek, SeekFrom, Write}; +use std::path::Path; -use anyhow::{bail, ensure, Context, Result}; +use anyhow::{anyhow, bail, ensure, Context, Result}; use camino::Utf8PathBuf; use indoc::{formatdoc, indoc, writedoc}; @@ -178,6 +179,11 @@ fn package_one_impl( "Running package script with package", &pkg_id.to_string(), )); + let script_path_string = script_definition.to_string(); + let script_path = Path::new(&script_path_string); + if !script_path.exists() || !script_path.is_file() { + return Err(anyhow!("Package script must be a path to an existing file")); + }; let absolute_path_script_definition = ScriptDefinition::new( pkg.root() .join(format!("{}", script_definition)) diff --git a/scarb/tests/package.rs b/scarb/tests/package.rs index 00d3cb279..3fc3d7fdc 100644 --- a/scarb/tests/package.rs +++ b/scarb/tests/package.rs @@ -14,7 +14,7 @@ use scarb::DEFAULT_TARGET_DIR_NAME; use scarb_build_metadata::CAIRO_VERSION; use scarb_test_support::cairo_plugin_project_builder::CairoPluginProjectBuilder; use scarb_test_support::command::Scarb; -use scarb_test_support::fsx::unix_paths_to_os_lossy; +use scarb_test_support::fsx::{make_executable, unix_paths_to_os_lossy}; use scarb_test_support::gitx; use scarb_test_support::project_builder::{Dep, DepBuilder, ProjectBuilder}; use scarb_test_support::registry::local::LocalRegistry; @@ -1525,3 +1525,57 @@ fn package_with_publish_disabled() { [..]Packaged [..] files, [..] ([..] compressed) "#}); } + +#[test] +fn package_with_package_script() { + let t = TempDir::new().unwrap(); + + #[cfg(not(windows))] + let script_name = "script.sh"; + #[cfg(not(windows))] + let script_code = indoc! { r#" + touch text.txt + "#}; + + #[cfg(windows)] + let script_name = "script.bat"; + #[cfg(windows)] + let script_code = indoc! { r#" + @echo off + copy NUL text.txt + "#}; + + ProjectBuilder::start() + .name("foo") + .version("1.0.0") + .manifest_extra(formatdoc! {r#" + [scripts] + package = "{script_name}" + "#}) + .src(script_name, script_code) + .build(&t); + + #[cfg(not(windows))] + make_executable(&t.to_path_buf().join(script_name)); + + Scarb::quick_snapbox() + .arg("package") + .arg("--no-verify") + .arg("--no-metadata") + .current_dir(&t) + .assert() + .success() + .stdout_matches(indoc! {r#" + [..]Packaging[..] + [..]Running package script with package foo v1.0.0[..] + [..]Packaged[..] + "#}); + + assert!(Path::new( + &t.to_path_buf() + .join("target") + .join("package") + .join("text.txt"), + ) + .exists()); +} From ef0450ff9d7bd9db8a14900eca66a0f5a8a019e9 Mon Sep 17 00:00:00 2001 From: Mateusz Kowalski Date: Mon, 2 Dec 2024 16:00:39 +0100 Subject: [PATCH 08/14] Make tests use the package --- scarb/tests/package.rs | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/scarb/tests/package.rs b/scarb/tests/package.rs index 3fc3d7fdc..22b4b7e54 100644 --- a/scarb/tests/package.rs +++ b/scarb/tests/package.rs @@ -1535,6 +1535,8 @@ fn package_with_package_script() { #[cfg(not(windows))] let script_code = indoc! { r#" touch text.txt + cd ../.. + cargo build "#}; #[cfg(windows)] @@ -1543,14 +1545,29 @@ fn package_with_package_script() { let script_code = indoc! { r#" @echo off copy NUL text.txt + cd ../.. + cargo build "#}; - ProjectBuilder::start() + CairoPluginProjectBuilder::start() .name("foo") - .version("1.0.0") - .manifest_extra(formatdoc! {r#" - [scripts] - package = "{script_name}" + .scarb_project(|b| { + b.name("foo") + .version("1.0.0") + .manifest_extra(formatdoc! {r#" + [cairo-plugin] + + [scripts] + package = "{script_name}" + "#}) + }) + .lib_rs(indoc! {r#" + use cairo_lang_macro::{ProcMacroResult, TokenStream, attribute_macro}; + + #[attribute_macro] + pub fn some(_attr: TokenStream, token_stream: TokenStream) -> ProcMacroResult { + ProcMacroResult::new(token_stream) + } "#}) .src(script_name, script_code) .build(&t); @@ -1560,16 +1577,9 @@ fn package_with_package_script() { Scarb::quick_snapbox() .arg("package") - .arg("--no-verify") - .arg("--no-metadata") .current_dir(&t) .assert() - .success() - .stdout_matches(indoc! {r#" - [..]Packaging[..] - [..]Running package script with package foo v1.0.0[..] - [..]Packaged[..] - "#}); + .success(); assert!(Path::new( &t.to_path_buf() @@ -1578,4 +1588,5 @@ fn package_with_package_script() { .join("text.txt"), ) .exists()); + assert!(Path::new(&t.to_path_buf().join("target").join("debug")).exists()) } From 9bd5a9152bf84c7cd968d983bd8ab2400248f1d2 Mon Sep 17 00:00:00 2001 From: Mateusz Kowalski Date: Thu, 5 Dec 2024 10:25:15 +0100 Subject: [PATCH 09/14] Set include field in scarb toml --- scarb/src/core/manifest/mod.rs | 1 + scarb/src/core/manifest/toml_manifest.rs | 8 ++++++++ scarb/src/core/publishing/manifest_normalization.rs | 1 + scarb/src/ops/package.rs | 3 +++ 4 files changed, 13 insertions(+) diff --git a/scarb/src/core/manifest/mod.rs b/scarb/src/core/manifest/mod.rs index bdeb0c175..ef41db954 100644 --- a/scarb/src/core/manifest/mod.rs +++ b/scarb/src/core/manifest/mod.rs @@ -75,6 +75,7 @@ pub struct ManifestMetadata { pub license_file: Option, pub readme: Option, pub repository: Option, + pub include: Option>, #[serde(rename = "tool")] pub tool_metadata: Option>, pub cairo_version: Option, diff --git a/scarb/src/core/manifest/toml_manifest.rs b/scarb/src/core/manifest/toml_manifest.rs index 64485e847..8f95ca26f 100644 --- a/scarb/src/core/manifest/toml_manifest.rs +++ b/scarb/src/core/manifest/toml_manifest.rs @@ -119,6 +119,7 @@ pub struct PackageInheritableFields { pub license_file: Option, pub readme: Option, pub repository: Option, + pub include: Option>, pub cairo_version: Option, } @@ -147,6 +148,7 @@ impl PackageInheritableFields { get_field!(license, String); get_field!(license_file, Utf8PathBuf); get_field!(repository, String); + get_field!(include, VecOfStrings); get_field!(edition, Edition); pub fn readme(&self, workspace_root: &Utf8Path, package_root: &Utf8Path) -> Result { @@ -197,6 +199,7 @@ pub struct TomlPackage { pub license_file: Option>, pub readme: Option>, pub repository: Option>, + pub include: Option>>, /// **UNSTABLE** This package does not depend on Cairo's `core`. pub no_core: Option, pub cairo_version: Option>, @@ -571,6 +574,11 @@ impl TomlManifest { .clone() .map(|mw| mw.resolve("repository", || inheritable_package.repository())) .transpose()?, + include: package + .include + .clone() + .map(|mw| mw.resolve("include", || inheritable_package.include())) + .transpose()?, cairo_version: package .cairo_version .clone() diff --git a/scarb/src/core/publishing/manifest_normalization.rs b/scarb/src/core/publishing/manifest_normalization.rs index 14e41e0ac..45e04e555 100644 --- a/scarb/src/core/publishing/manifest_normalization.rs +++ b/scarb/src/core/publishing/manifest_normalization.rs @@ -73,6 +73,7 @@ fn generate_package(pkg: &Package) -> Box { .clone() .map(|_| MaybeWorkspace::Defined((Utf8PathBuf::from(DEFAULT_README_FILE_NAME)).into())), repository: metadata.repository.clone().map(MaybeWorkspace::Defined), + include: metadata.include.clone().map(MaybeWorkspace::Defined), no_core: summary.no_core.then_some(true), cairo_version: metadata.cairo_version.clone().map(MaybeWorkspace::Defined), experimental_features: pkg.manifest.experimental_features.clone(), diff --git a/scarb/src/ops/package.rs b/scarb/src/ops/package.rs index 63588c6a5..0398acff0 100644 --- a/scarb/src/ops/package.rs +++ b/scarb/src/ops/package.rs @@ -173,6 +173,9 @@ fn package_one_impl( let recipe = prepare_archive_recipe(pkg, opts, ws)?; let num_files = recipe.len(); let target_dir = ws.target_dir().child("package"); + if let Some(include) = pkg.manifest.metadata.include.clone() { + println!("includy: {:?}", include); + } if let Some(script_definition) = pkg.manifest.scripts.get("package") { ws.config().ui().print(Status::new( From 98f60646eb35487c92d0525cce88dfc1d20decfd Mon Sep 17 00:00:00 2001 From: Mateusz Kowalski Date: Thu, 5 Dec 2024 16:10:29 +0100 Subject: [PATCH 10/14] Make package script support and put the result dlls inside tarball --- scarb/Cargo.toml | 1 + scarb/src/lib.rs | 2 + scarb/src/ops/package.rs | 114 ++++++++++++++++++++++++++++----------- scarb/tests/package.rs | 91 ++++++++++++++++++++++++------- 4 files changed, 157 insertions(+), 51 deletions(-) diff --git a/scarb/Cargo.toml b/scarb/Cargo.toml index d1d093ab7..dc41d39a4 100644 --- a/scarb/Cargo.toml +++ b/scarb/Cargo.toml @@ -89,6 +89,7 @@ windows-sys.workspace = true zstd.workspace = true cargo_metadata.workspace = true flate2.workspace = true +fs_extra.workspace = true [target.'cfg(not(target_os = "linux"))'.dependencies] reqwest = { workspace = true, default-features = true } diff --git a/scarb/src/lib.rs b/scarb/src/lib.rs index 673632428..97d1aee1d 100644 --- a/scarb/src/lib.rs +++ b/scarb/src/lib.rs @@ -42,3 +42,5 @@ pub const TEST_ASSERTS_PLUGIN_NAME: &str = "assert_macros"; pub const CAIRO_RUN_PLUGIN_NAME: &str = "cairo_run"; pub const CARGO_MANIFEST_FILE_NAME: &str = "Cargo.toml"; pub const CARGO_LOCK_FILE_NAME: &str = "Cargo.lock"; +pub static SHARED_LIBRARY_TARGET_DIRECTORY: LazyLock = + LazyLock::new(|| ["target", "scarb", "cairo-plugin"].iter().collect()); diff --git a/scarb/src/ops/package.rs b/scarb/src/ops/package.rs index 0398acff0..b17914172 100644 --- a/scarb/src/ops/package.rs +++ b/scarb/src/ops/package.rs @@ -1,13 +1,15 @@ +use anyhow::{anyhow, bail, ensure, Context, Result}; +use camino::{Utf8Path, Utf8PathBuf}; use core::str; +use fs::copy; +use fs_extra::dir::{copy as fs_extra_copy, CopyOptions}; +use fs_extra::error::{Error as FsExtraError, ErrorKind as FsExtraErrorKind}; +use indoc::{formatdoc, indoc, writedoc}; use std::collections::BTreeMap; -use std::fs::File; +use std::fs::{self, File}; use std::io::{Seek, SeekFrom, Write}; use std::path::Path; -use anyhow::{anyhow, bail, ensure, Context, Result}; -use camino::Utf8PathBuf; -use indoc::{formatdoc, indoc, writedoc}; - use crate::core::registry::package_source_store::PackageSourceStore; use crate::sources::client::PackageRepository; @@ -20,16 +22,16 @@ use crate::compiler::plugin::proc_macro::compilation::{ }; 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, ScriptDefinition, Target, TargetKind, Workspace, -}; +use crate::core::{Config, Package, PackageId, PackageName, Target, TargetKind, Workspace}; use crate::flock::{FileLockGuard, Filesystem}; 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, + MANIFEST_FILE_NAME, SHARED_LIBRARY_TARGET_DIRECTORY, VCS_INFO_FILE_NAME, }; +use super::execute_script; + const VERSION: u8 = 1; const VERSION_FILE_NAME: &str = "VERSION"; const ORIGINAL_MANIFEST_FILE_NAME: &str = "Scarb.orig.toml"; @@ -161,6 +163,7 @@ fn package_one_impl( ws: &Workspace<'_>, ) -> Result { let pkg = ws.fetch_package(&pkg_id)?; + let target_dir = ws.target_dir().child("package"); ws.config() .ui() @@ -170,37 +173,40 @@ fn package_one_impl( check_metadata(pkg, ws.config())?; } - let recipe = prepare_archive_recipe(pkg, opts, ws)?; - let num_files = recipe.len(); - let target_dir = ws.target_dir().child("package"); - if let Some(include) = pkg.manifest.metadata.include.clone() { - println!("includy: {:?}", include); - } + let mut recipe: Vec = Vec::default(); if let Some(script_definition) = pkg.manifest.scripts.get("package") { + let target_dir_path = target_dir.path_existent()?; ws.config().ui().print(Status::new( "Running package script with package", &pkg_id.to_string(), )); - let script_path_string = script_definition.to_string(); - let script_path = Path::new(&script_path_string); - if !script_path.exists() || !script_path.is_file() { - return Err(anyhow!("Package script must be a path to an existing file")); - }; - let absolute_path_script_definition = ScriptDefinition::new( - pkg.root() - .join(format!("{}", script_definition)) - .to_string(), - ); - ops::execute_script( - &absolute_path_script_definition, - &[], - ws, - target_dir.path_existent()?, - None, - )?; + + if let Some(includes) = pkg.manifest.metadata.include.clone() { + copy_items_to_target_dir(includes, target_dir_path)?; + } + + execute_script(script_definition, &[], ws, target_dir_path, None)?; + + let built_macros_target_dir = ws.target_dir().child("scarb").child("cairo-plugin"); + + if built_macros_target_dir.exists() { + let dynamic_library_files = + find_dynamic_library_files(built_macros_target_dir.path_unchecked()); + for dynamic_library_file in dynamic_library_files.into_iter() { + recipe.push(ArchiveFile { + path: SHARED_LIBRARY_TARGET_DIRECTORY + .as_path() + .join(dynamic_library_file.file_name().unwrap()), + contents: ArchiveFileContents::OnDisk(dynamic_library_file), + }) + } + } } + recipe.extend(prepare_archive_recipe(pkg, opts, ws)?); + let num_files = recipe.len(); + // Package up and test a temporary tarball and only move it to the final location if it actually // passes all verification checks. Any previously existing tarball can be assumed as corrupt // or invalid, so we can overwrite it if it exists. @@ -637,3 +643,47 @@ fn check_metadata(pkg: &Package, config: &Config) -> Result<()> { Ok(()) } + +fn copy_items_to_target_dir(paths: Vec, target_dir: &Utf8Path) -> Result<()> { + let target_path = Path::new(target_dir); + let options = CopyOptions::new().copy_inside(true).overwrite(true); + + for path_str in paths { + let source_path = Path::new(&path_str); + if source_path.exists() { + if source_path.is_dir() { + fs_extra_copy(source_path, target_path, &options)?; + } else { + let file_name = source_path.file_name().ok_or(FsExtraError::new( + fs_extra::error::ErrorKind::Other, + "failed to extract file name", + ))?; + let target_file_path = target_path.join(file_name); + copy(source_path, target_file_path).map_err(|err| { + FsExtraError::new(FsExtraErrorKind::Io(err), "failed to copy file") + })?; + } + } else { + return Err(anyhow!("path does not exist: {}", source_path.display())); + } + } + + Ok(()) +} + +fn find_dynamic_library_files(directory: &Utf8Path) -> Vec { + let mut files = Vec::new(); + if let Ok(entries) = fs::read_dir(directory) { + for entry in entries.filter_map(Result::ok) { + let path = entry.path(); + if let Ok(utf8_path) = Utf8PathBuf::from_path_buf(path) { + if let Some(extension) = utf8_path.extension() { + if extension == "dll" || extension == "so" || extension == "dylib" { + files.push(utf8_path); + } + } + } + } + } + files +} diff --git a/scarb/tests/package.rs b/scarb/tests/package.rs index 22b4b7e54..5e36dc39b 100644 --- a/scarb/tests/package.rs +++ b/scarb/tests/package.rs @@ -48,10 +48,11 @@ impl PackageChecker { .path() .expect("failed to get archive entry path") .into_owned(); - let mut contents = String::new(); - entry - .read_to_string(&mut contents) - .expect("failed to read archive entry contents"); + let mut contents = String::default(); + // Some files are not valid utf-8 contents, so we don't want to read them at all. + // We just want to track that they even exist. + let _ = entry.read_to_string(&mut contents); + (name, contents) }) .collect(); @@ -1534,9 +1535,9 @@ fn package_with_package_script() { let script_name = "script.sh"; #[cfg(not(windows))] let script_code = indoc! { r#" - touch text.txt - cd ../.. - cargo build + cargo build --release + mkdir -p ../scarb/cairo-plugin + cp target/release/libfoo.dylib ../scarb/cairo-plugin "#}; #[cfg(windows)] @@ -1544,9 +1545,9 @@ fn package_with_package_script() { #[cfg(windows)] let script_code = indoc! { r#" @echo off - copy NUL text.txt - cd ../.. - cargo build + cargo build --release + mkdir -p ../scarb/cairo-plugin + copy target\release\libfoo.dll ..\scarb\cairo-plugin "#}; CairoPluginProjectBuilder::start() @@ -1554,6 +1555,9 @@ fn package_with_package_script() { .scarb_project(|b| { b.name("foo") .version("1.0.0") + .manifest_package_extra(indoc! {r#" + include = ["Cargo.lock", "Cargo.toml", "src", "script.sh"] + "#}) .manifest_extra(formatdoc! {r#" [cairo-plugin] @@ -1576,17 +1580,66 @@ fn package_with_package_script() { make_executable(&t.to_path_buf().join(script_name)); Scarb::quick_snapbox() + .current_dir(&t) .arg("package") + .assert() + .failure(); + + PackageChecker::assert(&t.child("target/package/foo-1.0.0.tar.zst")) + .name_and_version("foo", "1.0.0") + .contents(&[ + "VERSION", + "Scarb.orig.toml", + "Scarb.toml", + "target/scarb/cairo-plugin/libfoo.dylib", + "Cargo.toml", + "Cargo.orig.toml", + "src/lib.rs", + "script.sh", + ]); +} + +#[test] +fn package_with_package_script_not_existing_script() { + let t = TempDir::new().unwrap(); + + #[cfg(not(windows))] + let script_name = "script.sh"; + + #[cfg(windows)] + let script_name = "script.bat"; + + CairoPluginProjectBuilder::start() + .name("foo") + .scarb_project(|b| { + b.name("foo") + .version("1.0.0") + .manifest_package_extra(indoc! {r#" + include = ["Cargo.lock", "Cargo.toml", "src", "script.sh"] + "#}) + .manifest_extra(formatdoc! {r#" + [cairo-plugin] + + [scripts] + package = "{script_name}" + "#}) + }) + .lib_rs(indoc! {r#" + use cairo_lang_macro::{ProcMacroResult, TokenStream, attribute_macro}; + + #[attribute_macro] + pub fn some(_attr: TokenStream, token_stream: TokenStream) -> ProcMacroResult { + ProcMacroResult::new(token_stream) + } + "#}) + .build(&t); + + let assert = Scarb::quick_snapbox() .current_dir(&t) + .arg("package") .assert() - .success(); + .failure(); - assert!(Path::new( - &t.to_path_buf() - .join("target") - .join("package") - .join("text.txt"), - ) - .exists()); - assert!(Path::new(&t.to_path_buf().join("target").join("debug")).exists()) + let stdout = String::from_utf8(assert.get_output().stdout.clone()).unwrap(); + assert!(stdout.contains(&formatdoc! {r#"error: path does not exist: {script_name}"#})); } From ab95831d5337b23bd77190529a0366b4e3cc1b16 Mon Sep 17 00:00:00 2001 From: Mateusz Kowalski Date: Thu, 5 Dec 2024 16:48:47 +0100 Subject: [PATCH 11/14] Fix test --- scarb/tests/package.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scarb/tests/package.rs b/scarb/tests/package.rs index 5e36dc39b..f0757aca9 100644 --- a/scarb/tests/package.rs +++ b/scarb/tests/package.rs @@ -1583,7 +1583,7 @@ fn package_with_package_script() { .current_dir(&t) .arg("package") .assert() - .failure(); + .success(); PackageChecker::assert(&t.child("target/package/foo-1.0.0.tar.zst")) .name_and_version("foo", "1.0.0") From 4009be95efb25b3ee69c24667ebf6cf42a165100 Mon Sep 17 00:00:00 2001 From: Mateusz Kowalski Date: Fri, 6 Dec 2024 10:59:53 +0100 Subject: [PATCH 12/14] Fixed tests --- scarb/tests/package.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/scarb/tests/package.rs b/scarb/tests/package.rs index f0757aca9..34a2d7d86 100644 --- a/scarb/tests/package.rs +++ b/scarb/tests/package.rs @@ -1555,8 +1555,8 @@ fn package_with_package_script() { .scarb_project(|b| { b.name("foo") .version("1.0.0") - .manifest_package_extra(indoc! {r#" - include = ["Cargo.lock", "Cargo.toml", "src", "script.sh"] + .manifest_package_extra(formatdoc! {r#" + include = ["Cargo.lock", "Cargo.toml", "src", "{script_name}"] "#}) .manifest_extra(formatdoc! {r#" [cairo-plugin] @@ -1595,7 +1595,7 @@ fn package_with_package_script() { "Cargo.toml", "Cargo.orig.toml", "src/lib.rs", - "script.sh", + script_name, ]); } @@ -1634,12 +1634,9 @@ fn package_with_package_script_not_existing_script() { "#}) .build(&t); - let assert = Scarb::quick_snapbox() + Scarb::quick_snapbox() .current_dir(&t) .arg("package") .assert() .failure(); - - let stdout = String::from_utf8(assert.get_output().stdout.clone()).unwrap(); - assert!(stdout.contains(&formatdoc! {r#"error: path does not exist: {script_name}"#})); } From 6a66b47d5dbd00da90b0e959ea3dcfc299e9c629 Mon Sep 17 00:00:00 2001 From: Mateusz Kowalski Date: Fri, 6 Dec 2024 13:16:01 +0100 Subject: [PATCH 13/14] Set expected shared library target extension to be correct --- scarb/tests/package.rs | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/scarb/tests/package.rs b/scarb/tests/package.rs index f05ea779f..df2cdeff1 100644 --- a/scarb/tests/package.rs +++ b/scarb/tests/package.rs @@ -1532,10 +1532,10 @@ fn package_with_package_script() { let t = TempDir::new().unwrap(); #[cfg(not(windows))] - let script_code = indoc! { r#"cargo build --release && mkdir -p ../scarb/cairo-plugin && cp target/release/libfoo.dylib ../scarb/cairo-plugin"#}; + let script_code = indoc! { r#"cargo build --release && mkdir -p ../scarb/cairo-plugin && cp target/release/libfoo.so ../scarb/cairo-plugin"#}; #[cfg(windows)] - let script_code = indoc! { r#"cargo build --release && mkdir -p ..\scarb\cairo-plugin && copy target\release\libfoo.dll ..\scarb\cairo-plugin"#}; + let script_code = indoc! { r#"cargo build --release && mkdir -p ../scarb/cairo-plugin && copy target/release/libfoo.dll ../scarb/cairo-plugin"#}; CairoPluginProjectBuilder::start() .name("foo") @@ -1543,23 +1543,23 @@ fn package_with_package_script() { b.name("foo") .version("1.0.0") .manifest_package_extra(formatdoc! {r#" - include = ["Cargo.lock", "Cargo.toml", "src"] - "#}) + include = ["Cargo.lock", "Cargo.toml", "src"] + "#}) .manifest_extra(formatdoc! {r#" - [cairo-plugin] - - [scripts] - package = "{script_code}" - "#}) + [cairo-plugin] + + [scripts] + package = "{script_code}" + "#}) }) .lib_rs(indoc! {r#" - use cairo_lang_macro::{ProcMacroResult, TokenStream, attribute_macro}; - - #[attribute_macro] - pub fn some(_attr: TokenStream, token_stream: TokenStream) -> ProcMacroResult { - ProcMacroResult::new(token_stream) - } - "#}) + use cairo_lang_macro::{ProcMacroResult, TokenStream, attribute_macro}; + + #[attribute_macro] + pub fn some(_attr: TokenStream, token_stream: TokenStream) -> ProcMacroResult { + ProcMacroResult::new(token_stream) + } + "#}) .build(&t); Scarb::quick_snapbox() @@ -1568,13 +1568,19 @@ fn package_with_package_script() { .assert() .success(); + #[cfg(not(windows))] + let expected_shared_lib_file = r#"target/scarb/cairo-plugin/libfoo.so"#; + + #[cfg(windows)] + let expected_shared_lib_file = r#"target/scarb/cairo-plugin/libfoo.dll"#; + PackageChecker::assert(&t.child("target/package/foo-1.0.0.tar.zst")) .name_and_version("foo", "1.0.0") .contents(&[ "VERSION", "Scarb.orig.toml", "Scarb.toml", - "target/scarb/cairo-plugin/libfoo.dylib", + expected_shared_lib_file, "Cargo.toml", "Cargo.orig.toml", "src/lib.rs", From 81dcccccb3b99f56e7c924a0fd4f663e02ebc42c Mon Sep 17 00:00:00 2001 From: Mateusz Kowalski Date: Fri, 6 Dec 2024 13:51:37 +0100 Subject: [PATCH 14/14] Use cp instead of copy for windows tests --- scarb/tests/package.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scarb/tests/package.rs b/scarb/tests/package.rs index df2cdeff1..e30315425 100644 --- a/scarb/tests/package.rs +++ b/scarb/tests/package.rs @@ -1535,7 +1535,7 @@ fn package_with_package_script() { let script_code = indoc! { r#"cargo build --release && mkdir -p ../scarb/cairo-plugin && cp target/release/libfoo.so ../scarb/cairo-plugin"#}; #[cfg(windows)] - let script_code = indoc! { r#"cargo build --release && mkdir -p ../scarb/cairo-plugin && copy target/release/libfoo.dll ../scarb/cairo-plugin"#}; + let script_code = indoc! { r#"cargo build --release && mkdir -p ../scarb/cairo-plugin && cp target/release/libfoo.dll ../scarb/cairo-plugin"#}; CairoPluginProjectBuilder::start() .name("foo")