FS#63895 - [makepkg] provides with specified versions are rejected by makepkg

Attached to Project: Pacman
Opened by Ferdinand Bachmann (Ferdi265) - Monday, 23 September 2019, 19:00 GMT
Last edited by Allan McRae (Allan) - Monday, 25 November 2019, 12:56 GMT
Task Type Bug Report
Category makepkg
Status Unconfirmed   Reopened
Assigned To No-one
Architecture All
Severity Low
Priority Normal
Reported Version 5.1.3
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No

Details

# Summary

PKGBUILDs with a provides section that specifies an exact version are rejected by the lint_pkgbuild step in makepkg.

# Reproducing

Any PKGBUILD with a provides line with specified versions will exhibit this bug:
example line from a PKGBUILD:

```sh
provides=('wlroots==0.7.0')
```

response from makepkg:

```
==> ERROR: provides contains invalid characters: '='
```

# Root cause

in /usr/share/makepkg/lint_pkgbuild/provides.sh:

```
lint_provides() {
# *snip*

name=${provide%=*}
# remove optional epoch in version specifier
ver=${provide##$name=?(+([0-9]):)}
lint_one_pkgname provides "$name" || ret=1
if [[ $ver != $provide ]]; then
# remove optional pkgrel in version specifier
check_pkgver "${ver%-+([0-9])?(.+([0-9]))}" provides || ret=1
fi

# *snip*
}
```

the line `name=${provide%=*}` is the culprit.

from the bash manual:
- `${str%pattern}` - Deletes shortest match of substr from back of str
- `${str%%pattern}` - Deletes longest match of substr from back of str

in order to correctly remove the `==`, we should use the `%%` syntax.

# Fix

A patch that fixes this issue is attached below.
This task depends upon

Comment by Eli Schwartz (eschwartz) - Monday, 23 September 2019, 19:08 GMT
This is incorrect, you are stating that your package provides the package wlroots with a version = to the version number "=0.7.0". Version numbers are not (should not be) permitted to contain the character "=".

Do not use double equals signs, as makepkg told you, it is invalid characters. This is in fact the reason I implemented the fatal error which you just saw. People were incorrectly specifying incorrect provides values.
Comment by Ferdinand Bachmann (Ferdi265) - Monday, 23 September 2019, 19:43 GMT
It's interesting though that pacman accepts the double equals characters. I also didn't find documentation on it, so I assumed the syntax for provides and dependencies was the same.

EDIT: Sorry, my bad. Seems double equals is wrong in all places. I still think this should be documented a little better.
Comment by Eli Schwartz (eschwartz) - Monday, 23 September 2019, 19:57 GMT
Actually, double checking and thinking back... the error message is that we are splitting it and pushing the = to the name component, not the version component. Version components can contain the equals character, PKGBUILD(5) does not forbid it even though it should... though it is quite useless either way.

$ vercmp '=0.7.0' 99999
1

It's considered "newer" than the very high version number indeed, of 99999.

> I also didn't find documentation on it, so I assumed the syntax for provides and dependencies was the same.

The syntax *is* the same. Using a double equals sign is never, ever, ever valid. The double equals sign is a way of denoting an equality check in the C language and other languages inspired by the same syntax. There are multiple problems with using the double equals in a PKGBUILD:

- A PKGBUILD is written in bash, and bash uses a single equals sign for both equality checks and assignment expressions, distinguishing between the two based on whether it is surrounded by whitespace plus in a comparison context (the [[ keyword). Outside of a comparison context, C considers spaces to be non-significant, whereas in bash, the spaces remove the assignment context too! and turn "key = val" into a simple command named "key", followed by the argv parameters "=" and "val".

- This isn't an equality check, assigning the provides metadata doesn't inherently do comparison checks, you are declaring that the package provides wlroots and assigning the provided wlroots as "0.7.0". In both C and bash, this is a single equals (though in C, you are permitted to add spaces).
Comment by Eli Schwartz (eschwartz) - Monday, 23 September 2019, 19:58 GMT
The only thing I can think of doing here is adding a special check just for double equals, and raising the error "this is surely not what you meant", and/or documenting that pkgver is not permitted to begin with a "=" character.
Comment by Ferdinand Bachmann (Ferdi265) - Monday, 23 September 2019, 20:10 GMT
Thanks for clarifying.

Another point to add here is that many other package management systems (e.g. python's pip, node's npm, rust's cargo, ...) use == to select a package version.
Since specifying exact package versions is not widespread, some people might assume the syntax is similar to other package managers upon seeing the >= and <= syntax, and would probably try == at some point (as I did).

I do think that the error message is confusing though: "provides contains invalid characters: '='" seems to imply that the = character is invalid _ANYWHERE_, while using it once is allowed.
Comment by Eli Schwartz (eschwartz) - Tuesday, 24 September 2019, 05:35 GMT
PKGBUILD(5) itself directly documents what a "version requirement" is:

depends (array)
An array of packages this package depends on to run. Entries in this list should be surrounded with single quotes and contain at least the package name.
Entries can also include a version requirement of the form name<>version, where <> is one of five comparisons: >= (greater than or equal to), <= (less
than or equal to), = (equal to), > (greater than), or < (less than).

The syntax is also the same one described in the pacman manpage for valid arguments to -S:
> You can also specify version requirements: pacman -S "bash>=3.2".

Version ranges are permitted for -S, but, alternatively, the documentation for

--assume-installed <package=version>

uses one equals.

...

Disregarding programming language package managers, let us look at comparative operating system package managers.

Gentoo: emerge "=app-shells/bash-5.0_p11"
Same grammar is valid in a .ebuild recipe.
So that is a leading single equals, followed by "category/pkgname-fullpkgver". Supported comparisons match Arch Linux (you may use ">", "<", ">=", "<=", "=").

Fedora: dnf install "bash-5.0.7-1.fc30"
You can specify either a "pkgname", or a "pkgname-fullpkgver", no comparison ranges.
A .spec recipe can additionally use the comparisons matching Arch Linux (">", "<", ">=", "<=", "=") via the grammar:
Requires: pkgname = fullpkgver

Debian: apt install "bash=5.0-4".
You can specify either a "pkgname", or a "pkgname=fullpkgver", no comparison ranges.
In a debian/control recipe, you can additionally use the comparisons not matching Arch Linux: <<, <=, =, >= and >> for strictly earlier, earlier or equal, exactly equal, later or equal and strictly later, respectively, via the grammar:
Depends: pkgname (= fullpkgver)

Void Linux: xbps-install "bash-5.0.009"
You may also use the comparisons otherwise matching Arch Linux (">", "<", ">=", "<=") e.g. "bash>=5.0.009"
In a template recipe, you may use the same grammar:
depends="pkgname-fullpkgver otherpkgname>=otherfullpkgver"

I haven't looked around further, but as far as I can tell, none use ==, some use "-" instead of "=", and most support the same grammar across build recipes and installation options.
Comment by Eli Schwartz (eschwartz) - Tuesday, 24 September 2019, 05:36 GMT
I agree the error message is confusing, if not because of missing documentation on what a dependency specifier is, then simply because it's not obvious the linter is "assuming" that the leading `=` is part of the pkgname. One solution is to indeed use `%%`, document that pkgver is not permitted to contain a leading `=` (or any?) -- pkgname already cannot contain a `=` -- and update the linter to reject the newly documented forbidden `pkgver='='`.

On the other hand, it's a bit arbitrary which side you list the extraneous <>= characters which are used for field splitting. Perhaps we should make that its own error. Perhaps we should just be more conservative about what is allowed in a pkgver -- currently, it's much more forgiving than pkgname is!

There's precedent for being more thorough in our error messages, e.g. https://git.archlinux.org/pacman.git/commit/scripts/libmakepkg/lint_pkgbuild?id=f7efa6a93d5361af610827d41045d87c7a72f2b5
It might be possible to introduce better errors here too.
Comment by Allan McRae (Allan) - Monday, 25 November 2019, 12:56 GMT
oops... closed wrong bug!

Loading...