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

--bumped-version fails if commit is skipped by commit_parsers #816

Open
colinmarc opened this issue Aug 25, 2024 · 17 comments
Open

--bumped-version fails if commit is skipped by commit_parsers #816

colinmarc opened this issue Aug 25, 2024 · 17 comments
Labels
bug Something isn't working

Comments

@colinmarc
Copy link

colinmarc commented Aug 25, 2024

I use git-cliff both for generating changelogs and detecting the next version, with --bumped-version. Here is my config:

[changelog]
body = """
{% if version %}\
    ## [{{ version | trim_start_matches(pat="v") }}] - {{ timestamp | date(format="%Y-%m-%d") }}
{% else %}\
    ## [unreleased]
{% endif %}\
{% for group, commits in commits | group_by(attribute="group") %}
    ### {{ group | striptags | trim | upper_first }}
    {% for commit in commits %}
	{% if commit.breaking %}[**breaking**] {% endif %}\
          - {{ commit.message | upper_first }} \
	({{ commit.id }})\
    {% endfor %}
{% endfor %}\n
"""

[git]
commit_parsers = [
  { message = "^feat", group = "<!-- 0 -->New Features" },
  { message = "^fix", group = "<!-- 1 -->Bugfixes" },
  { message = "^doc", skip = true },
  { message = "^perf", skip = true },
  { message = "^refactor", skip = true },
  { message = "^style", skip = true },
  { message = "^test", skip = true },
  { message = "^chore\\(release\\): prepare for", skip = true },
  { message = "^chore\\(deps.*\\)", skip = true },
  { message = "^chore\\(pr\\)", skip = true },
  { message = "^chore\\(pull\\)", skip = true },
  { message = "^chore|^ci", skip = true },
  { message = "build", skip = true },
  { body = ".*security", skip = true },
  { message = "^revert", skip = true },
]

[bump]
features_always_bump_minor = true
breaking_always_bump_major = false
% git cliff --version
git-cliff 2.5.0

Unfortunately, setting skip = true on build: commits seems to cause --bumped-version --unreleased to fail if the latest tag points to a build commit, presumably because it's skipping over that commit.

For example, on d888ab of https://github.com/colinmarc/magic-mirror:

% git show --oneline --quiet mmserver-v0.4.1
3a0d796 (tag: mmserver-v0.4.1) build: fix typo in release workflow
% git cliff --bumped-version --tag-pattern "server" --unreleased
 WARN  git_cliff > "cliff.toml" is not found, using the default configuration.
mmserver-v0.4.2 # this is correct
% git cliff --bumped-version --tag-pattern "server" --unreleased -c .github/workflows/cliff.toml
 WARN  git_cliff_core::config > No releases found, using 0.1.0 as the next version.
0.1.0

I'm not sure what the right behavior is, here. Ideally, I'd like to skip those commits for the purpose of determining whether the version should be bumped, but not fail if there are legitimate changes since a tagged build: version.

In other words, on the following history:

build: foo 
build: bar <- v1.2.3
build: baz

Then I would like --bumped-version --unreleased to return an empty string, as it currently does if the tag points to a non-skipped commit, like so:

build: foo
fix: blah blah blah <-v1.2.3
build: bar
Copy link

welcome bot commented Aug 25, 2024

Thanks for opening your first issue at git-cliff! Be sure to follow the issue template! ⛰️

@orhun orhun added the bug Something isn't working label Aug 26, 2024
@orhun
Copy link
Owner

orhun commented Aug 26, 2024

Hello 👋🏼 thanks for the issue,

I reproduced this with a minimal example and it seems like the correct behavior. However, I agree that the warning message is a bit misleading in this case.

I'm not sure what the right behavior is, here. Ideally, I'd like to skip those commits for the purpose of determining whether the version should be bumped, but not fail if there are legitimate changes since a tagged build: version.

Can you expand this a bit? I would like to better understand your use case to find a middleground here and potentially change the behavior.

Are you using --bumped-version as a mechanism to determine if the project needs a bump? If so, how? 🙂

@colinmarc
Copy link
Author

Are you using --bumped-version as a mechanism to determine if the project needs a bump? If so, how? 🙂

Yes, and I thought that was intended as a feature :)

Here is the github actions workflow I use to create an automated "bump the version" PR, e.g. this one: colinmarc/magic-mirror#12

I'm using --bumped-version to determine the new version. If there were no unreleased commits (that were not skipped), it exits without printing a version. That's the behavior I was relying on - I skip the rest of the job if the output of git cliff is empty.

@orhun
Copy link
Owner

orhun commented Aug 30, 2024

Thanks for sharing your use case, it's all clear now 🙂

If there were no unreleased commits (that were not skipped), it exits without printing a version.

I see why that happens:

return Ok(());

Maybe we can change this behavior a bit and use exit codes instead - that would make the integration easier I reckon. This way we also don't need to change the output and just get away with a better warning message.

Needs a bit more thought maybe. What do you think?

@colinmarc
Copy link
Author

While a different exit code would be more explicit, it wouldn't actually solve this issue. --bumped-version is less useful if it ignores the last version just because the last tag points to a skipped commit.

Said another way, I would like this sequence of commits to report a (correct) bump:

build: foo
build: bar <- v1.2.3
fix: baz

I would like --bumped-version to report v1.2.4 here, because of the fix. Currently it returns 0.1.0, since the last version is a skipped commit, if I understand what's happening correctly.

@orhun
Copy link
Owner

orhun commented Sep 15, 2024

Yeah, that sounds correct. However, not sure if that's the same as the first issue you've described.

I tried to reproduce with the following configuration and commit history:

# cliff.toml

[git]
# parse the commits based on https://www.conventionalcommits.org
conventional_commits = true
# filter out the commits that are not conventional
filter_unconventional = true
# regex for parsing and grouping commits
commit_parsers = [
  { message = "build", skip = true },
]
git init
git commit --allow-empty -m "build: foo"
git commit --allow-empty -m "build: bar"
git tag v1.2.3
git commit --allow-empty -m "fix: baz"
* 06d3252 (HEAD -> master) fix: baz
* 42814ac (tag: v1.2.3) build: bar
* 2c3b608 build: foo

And it works fine:

$ git cliff --bumped-version
v1.2.4

Not sure if I'm missing something here, can you take a look? 👀

@colinmarc
Copy link
Author

colinmarc commented Sep 15, 2024

I took a fresh look. I think you're right, and I got confused in my last message. The issue is indeed being able to reliably detect whether a version bump is necessary at all, and exit codes would help.

Details:

/tmp % git clone git@github.com:colinmarc/magic-mirror
Cloning into 'magic-mirror'...
/tmp % cd magic-mirror && git checkout d888ab
HEAD is now at d888ab6 build: pass GITHUB_TOKEN to git cliff

At that commit, the last version of the server was 0.4.1, and there have only been build commits since then:

[HEAD] /tmp/magic-mirror % git log --oneline --quiet mmserver-v0.4.1...HEAD
d888ab6 (HEAD) build: pass GITHUB_TOKEN to git cliff
6cd7928 build: fix tags in bump-version job
212a55b build: fix yaml typo
3a0d796 (tag: mmserver-v0.4.1) build: fix typo in release workflo

But I get 0.1.0 which is difficult to deal with:

[HEAD] /tmp/magic-mirror % git cliff --bumped-version --tag-pattern "server" -c .github/workflows/cliff.toml
mmserver-v0.4.1
[HEAD] /tmp/magic-mirror % git cliff --bumped-version --tag-pattern "server" --unreleased -c .github/workflows/cliff.toml
 WARN  git_cliff_core::config > No releases found, using 0.1.0 as the next version.
0.1.0

I'm a little bit confused about whether the --unreleased is necessary, but both answers are "incorrect".

If I add a fix commit, it starts working as expected:

[HEAD] /tmp/magic-mirror % git commit --allow-empty -m "fix(server): foo bar"
[HEAD] /tmp/magic-mirror % git cliff --bumped-version --tag-pattern "server" -c .github/workflows/cliff.toml
mmserver-v0.4.1
[HEAD] /tmp/magic-mirror % git cliff --bumped-version --tag-pattern "server" --unreleased -c .github/workflows/cliff.toml
mmserver-v0.4.2

I actually can't figure out how to get it to exit without printing anything now 😓 . I was using that behavior at some point, though.

So, long story short, yes, exit codes would solve my problem.

@colinmarc
Copy link
Author

And it works fine:

$ git cliff --bumped-version
v1.2.4

By the way, not sure if this is a separate issue, but I need to use --unreleased to get it to do that. Maybe because of the combination with --tag-pattern?

@orhun
Copy link
Owner

orhun commented Sep 21, 2024

Let's take a step back and try to identify the issues that we are having first. I feel like we are describing a new/different issue with each comment.


1.

git init
git commit --allow-empty -m "build: foo"
git commit --allow-empty -m "build: bar"
git tag v1.2.3
git commit --allow-empty -m "fix: baz"
git cliff --bumped-version # v1.2.4 ✅
git cliff -u --bumped-version # v1.2.4 ✅

2.

git init
git commit --allow-empty -m "build: foo"
git commit --allow-empty -m "build: bar"
git tag v1.2.3
git commit --allow-empty -m "build: baz"
[git]
commit_parsers = [
  { message = "build", skip = true }
]
git cliff --bumped-version # v0.1.0 ❌
git cliff -u --bumped-version # v0.1.0 ❌

3.

git init
git commit --allow-empty -m "build: foo"
git commit --allow-empty -m "fix: bar"
git tag v1.2.3
git commit --allow-empty -m "build: baz"
[git]
commit_parsers = [
  { message = "build", skip = true }
]
git cliff --bumped-version # v1.2.3 🤔 (there is nothing to bump)
git cliff -u --bumped-version # v0.1.0 ❌

4.

git init
git commit --allow-empty -m "build: foo"
git commit --allow-empty -m "fix: bar"
git tag v1.2.3
git cliff --bumped-version # v1.2.3 🤔 (there is nothing to bump)
git cliff -u --bumped-version # v0.1.0 ❌

And yes, I also can't figure out how to get it to exit without printing anything...

Can you take a look again and help me with that and also identify which one you are currently hitting?

@colinmarc
Copy link
Author

colinmarc commented Sep 21, 2024

Thanks for all your time and attention on this, and sorry it's so convoluted.

The issue at hash d888ab in my repo most closely corresponds to 2. in the list above. However, I'm not getting the same result.

Just to repeat the git history at that point:

% git log --oneline --quiet mmserver-v0.4.1...HEAD
d888ab6 (HEAD) build: ...
6cd7928 build: ...
212a55b build: ...
3a0d796 (tag: mmserver-v0.4.1) build: ...

And the output:

% git cliff --bumped-version --tag-pattern "server" -c .github/workflows/cliff.toml # mmserver-v0.4.1 🤔 (there is nothing to bump)
% git cliff --bumped-version --tag-pattern "server" --unreleased -c .github/workflows/cliff.toml # 0.1.0 ❌

In your version 2, you get 0.1.0 for both, whereas I get 🤔 without --unreleased. The only difference I see is that I'm using --tag-pattern, since there are two release timelines in one repo. However, this really seems like a second issue and maybe even a distraction. Fixing the cases where git-cliff returns 0.1.0 would make my workflow function again.

@orhun
Copy link
Owner

orhun commented Sep 21, 2024

Yeah I agree, there might be other things that are affecting the output here. We can come back to those details later on.

Fixing the cases where git-cliff returns 0.1.0 would make my workflow function again.

So should we return something else or nothing in that case? Also using exit codes would be nice I guess.

@colinmarc
Copy link
Author

So should we return something else or nothing in that case? Also using exit codes would be nice I guess.

I think the two best options (other than exit codes) are either the empty string or the current latest version.

The empty string is reasonably communicative, especially if it's accompanied by a WARN: no matched commits would cause a version bump or a similar message on stderr. Printing the current version is slightly harder to handle in a script, but maybe better in a vacuum (it means the tool always prints a version).

@orhun
Copy link
Owner

orhun commented Sep 21, 2024

Sounds reasonable.

I just realized something, you can actually achieve this when #859 goes out, you simply need to add [changelog].always_render = true to your configuration.

So 2 becomes:

git init
git commit --allow-empty -m "build: foo"
git commit --allow-empty -m "build: bar"
git tag v1.2.3
git commit --allow-empty -m "build: baz"
[git]
commit_parsers = [
  { message = "build", skip = true }
]

[changelog]
always_render = true
git cliff --bumped-version # v1.2.3 🤔 (there is nothing to bump)
git cliff -u --bumped-version # v1.2.3 ✅

Can you test it out? It works because always_render always adds an empty release to the release list.

Not sure if that solution fits the description of this issue but... yeah.

@colinmarc
Copy link
Author

Yeah, that works!

[HEAD *] dev/magic-mirror % git cliff --bumped-version --tag-pattern "server" -c .github/workflows/cliff.toml
mmserver-v0.4.1
[HEAD *] dev/magic-mirror % git cliff --bumped-version --tag-pattern "server" -u -c .github/workflows/cliff.toml
mmserver-v0.4.1

@orhun
Copy link
Owner

orhun commented Sep 21, 2024

Awesome! Do you think the name of the option is intuitive? I think I'm going to yolo that for the next release :)

@orhun
Copy link
Owner

orhun commented Sep 21, 2024

Btw I renamed it to render_always

@colinmarc
Copy link
Author

Hm, I guess from my perspective it's changing something unrelated to rendering, but given that the main reason git cliff exists is to render changelogs, it's not a bad name :)

If I was naming it, I'd probably go with always_render_unreleased, but that's pure bikeshedding.

colinmarc added a commit to colinmarc/magic-mirror that referenced this issue Oct 15, 2024
colinmarc added a commit to colinmarc/magic-mirror that referenced this issue Oct 21, 2024
colinmarc added a commit to colinmarc/magic-mirror that referenced this issue Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants