crate-ci / cargo-release

Cargo subcommand `release`: everything about releasing a rust crate.
Apache License 2.0
1.33k stars 112 forks source link

`pre-release-commit-message` does not replace `{{version}}` despite `consolidate-commits = false` #680

Closed LeoniePhiline closed 1 year ago

LeoniePhiline commented 1 year ago

I have set consolidate-commits = false.

Yet still cargo release commit -x created a commit with the following message:

release: {{version}}

Instead of

release: 0.3.0

Single-package project. No workspace.

Am I doing something wrong or might this be a bug?

Config:

allow-branch = ["main"]
consolidate-commits = false
pre-release-commit-message = "release: {{version}}"

Using cargo-release 0.24.10

epage commented 1 year ago

Sorry for the delay

Do you have a public repo I can reproduce this on? I want to make sure I am reproducing what you are seeing rather than my guess.

LeoniePhiline commented 1 year ago

Hi! Thank you for your reply! :)

Yes, here's once with workspace and once without.

Using: cargo-release 0.24.10

Without workspace

Steps to reproduce

cd /tmp
git clone git@github.com:LeoniePhiline/basispoort-sync-client.git
cd basispoort-sync-client
cargo release version patch --execute
cargo release commit --execute

Expected result

One commit reading release: 0.5.2

Actual result

One commit reading release: {{version}}

Configuration

Configuration

allow-branch = ["main"]
consolidate-commits = false
pre-release-replacements = [
  {file="README.md", search="basispoort-sync-client = .*", replace="{{crate_name}} = \"{{version}}\""},
  # {file="src/lib.rs", search="basispoort-sync-client = .*", replace="{{crate_name}} = \"{{version}}\""},
  {file="CHANGELOG.md", search="Unreleased", replace="{{version}}"},
  {file="CHANGELOG.md", search="\\.\\.\\.HEAD", replace="...{{tag_name}}", exactly=1},
  {file="CHANGELOG.md", search="<!-- release-date -->", replace="- {{date}}"},
  {file="CHANGELOG.md", search="<!-- next-header -->", replace="<!-- next-header -->\n\n## [Unreleased] <!-- release-date -->", exactly=1},
  {file="CHANGELOG.md", search="<!-- next-url -->", replace="<!-- next-url -->\n[Unreleased]: https://github.com/LeoniePhiline/basispoort-sync-client/compare/{{tag_name}}...HEAD", exactly=1},
]
pre-release-commit-message = "release: {{version}}"
pre-release-hook = ["cargo", "test"]

With workspace

Steps to reproduce

cd /tmp
git clone git@github.com:LeoniePhiline/async-mailer.git
cd async-mailer
cargo release version --workspace patch --execute
cargo release commit

Expected result

Four commits:

Actual result

A single commit:

Configuration

Configuration

allow-branch = ["main"]
consolidate-commits = false
pre-release-replacements = [
  {file="README.md", search="async-mailer = .*", replace="{{crate_name}} = \"{{version}}\""},
  {file="src/lib.rs", search="async-mailer = .*", replace="{{crate_name}} = \"{{version}}\""},
  {file="CHANGELOG.md", search="Unreleased", replace="{{version}}"},
  {file="CHANGELOG.md", search="\\.\\.\\.HEAD", replace="...{{tag_name}}", exactly=1},
  {file="CHANGELOG.md", search="<!-- release-date -->", replace="- {{date}}"},
  {file="CHANGELOG.md", search="<!-- next-header -->", replace="<!-- next-header -->\n\n## [Unreleased] <!-- release-date -->", exactly=1},
  {file="CHANGELOG.md", search="<!-- next-url -->", replace="<!-- next-url -->\n[Unreleased]: https://github.com/LeoniePhiline/async-mailer/compare/{{tag_name}}...HEAD", exactly=1},
]
pre-release-commit-message = "release: {{crate_name}} {{version}}"
pre-release-hook = ["cargo", "test"]
epage commented 1 year ago

cargo release commit can only do consolidated commits, so this is expected behavior, especially with a workspace.

I have #689 which makes this work for non-workspaces and adds a warning for the workspace case.

LeoniePhiline commented 1 year ago

Thank you for your response!

I must be severely misunderstanding something. If cargo release commit can only do consolidated commits, then what is the consolidate-commits option used for?

Or in other words:

epage commented 1 year ago

cargo release commit gives you access to just the commit step of the release process right now. The problem is we commit the whole repo for each commit because we do not know what other files parts of the release process might have touched (and for lockfiles, it changes on every package version bump). If we did this without a consolidated commit, the first package would commit everything and subsequent commits would have nothing to commit.

This is different than cargo release which can do a single commit per workspace or individual commits per package as it bubbles up the stack, going through the release process for each package. I ran cargo release --workspace -vv and it showed that the commits that would happen would not be consolidated and showed the proper commit messages.

LeoniePhiline commented 1 year ago

Thank you!

I falsely assumed I could carefully run the release process step by step manually (to validate the intermediate steps before moving on), with the same result as if I had run cargo release.

I get good results with cargo release -vv patch -x and cargo release -vv --workspace patch -x. Thank you for the clarification and the UX improvement at #689!

I had been confused by the commit step not accepting a --package option. Do you think it would make sense for cargo release commit to take a package spec (like cargo release {version,replace,hook,publish,owner,tag,push} -p pkgid do)?

This way, one could still run individual steps for individual workspace packages.

epage commented 1 year ago

I had been confused by the commit step not accepting a --package option. Do you think it would make sense for cargo release commit to take a package spec (like cargo release {version,replace,hook,publish,owner,tag,push} -p pkgid do)?

This way, one could still run individual steps for individual workspace packages.

I might be able to be convinced but I feel like if the others have more legitimate use cases for doing individually but when you get to committing, its easier to make a mistake and if you really are stepping through the whole process on a per-package basis in a workspace then that is likely complicated enough to not support.

LeoniePhiline commented 1 year ago

I understand. Thank you so much for your time and effort, here in this crate and issue, and in the general ecosystem!

I will close this issue - there is no need to convince you to do even more work. :)