crate-ci / cargo-release

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

Allow specifying options for `git push`? #704

Closed jyn514 closed 1 year ago

jyn514 commented 1 year ago

Currently, cargo release allows specifying options for the server with push-options, but not options for the client: https://github.com/crate-ci/cargo-release/blob/f2307121c187cab3ed694c4536d11340904907b8/src/ops/git.rs#L223-L228 It would be nice to allow passing flags to the client as well.

My original use-case I am using a separate branch for releases to avoid relaxing the branch protections on `main` (see https://github.com/crate-ci/cargo-release/issues/658#issuecomment-1481479950 for some previous discussion). To avoid having to sync changes between `release` and `main`, I am only pushing the version update commits to `release`, not to `main` (i.e. main always has `version = "dev"` set in Cargo.toml). This *almost* works out-of-the-box with `allow-branch = ["releases"]` set, but not quite. Consider the following sequence: 1. `git checkout release` 2. `git reset --hard origin/main` 3. `cargo release 0.1.0` # this first release works fine ``` [release e7bb54b] chore: Release test-release-process version 0.1.1 1 file changed, 1 insertion(+), 1 deletion(-) Pushing Pushing release, v0.1.1 to origin ``` 5. ... some time passes and commits are made to the main branch ... 6. `git reset --hard origin/main` 7. `cargo release 0.1.1` The *second* release breaks because `cargo release` made a commit bumping to 0.1.1 at the time it created the tag: ``` [release a9d4e73] chore: Release test-release-process version 0.1.2 1 file changed, 1 insertion(+), 1 deletion(-) Pushing Pushing release, v0.1.2 to origin error: atomic push failed for ref refs/heads/release. status: 2 To github.com:jyn514/test-release-process.git ! [rejected] release -> release (non-fast-forward) ! [rejected] v0.1.2 -> v0.1.2 (atomic push failed) error: failed to push some refs to 'github.com:jyn514/test-release-process.git' hint: Updates were rejected because the tip of your current branch is behind hint: its remote counterpart. Integrate the remote changes (e.g. hint: 'git pull ...') before pushing again. ``` If cargo release allowed me to pass custom flags, I could add `--force` which would overwrite `release`.
jyn514 commented 1 year ago

I've since figured out a way to avoid needing this, but it could be useful for other people who want to pass --progress or something like that.

I can avoid needing to pass `--force` by adding a pre-commit hook that deletes the `releases` branch. ``` if ! [ "$DRY_RUN" = false ]; then git push --delete origin release fi ```
epage commented 1 year ago

The idea of allowing people to pass in --force makes me not want to support this feature at all :)

Couldn't this specific use case be covered by doing a merge-commit between the two branches rather than git reset --hard?

jyn514 commented 1 year ago

Well, yes, but then there's a possibility of merge conflicts between the two branches - I don't want people running cargo release on a merge commit with manual changes that never went through CI.

anyway, feel free to close this, the workaround in https://github.com/crate-ci/cargo-release/issues/704#issuecomment-1725837600 fine for my use case.