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

Fix martin-ui crate packaging #1668

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix martin-ui crate packaging #1668

wants to merge 1 commit into from

Conversation

nyurik
Copy link
Member

@nyurik nyurik commented Jan 27, 2025

STATUS: this PR can use some help from other folks - it deals with NPM integration, which turns out is not working as intended... :(

The martin-ui dir must be part of the martin package, otherwise it does not get included in the cargo publish -p martin. We could also do it with a symlink from inside the /martin dir, but that gets a bit confusing - easier to just keep ui inside martin itself, as it is not used anywhere outside of that crate.

In order to prevent these bugs in the future, we might want to run cargo +nightly publish --workspace -Z package-workspace --dry-run as part of some CI -- this will take into account that some crates depend on the other, and that some/all of them have not been published yet

TODO

  • modify martin/build.rs to install node files inside the standard build dir. Current dry run shows this error:
error: failed to verify package tarball

Caused by:
  Source directory was modified by build.rs during cargo publish. Build scripts should not modify anything outside of OUT_DIR.
  Added: <...>/martin/target/package/martin-0.15.0/martin-ui/dist
  	<...>/martin/target/package/martin-0.15.0/martin-ui/dist/_
  	<...>/martin/target/package/martin-0.15.0/martin-ui/dist/_/assets
  	<...>/martin/target/package/martin-0.15.0/martin-ui/dist/_/assets/favicon.ico
  	<...>/martin/target/package/martin-0.15.0/martin-ui/dist/_/assets/index-Cu-91x5d.js
  	<...>/martin/target/package/martin-0.15.0/martin-ui/dist/_/assets/index-DZmjP79K.css
  	<...>/martin/target/package/martin-0.15.0/martin-ui/dist/_/assets/logo-BlOKpv3a.png
  	<...>/martin/target/package/martin-0.15.0/martin-ui/dist/index.html
  	<...>/martin/target/package/martin-0.15.0/martin-ui/node_modules
  	           (and many files in this dir)

  To proceed despite this, pass the `--no-verify` flag.

I am not sure we should use --no-verify

fixes #1667

The martin-ui dir must be part of the martin package, otherwise it does not get included in the `cargo publish -p martin`.  We could also do it with a symlink from inside the `/martin` dir, but that gets a bit confusing - easier to just keep ui inside martin itself, as it is not used anywhere outside of that crate.
@nyurik
Copy link
Member Author

nyurik commented Jan 27, 2025

cc: @paigewilliams - do you know how to best solve the TODO?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cargo publish -p martin does not work due to Node.js package issue
1 participant