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.

Sign up for GitHub

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

Jump to bottom

Support packaging cairo-plugins #1605

Merged
merged 52 commits into from
Oct 14, 2024
Merged

Support packaging cairo-plugins #1605

merged 52 commits into from
Oct 14, 2024

Conversation

DelevoXDG
Copy link
Contributor

@DelevoXDG DelevoXDG commented Sep 18, 2024 •
edited
Loading

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

@DelevoXDG
Add cargo action for packaging
4569059
@DelevoXDG
Skip Cargo.toml when sourcing files for recipe
058a698
@DelevoXDG
Add conversion from Target to TomlTarget with external params
bcdeaaf
@DelevoXDG
Support cairo plugin in manifest normalization
1ba6c14
@DelevoXDG
Update toml manifest target collection to handle cairo plugin target
b9904ba
@DelevoXDG
Add initial support for packaging plugins
2e8fc39 
- Allow package with `[cairo-plugin]` target to be packaged
- Run `cargo package` to build + normalize `Cargo.toml`
- Add normalized and original `Cargo.toml` to the package archive
- Add `get_cargo_package_name` helper to read package name from `Cargo.toml`
  - TODO: consider sharing duplicated logic in `check_corelib_version`
@DelevoXDG DelevoXDG mentioned this pull request Sep 18, 2024
Closed
@DelevoXDG
Fix no_lib_target test; Rename to no_target
67b5a2a
@DelevoXDG
Move CairoPluginProjectBuilder to utils/scarb-test-support/
343810d
@DelevoXDG
Retrieve name+version from Cargo.toml for path resolution
623357a 
Don't use version as used in Scarb.toml, use the one in Cargo.toml instead to ensure path can be resolved
@DelevoXDG
Allow packaging via Cargo without verification
7d8001c
@DelevoXDG
Format
428b99f
@DelevoXDG
Specify dependency version in CairoPluginProjectBuilder
f1f94d1
@DelevoXDG
Revert "Allow packaging via Cargo without verification"
869a0be 
This reverts commit 7d8001c.
@DelevoXDG
Fix missing whitespace in cairo_plugin_project_builder.rs
fd676e3
@DelevoXDG
Add plugin packaging basic test
9f64c1b
@DelevoXDG
Skip Scarb.lock when packaging
662768c
@DelevoXDG
Pass relevant arguments to Cargo actions
bdcf32b 
- Pass `--offline` for all cmds
- Pass `--no-metadata`, `--verify`, `--allow-dirty` for `package`
@DelevoXDG
Deduplicate --offline flag addition
ade5aab
@DelevoXDG
Move cairo_plugin packaging test
b7cfd68
@DelevoXDG
Allow checking that target and package are builtin
20acc00
@DelevoXDG
Don't cargo package and don't verify builtin plugins
ab9a86d
@DelevoXDG
Add test for packing builtin cairo plugins
be9fd63
@DelevoXDG
Simplify cairo-plugin, lib, and builtin checks
13a0929
@DelevoXDG
Normalize cairo-plugin in a typed manner; Skip builtin = false
656a61a
@DelevoXDG
Remove Target.to_toml_target
5b5941b
@DelevoXDG
Format
8559e17
@DelevoXDG DelevoXDG marked this pull request as ready for review September 25, 2024 15:36
@maciektr maciektr mentioned this pull request Oct 8, 2024
Closed
maciektr
maciektr reviewed Oct 10, 2024
View reviewed changes
Copy link
Contributor

@maciektr maciektr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!
Some comments attached.

scarb/src/compiler/plugin/proc_macro/compilation.rs Outdated Show resolved Hide resolved
scarb/src/compiler/plugin/proc_macro/compilation.rs Outdated Show resolved Hide resolved
scarb/src/compiler/plugin/proc_macro/compilation.rs Outdated Show resolved Hide resolved
scarb/src/ops/package.rs Outdated Show resolved Hide resolved
scarb/src/compiler/plugin/proc_macro/compilation.rs Outdated Show resolved Hide resolved
@franciszekjob franciszekjob mentioned this pull request Oct 11, 2024
Draft
5 tasks
@DelevoXDG DelevoXDG mentioned this pull request Oct 13, 2024
Merged
maciektr
maciektr approved these changes Oct 14, 2024
View reviewed changes
scarb/src/compiler/plugin/proc_macro/compilation.rs Outdated Show resolved Hide resolved
scarb/src/compiler/plugin/proc_macro/compilation.rs Outdated Show resolved Hide resolved
scarb/src/compiler/plugin/proc_macro/compilation.rs Outdated Show resolved Hide resolved
@maciektr maciektr added this pull request to the merge queue Oct 14, 2024
Merged via the queue into main with commit d01ccee Oct 14, 2024
22 checks passed
@maciektr maciektr deleted the zdobnikau/1527-package-cairo-plugins branch October 14, 2024 16:30
github-merge-queue bot pushed a commit that referenced this pull request Oct 16, 2024
@DelevoXDG
Fix packaging plugins when --no-verify is passed (#1643)
0ee3bc5 
Resolves the issue brought up in
#1605 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maciektr maciektr maciektr approved these changes

@wawel37 wawel37 Awaiting requested review from wawel37 wawel37 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support packaging cairo-plugins in Scarb
2 participants
@DelevoXDG @maciektr