Please use "<package>: bump to <version>" for version changes

[aparcar]

With the recent commit openwrt/openwrt@9ae3c6f a new variable $(AUTORELEASE) is added to search the commit subject log of a package for the ": [bB]ump to " and ": [uU]pdate to ". The variable contains the number of commits since the latest bump, making manual changes of PKG_RELEASE obsolete.

Please use <package>: bump to <version> as commit message subject and set PKG_RELEASE to $(AUTORELEASE) to use this feature.

[karlp]

Why would you not just look for commits that changed the PKG_VERSION instead of this string matching? Especially if you have to opt in via AUTORELEASE anyway?

[aparcar]

Some packages don't use PKG_VERSION, e.g. odhcpd.

[PolynomialDivision]

@aparcar If source code is checked in here (like net/owipcalc), I can also use this feature, or? But there will never be a bump or update commit.

There is a problem that owipcalc was moved from openwrt to packages repro. Now the counter does not work since it is resetted? :/

[PolynomialDivision]

I changed all the packages that I maintain officially, but the corresponding packages do not have the release identifier in their name? Maybe I did a mistake.

[aparcar]

I changed all the packages that I maintain officially, but the corresponding packages do not have the release identifier in their name? Maybe I did a mistake.

I tried it locally and the release is attached:

Package: generate-ipv6-address
Version: 0.1-2
Depends: libc
License: MIT

generate-ipv6-address_0.1-2_x86_64.ipk

[PolynomialDivision]

I tried it locally and the release is attached:

I only looked at the output of github builds. Is there a difference?

[aparcar]

Maybe the SDKs did not yet pickup that change.

[aparcar]

@openwrt/package-maintainers I'd bin this issue if nobody objects.

[cotequeiroz]

OK with pinning this. Additionally, we could perhaps add this information to the PR template text, where it is even more visible.

[jefferyto]

I have only briefly skimmed through the main repo commit, but it seems to me that:

• If I make a change that does not affect the output package, e.g. fix a spelling mistake in the Makefile, $(AUTORELEASE) would still be incremented
• If I separate a change into multiple commits in one PR, because it makes logical sense to do so, $(AUTORELEASE) would advance by the number commits in the PR

Is my understanding correct? If so, I would prefer to continue manually managing PKG_RELEASE for my own packages.

I don't want to discourage other people from using $(AUTORELEASE), but I think part of being a maintainer is being cognizant of when PKG_RELEASE should or should not be incremented. Commits are not the same as releases.

[diizzyy]

I agree that this shouldn't be an "automatic" function.

[aparcar]

If I make a change that does not affect the output package, e.g. fix a spelling mistake in the Makefile, $(AUTORELEASE) would still be incremented

In my opinion spelling fixups are not worth a commit, rather wait until the next release and fixup spelling or cosmetics within that commit. Skimming through openwrt.git there are also spelling mistakes, fixing them in single commits just makes the log less readable.

If I separate a change into multiple commits in one PR, because it makes logical sense to do so, $(AUTORELEASE) would advance by the number commits in the PR

In that case I'd consider the single commits as incomplete. Each commit should represent a consistent state of the repository. Bumping the release only in some commits results in functional (and checksum) varying packages with the same version-release combination.

If so, I would prefer to continue manually managing PKG_RELEASE for my own packages.

You surely can do so. It's opt-in by setting PKG_RELEASE to $(AUTORELEASE), your package your choice. The motivation was to lower the number of review loops like here.

I agree that this shouldn't be an "automatic" function.

It's automatic if you enable it. If not, not.

[jefferyto]

In my opinion spelling fixups are not worth a commit, rather wait until the next release and fixup spelling or cosmetics within that commit.

Changes waiting to be committed in the future are changes that will be forgotten.

In that case I'd consider the single commits as incomplete. Each commit should represent a consistent state of the repository.

I didn't say the commits do not represent consistent states. It is entirely possible for a large change to be broken into separate commits while also being consistent at every step.

If commits are meant to be read by people, then it is a requirement to break down a large change into discrete steps that can be read and understood.

Bumping the release only in some commits results in functional (and checksum) varying packages with the same version-release combination.

If commits are part of one PR, when the PR is merged, all of the commits are merged at the same time. Assuming PKG_RELEASE was incremented in the PR, when buildbots build the package again, it will build with the new release number.

The only way someone can get a package with an old release number and extra commits is if they purposefully checked out a commit from the middle of the PR and built the package from there.

It's opt-in by setting PKG_RELEASE to $(AUTORELEASE), your package your choice.

Having this issue pinned, the PR template text possibly being amended, $(AUTORELEASE) being recommended to maintainers, one might wonder if using $(AUTORELEASE) will become a requirement soon.

The motivation was to lower the number of review loops like here.

Part of on-boarding new contributors is teaching them what needs to be done and when.

[aparcar]

Changes waiting to be committed in the future are changes that will be forgotten.

That's why I have local staging branches with minor changes.

If commits are meant to be read by people, then it is a requirement to break down a large change into discrete steps that can be read and understood.

If your PR contains 5 commits and only one of them touches anything in the specific package folder, the release is bumped by one. If you have multiple logical changes to the package, it makes sense to me treating them as separate releases. Imagine on of the logical changes breaks something.

The only way someone can get a package with an old release number and extra commits is if they purposefully checked out a commit from the middle of the PR and built the package from there.

Which is a terrible unlikely corner cases which makes it the more annoying to debug if it ever happens.

Having this issue pinned, the PR template text possibly being amended, $(AUTORELEASE) being recommended to maintainers, one might wonder if using $(AUTORELEASE) will become a requirement soon.

It's meant as a convenience function for maintainers. There were requests to add a PKG_RELEASE bump check in the CI, adding AUTORELEASE seemed like al cleaner solution to me. There are no plans to make this a requirement, although the idea exists to use it per default IFF no PKG_RELEASE is defined. In any case, maintainers can ignore all that by simply keeping the PKG_RELEASE in place.

Part of on-boarding new contributors is teaching them what needs to be done and when.

More coding less chore :)

[zyxmon]

IMHO AUTORELEASE is not defined correctly for feeds, because they are cloned with --depth 1 param. More info is here - openwrt/openwrt@9ae3c6f#commitcomment-50277872

[nobk]

IMHO AUTORELEASE is not defined correctly for feeds, because they are cloned with --depth 1 param. More info is here - openwrt/openwrt@9ae3c6f#commitcomment-50277872

Yes, if a feed is clone by git clone --depth 1, AUTORELEASE will get wrong result, when using git rev-list --count. And repo more huge, git rev-list will more slow.
git log -1 --format=%ct Makefile as AUTORELEASE is enough, and uniq.

[jefferyto]

Ironically, the use of $(AUTORELEASE) has made reviewing PRs more time consuming, since when I don't see PKG_RELEASE changed in the PR, I have to go find the package Makefile to check if it is using $(AUTORELEASE).

[neheb]

Yeah, unfortunately

[aparcar]

Im sorry for that, should we extend the CI to check for release bumps and fade out AUTORELEASE again?

[hnyman]

should we extend the CI to check for release bumps and fade out AUTORELEASE again?

No, autorelease is mostly good.
Sure, it has drawbacks in PR verification, but as packages have gradually adopted autorelease, the mistakes regarding missing bumps have been largely eliminated.

[jefferyto]

I think it is too late to remove AUTORELEASE, but we have traded one thing package changes must do (bump PKG_RELEASE) with another (version upgrades must have "update to" or "bump to" in the commit title).

I don't think anyone wants me to go into detail on why one is better than the other, but let's just say that "mistakes" still happen, e.g. #18483.

I don't want to be so negative on this feature and I think AUTORELEASE can be good eventually, but I think there still a lot of work to do before it gets there.

[aparcar]

I'll try to find the time to check for AUTORELEASE Via CI and if a number is used and not bumped, an error is reported. That should improve the workflow for reviews

[hnyman]

Would it be possible to skip the whole PKG_RELEASE variable, and always calculate and set a corresponding automatic counter (identical to the AUTORELEASE formula) to the package version?

That would avoid both bump, and the check for AUTORELEASE.

[jefferyto]

I'll try to find the time to check for AUTORELEASE Via CI and if a number is used and not bumped, an error is reported. That should improve the workflow for reviews

I know you don't agree, but not every change requires a bump to PKG_RELEASE. Please don't give me CI errors when I use my judgement on how to manage my packages.

I think your time would be better spent improving the AUTORELEASE detection so that it doesn't require "update to" or "bump to" in the commit title. They lead to false negatives (contributors forgetting to use the magic strings when they should) and false positives (contributors using the magic strings when they shouldn't, e.g. "pinball: bump to bottom edge should not cause ball to levitate")

Would it be possible to skip the whole PKG_RELEASE variable, and always calculate and set a corresponding automatic counter (identical to the AUTORELEASE formula) to the package version?

Please don't, at least not until the AUTORELEASE code is better.

[aparcar]

I'm happy to look into that but that involves more Git juggling which eventually slows down things. I'll try to run a benchmark and see how much things get worse.

[aparcar]

@jefferyto could you please try the following?

diff --git a/rules.mk b/rules.mk
index 8d4f619211..27b8079d40 100644
--- a/rules.mk
+++ b/rules.mk
@@ -417,9 +417,10 @@ define commitcount
 $(shell \
   if git log -1 >/dev/null 2>/dev/null; then \
     if [ -n "$(1)" ]; then \
-      last_bump="$$(git log --pretty=format:'%h %s' . | \
-        grep --max-count=1 -e ': [uU]pdate to ' -e ': [bB]ump to ' | \
-        cut -f 1 -d ' ')"; \
+      last_bump=""; \
+      for last_bump in $$(git log --pretty=format:'%h' .); do \
+        git show "$$last_bump" -- Makefile | grep -q -e "+PKG_VERSION" -e "+PKG_SOURCE_DATE" && break; \
+      done; \
     fi; \
     if [ -n "$$last_bump" ]; then \
       echo -n $$(($$(git rev-list --count "$$last_bump..HEAD" .) + 1)); \

[aparcar]

Related openwrt/openwrt#9893

[aparcar]

Also related openwrt/openwrt#4218