-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add CI workflow to publish crates on tag/release #14
Conversation
This commit adds a new github actions work to publish the package's crates to crates.io. This new CI job run cargo publish on the respective crates in the correct order to publish the entire workspace.
Thanks! Also, as discussed. I'll remove the pyo3 related crate and edit this PR. |
See #13 which prepares the packages for publishing. This just is adding the CI jobs to do the publishing when we're ready to tag 0.0.1. |
.github/workflows/release.yml
Outdated
name: Release openqasm3_parser | ||
on: | ||
push: | ||
tags: | ||
- '*' | ||
|
||
jobs: | ||
publish_lexer: | ||
name: Publish oq3_lexer crate | ||
runs-on: ubuntu-latest | ||
strategy: | ||
fail-fast: false | ||
matrix: | ||
crate: [oq3_lexer, oq3_parser, oq3_semantics, oq3_sourcegen, oq3_syntax, oq3_source_file] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name: openqasm3_parser
jobs.publish_lexer.name: oq3_lexer crate
matrix: [...]
three different things haha
steps: | ||
- uses: actions/checkout@v4 | ||
- uses: dtolnay/rust-toolchain@stable | ||
- name: Run cargo publish | ||
run: | | ||
cd crates/${{ matrix.crate }} | ||
cargo publish | ||
env: | ||
CARGO_REGISTRY_TOKEN: ${{ secrets.CARGO_REGISTRY_TOKEN }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do the crates get verified crates.io-side at all? If so, we might have to force the order of them to some topological iteration through them to get them uploaded.
Seems a shame that there's no workspace-wide cargo publish
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh that's a good point, yeah we'll need to do this manually in topological order otherwise cargo's validation will error when the dependencies aren't resolvable with crates.io. You get an error like:
error: failed to prepare local package for uploading
Caused by:
no matching package named `oq3_lexer` found
location searched: registry `crates-io`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is true rust-lang/cargo#1169 (comment) , then there is no problem publishing out of order, as long as the crates have been published once in the correct order (say manually).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a ton of discussion around this and many tools available. This tool looks to me (fwiw) to be the most serious and perhaps relevant.
https://github.com/crate-ci/cargo-release
I originally had prefixed package names in This appears to be what rust analyzer has done. For example the crate in rust analyzer directory And the |
The discussion around the name should probably be in #13 which actually renames things. In general they can be different but I would strongly argue against it. Besides the obvious consistency issues with potentially have 3 different names (crate, module, and directory) for the same thing, the obvious conflict concerns with having such a generic name like |
From the perspective of using it outside a single application (like building on top of it in Qiskit), it's better to have the prefixes, and that aligns closer with how it's actually packaged. For example, see how PyO3 (a library, rather than an application) works and packages itself. Within itself, you can always |
Another thing that's becoming clear. If find it convenient for developing to sometimes organize code on the level of crates. For various reasons. For example, but not only, the code can easily be built and tested separately. However when installing or using crates, it looks like the namespace is flat. So each crate in a workspace has to be installed as a separate crate in If so, its worth considering things like moving the code in |
This is correct. I was busily editing my last comment, then saw that I had two response comments in the mean time. |
Yeah, since we have to hit a flat namespace, the |
We shouldn't have any gratuitous or accidental inconsistency. But it's common in the PL world to have a regular rule for deriving names. Eg. package 'the-package
I don't follow. The flat namespace is present at the level of crates.io ? But it is possible to publish crates under a prefix, for example
Doing Or we can do what r-a does: #![cfg_attr(feature = "in-rust-tree", feature(rustc_private))]
#[cfg(not(feature = "in-rust-tree"))]
extern crate ra_ap_rustc_lexer as rustc_lexer;
#[cfg(feature = "in-rust-tree")]
extern crate rustc_lexer; Beautiful! =( |
@mtreinish and @jakelishman This PR is no longer blocked by anything. I spent an hour or so investigating how to publish, but did not see an obvious best way. Apparently publishing is problematic enough that there are several tools available to assist. Many issues are discussed. For example, after a crate is published, it takes an indeterminate amount of time before it is available to satisfy a dependency of a subsequently published crate. It seems there may be an automatic sleep interval inserted into some publishing tools. And I saw some talk of an adjustable period because no single period was satisfactory. However, someone claimed that versions are not checked when determining if dependencies are satisfied. |
There is specifically an issue with the first publishing of this though. The name needs to exist on |
Right. I meant we can figure out the dependence manually and publish them by hand once. I don't have the secret stuff to publish. I am pretty sure the order is
|
John: yeah, at least for the Qiskit package I have with a direct dependency on
which agrees with the order you said. |
This commit adds a new github actions work to publish the package's crates to crates.io. This new CI job run cargo publish on the respective crates in the correct order to publish the entire workspace.