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

Extend version to four numbers (5.x.y.z) #1247

Merged
merged 4 commits into from
Mar 6, 2024

Conversation

kontura
Copy link
Contributor

@kontura kontura commented Feb 14, 2024

Unfortunately this is a breaking change because it is changing a struct in include/libdnf5/version.hpp

Therefore I have targeted the 5.2.0.0 branch.
This means it will not get into rawhide before dnf5 is made default.

I have called the new version number PRIME but I am open to any other naming.

@kontura kontura force-pushed the dnf5-5.2.0.0 branch 2 times, most recently from 1577163 to 599f7b5 Compare February 28, 2024 08:26
The problem is that cmake call to `project()` sets (overrides) the:
`PROJECT_VERSION_MAJOR/MINOR/PATCH/TWEAK` with the values provided.
https://cmake.org/cmake/help/latest/command/project.html#options

This could cause confusion when we want to name some of the variables
differently and use differnt order (for example the
PROJECT_VERSION_MAJOR variable woudn't be the first).
Dnf5 and libdnf5 call the smallest version number `micro` (on the API as
well) unify cmake variable to be the same.
@ppisar ppisar self-assigned this Feb 28, 2024
@ppisar
Copy link
Contributor

ppisar commented Feb 28, 2024

The overall idea and the code look good. However CI fails on cmake invocation:

CMake Error at CMakeLists.txt:16 (message):
  Variables PROJECT_VERSION and PACKAGE_VERSION differ.  Make sure the
  versions in VERSION.cmake and the value provided via -DPACKAGE_VERSION are
  in sync.

Is it possible to fix it? I'd like see the code buildable and tests passsing before merging this change. That also implies rebasing the target (i.e. merging commits from main branch) first.

We want to keep the most important version number stable (as a 5 because
we are calling the project dnf5) however we also want to mostly adhere
to the SEMVER system. Therefore we switch to a 4 number scheme,
incrementing the:

PRIME version when completely changing API and everything in dnf
MAJOR version when you make incompatible API changes
MINOR version when you add functionality in a backward compatible manner
MICRO version when you make backward compatible bug fixes

The PRIME version should hopefully stay as a 5 for the foreseeable future.
@kontura kontura force-pushed the version_scheme branch 2 times, most recently from 4f6c4b4 to f09982c Compare March 1, 2024 12:40
@kontura
Copy link
Contributor Author

kontura commented Mar 1, 2024

/packit test

@ppisar
Copy link
Contributor

ppisar commented Mar 1, 2024

Your last commit broke packit:

2024-03-01 13:26:25.665 base_git.py       INFO   Using user-defined script for ActionName.get_current_version: [['rpmspec', '-q', '--queryformat', '%{VERSION}\\n', '*spec', '|head', '-n1']]
2024-03-01 13:26:25.665 commands.py       DEBUG  Command: rpmspec -q --queryformat %{VERSION}\n *spec |head -n1
2024-03-01 13:26:25.668 logging.py        INFO   rpmspec: -n1: unknown option
2024-03-01 13:26:25.669 commands.py       ERROR  Command 'rpmspec -q --queryformat %{VERSION}\\n *spec |head -n1' failed.
2024-03-01 13:26:25.669 commands.py       DEBUG  Command stderr: rpmspec: -n1: unknown option

@kontura
Copy link
Contributor Author

kontura commented Mar 5, 2024

In an attempt to avoid future conflicts I moved the lines around a bit, it should match: #1296

Anyway the PR should be ready and the builds should work now.

@ppisar
Copy link
Contributor

ppisar commented Mar 6, 2024

The failing test "dnf/installonly.feature:414 Handle over-limit custom kernel without installonlypkg(kernel) provide" is unrelated. Otherwise, the package builds and the tests are fine.

@ppisar ppisar merged commit e34e0aa into rpm-software-management:dnf5-5.2.0.0 Mar 6, 2024
9 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants