Skip to content

Commit

Permalink
rules: add AUTORELEASE and COMMITCOUNT variables
Browse files Browse the repository at this point in the history 
The lack of bumped PKG_RELEASE variables is a recurring theme on the
mailing list and in GitHub comments. This costs precious review time,
a rare good within the OpenWrt project.

Instead of relying on a manually set PKG_RELEASE this commit adds a
`commitcount` function that uses the number of Git commits to determine
the release. The function is called via the variables `$(AUTORELEASE)`
or `$(COMMITCOUNT)`. The `PKG_RELEASE` variable can be set to either of
the two.

- $(AUTORELEASE):

Release is automagically set to the number of commits since the last
commit containing either ": update to " or ": bump to ".

Example below:

    $ git log packages/foobar/
    foobar: fixup file location
    foobar: disable docs
    foobar: bump to 5.3.2
    foobar: fixup copyright

Resulting package name: foobar_5.3.2-3_all.ipk, two package changes
since the last upstream version change, using a 1 based counter.

- $(COMMITCOUNT):

For non-traditional versioning (x.y.z), most prominent `base-files`,
this variable contains the total number of package commits.

The new functionality can also be used by other feeds like packages.git.

In case no build information is available, e.g. when using release
tarballs, the SOURCE_DATE_EPOCH is used to have a reproducible release
identifier.

Suggested-by: Daniel Golle <daniel@makrotopia.org>
Signed-off-by: Paul Spooren <mail@aparcar.org>
  • Loading branch information
@aparcar
aparcar committed Jan 23, 2021
1 parent e779d30 commit 9ae3c6f
Showing 1 changed file with 26 additions and 0 deletions.
26 changes: 26 additions & 0 deletions rules.mk
View file
Expand Up @@ -408,6 +408,32 @@ endef
# file extension
ext=$(word $(words $(subst ., ,$(1))),$(subst ., ,$(1)))

# Count Git commits of a package
# $(1) => if non-empty: count commits since last ": [uU]pdate to " or ": [bB]ump to " in commit message
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 ' ')"; \
fi; \
if [ -n "$$last_bump" ]; then \
echo -n $$(($$(git rev-list --count "$$last_bump..HEAD" .) + 1)); \
else \
echo -n $$(($$(git rev-list --count HEAD .) + 1)); \
fi; \
else \
secs="$$(($(SOURCE_DATE_EPOCH) % 86400))"; \
date="$$(date --utc --date="@$(SOURCE_DATE_EPOCH)" "+%y%m%d")"; \
printf '%s.%05d' "$$date" "$$secs"; \
fi; \
)
endef

COMMITCOUNT = $(if $(DUMP),,$(call commitcount))
AUTORELEASE = $(if $(DUMP),,$(call commitcount,1))

all:
FORCE: ;
.PHONY: FORCE
Expand Down

19 comments on commit 9ae3c6f

@zyxmon
Copy link

@zyxmon zyxmon commented on 9ae3c6f May 2, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not work as expected for feeds. script/feeds/update uses git clone --depth 1 when cloning and the git log is truncated and odes not have the needed lines
Just an example. in case I do git clone https://git.openwrt.org/feed/packages.git the output of git log is (truncated) for dbus package is

packages/utils/dbus$ git log --pretty=format:'%h %s' . 
75e623710 dbus: fix new cmake build
0fb5d3ed2 dbus: update to 1.13.18
1edad0400 dbus: remove host build
c73fd179e dbus: update to 1.13.12
299e5b0a9 treewide: add PKG_CPE_ID for better cvescanner coverage
13f713d08 utils/dbus: Update to 1.12.12
5988204db utils/dbus: Update to 1.12.10
113a34788 dbus: Update to 1.12.8
4006865ae treewide: run "make check FIXUP=1
`

In case I clone with scripts/feeds it is

openwrt/feeds/packages/utils/dbus$ git log --pretty=format:'%h %s' . 
765e986 nano: update version to 5.7

This will give AUTORELEASE value 2 for dbus (and other packages).

@aparcar
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The buildbots use the full git history so upstream packages always have the correct release. Without the full git log you won't be able to set correct "last modified" dates within packages, see here for further information on package reproducibility.

In short, use full history clones if you need upstream release numbers and have reproducible packages. To do so edit your feeds.config and replace src-git with src-git-full.

@champtar
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should just default to full then ?

@aparcar
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine with me

@nobk
Copy link

@nobk nobk commented on 9ae3c6f Jul 24, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@aparcar
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But files of the package may change which do not touch the Makefile

@nobk
Copy link

@nobk nobk commented on 9ae3c6f Jul 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, how about git log -1 --format=%ct -- ./, if ./ is the directory where Makefile at.

@aparcar
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look into that, thank you

@aparcar
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this is faster the numbers are not really "human readable", what do you think?

@nobk
Copy link

@nobk nobk commented on 9ae3c6f Jul 26, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may using YYYYMMDDThhmmss format, it will longer but human easy to read than C time.
such as vim-8.2-20210720T162259_x86_64.ipk, 20210720T162259 is ISO time string of the last git commit time.

@nobk
Copy link

@nobk nobk commented on 9ae3c6f Jul 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like this, or you can find only one git command method.
git log -1 --date=format:%Y%m%dT%H%M%S -- ./ | grep -Po '\d{8}T\d{6}'
20210723T204843

@nobk
Copy link

@nobk nobk commented on 9ae3c6f Jul 26, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or you can give the option to the user, provide 3 AUTORELEASE Macros
AUTORELEASE_123 # Number of releases, not support git clone --depth 1, it will become slower when repo grows
AUTORELEASE_CTIME # faster but not human readable
AUTORELEASE_ISOTIME # faster, readable but longer filename
and define one of them as default AUTORELEASE.

@nobk
Copy link

@nobk nobk commented on 9ae3c6f Jul 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a simply speed test

$ cd feeds/packages/utils/vim
$ /bin/time -f "real %E, user %U, sys %S" git rev-list --count HEAD ./
24
real 0:00.22, user 0.21, sys 0.00
$ /bin/time -f "real %E, user %U, sys %S" git log -1 --date=format:%Y%m%dT%H%M%S ./ | grep -Po '\d{8}T\d{6}'
20210505T213956
real 0:00.02, user 0.00, sys 0.02

@aparcar
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll have the git clone --depth 1 issue for all your other solution too, unless you coincidentally updated often enough so that every package has at least one affecting commit. Running your commands on a fresh copy with a depth of one results in every package having the same date.

Is the speed impact very relevant? I see how this could become an issue for millions of packages but the point was after all to save human time by using computer time.

@nobk
Copy link

@nobk nobk commented on 9ae3c6f Jul 26, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll have the git clone --depth 1 issue for all your other solution too, unless you coincidentally updated often enough so that every package has at least one affecting commit. Running your commands on a fresh copy with a depth of one results in every package having the same date.

Thats different, when using rev-list --count on repo depth 1, you got small release number than expect, that will cause problem. But git log -1 --date you will got the latest time string, that is OK.

Is the speed impact very relevant? I see how this could become an issue for millions of packages but the point was after all to save human time by using computer time.

One pass full package compile will slower more than 20 seconds if there are 100 packages Makefile using AUTORELEASE_123. After 5 years, maybe 200+ seconds if repos grows and packages grows.

@G-M0N3Y-2503
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any objections if we also support the past tense for this too?
e.g. ": Updated to x.x.x"

@adschm
Copy link
Member

@adschm adschm commented on 9ae3c6f Oct 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any objections if we also support the past tense for this too? e.g. ": Updated to x.x.x"

The general practice is to write commit titles (and message where appropriate) in imperative form. I do not see a reason to deviate from that, particularly since this feature will only be relevant for present and future commit titles, and does not care about the past (mostly).

@G-M0N3Y-2503
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The general practice is to write commit titles (and message where appropriate) in imperative form. I do not see a reason to deviate from that, particularly since this feature will only be relevant for present and future commit titles, and does not care about the past (mostly).

Sounds fair to me, for similar motivations to this feature. It might be nice to error or warn if a different tense is detected then 🤔

@aparcar
Copy link
Member Author

@aparcar aparcar commented on 9ae3c6f Oct 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That’s CI stuff

Please sign in to comment.