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

Revert a mistake in v0.22 that prevents cargo publish #1065

Merged
merged 3 commits into from
Jan 11, 2024

Conversation

qinsoon
Copy link
Member

@qinsoon qinsoon commented Jan 11, 2024

#1055 removed the version for mmtk-macros in Cargo.toml. I thought it was fine, as it uses a local path as the dependency. But cargo publish actually requires all the dependencies to specify a version. So cargo publish failed for https://github.com/mmtk/mmtk-core/releases/tag/v0.22.0 in https://github.com/mmtk/mmtk-core/actions/runs/7284255107/job/19849376243. The action did not return a non-zero code so I was not aware of the issue. v0.22 was not published to crates.io.

This PR adds back the version for mmtk-macros, adds a check for cargo publish dry run in the style check, and fixes the publish script so the script properly fails if all the attempts of cargo publish fail.

@qinsoon qinsoon requested a review from wks January 11, 2024 04:31
echo "Wait for Retry #"$n
sleep 60
echo "Attempt #"$n
false && { success=true; break; }
Copy link
Member

Choose a reason for hiding this comment

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

false is probably from testing but it should be cargo publish

Copy link
Member Author

Choose a reason for hiding this comment

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

Right. Sorry for that.

@qinsoon qinsoon added this pull request to the merge queue Jan 11, 2024
Merged via the queue into mmtk:master with commit f0b472c Jan 11, 2024
19 checks passed
@qinsoon qinsoon deleted the add-mmtk-macros-version branch January 11, 2024 06:17
@qinsoon qinsoon mentioned this pull request Jan 11, 2024
github-merge-queue bot pushed a commit that referenced this pull request Jan 11, 2024
Bumps the version to v0.22.1 for
#1065.
qinsoon added a commit that referenced this pull request Jan 12, 2024
#1055 removed the version for
`mmtk-macros` in `Cargo.toml`. I thought it was fine, as it uses a local
path as the dependency. But `cargo publish` actually requires all the
dependencies to specify a version. So `cargo publish` failed for
https://github.com/mmtk/mmtk-core/releases/tag/v0.22.0 in
https://github.com/mmtk/mmtk-core/actions/runs/7284255107/job/19849376243.
The action did not return a non-zero code so I was not aware of the
issue. `v0.22` was not published to `crates.io`.

This PR adds back the version for `mmtk-macros`, adds a check for `cargo
publish` dry run in the style check, and fixes the publish script so the
script properly fails if all the attempts of `cargo publish` fail.
qinsoon added a commit that referenced this pull request Jan 12, 2024
Bumps the version to v0.22.1 for
#1065.
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.

None yet

2 participants