Skip to content

makefiles/utils/strings.mk: Fix version_is_greater_or_equal #19133

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

Merged
merged 1 commit into from
Jan 12, 2023

Conversation

maribu
Copy link
Member

@maribu maribu commented Jan 12, 2023

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.
@maribu maribu added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch labels Jan 12, 2023
@maribu maribu requested a review from kaspar030 January 12, 2023 11:12
@github-actions github-actions bot added the Area: build system Area: Build system label Jan 12, 2023
@riot-ci
Copy link

riot-ci commented Jan 12, 2023

Murdock results

✔️ PASSED

8c055f0 makefiles/utils/strings.mk: Fix version_is_greater_or_equal

Success Failures Total Runtime
6770 0 6770 14m:33s

Artifacts

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.

ACK.

bors merge

@maribu
Copy link
Member Author

maribu commented Jan 12, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Jan 12, 2023

Already running a review

@maribu
Copy link
Member Author

maribu commented Jan 12, 2023

bors cancel
bors merge

@bors
Copy link
Contributor

bors bot commented Jan 12, 2023

Canceled.

@maribu
Copy link
Member Author

maribu commented Jan 12, 2023

bors merge already!

@bors
Copy link
Contributor

bors bot commented Jan 12, 2023

Already running a review

@maribu
Copy link
Member Author

maribu commented Jan 12, 2023

sudo bors merge

@maribu
Copy link
Member Author

maribu commented Jan 12, 2023

bors merge --force

@bors
Copy link
Contributor

bors bot commented Jan 12, 2023

Already running a review

@maribu
Copy link
Member Author

maribu commented Jan 12, 2023

bors cancel
bors merge

@bors
Copy link
Contributor

bors bot commented Jan 12, 2023

Canceled.

bors bot added a commit that referenced this pull request Jan 12, 2023
19128: boards: common: stdio_cdc_acm: let tests wait a bit for serial port r=aabadie a=miri64



19133: makefiles/utils/strings.mk: Fix version_is_greater_or_equal r=maribu a=maribu

### 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: Martine Lenders <m.lenders@fu-berlin.de>
Co-authored-by: Marian Buschsieweke <marian.buschsieweke@ovgu.de>
@bors
Copy link
Contributor

bors bot commented Jan 12, 2023

Build failed (retrying...):

bors bot added a commit that referenced this pull request Jan 12, 2023
19133: makefiles/utils/strings.mk: Fix version_is_greater_or_equal r=maribu a=maribu

### 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>
@bors
Copy link
Contributor

bors bot commented Jan 12, 2023

Build failed:

@maribu
Copy link
Member Author

maribu commented Jan 12, 2023

bors retry

@bors
Copy link
Contributor

bors bot commented Jan 12, 2023

Build succeeded:

@bors bors bot merged commit 1dcccdc into RIOT-OS:master Jan 12, 2023
@maribu maribu deleted the makefiles/utils/strings.mk branch January 12, 2023 22:05
@maribu
Copy link
Member Author

maribu commented Jan 12, 2023

Thx :)

@maribu
Copy link
Member Author

maribu commented Jan 12, 2023

Backport provided in #19135

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>
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=kaspar030 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>
@jia200x jia200x added this to the Release 2023.04 milestone Apr 25, 2023
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: needs backport Integration Process: The PR is required to be backported to a release or feature branch Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants