Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support packaging cairo-plugins #1605

Merged
merged 52 commits into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
4569059
Add cargo action for packaging
DelevoXDG Sep 17, 2024
058a698
Skip `Cargo.toml` when sourcing files for recipe
DelevoXDG Sep 17, 2024
bcdeaaf
Add conversion from `Target` to `TomlTarget` with external params
DelevoXDG Sep 17, 2024
1ba6c14
Support cairo plugin in manifest normalization
DelevoXDG Sep 17, 2024
b9904ba
Update toml manifest target collection to handle cairo plugin target
DelevoXDG Sep 17, 2024
2e8fc39
Add initial support for packaging plugins
DelevoXDG Sep 18, 2024
67b5a2a
Fix `no_lib_target` test; Rename to `no_target`
DelevoXDG Sep 19, 2024
343810d
Move `CairoPluginProjectBuilder` to `utils/scarb-test-support/`
DelevoXDG Sep 19, 2024
623357a
Retrieve name+version from Cargo.toml for path resolution
DelevoXDG Sep 24, 2024
7d8001c
Allow packaging via Cargo without verification
DelevoXDG Sep 24, 2024
428b99f
Format
DelevoXDG Sep 24, 2024
f1f94d1
Specify dependency version in `CairoPluginProjectBuilder`
DelevoXDG Sep 24, 2024
869a0be
Revert "Allow packaging via Cargo without verification"
DelevoXDG Sep 25, 2024
fd676e3
Fix missing whitespace in cairo_plugin_project_builder.rs
DelevoXDG Sep 25, 2024
9f64c1b
Add plugin packaging basic test
DelevoXDG Sep 25, 2024
662768c
Skip `Scarb.lock` when packaging
DelevoXDG Sep 25, 2024
bdcf32b
Pass relevant arguments to Cargo actions
DelevoXDG Sep 25, 2024
ade5aab
Deduplicate `--offline` flag addition
DelevoXDG Sep 25, 2024
b7cfd68
Move `cairo_plugin` packaging test
DelevoXDG Sep 25, 2024
20acc00
Allow checking that `target` and `package` are builtin
DelevoXDG Sep 25, 2024
ab9a86d
Don't `cargo package` and don't verify builtin plugins
DelevoXDG Sep 25, 2024
be9fd63
Add test for packing builtin cairo plugins
DelevoXDG Sep 25, 2024
13a0929
Simplify cairo-plugin, lib, and builtin checks
DelevoXDG Sep 25, 2024
656a61a
Normalize `cairo-plugin` in a typed manner; Skip `builtin = false`
DelevoXDG Sep 25, 2024
5b5941b
Remove `Target.to_toml_target`
DelevoXDG Sep 25, 2024
8559e17
Format
DelevoXDG Sep 25, 2024
d210532
Merge branch 'main' into zdobnikau/1527-package-cairo-plugins
DelevoXDG Sep 26, 2024
cdb2ce0
Fix test after merge
DelevoXDG Sep 26, 2024
8464751
Update cargo package related methods name
DelevoXDG Sep 27, 2024
af8dc5b
Move `is_builtin` method to fn
DelevoXDG Sep 29, 2024
523c30f
Remove source path
DelevoXDG Sep 29, 2024
6a157d2
Change Cargo manifest path resolution to extract version from `cargo …
DelevoXDG Sep 29, 2024
cc1c388
Move Cargo manifest path resolution to compilation.rs
DelevoXDG Sep 29, 2024
50296a8
Refactor Cargo manifest path resolution
DelevoXDG Sep 29, 2024
fa8d26f
Merge branch 'main' into zdobnikau/1527-package-cairo-plugins
DelevoXDG Sep 29, 2024
688dcd0
Update `shared_lib_path` to fetch package name from Cargo manifest
DelevoXDG Sep 29, 2024
dec41e2
Simplify error help message for missing target
DelevoXDG Oct 9, 2024
c5d6cff
Emit warning on cargo / scarb name and version mismatch
DelevoXDG Oct 9, 2024
838993c
Use name of `cdylib` target in `shared_lib_path`
DelevoXDG Oct 9, 2024
2bf0149
Run `cargo metadata` at package root
DelevoXDG Oct 9, 2024
c02dceb
Format
DelevoXDG Oct 9, 2024
32c5b7f
Move `is_builtin(target)`
DelevoXDG Oct 9, 2024
bdc9954
Format
DelevoXDG Oct 9, 2024
de1ac69
Return `Result` instead of panic; Fix error messages style
DelevoXDG Oct 10, 2024
f58b098
Add warning on package name mismatch in `get_cargo_library_name`
DelevoXDG Oct 10, 2024
618f064
Update warning message
DelevoXDG Oct 10, 2024
fb62138
Format
DelevoXDG Oct 10, 2024
83b3c09
Merge branch 'zdobnikau/1527-package-cairo-plugins' of https://github…
DelevoXDG Oct 10, 2024
405e48a
Remove unnecessary whitespace in the warning
DelevoXDG Oct 10, 2024
ed39105
Update test to match new warning msg
DelevoXDG Oct 10, 2024
e44a218
Update `shared_lib_path` to return `Result<Utf8PathBuf>`
DelevoXDG Oct 10, 2024
8019bdc
Apply suggestions from code review
maciektr Oct 14, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions scarb/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ walkdir.workspace = true
which.workspace = true
windows-sys.workspace = true
zstd.workspace = true
cargo_metadata.workspace = true

[target.'cfg(not(target_os = "linux"))'.dependencies]
reqwest = { workspace = true, default-features = true }
Expand Down
2 changes: 1 addition & 1 deletion scarb/src/compiler/plugin/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub fn fetch_cairo_plugin(package: &Package, ws: &Workspace<'_>) -> Result<()> {
let props: CairoPluginProps = target.props()?;
// No need to fetch for buildin plugins.
if !props.builtin {
proc_macro::fetch_package(package, ws)?;
proc_macro::fetch_crate(package, ws)?;
}
Ok(())
}
Expand Down
132 changes: 122 additions & 10 deletions scarb/src/compiler/plugin/proc_macro/compilation.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
use crate::compiler::ProcMacroCompilationUnit;
use crate::core::{Config, Package, Workspace};
use crate::flock::Filesystem;
use crate::ops::PackageOpts;
use crate::process::exec_piping;
use anyhow::Result;
use crate::CARGO_MANIFEST_FILE_NAME;
use anyhow::{anyhow, Context, Result};
use camino::Utf8PathBuf;
use cargo_metadata::MetadataCommand;
use indoc::formatdoc;
use libloading::library_filename;
use ra_ap_toolchain::Tool;
use scarb_ui::{Message, OutputFormat};
use serde::{Serialize, Serializer};
use serde_json::value::RawValue;
use std::fmt::Display;
use std::fs;
use std::process::Command;
use tracing::trace_span;

Expand All @@ -20,7 +25,7 @@ pub trait SharedLibraryProvider {
/// Location of Cargo `target` directory.
fn target_path(&self, config: &Config) -> Filesystem;
/// Location of the shared library for the package.
fn shared_lib_path(&self, config: &Config) -> Utf8PathBuf;
fn shared_lib_path(&self, config: &Config) -> Result<Utf8PathBuf>;
}

impl SharedLibraryProvider for Package {
Expand All @@ -36,17 +41,20 @@ impl SharedLibraryProvider for Package {
.into_child("target")
}

fn shared_lib_path(&self, config: &Config) -> Utf8PathBuf {
let lib_name = library_filename(self.id.name.to_string());
fn shared_lib_path(&self, config: &Config) -> Result<Utf8PathBuf> {
let lib_name =
get_cargo_library_name(self, config).context("could not resolve library name")?;
let lib_name = library_filename(lib_name);
let lib_name = lib_name
.into_string()
.expect("library name must be valid UTF-8");
// Defines the shared library path inside the target directory, as:
// `/(..)/target/release/[lib]<package_name>.[so|dll|dylib]`
self.target_path(config)
Ok(self
.target_path(config)
.into_child(PROC_MACRO_BUILD_PROFILE)
.path_unchecked()
.join(lib_name)
.join(lib_name))
}
}

Expand All @@ -60,10 +68,96 @@ pub fn check_unit(unit: ProcMacroCompilationUnit, ws: &Workspace<'_>) -> Result<
run_cargo(CargoAction::Check, &package, ws)
}

pub fn fetch_package(package: &Package, ws: &Workspace<'_>) -> Result<()> {
fn get_cargo_package_name(package: &Package) -> Result<String> {
let cargo_toml_path = package.root().join(CARGO_MANIFEST_FILE_NAME);

let cargo_toml: toml::Value = toml::from_str(
&fs::read_to_string(cargo_toml_path).context("could not read `Cargo.toml`")?,
)
.context("could not convert `Cargo.toml` to toml")?;

let package_section = cargo_toml
.get("package")
.ok_or_else(|| anyhow!("could not get `package` section from Cargo.toml"))?;

let package_name = package_section
.get("name")
.ok_or_else(|| anyhow!("could not get `name` field from Cargo.toml"))?
.as_str()
.ok_or_else(|| anyhow!("could not convert package name to string"))?;

Ok(package_name.to_string())
}

fn get_cargo_library_name(package: &Package, config: &Config) -> Result<String> {
let metadata = MetadataCommand::new()
.cargo_path(Tool::Cargo.path())
.current_dir(package.root())
.exec()
.context("could not get Cargo metadata")?;

let cargo_package_name = get_cargo_package_name(package)?;

if cargo_package_name != package.id.name.to_string() {
config.ui().warn(formatdoc!(
r#"
package name differs between Cargo and Scarb manifest
cargo: `{cargo_name}`, scarb: `{scarb_name}`
this might become an error in future Scarb releases
"#,
cargo_name = cargo_package_name,
scarb_name = package.id.name,
));
}

let package = metadata
.packages
.iter()
.find(|pkg| pkg.name == cargo_package_name)
.ok_or_else(|| anyhow!("could not get `{cargo_package_name}` package from metadata"))?;

let cdylib_target = package
.targets
.iter()
.find(|target| target.kind.contains(&"cdylib".to_string()))
.ok_or_else(|| anyhow!("no target of `cdylib` kind found in package"))?;

Ok(cdylib_target.name.clone())
}

fn get_cargo_package_version(package: &Package) -> Result<String> {
let metadata = MetadataCommand::new()
.cargo_path(Tool::Cargo.path())
.current_dir(package.root())
.exec()
.context("could not get Cargo metadata")?;

let cargo_package_name = get_cargo_package_name(package)?;

let package = metadata
.packages
.iter()
.find(|pkg| pkg.name == cargo_package_name)
.ok_or_else(|| anyhow!("could not get `{cargo_package_name}` package from metadata"))?;

Ok(package.version.to_string())
}

pub fn get_crate_archive_basename(package: &Package) -> Result<String> {
let package_name = get_cargo_package_name(package)?;
let package_version = get_cargo_package_version(package)?;

Ok(format!("{}-{}", package_name, package_version))
}

pub fn fetch_crate(package: &Package, ws: &Workspace<'_>) -> Result<()> {
run_cargo(CargoAction::Fetch, package, ws)
}

pub fn package_crate(package: &Package, opts: &PackageOpts, ws: &Workspace<'_>) -> Result<()> {
run_cargo(CargoAction::Package(opts.clone()), package, ws)
}

fn run_cargo(action: CargoAction, package: &Package, ws: &Workspace<'_>) -> Result<()> {
let cmd = CargoCommand {
action,
Expand All @@ -73,6 +167,7 @@ fn run_cargo(action: CargoAction, package: &Package, ws: &Workspace<'_>) -> Resu
.target_path(ws.config())
.path_unchecked()
.to_path_buf(),
config: ws.config(),
};
{
let _ = trace_span!("proc_macro").enter();
Expand All @@ -81,17 +176,20 @@ fn run_cargo(action: CargoAction, package: &Package, ws: &Workspace<'_>) -> Resu
Ok(())
}

#[derive(Clone)]
enum CargoAction {
Build,
Check,
Fetch,
Package(PackageOpts),
}

struct CargoCommand {
struct CargoCommand<'c> {
current_dir: Utf8PathBuf,
target_dir: Utf8PathBuf,
output_format: OutputFormat,
action: CargoAction,
config: &'c Config,
}

enum CargoOutputFormat {
Expand All @@ -117,17 +215,31 @@ impl From<OutputFormat> for CargoOutputFormat {
}
}

impl From<CargoCommand> for Command {
fn from(args: CargoCommand) -> Self {
impl<'c> From<CargoCommand<'c>> for Command {
fn from(args: CargoCommand<'c>) -> Self {
let mut cmd = Command::new(Tool::Cargo.path());
cmd.current_dir(args.current_dir);
match args.action {
CargoAction::Fetch => cmd.arg("fetch"),
CargoAction::Build => cmd.arg("build"),
CargoAction::Check => cmd.arg("check"),
CargoAction::Package(_) => cmd.arg("package"),
};
if args.config.offline() {
cmd.arg("--offline");
}
DelevoXDG marked this conversation as resolved.
Show resolved Hide resolved
match args.action {
CargoAction::Fetch => (),
CargoAction::Package(ref opts) => {
cmd.arg("--target-dir");
cmd.arg(args.target_dir);
if !opts.check_metadata {
cmd.arg("--no-metadata");
}
if opts.allow_dirty {
cmd.arg("--allow-dirty");
}
}
_ => {
cmd.arg("--release");
cmd.arg("--message-format");
Expand Down
4 changes: 3 additions & 1 deletion scarb/src/compiler/plugin/proc_macro/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ impl Debug for ProcMacroInstance {
impl ProcMacroInstance {
/// Load shared library
pub fn try_new(package: Package, config: &Config) -> Result<Self> {
let lib_path = package.shared_lib_path(config);
let lib_path = package
.shared_lib_path(config)
.context("could not resolve shared library path")?;
let plugin = unsafe { Plugin::try_new(lib_path.to_path_buf())? };
Ok(Self {
expansions: unsafe { Self::load_expansions(&plugin, package.id)? },
Expand Down
2 changes: 1 addition & 1 deletion scarb/src/compiler/plugin/proc_macro/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ pub mod compilation;
mod ffi;
mod host;

pub use compilation::{check_unit, compile_unit, fetch_package};
pub use compilation::{check_unit, compile_unit, fetch_crate};
pub use ffi::*;
pub use host::*;
8 changes: 7 additions & 1 deletion scarb/src/core/manifest/toml_manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub struct TomlManifest {
pub dependencies: Option<BTreeMap<PackageName, MaybeTomlWorkspaceDependency>>,
pub dev_dependencies: Option<BTreeMap<PackageName, MaybeTomlWorkspaceDependency>>,
pub lib: Option<TomlTarget<TomlLibTargetParams>>,
pub cairo_plugin: Option<TomlTarget<TomlExternalTargetParams>>,
pub cairo_plugin: Option<TomlTarget<TomlCairoPluginTargetParams>>,
pub test: Option<Vec<TomlTarget<TomlExternalTargetParams>>>,
pub target: Option<BTreeMap<TargetKind, Vec<TomlTarget<TomlExternalTargetParams>>>>,
pub cairo: Option<TomlCairo>,
Expand Down Expand Up @@ -294,6 +294,12 @@ pub struct TomlLibTargetParams {
pub sierra_text: Option<bool>,
}

#[derive(Debug, Default, Deserialize, Serialize)]
#[serde(rename_all = "kebab-case")]
pub struct TomlCairoPluginTargetParams {
pub builtin: Option<bool>,
}

maciektr marked this conversation as resolved.
Show resolved Hide resolved
pub type TomlExternalTargetParams = BTreeMap<SmolStr, toml::Value>;

#[derive(Debug, Default, Deserialize, Serialize, Clone)]
Expand Down
19 changes: 15 additions & 4 deletions scarb/src/core/publishing/manifest_normalization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use anyhow::{bail, Result};
use camino::Utf8PathBuf;
use indoc::formatdoc;

use crate::core::{TomlCairoPluginTargetParams, TomlTarget};
use crate::{
core::{
DepKind, DependencyVersionReq, DetailedTomlDependency, ManifestDependency, MaybeWorkspace,
Expand All @@ -29,10 +30,7 @@ pub fn prepare_manifest_for_publish(pkg: &Package) -> Result<TomlManifest> {
.collect()
});

let cairo_plugin = match pkg.target(&TargetKind::CAIRO_PLUGIN) {
None => None,
Some(_) => todo!("Packaging Cairo plugins is not implemented yet."),
};
let cairo_plugin = generate_cairo_plugin(pkg);

Ok(TomlManifest {
package,
Expand Down Expand Up @@ -146,3 +144,16 @@ fn generate_dependency(dep: &ManifestDependency) -> Result<TomlDependency> {
},
})))
}

fn generate_cairo_plugin(pkg: &Package) -> Option<TomlTarget<TomlCairoPluginTargetParams>> {
let target = pkg.target(&TargetKind::CAIRO_PLUGIN)?;
let params = target.props::<TomlCairoPluginTargetParams>().ok()?;

Some(TomlTarget {
name: Some(target.name.clone()),
source_path: None,
params: TomlCairoPluginTargetParams {
builtin: params.builtin.and_then(|b| b.then_some(true)),
},
maciektr marked this conversation as resolved.
Show resolved Hide resolved
})
}
13 changes: 10 additions & 3 deletions scarb/src/core/publishing/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ use ignore::{DirEntry, WalkBuilder};

use crate::core::Package;
use crate::internal::fsx::PathBufUtf8Ext;
use crate::{DEFAULT_TARGET_DIR_NAME, LOCK_FILE_NAME, MANIFEST_FILE_NAME, SCARB_IGNORE_FILE_NAME};
use crate::{
CARGO_LOCK_FILE_NAME, CARGO_MANIFEST_FILE_NAME, DEFAULT_TARGET_DIR_NAME, LOCK_FILE_NAME,
MANIFEST_FILE_NAME, SCARB_IGNORE_FILE_NAME,
};

/// List all files relevant to building this package inside this source.
///
Expand Down Expand Up @@ -52,11 +55,15 @@ fn push_worktree_files(pkg: &Package, ret: &mut Vec<Utf8PathBuf>) -> Result<()>
return false;
}

// Skip `Scarb.toml`, `Scarb.lock` and `target` directory.
// Skip `Scarb.toml`, `Scarb.lock`, 'Cargo.toml`, 'Cargo.lock', and `target` directory.
if entry.depth() == 1
&& ({
let f = entry.file_name();
f == MANIFEST_FILE_NAME || f == LOCK_FILE_NAME || f == DEFAULT_TARGET_DIR_NAME
f == MANIFEST_FILE_NAME
|| f == LOCK_FILE_NAME
|| f == CARGO_MANIFEST_FILE_NAME
|| f == CARGO_LOCK_FILE_NAME
|| f == DEFAULT_TARGET_DIR_NAME
})
{
return false;
Expand Down
2 changes: 2 additions & 0 deletions scarb/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,5 @@ pub const STARKNET_PLUGIN_NAME: &str = "starknet";
pub const TEST_PLUGIN_NAME: &str = "cairo_test";
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";
Loading
Loading