Skip to content

Commit

Permalink
Support packaging cairo-plugins (#1605)
Browse files Browse the repository at this point in the history
## Changes
- Allow packaging cairo-plugins
- Run `cargo package` to normalize `Cargo.toml` and verify rust code for
proc macros
  - Skip `cargo package` and verification for builtin plugins
- Pass relevant configuration options from `scarb package` to `cargo
package`
- Update manifest normalization to include normalized `[cairo-plugin]`
section
- Fix `shared_lib_path` resolution to support packages mismatched scarb
/ cargo name and version

## Related issues
Closes #1527

---------

Signed-off-by: Maksim Zdobnikau <[email protected]>
Signed-off-by: maciektr <[email protected]>
Co-authored-by: maciektr <[email protected]>
  • Loading branch information
DelevoXDG and maciektr authored Oct 14, 2024
1 parent 302aaf4 commit d01ccee
Show file tree
Hide file tree
Showing 15 changed files with 506 additions and 154 deletions.
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");
}
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>,
}

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)),
},
})
}
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

0 comments on commit d01ccee

Please sign in to comment.