Skip to content

Conversation

maribu
Copy link
Member

@maribu maribu commented Jan 12, 2023

Backport of #19133

Contribution description

The Makefile function version_is_greater_or_equal is used to check if a version of GNU Make is at least the required one. However, it has the built-in assumption the version numbers have to format x.y.z, but Alpine Linux currently ships GNU Make 4.4. This results in $(call _pad_number,3,) which runs printf '$03d' '' in the shell, which is not valid.

This fixes the issue by making _pad_number more robust by fall back to printing 0 with the given padding, if the number given to print is empty.

Testing procedure

Append

$(info A=$(call version_is_greater_or_equal,4.2.0,4.2.0))
$(info B=$(call version_is_greater_or_equal,4.2,4.2.0))
$(info C=$(call version_is_greater_or_equal,4.1,4.2.0))
$(info D=$(call version_is_greater_or_equal,4.1.9,4.2.0))
$(info E=$(call version_is_greater_or_equal,5.1.9,4.2.0))
$(info F=$(call version_is_greater_or_equal,5.0.0,4.2.0))
$(info G=$(call version_is_greater_or_equal,4.2.1,4.2.0))
$(info H=$(call version_is_greater_or_equal,4.3.1,4.2.0))

e.g. to makefiles/utils/strings.mk, build something and observe the info output.

This yields

A=1
B=1
C=
D=
E=1
F=1
G=1
H=1

for me and does not complain about invalid printf invocations.

Issues/PRs references

None

The Makefile function `version_is_greater_or_equal` is used to check if
a version of GNU Make is at least the required one. However, it has
the built-in assumption the version numbers have to format x.y.z,
but Alpine Linux currently ships GNU Make 4.4. This results in
`$(call _pad_number,3,)` which runs `printf '$03d' ''` in the shell,
which is not valid.

This fixes the issue by making `_pad_number` more robust by fall back to
printing `0` with the given padding, if the number given to print is
empty.

(cherry picked from commit 8c055f0)
@maribu maribu added Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Jan 12, 2023
Copy link
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

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

backport-ACK.

bors merge

@bors
Copy link
Contributor

bors bot commented Jan 12, 2023

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

@riot-ci
Copy link

riot-ci commented Jan 13, 2023

Murdock results

✔️ PASSED

2ef49ed makefiles/utils/strings.mk: Fix version_is_greater_or_equal

Success Failures Total Runtime
6770 0 6770 15m:13s

Artifacts

@benpicco
Copy link
Contributor

bors merge

@bors
Copy link
Contributor

bors bot commented Jan 13, 2023

Already running a review

@benpicco
Copy link
Contributor

bors cancel
bors merge

@bors
Copy link
Contributor

bors bot commented Jan 13, 2023

Canceled.

@benpicco
Copy link
Contributor

bors merge

@bors
Copy link
Contributor

bors bot commented Jan 13, 2023

Already running a review

bors bot added a commit that referenced this pull request Jan 13, 2023
19135: makefiles/utils/strings.mk: Fix version_is_greater_or_equal [backport 2023.01] r=benpicco a=maribu

# Backport of #19133

### Contribution description

The Makefile function `version_is_greater_or_equal` is used to check if a version of GNU Make is at least the required one. However, it has the built-in assumption the version numbers have to format x.y.z, but Alpine Linux currently ships GNU Make 4.4. This results in `$(call _pad_number,3,)` which runs `printf '$03d' ''` in the shell, which is not valid.

This fixes the issue by making `_pad_number` more robust by fall back to printing `0` with the given padding, if the number given to print is empty.

### Testing procedure

Append

```Makefile

$(info A=$(call version_is_greater_or_equal,4.2.0,4.2.0))
$(info B=$(call version_is_greater_or_equal,4.2,4.2.0))
$(info C=$(call version_is_greater_or_equal,4.1,4.2.0))
$(info D=$(call version_is_greater_or_equal,4.1.9,4.2.0))
$(info E=$(call version_is_greater_or_equal,5.1.9,4.2.0))
$(info F=$(call version_is_greater_or_equal,5.0.0,4.2.0))
$(info G=$(call version_is_greater_or_equal,4.2.1,4.2.0))
$(info H=$(call version_is_greater_or_equal,4.3.1,4.2.0))
```

e.g. to `makefiles/utils/strings.mk`, build something and observe the info output.

This yields

```
A=1
B=1
C=
D=
E=1
F=1
G=1
H=1
```

for me and does not complain about invalid `printf` invocations.

### Issues/PRs references

None

Co-authored-by: Marian Buschsieweke <marian.buschsieweke@ovgu.de>
@kaspar030
Copy link
Contributor

bors cancel
bors merge

@bors
Copy link
Contributor

bors bot commented Jan 13, 2023

Canceled.

@bors
Copy link
Contributor

bors bot commented Jan 13, 2023

Build succeeded:

@bors bors bot merged commit 88bdbcc into RIOT-OS:2023.01-branch Jan 13, 2023
@maribu maribu deleted the backport/2023.01/makefiles/utils/strings.mk branch April 25, 2023 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
No open projects
Status: Fix in release branch
Development

Successfully merging this pull request may close these issues.

4 participants