Re: [pacman-dev] [PATCH] makepkg: Treat pkgrel more similarly to pkgver

2018-06-04 Thread Allan McRae
On 04/06/18 06:01, Luke Shumaker wrote:
> From: Luke Shumaker 
> 
> This is perfectly fine with libalpm; it was only makepkg that was more
> strict with pkgrel than pkgver.
> 
> Further, the former error message about invalid pkgrel formats claimed that
> pkgrel was a "decimal", which would mean that `1.1 == 1.10`.  This is not
> the case; alpm parses pkgrel as a version, not as a decimal.
> 
> Signed-off-by: Luke Shumaker 
> ---

I am still very much against going beyond x.y for pkgrel.   In fact, I
only begrudgingly accept the need for .y in there.

A


[pacman-dev] [PATCH] makepkg: Treat pkgrel more similarly to pkgver

2018-06-03 Thread Luke Shumaker
From: Luke Shumaker 

This is perfectly fine with libalpm; it was only makepkg that was more
strict with pkgrel than pkgver.

Further, the former error message about invalid pkgrel formats claimed that
pkgrel was a "decimal", which would mean that `1.1 == 1.10`.  This is not
the case; alpm parses pkgrel as a version, not as a decimal.

Signed-off-by: Luke Shumaker 
---

This is a patch from Parabola, that we've been maintaining and
applying to our pacman package for a while.  I originally wrote it
against version 4.1.2, and have submitted upstream before:
https://lists.archlinux.org/pipermail/pacman-dev/2014-August/019340.html

Dan McGee and Allan McRae weren't fans of it then.
https://lists.archlinux.org/pipermail/pacman-dev/2014-August/019340.html
https://lists.archlinux.org/pipermail/pacman-dev/2014-September/019351.html

Allan was "of the opinion that the pkgrel should be a positive integer
and nothing else."  Dan thought it shouldn't be complicated, as "we're
not trying to copy or match an upstream value."  Well, in Arch, you're
not trying to copy an upstream value, but in Parabola and in Arch
Linux 32, we are.

Currently in Arch Linux 32 they take advantage of makepkg's current
behavior: when modifying a PKGBUILD from Arch, they append ".$N to the
pkgrel from Arch, and don't modify the existing number, so that it is
easy to tell that "foo-X-Y.Z" is based on "foo-X-Y" from Arch (and Z
is the Arch 32-specific pkgrel); they are matching an upstream value.

Prior to applying this patch, Parabola did the same thing.  Since
applying this patch, we've transitioned to "foo-X-Y.parabolaZ".  This
is even more relevant now, since we also import packages from Arch 32,
which may already have a ".Z".  When building foo-static, we might use
"foo-static-X-Y.staticZ" to clearly indicate that it is based on the
PKGBUILD for "foo-X-Y".

Allan had said that he would consider "Inclusion of a 'dist' keyword
in PKGBUILDs.  If present, this gets added onto the end of the
pkgrel."  I think that's introducing new weird semantics where we
don't need any.  Normal versioning semantics do fine; pkgrel is a
version number of the packaging metadata, and "Y.distZ" says "version
Y, and then Z patches", same as any other patchlevel; similar to
".gitZ" in a pkgver.

 doc/PKGBUILD.5.asciidoc   |  3 ++-
 scripts/libmakepkg/lint_pkgbuild/pkgrel.sh.in |  4 ++--
 test/util/vercmptest.sh   | 23 ++-
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/doc/PKGBUILD.5.asciidoc b/doc/PKGBUILD.5.asciidoc
index 9634bd15..a75b40fe 100644
--- a/doc/PKGBUILD.5.asciidoc
+++ b/doc/PKGBUILD.5.asciidoc
@@ -60,7 +60,8 @@ systems (see below).
allows package maintainers to make updates to the package's configure
flags, for example. This is typically set to '1' for each new upstream
software release and incremented for intermediate PKGBUILD updates. The
-   variable is not allowed to contain hyphens.
+   variable is not allowed to contain colons, forward slashes, hyphens or
+   whitespace.

 *epoch*::
Used to force the package to be seen as newer than any previous versions
diff --git a/scripts/libmakepkg/lint_pkgbuild/pkgrel.sh.in 
b/scripts/libmakepkg/lint_pkgbuild/pkgrel.sh.in
index f294a3bf..07292613 100644
--- a/scripts/libmakepkg/lint_pkgbuild/pkgrel.sh.in
+++ b/scripts/libmakepkg/lint_pkgbuild/pkgrel.sh.in
@@ -35,8 +35,8 @@ lint_pkgrel() {
return 1
fi

-   if [[ $pkgrel != +([0-9])?(.+([0-9])) ]]; then
-   error "$(gettext "%s must be a decimal, not %s.")" "pkgrel" 
"$pkgrel"
+   if [[ $pkgrel = *[[:space:]/:-]* ]]; then
+   error "$(gettext "%s is not allowed to contain colons, forward 
slashes, hyphens or whitespace.")" "pkgrel" "$pkgrel"
return 1
fi
 }
diff --git a/test/util/vercmptest.sh b/test/util/vercmptest.sh
index 1541e7ae..f41a0d1e 100755
--- a/test/util/vercmptest.sh
+++ b/test/util/vercmptest.sh
@@ -39,7 +39,7 @@ tap_runtest() {
tap_is_str "$($bin "$ver2" "$ver1")" "$exp" "$ver2 $ver1"
 }

-tap_plan 92
+tap_plan 124

 # all similar length, no pkgrel
 tap_runtest 1.5.0 1.5.0  0
@@ -113,4 +113,25 @@ tap_runtest 1:1.01.0   1
 tap_runtest 1:1.01.1   1
 tap_runtest 1:1.11.1   1

+# complex pkgrel values
+tap_runtest 1-1.5.01-1.5.0 0
+tap_runtest 1-1.5.11-1.5.0 1
+tap_runtest 1-1.5.11-1.5   1
+tap_runtest 1-1.5b 1-1.5  -1
+tap_runtest 1-1.5b 1-1.5.1-1
+# based on "from the manpage"
+tap_runtest 1-1.0a 1-1.0alpha -1
+tap_runtest 1-1.0alpha 1-1.0b -1
+tap_runtest 1-1.0b 1-1.0beta  -1
+tap_runtest 1-1.0beta  1-1.0rc-1
+tap_runtest 1-1.0rc1-1.0  -1
+# based on "going crazy? alpha-dotted versions"
+tap_runtest 1-1.5.a1-1.5   1
+tap_runtest 1-1.5.b1-1.5.a 1
+tap_runtest 1-1.5.11-1.5.b 1
+# based on Parabola usage
+tap_runtest 1-11-2.par1   -1
+tap_ru

Re: [pacman-dev] [PATCH] makepkg: treat pkgrel more similarly to pkgver

2014-09-14 Thread Allan McRae
On 30/08/14 13:56, Luke Shumaker wrote:
> At Fri, 29 Aug 2014 17:16:11 -0500,
> Dan McGee wrote:
>>
>> On Fri, Aug 29, 2014 at 1:07 PM, Luke Shumaker 
>> wrote:
>>
>>> This is perfectly fine with libalpm; it was only makepkg that was more
>>> strict with pkgrel than pkgver.
>>>
>> Correct.
>>
>> However, did you look at the NEWS file? This was an explicit change made in
>> pacman 4.1.0 (commit 708a22757) to tighten the format of this value. I'd be
>> -1 on this change, unless someone can show me a real reason pkgrel should
>> be complicated, given this is something the packager influences and we're
>> not trying to copy or match an upstream value.
> 
> Sorry, I did not catch that in NEWS.
> 
> In my opinion, this is most useful to other distros that are
> downstream from Arch.  For example, in Parabola, packages that are
> repackaged/modified from Arch, they like to set it like
> `pkgrel=${archrel}.${parabolarel}`.  If an Arch package uses both
> places, then Parabola's scheme breaks.  There's also been discussion
> that it would be nice to be able to do
> `pkgrel=${archrel}.parabola${parabolarel}`.
> 
> For kernel modules that must be built against a specific version of
> the kernel, it would be nice to be able to do
> `pkgrel=${_pkgrel}.${_basekernel}`, which would make it so one
> wouldn't have to mess around with pkgrel when just bumping
> _basekernel.
> 
> Sans the desire to stick 'parabola' into pkgrel, I guess at a minimum,
> that really advocates changing it from
> 
> [[ $i != +([0-9])?(.+([0-9])) ]]
> 
> to
> 
> [[ $i != +([0-9])*(.+([0-9])) ]]
> 

So that would allow x.y.z pkrels?

>>> Further, the former error message about invalid pkgrel formats claimed that
>>> pkgrel was a "decimal", which would mean that `1.1 == 1.10`.  This was not
>>> the case; alpm parsed pkgrel as a version, not a decimal.  In that light,
>>> enforcing /[0-9]+(\.([0-9]+)?/ on a version spec seems silly.
> 
> If you do reject this change, would you at least accept a patch
> clarifying that it is a simplified 'version', not a decimal?
> 

I'm personally of the opinion that the pkgrel should be a positive
integer and nothing else.  However, I have seen what could be considered
valid uses of the x.y pkgrel - preparing a release and packaging with
0.1, 0.2,...,  and rebuilds that are architecture specific (which is
more of an issue for distributions that have more architectures than
Arch has). So decimals need to stay.g

Saying all of this...   spec files (rpm) often have something like:
Release: 1%{?dist}
where dist is a distribution specific tag.

What I will consider (my mind is changeable):
1) A documentation patch saying pkgrel is an (positive) integer.  A
sub-release (or better name) can be specified by adding another interger
after a period.

2) Inclusion of a "dist" keyword in PKGBUILDs.  If present, this gets
added onto the end of the pkgrel.

Allan



Re: [pacman-dev] [PATCH] makepkg: treat pkgrel more similarly to pkgver

2014-08-29 Thread Luke Shumaker
At Fri, 29 Aug 2014 17:16:11 -0500,
Dan McGee wrote:
> 
> On Fri, Aug 29, 2014 at 1:07 PM, Luke Shumaker 
> wrote:
> 
> > This is perfectly fine with libalpm; it was only makepkg that was more
> > strict with pkgrel than pkgver.
> >
> Correct.
> 
> However, did you look at the NEWS file? This was an explicit change made in
> pacman 4.1.0 (commit 708a22757) to tighten the format of this value. I'd be
> -1 on this change, unless someone can show me a real reason pkgrel should
> be complicated, given this is something the packager influences and we're
> not trying to copy or match an upstream value.

Sorry, I did not catch that in NEWS.

In my opinion, this is most useful to other distros that are
downstream from Arch.  For example, in Parabola, packages that are
repackaged/modified from Arch, they like to set it like
`pkgrel=${archrel}.${parabolarel}`.  If an Arch package uses both
places, then Parabola's scheme breaks.  There's also been discussion
that it would be nice to be able to do
`pkgrel=${archrel}.parabola${parabolarel}`.

For kernel modules that must be built against a specific version of
the kernel, it would be nice to be able to do
`pkgrel=${_pkgrel}.${_basekernel}`, which would make it so one
wouldn't have to mess around with pkgrel when just bumping
_basekernel.

Sans the desire to stick 'parabola' into pkgrel, I guess at a minimum,
that really advocates changing it from

[[ $i != +([0-9])?(.+([0-9])) ]]

to

[[ $i != +([0-9])*(.+([0-9])) ]]

> > Further, the former error message about invalid pkgrel formats claimed that
> > pkgrel was a "decimal", which would mean that `1.1 == 1.10`.  This was not
> > the case; alpm parsed pkgrel as a version, not a decimal.  In that light,
> > enforcing /[0-9]+(\.([0-9]+)?/ on a version spec seems silly.

If you do reject this change, would you at least accept a patch
clarifying that it is a simplified 'version', not a decimal?

-- 
Happy hacking,
~ Luke Shumaker



Re: [pacman-dev] [PATCH] makepkg: treat pkgrel more similarly to pkgver

2014-08-29 Thread Dan McGee
On Fri, Aug 29, 2014 at 1:07 PM, Luke Shumaker 
wrote:

> This is perfectly fine with libalpm; it was only makepkg that was more
> strict with pkgrel than pkgver.
>
Correct.

However, did you look at the NEWS file? This was an explicit change made in
pacman 4.1.0 (commit 708a22757) to tighten the format of this value. I'd be
-1 on this change, unless someone can show me a real reason pkgrel should
be complicated, given this is something the packager influences and we're
not trying to copy or match an upstream value.


>
> Further, the former error message about invalid pkgrel formats claimed that
> pkgrel was a "decimal", which would mean that `1.1 == 1.10`.  This was not
> the case; alpm parsed pkgrel as a version, not a decimal.  In that light,
> enforcing /[0-9]+(\.([0-9]+)?/ on a version spec seems silly.
> ---
>  doc/PKGBUILD.5.txt  |  4 ++--
>  scripts/makepkg.sh.in   |  5 +
>  test/util/vercmptest.sh | 18 ++
>  3 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/doc/PKGBUILD.5.txt b/doc/PKGBUILD.5.txt
> index e2389cb..159adbb 100644
> --- a/doc/PKGBUILD.5.txt
> +++ b/doc/PKGBUILD.5.txt
> @@ -48,7 +48,7 @@ similar to `$_basekernver`.
>
>  *pkgver*::
> The version of the software as released from the author (e.g.,
> '2.7.1').
> -   The variable is not allowed to contain colons or hyphens.
> +   The variable is not allowed to contain colons, hyphens or
> whitespace.
>  +
>  The `pkgver` variable can be automatically updated by providing a
> `pkgver()`
>  function in the PKGBUILD that outputs the new package version.
> @@ -62,7 +62,7 @@ below).
> allows package maintainers to make updates to the package's
> configure
> flags, for example. This is typically set to '1' for each new
> upstream
> software release and incremented for intermediate PKGBUILD
> updates. The
> -   variable is not allowed to contain hyphens.
> +   variable is not allowed to contain colons, hyphens or whitespace.
>
>  *pkgdesc*::
> This should be a brief description of the package and its
> functionality.
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 8e8a64c..552d305 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -2337,10 +2337,7 @@ lint_pkgrel() {
> return 1
> fi
>
> -   if [[ $pkgrel != +([0-9])?(.+([0-9])) ]]; then
> -   error "$(gettext "%s must be a decimal, not %s.")"
> "pkgrel" "$pkgrel"
> -   return 1
> -   fi
> +   validate_pkgver "$pkgrel"
>  }
>
>  lint_pkgver() {
> diff --git a/test/util/vercmptest.sh b/test/util/vercmptest.sh
> index 9032cbf..32fb3e7 100755
> --- a/test/util/vercmptest.sh
> +++ b/test/util/vercmptest.sh
> @@ -142,6 +142,24 @@ runtest 1:1.01.0   1
>  runtest 1:1.01.1   1
>  runtest 1:1.11.1   1
>
> +# complex pkgrel values
> +runtest 1-1.5.01-1.5.0 0
> +runtest 1-1.5.11-1.5.0 1
> +runtest 1-1.5.11-1.5   1
> +runtest 1-1.5b 1-1.5  -1
> +runtest 1-1.5b 1-1.5.1-1
> +runtest 1-1.0a 1-1.0alpha -1
> +runtest 1-1.0alpha 1-1.0b -1
> +runtest 1-1.0b 1-1.0beta  -1
> +runtest 1-1.0beta  1-1.0rc-1
> +runtest 1-1.0rc1-1.0  -1
> +runtest 1-1.5.a1-1.5   1
> +runtest 1-1.5.b1-1.5.a 1
> +runtest 1-1.5.11-1.5.b 1
> +runtest 1-21-2.par11
> +runtest 1-21-2.par11
> +runtest 1-31-2.par1   -1
> +
>  #END TESTS
>
>  if [[ $failure -eq 0 ]]; then
> --
> 2.1.0
>
>



[pacman-dev] [PATCH] makepkg: treat pkgrel more similarly to pkgver

2014-08-29 Thread Luke Shumaker
This is perfectly fine with libalpm; it was only makepkg that was more
strict with pkgrel than pkgver.

Further, the former error message about invalid pkgrel formats claimed that
pkgrel was a "decimal", which would mean that `1.1 == 1.10`.  This was not
the case; alpm parsed pkgrel as a version, not a decimal.  In that light,
enforcing /[0-9]+(\.([0-9]+)?/ on a version spec seems silly.
---
 doc/PKGBUILD.5.txt  |  4 ++--
 scripts/makepkg.sh.in   |  5 +
 test/util/vercmptest.sh | 18 ++
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/doc/PKGBUILD.5.txt b/doc/PKGBUILD.5.txt
index e2389cb..159adbb 100644
--- a/doc/PKGBUILD.5.txt
+++ b/doc/PKGBUILD.5.txt
@@ -48,7 +48,7 @@ similar to `$_basekernver`.
 
 *pkgver*::
The version of the software as released from the author (e.g., '2.7.1').
-   The variable is not allowed to contain colons or hyphens.
+   The variable is not allowed to contain colons, hyphens or whitespace.
 +
 The `pkgver` variable can be automatically updated by providing a `pkgver()`
 function in the PKGBUILD that outputs the new package version.
@@ -62,7 +62,7 @@ below).
allows package maintainers to make updates to the package's configure
flags, for example. This is typically set to '1' for each new upstream
software release and incremented for intermediate PKGBUILD updates. The
-   variable is not allowed to contain hyphens.
+   variable is not allowed to contain colons, hyphens or whitespace.
 
 *pkgdesc*::
This should be a brief description of the package and its functionality.
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 8e8a64c..552d305 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -2337,10 +2337,7 @@ lint_pkgrel() {
return 1
fi
 
-   if [[ $pkgrel != +([0-9])?(.+([0-9])) ]]; then
-   error "$(gettext "%s must be a decimal, not %s.")" "pkgrel" 
"$pkgrel"
-   return 1
-   fi
+   validate_pkgver "$pkgrel"
 }
 
 lint_pkgver() {
diff --git a/test/util/vercmptest.sh b/test/util/vercmptest.sh
index 9032cbf..32fb3e7 100755
--- a/test/util/vercmptest.sh
+++ b/test/util/vercmptest.sh
@@ -142,6 +142,24 @@ runtest 1:1.01.0   1
 runtest 1:1.01.1   1
 runtest 1:1.11.1   1
 
+# complex pkgrel values
+runtest 1-1.5.01-1.5.0 0
+runtest 1-1.5.11-1.5.0 1
+runtest 1-1.5.11-1.5   1
+runtest 1-1.5b 1-1.5  -1
+runtest 1-1.5b 1-1.5.1-1
+runtest 1-1.0a 1-1.0alpha -1
+runtest 1-1.0alpha 1-1.0b -1
+runtest 1-1.0b 1-1.0beta  -1
+runtest 1-1.0beta  1-1.0rc-1
+runtest 1-1.0rc1-1.0  -1
+runtest 1-1.5.a1-1.5   1
+runtest 1-1.5.b1-1.5.a 1
+runtest 1-1.5.11-1.5.b 1
+runtest 1-21-2.par11
+runtest 1-21-2.par11
+runtest 1-31-2.par1   -1
+
 #END TESTS
 
 if [[ $failure -eq 0 ]]; then
-- 
2.1.0