[pacman-dev] [PATCH 1/3] makepkg: Better error messages for versions in (check, make, opt)depends/provides/conflicts

[Luke Shumaker]

On Wed, 08 Aug 2018 00:05:25 -0400,
Eli Schwartz wrote:
> 
> On 08/07/2018 11:16 PM, Luke Shumaker wrote:
> > +check_fullpkgver() {
> > +	local fullver=$1 type=$2
> > +	local ret=0
> > +
> > +	# If there are multiple colons or multiple hyphens, there's a
> > +	# question of how we split it--it's invalid either way, but it
> > +	# will affect error messages.  Let's mimic version.c:parseEVR().
> > +
> > +	if [[ $fullver = *:* ]]; then
> > +		# split at the *first* colon
> > +		check_epoch "${fullver%%:*}" "$type" || ret=1
> > +		fullver=${fullver#*:}
> > +	fi
> > +
> > +	if [[ $fullver = *-* ]]; then
> > +		# split at the *last* hyphen
> > +		check_pkgrel "${fullver##*-}" "$type" || ret=1
> > +		fullver=${fullver%-*}
> > +	fi
> 
> Allan and I discussed on IRC that this does interesting things if
> someone uses e.g. makedepends=('perl-test-fatal>=-0.003')
> This was a real example discovered during the perl rebuild...
> 
> The resulting error message is:
> ERROR: pkgver in makedepends is not allowed to be empty.
> 
> Your patch improves error reporting in several ways, but it does nothing
> for this. So while we are at it, this would be a great time to check
> when splitting off the epoch/pkgrel, whether there's actually a pkgver
> on the other side.

Hmm, I'll have to ponder how to best handle that.  This is a problem
of "given a malformed input, what's the most likely thing that the
user intended?", which is a problem with no robust answer.

Perhaps change the check to:

    if [[ $fullver = ?*-* ]]; then

> > -lint_pkgrel() {

> > +lint_pkgrel() {
> > +	if (( PKGVERFUNC )); then
> > +		# defer check to after getting version from pkgver function
> > +		return 0
> > +	fi
> 
> Why are you delaying this? I assume to match how we do pkgver. But
> that's a change in how we currently handle it. I suppose we should
> consistently treat both variables which makepkg can auto-update... but
> it shouldn't be silently changed without mention in the commit logs. And
> it probably deserves its own commit.

You're right, this should have at least been a separate commit.

Since if pkgver() changes the value of pkgver, it also resets pkgrel
to '1', I figured that there's no point in linting the old value.

> And I'm not positive why we do so for pkgver either TBH. It's been like
> that since 4b129d484394ce6090a9ed21782fe1df2227ad18 when we added
> validation after running pkgver() at all, but the commit logs are not
> clear on why. Maybe to allow the initial author to use `pkgver=` and set
> the initial value using makepkg?

I'll study how it uses PKGVERFUNC and see if I can come up with an answer.

-- 
Happy hacking,
~ Luke Shumaker