git-cliff icon indicating copy to clipboard operation
git-cliff copied to clipboard

Version tags don't get sorted topologically?

Open dpad opened this issue 9 months ago • 5 comments

Is there an existing issue for this?

  • [x] I have searched the existing issues

Description of the bug

Sorry if this is just a mistake, I'm not sure if I'm just misunderstanding things or if there is some weird behaviour with the "topo_order" option...

Basically, I have 3 commits tagged with version numbers like "v0.1.0", "v0.9.0", "v0.10.0" respectively. Topologically, the latest version (on the HEAD commit) is v0.10.0. However, running git-cliff --topo-order --latest will print out the "v0.9.0" changelog, and git-cliff --topo-order --unreleased will print out the "v0.10.0" changelog. Removing the --topo-order argument produces the expected result.

Steps To Reproduce

  1. git init
  2. git commit --allow-empty -m "feat: new"
  3. git commit --allow-empty -m "fix: whatever"
  4. git commit --allow-empty -m "doc: stuff"
  5. git tag v0.1.0 HEAD~2
  6. git tag v0.9.0 HEAD~
  7. git tag v0.10.0
$ git log --pretty=oneline
871adbf82b0dba2a4719fff11793387934a3cc11 (HEAD -> master, tag: v0.10.0) doc: stuff
b2c33a4df36176363f8a77421c917f212bfdcf98 (tag: v0.9.0) fix: whatever
457b3c4e0fadb9c8801e44d16d860054d9331609 (tag: v0.1.0) feat: new
$ git tag -l
v0.1.0
v0.10.0
v0.9.0
$ git cliff --topo-order --latest 
 WARN  git_cliff > "cliff.toml" is not found, using the default configuration.
# Changelog

All notable changes to this project will be documented in this file.

<!-- generated by git-cliff -->
$ git cliff --topo-order --unreleased
 WARN  git_cliff > "cliff.toml" is not found, using the default configuration.
# Changelog

All notable changes to this project will be documented in this file.

## [0.10.0] - 2025-03-06

### 📚 Documentation

- Stuff

<!-- generated by git-cliff -->

Generating the full changelog seems to have the tags in the correct order:

$ git cliff --topo-order
 WARN  git_cliff > "cliff.toml" is not found, using the default configuration.
# Changelog

All notable changes to this project will be documented in this file.

## [0.10.0] - 2025-03-06

### 📚 Documentation

- Stuff

## [0.9.0] - 2025-03-06

### 🐛 Bug Fixes

- Whatever

## [0.1.0] - 2025-03-06

### 🚀 Features

- New

<!-- generated by git-cliff -->

Expected behavior

$ git cliff --latest 
 WARN  git_cliff > "cliff.toml" is not found, using the default configuration.
# Changelog

All notable changes to this project will be documented in this file.

## [0.10.0] - 2025-03-06

### 📚 Documentation

- Stuff

<!-- generated by git-cliff -->
$ git cliff --unreleased
 WARN  git_cliff > "cliff.toml" is not found, using the default configuration.
# Changelog

All notable changes to this project will be documented in this file.

<!-- generated by git-cliff -->

Screenshots / Logs

No response

Software information

  • Operating system: Linux
  • Rust version:
  • Project version: 2.8.0

Additional context

Based on the behaviour above, it seems to me like the tags don't actually get sorted topologically when using --topo-order.

If you change the last tag from v0.10.0 to v0.9.1 instead, you get the same results both with and without --topo-order. So, I suspect there's some issue when the v0.10.0 tag gets sorted alphabetically before the v0.9.0 tag. (In other words, it looks like --topo-order is actually doing an alphabetical order, not a topological one??)

Am I just misunderstanding the purpose of --topo-order or --latest/--unreleased, or is there something weird going on here?

dpad avatar Mar 06 '25 10:03 dpad

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

welcome[bot] avatar Mar 06 '25 10:03 welcome[bot]

Hello, thanks for reporting and detailed debugging steps! 👋🏼

In other words, it looks like --topo-order is actually doing an alphabetical order, not a topological one??

You might be right... We are not sorting the tags topologically but the commits:

https://github.com/orhun/git-cliff/blob/9babe0649c365f6c499ec6db62450625d8a21987/git-cliff-core/src/repo.rs#L122-L123

Thus I was hoping that the tags will be sorted too, since they are associated with a commit.

For the tags, I just do a time based sorting if topo_order is false:

https://github.com/orhun/git-cliff/blob/9babe0649c365f6c499ec6db62450625d8a21987/git-cliff-core/src/repo.rs#L427-L429

Relatedly, there is an issue for disabling topological order for commits: #804

TBH it's been numerous cases got affected by this so I would really appreciate some other eyes on these sorting issues. Would you be possibly interested in debugging this with the references I provided and confirming the behavior/bug?

orhun avatar Mar 07 '25 11:03 orhun

@orhun Thanks very much for checking this issue (and confirming I'm possibly not crazy 😆). Unfortunately I know nothing about Rust so I'm not really capable of debugging directly, but I did have a look at the sources you provided. Not sure how helpful my comments will be, but here's some in any case.

Based on what I can understand, I am guessing that this line returns the tags in the same order as git tag -l:

https://github.com/orhun/git-cliff/blob/9babe0649c365f6c499ec6db62450625d8a21987/git-cliff-core/src/repo.rs#L385

You then use these tags to compute the "unreleased" case above:

https://github.com/orhun/git-cliff/blob/9babe0649c365f6c499ec6db62450625d8a21987/git-cliff/src/lib.rs#L171

So I suspect that the solution is to explicitly sort these tags topologically. I guess the easiest way might be to sort the tags either during or based on the results of the topological ordering of the commits?

Sorry that I can't be of much help. In any case, git-cliff is fantastic in general, and I'm getting good usage out of it. Thanks for all your work on it!

dpad avatar Mar 07 '25 12:03 dpad

Hey again @dpad - that sounds correct to me. We probably need to manually/explicitly sort the tags as you've pointed out.

I might get to implementing that in the following weeks if it's urgent for your use case. Just LMK!

orhun avatar Mar 23 '25 09:03 orhun

@orhun Thanks for looking into it, no rush from my end though :)

dpad avatar Mar 23 '25 13:03 dpad