Version generation for PRs does not guarantee increasing values

[sgallagher]

If a pull request involves a rebase (such as to squash in some fixup commits or drop some unneeded changes), there is no guarantee that the NEVRA of the resulting RPM will sort higher than the previous push. This is because the version logic uses git describe, which adds a new version section based on the number of commits since the most recent tag. If we squash the patches, this number goes down.

For example, I build the following two libmodulemd builds from the same PR, the first with a number of patches and the second with them all squashed in together:

• libmodulemd.2.8.3.39.g7b928e6-0.fc30.20200130141552
• libmodulemd.2.8.3.32.gdd69720-0.fc30.20200130144520

Using git describe is useful for uniqueness, but it should not be part of the Version: field in RPM. That should be reserved for the output of git describe --abbrev=0 which would result in libmodulemd-2.8.3. The remainder of the git description should be moved to the Release: field in RPM. For maximum effect, it should probably be something like: `Release: 0.%{current_time}.%{git_desc_suffix}.

The result would be something that looks like libmodulemd-2.8.3-0.20200130144520.32.gdd69720.fc30. Because the timestamp precedes the description, it will be preferred when doing NEVRA comparisons.

[lachmanfrantisek]

@sgallagher

Thanks for the detailed description. The example format is pretty long but solves the problem without losing the git info. What do others think?

---

Note for us: We need to take into consideration also the custom version commands.

[sgallagher]

A more complete analysis of the problem is documented for Fedora packaging guidelines at https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_snapshots

[dhodovsk]

Related to: oamg/convert2rhel#11

[sturivny]

Hi, guys do you have any updates?

[TomasTomecek]

Good idea, we should do that. @sgallagher very nice write-up.

[dmach]

I have a bad feeling about packit touching the version.
The best practice is that downstream packaging (packit falls into this category) never touches version and uses the release fields for increasing the build numbers.

That's why I'd prefer:
Name: libmodulemd
Version: 2.8.3 (equal to upstream version)
Release: <original release number>.git.39.g7b928e6%{?dist} (+ eventually any additional details such as build time as @sgallagher suggested)

Pre-release versioning with tilde should also work, but I'm not sure if it's possible to fully automate it. And as I understand it, it's primarily meant for Alphas, Betas, RCs and other named milestones, not for generic pre-release builds.

[sgallagher]

@dmach You've described almost the same solution I provided, except that your approach doesn't solve the specific problem of "squashed patches means the NEVRA goes backwards". That's why the build time needs to be ahead of the git.39.<hash> portion.

[TomasTomecek]

I'd say this is not that hard to do on packit's side. I'm gonna give it a shot.

[TomasTomecek]

@sgallagher

0.%{current_time}...

The zero is static right? It's the "basic release number" - we probably don't want to start with the current time number since it's too big. Is that correct?

Edit: okay, Dan suggests original release number, that makes sense, gonna do that

[TomasTomecek]

This is now getting to be really complicated: I agree to use release for tracibility. But it means this is solved on the RPM-level and not on the upstream level (or ecosystem-specific packaging level).

Example: we are using setuptools scm for packit which uses git describe style of versions. Hence our tarballs are expected to use git-describe style of naming - should this be reflected in the spec file?

The way I am planning to solve this:

1. Implement the proposed solution to not tamper version by default and stuff git-desc into release
2. change default get latest upstream version to git describe --abbrev=0 which people can override and get the old behaviour (which packit itself needs actually)

[sgallagher]

@sgallagher

0.%{current_time}...

The zero is static right? It's the "basic release number" - we probably don't want to start with the current time number since it's too big. Is that correct?

Edit: okay, Dan suggests original release number, that makes sense, gonna do that

Right, the purpose there is to ensure a "real" release sorts higher.