[pacman-dev] [PATCH 0/3] makepkg: Alternate implementation of VCS URLs in sources array.
A while ago I started working on a derivative of makepkg to support having 'git://...' type urls in the sources=() array. When preparing to file this patch, I did a `git rebase`, and noticed that Allan McRae began working on a similar feature. Our implementations are in many ways similar. Hopefully mine will be useful. My implementation makes minimal changes to makepkg itself (only adding blob expansion to DLAGENTS, allowing for things like "git+*::""). Instead I added a `vcsget` tool which generates a tarball from the VCS repo, in a very similar manner to the way Allan's implementation does so within makepkg. It looks as if Allan's download_*() functions are more verbose than mine about what failed when there is an error. His svn and hg handlers are likely more robust--though my git is pretty solid. I also have a half-written handler for for bzr. An advantage of my design is that it does allow for integrity checks of VCS packages, rather than inserting 'SKIP' into the md5sums array. This is very important to the derivative distribution Parabola. (However, the 'SKIP' option is still valuable for URLs that track a branch) Happy hacking, ~ Luke Shumaker Luke Shumaker (3): Add a `vcsget` tool to download source from VCS repositories. makepkg: do glob expansion in DLAGENTS maps makepkg.conf: add vcsget DLAGENTS etc/makepkg.conf.in | 8 +- scripts/.gitignore| 1 + scripts/Makefile.am | 4 +- scripts/makepkg.sh.in | 13 ++- scripts/vcsget.sh.in | 294 ++ 5 files changed, 316 insertions(+), 4 deletions(-) create mode 100644 scripts/vcsget.sh.in -- 1.7.12
[pacman-dev] [PATCH 1/3] Add a `vcsget` tool to download source from VCS repositories.
It would support
* cvs
* git
* svn
* bzr
* hg
But cvs support is yet to be written, and bzr support is half-way written.
Git is the only mode that I'm thoroughly confident in, the others need
testing.
Signed-off-by: Luke Shumaker
---
scripts/.gitignore | 1 +
scripts/Makefile.am | 4 +-
scripts/vcsget.sh.in | 294 +++
3 files changed, 298 insertions(+), 1 deletion(-)
create mode 100644 scripts/vcsget.sh.in
diff --git a/scripts/.gitignore b/scripts/.gitignore
index 9e403bf..e59551e 100644
--- a/scripts/.gitignore
+++ b/scripts/.gitignore
@@ -6,3 +6,4 @@ pkgdelta
repo-add
repo-elephant
repo-remove
+vcsget
diff --git a/scripts/Makefile.am b/scripts/Makefile.am
index 29c81aa..8e6d037 100644
--- a/scripts/Makefile.am
+++ b/scripts/Makefile.am
@@ -14,7 +14,8 @@ OURSCRIPTS = \
pacman-key \
pacman-optimize \
pkgdelta \
- repo-add
+ repo-add \
+ vcsget
EXTRA_DIST = \
makepkg.sh.in \
@@ -23,6 +24,7 @@ EXTRA_DIST = \
pacman-optimize.sh.in \
pkgdelta.sh.in \
repo-add.sh.in \
+ vcsget.sh.in \
$(LIBRARY)
LIBRARY = \
diff --git a/scripts/vcsget.sh.in b/scripts/vcsget.sh.in
new file mode 100644
index 000..f8f6e4b
--- /dev/null
+++ b/scripts/vcsget.sh.in
@@ -0,0 +1,294 @@
+#!/bin/bash
+#
+# vcsget - downloader agent that generates tarballs from VCS repos
+# @configure_input@
+#
+# Copyright (c) 2012 Luke Shumaker
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+# gettext initialization
+export TEXTDOMAIN='pacman-scripts'
+export TEXTDOMAINDIR='@localedir@'
+
+unset CDPATH
+
+m4_include(library/output_format.sh)
+
+##
+# usage : in_array( $needle, $haystack )
+# return : 0 - found
+# 1 - not found
+##
+in_array() {
+ local needle=$1; shift
+ local item
+ for item in "$@"; do
+ [[ $item = "$needle" ]] && return 0 # Found
+ done
+ return 1 # Not Found
+}
+
+usage() {
+ echo "Usage: $0 "
+ echo ""
+ echo "Talks with multiple version control systems (VCSs) to create a"
+ echo "tarball of a specific commit."
+ echo ""
+ echo " is a plain, uncompressed tarball. Given the same"
+ echo "commit, the tarball will always have the same checksum."
+ echo ""
+ echo " is the repository, and possibly commit to download and tar."
+ echo "The following schemes are recognized:"
+ echo " * cvs://"
+ echo " * git://"
+ echo " * git+PROTO://"
+ echo " * svn+PROTO://"
+ echo " * bzr://"
+ echo " * hg+PROTO://"
+ echo ""
+ echo "URLs to be consumed by $0 are not always in the format of the"
+ echo "relevent VCS program, but normalized to proper URLs. Simply, this"
+ echo "means:"
+ echo ""
+ echo "scheme://[authinfo@]host[:port]/path[#fragment]"
+ echo ""
+ echo "Where is in the format \"username[:password]\" and"
+ echo " is the commit ID, branch, or tag to be returned."
+}
+
+##
+# Get the column "$2" from the space-delimited string "$1".
+# This has the advantage over `set -- $1` and `arr=($1)` that it separates on
+# a single space, instead of '\s+', which allows it to have empty columns.
+##
+get_field() {
+ local str=$1
+ local i=$2
+ echo "$str"|cut -d' ' -f $i
+}
+
+##
+# Parse a URL into parts, according to RFC 3986, "URI Generic Syntax".
+# Sets the variables `scheme`, `authority`, `path`, `query` and `fragment`.
+##
+parse_url() {
+ local url=$1
+
+ # This regex is copied from RFC 3986
+ local regex='^(([^:/?#]+):)?(//([^/?#]*))?([^?#]*)(\?([^#]*))?(#(.*))?'
+ local parts=$(echo "$url"|sed -r "s@$regex@\2 \4 \5 \7 \9@")
+
+ scheme=$( get_field "$parts" 1)
+ authority=$(get_field "$parts" 2)
+ path=$( get_field "$parts" 3)
+ query=$(
[pacman-dev] [PATCH 2/3] makepkg: do glob expansion in DLAGENTS maps
This allows vcs-* schemes to be used for vcsget.
Signed-off-by: Luke Shumaker
---
scripts/makepkg.sh.in | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index b30e9d0..f2a5008 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -394,16 +394,25 @@ source_has_signatures() {
return 1
}
+string_matches_glob() {
+ local string=$1
+ local glob=$2
+ case "$string" in
+ $glob) return 0;;
+ esac
+ return 1
+}
+
get_downloadclient() {
# $1 = URL with valid protocol prefix
local url=$1
local proto="${url%%://*}"
- # loop through DOWNLOAD_AGENTS variable looking for protocol
+ # loop through DLAGENTS (download agents) variable looking for protocol
local i
for i in "${DLAGENTS[@]}"; do
local handler="${i%%::*}"
- if [[ $proto = "$handler" ]]; then
+ if string_matches_glob "$proto" "$handler"; then
local agent="${i##*::}"
break
fi
--
1.7.12
[pacman-dev] [PATCH 2/3] makepkg: do glob expansion in DLAGENTS maps
This allows vcs-* schemes to be used for vcsget.
Signed-off-by: Luke Shumaker
---
scripts/makepkg.sh.in | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index b30e9d0..f2a5008 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -394,16 +394,25 @@ source_has_signatures() {
return 1
}
+string_matches_glob() {
+ local string=$1
+ local glob=$2
+ case "$string" in
+ $glob) return 0;;
+ esac
+ return 1
+}
+
get_downloadclient() {
# $1 = URL with valid protocol prefix
local url=$1
local proto="${url%%://*}"
- # loop through DOWNLOAD_AGENTS variable looking for protocol
+ # loop through DLAGENTS (download agents) variable looking for protocol
local i
for i in "${DLAGENTS[@]}"; do
local handler="${i%%::*}"
- if [[ $proto = "$handler" ]]; then
+ if string_matches_glob "$proto" "$handler"; then
local agent="${i##*::}"
break
fi
--
1.7.12
[pacman-dev] [PATCH 3/3] makepkg.conf: add vcsget DLAGENTS
Signed-off-by: Luke Shumaker
---
etc/makepkg.conf.in | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/etc/makepkg.conf.in b/etc/makepkg.conf.in
index dcec6f5..7d19dfb 100644
--- a/etc/makepkg.conf.in
+++ b/etc/makepkg.conf.in
@@ -12,7 +12,13 @@ DLAGENTS=('ftp::/usr/bin/curl -qfC - --ftp-pasv --retry 3
--retry-delay 3 -o %o
'http::/usr/bin/curl -qb "" -fLC - --retry 3 --retry-delay 3 -o %o
%u'
'https::/usr/bin/curl -qb "" -fLC - --retry 3 --retry-delay 3 -o %o
%u'
'rsync::/usr/bin/rsync --no-motd -z %u %o'
- 'scp::/usr/bin/scp -C %u %o')
+ 'scp::/usr/bin/scp -C %u %o'
+ 'cvs::/usr/bin/vcsget %u %o'
+ 'git::/usr/bin/vcsget %u %o'
+ 'git+*::/usr/bin/vcsget %u %o'
+ 'svn+*::/usr/bin/vcsget %u %o'
+ 'bzr::/usr/bin/vcsget %u %o'
+ 'hg+*::/usr/bin/vcsget %u %o')
# Other common tools:
# /usr/bin/snarf
--
1.7.12
Re: [pacman-dev] [PATCH v2] makepkg: Respect XDG_CONFIG_HOME
At Wed, 2 Jul 2014 17:04:13 +0200,
Johannes Löthberg wrote:
> If no $XDG_CONFIG_HOME/pacman/makepkg.conf file exists we fall back to
> sourcing $HOME/.makepkg.conf
>
> # Source user-specific makepkg.conf overrides, but only if no override config
> # file was specified
> -if [[ $MAKEPKG_CONF = "$confdir/makepkg.conf" && -r ~/.makepkg.conf ]]; then
> - source_safe ~/.makepkg.conf
> +XDG_PACMAN_DIR="${XDG_CONFIG_HOME:-$HOME/.config}/pacman"
> +if [[ "$MAKEPKG_CONF" = "$confdir/makepkg.conf" ]]; then
> + if [[ -r "$XDG_PACMAN_DIR/makepkg.conf" ]]; then
> + source_safe "$XDG_PACMAN_DIR/makepkg.conf"
> + elif [[ -r "$HOME/.makepkg.conf" ]]; then
> + source_safe "$HOME/.makepkg.conf"
> + fi
> fi
Is "falling back" the right thing? My gut sort of tells me that if
they both exist, they should both be loaded, but that's just me.
Happy hacking,
~ Luke Shumaker
[pacman-dev] [PATCH] makepkg: treat pkgrel more similarly to pkgver
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
Re: [pacman-dev] [PATCH] makepkg: treat pkgrel more similarly to pkgver
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
[pacman-dev] [PATCH] makepkg: Treat pkgrel more similarly to pkgver
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.0
[pacman-dev] [PATCH] pacman.conf: Fixup the XferCommand example for curl
From: Luke Shumaker 1. Without `-L`, curl doesn't follow redirects. This is different than both the default behavior of pacman, and from the wget example. So add `-L`. 2. It uses `-C -` to supposedly allow resuming partial downloads; but that doesn't work if we use `> %o` to direct the output to the file. Instead, use `-o %o` so that `-C -` actually works. Signed-off-by: Luke Shumaker --- etc/pacman.conf.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/etc/pacman.conf.in b/etc/pacman.conf.in index 53071e52..8e967fbb 100644 --- a/etc/pacman.conf.in +++ b/etc/pacman.conf.in @@ -16,7 +16,7 @@ #GPGDir = @sysconfdir@/pacman.d/gnupg/ #HookDir = @sysconfdir@/pacman.d/hooks/ HoldPkg = pacman glibc -#XferCommand = /usr/bin/curl -C - -f %u > %o +#XferCommand = /usr/bin/curl -L -C - -f -o %o %u #XferCommand = /usr/bin/wget --passive-ftp -c -O %o %u #CleanMethod = KeepInstalled #UseDelta= 0.7 -- 2.17.1
Re: [pacman-dev] [PATCH v2 1/3] configure: bump the minimum version of bash to 4.4
On Tue, 19 Jun 2018 16:26:33 -0400,
Eli Schwartz wrote:
> This is required in order to use declare -g and ${var@a}
Huh? It's needed for @a, but wasn't `declare -g` in Bash 4.2?
--
Happy hacking,
~ Luke Shumaker
[pacman-dev] [PATCH 1/3] makepkg: Better error messages for versions in (check, make, opt)depends/provides/conflicts
From: Luke Shumaker
Given the depends
depends=('foo>=1.2-1.par2')
and the error message
==> ERROR: pkgver in depends is not allowed to contain colons, forward
slashes, hyphens or whitespace.
One would be lead to believe that the problem is that they gave a pkgrel in
depends at all, not that the pkgrel contains letters.
Each of the (check,make,opt)depends, conflicts, and provides linters use a
glob to trim off properly formed epoch an rel from the full version string,
and pass the remainder to check_pkgver(). This does a good job of
accepting/rejecting full versions, but doesn't do a good job of generating
good error messages when rejecting if it's because of the epoch or rel.
1. Factor out check_epoch() and check_pkgrel() from lint_epoch() and
lint_pkgrel(), similarly to check_pkgver().
2. Add a check_fullpkgver() that takes a full [epoch:]ver[-rel] string and
splits it in to epoch/ver/rel, and calls the appropriate check_ function
on each.
3. Use check_fullpkgver() in the {,check,make,opt}depends, conflicts, and
provides linters.
---
scripts/Makefile.am | 1 +
.../lint_pkgbuild/checkdepends.sh.in | 10 ++--
.../libmakepkg/lint_pkgbuild/conflicts.sh.in | 10 ++--
.../libmakepkg/lint_pkgbuild/depends.sh.in| 14 ++---
scripts/libmakepkg/lint_pkgbuild/epoch.sh.in | 10 +++-
.../libmakepkg/lint_pkgbuild/fullpkgver.sh.in | 54 +++
.../lint_pkgbuild/makedepends.sh.in | 10 ++--
.../libmakepkg/lint_pkgbuild/optdepends.sh.in | 10 ++--
scripts/libmakepkg/lint_pkgbuild/pkgrel.sh.in | 20 +--
.../libmakepkg/lint_pkgbuild/provides.sh.in | 10 ++--
10 files changed, 106 insertions(+), 43 deletions(-)
create mode 100644 scripts/libmakepkg/lint_pkgbuild/fullpkgver.sh.in
diff --git a/scripts/Makefile.am b/scripts/Makefile.am
index f759e149..7cf8bed0 100644
--- a/scripts/Makefile.am
+++ b/scripts/Makefile.am
@@ -73,6 +73,7 @@ LIBMAKEPKG_IN = \
libmakepkg/lint_pkgbuild/conflicts.sh \
libmakepkg/lint_pkgbuild/depends.sh \
libmakepkg/lint_pkgbuild/epoch.sh \
+ libmakepkg/lint_pkgbuild/fullpkgver.sh \
libmakepkg/lint_pkgbuild/install.sh \
libmakepkg/lint_pkgbuild/makedepends.sh \
libmakepkg/lint_pkgbuild/optdepends.sh \
diff --git a/scripts/libmakepkg/lint_pkgbuild/checkdepends.sh.in
b/scripts/libmakepkg/lint_pkgbuild/checkdepends.sh.in
index d5648bd4..0a9ddf67 100644
--- a/scripts/libmakepkg/lint_pkgbuild/checkdepends.sh.in
+++ b/scripts/libmakepkg/lint_pkgbuild/checkdepends.sh.in
@@ -23,8 +23,8 @@ LIBMAKEPKG_LINT_PKGBUILD_CHECKDEPENDS_SH=1
LIBRARY=${LIBRARY:-'@libmakepkgdir@'}
+source "$LIBRARY/lint_pkgbuild/fullpkgver.sh"
source "$LIBRARY/lint_pkgbuild/pkgname.sh"
-source "$LIBRARY/lint_pkgbuild/pkgver.sh"
source "$LIBRARY/util/message.sh"
source "$LIBRARY/util/pkgbuild.sh"
@@ -43,12 +43,10 @@ lint_checkdepends() {
for checkdepend in "${checkdepends_list[@]}"; do
name=${checkdepend%%@(<|>|=|>=|<=)*}
- # remove optional epoch in version specifier
- ver=${checkdepend##$name@(<|>|=|>=|<=)?(+([0-9]):)}
lint_one_pkgname checkdepends "$name" || ret=1
- if [[ $ver != $checkdepend ]]; then
- # remove optional pkgrel in version specifier
- check_pkgver "${ver%-+([0-9])?(.+([0-9]))}"
checkdepends || ret=1
+ if [[ $name != $checkdepend ]]; then
+ ver=${checkdepend##$name@(<|>|=|>=|<=)}
+ check_fullpkgver "$ver" checkdepends || ret=1
fi
done
diff --git a/scripts/libmakepkg/lint_pkgbuild/conflicts.sh.in
b/scripts/libmakepkg/lint_pkgbuild/conflicts.sh.in
index a18c25fa..b61459e1 100644
--- a/scripts/libmakepkg/lint_pkgbuild/conflicts.sh.in
+++ b/scripts/libmakepkg/lint_pkgbuild/conflicts.sh.in
@@ -23,8 +23,8 @@ LIBMAKEPKG_LINT_PKGBUILD_CONFLICTS_SH=1
LIBRARY=${LIBRARY:-'@libmakepkgdir@'}
+source "$LIBRARY/lint_pkgbuild/fullpkgver.sh"
source "$LIBRARY/lint_pkgbuild/pkgname.sh"
-source "$LIBRARY/lint_pkgbuild/pkgver.sh"
source "$LIBRARY/util/message.sh"
source "$LIBRARY/util/pkgbuild.sh"
@@ -43,12 +43,10 @@ lint_conflicts() {
for conflict in "${conflicts_list[@]}"; do
name=${conflict%%@(<|>|=|>=|<=)*}
- # remove optional epoch in version specifier
- ver=${conflict##$name@(<|>|=|>=|<=)?(+([0-9]):)}
lint_one_pkgname conflicts "$name" || ret=1
- if [[ $ver != $conflict ]]; then
- # remove optional pkgrel in version specifier
- check_pkgver "${ver%-+([0
[pacman-dev] [PATCH 3/3] makepkg: check_pkgver: Report what the bad pkgver is
From: Luke Shumaker
For consistency with check_epoch and check_pkgrel.
I think that this is important because if there are multiple
provides/depends/whatever that include a version, and one of them is
malformed, including the bad version in the error message identified
which one is the problem.
---
scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in
b/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in
index c105212b..65216b64 100644
--- a/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in
+++ b/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in
@@ -38,7 +38,7 @@ check_pkgver() {
fi
if [[ $ver = *[[:space:]/:-]* ]]; then
- error "$(gettext "%s is not allowed to contain colons, forward
slashes, hyphens or whitespace.")" "pkgver${type:+ in $type}"
+ error "$(gettext "%s is not allowed to contain colons, forward
slashes, hyphens or whitespace; got %s.")" "pkgver${type:+ in $type}" "$ver"
return 1
fi
}
--
2.18.0
[pacman-dev] [PATCH 0/3] makepkg: Improve lint_pkgbuild error messages for epoch/ver/rel
From: Luke Shumaker The error messages that makepkg prints when encountering a bad epoch/pkgver/pkgrel are often somewhat misleading. These patchset tries to improve that. The behavior is intended to be unchanged, other than what the messages say. Luke Shumaker (3): makepkg: Better error messages for versions in (check,make,opt)depends/provides/conflicts makepkg: check_pkgrel: Don't say "decimal" in the error message makepkg: check_pkgver: Report what the bad pkgver is scripts/Makefile.am | 1 + .../lint_pkgbuild/checkdepends.sh.in | 10 ++-- .../lint_pkgbuild/conflicts.sh.in | 10 ++-- .../lint_pkgbuild/depends.sh.in | 14 ++--- .../lint_pkgbuild/epoch.sh.in | 10 +++- .../lint_pkgbuild/fullpkgver.sh.in| 54 +++ .../lint_pkgbuild/makedepends.sh.in | 10 ++-- .../lint_pkgbuild/optdepends.sh.in| 10 ++-- .../lint_pkgbuild/pkgrel.sh.in| 20 +-- .../lint_pkgbuild/pkgver.sh.in| 2 +- .../lint_pkgbuild/provides.sh.in | 10 ++-- 11 files changed, 107 insertions(+), 44 deletions(-) create mode 100644 scripts/libmakepkg/lint_pkgbuild/fullpkgver.sh.in -- Happy hacking, ~ Luke Shumaker 2.18.0
[pacman-dev] [PATCH 2/3] makepkg: check_pkgrel: Don't say "decimal" in the error message
From: Luke Shumaker
If you have a malformed pkgrel, the error message says that it must be a
"decimal". That isn't quite true, as that would mean that `1.1 == 1.10`.
---
scripts/libmakepkg/lint_pkgbuild/pkgrel.sh.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/libmakepkg/lint_pkgbuild/pkgrel.sh.in
b/scripts/libmakepkg/lint_pkgbuild/pkgrel.sh.in
index 762f054a..30fa6d71 100644
--- a/scripts/libmakepkg/lint_pkgbuild/pkgrel.sh.in
+++ b/scripts/libmakepkg/lint_pkgbuild/pkgrel.sh.in
@@ -37,7 +37,7 @@ check_pkgrel() {
fi
if [[ $rel != +([0-9])?(.+([0-9])) ]]; then
- error "$(gettext "%s must be a decimal, not %s.")"
"pkgrel${type:+ in $type}" "$rel"
+ error "$(gettext "%s must be of the form 'integer[.integer]',
not %s.")" "pkgrel${type:+ in $type}" "$rel"
return 1
fi
}
--
2.18.0
Re: [pacman-dev] [PATCH 1/3] makepkg: Better error messages for versions in (check, make, opt)depends/provides/conflicts
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
[pacman-dev] [PATCH v2 0/3] makepkg: Improve lint_pkgbuild error messages for epoch/ver/rel
From: Luke Shumaker The error messages that makepkg prints when encountering a bad epoch/pkgver/pkgrel are often somewhat misleading. These patchset tries to improve that. The behavior is intended to be unchanged, other than what the messages say. v2: - Don't skip lint_pkgrel if (( PKGVERFUNC )), I should have never included that bit in v1. - check_fullpkgver: Don't let rel strip ver down to nothing Luke Shumaker (3): makepkg: Better error messages for versions in (check,make,opt)depends/provides/conflicts makepkg: check_pkgrel: Don't say "decimal" in the error message makepkg: check_pkgver: Report what the bad pkgver is scripts/Makefile.am | 1 + .../lint_pkgbuild/checkdepends.sh.in | 10 ++-- .../libmakepkg/lint_pkgbuild/conflicts.sh.in | 10 ++-- .../libmakepkg/lint_pkgbuild/depends.sh.in| 14 ++--- scripts/libmakepkg/lint_pkgbuild/epoch.sh.in | 10 +++- .../libmakepkg/lint_pkgbuild/fullpkgver.sh.in | 58 +++ .../lint_pkgbuild/makedepends.sh.in | 10 ++-- .../libmakepkg/lint_pkgbuild/optdepends.sh.in | 10 ++-- scripts/libmakepkg/lint_pkgbuild/pkgrel.sh.in | 15 +++-- scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in | 2 +- .../libmakepkg/lint_pkgbuild/provides.sh.in | 10 ++-- 11 files changed, 106 insertions(+), 44 deletions(-) create mode 100644 scripts/libmakepkg/lint_pkgbuild/fullpkgver.sh.in -- Happy hacking, ~ Luke Shumaker 2.18.0
[pacman-dev] [PATCH v2 1/3] makepkg: Better error messages for versions in (check, make, opt)depends/provides/conflicts
From: Luke Shumaker
Given the depends
depends=('foo>=1.2-1.par2')
and the error message
==> ERROR: pkgver in depends is not allowed to contain colons, forward
slashes, hyphens or whitespace.
One would be lead to believe that the problem is that they gave a pkgrel in
depends at all, not that the pkgrel contains letters.
Each of the (check,make,opt)depends, conflicts, and provides linters use a
glob to trim off properly formed epoch an rel from the full version string,
and pass the remainder to check_pkgver(). This does a good job of
accepting/rejecting full versions, but doesn't do a good job of generating
good error messages when rejecting if it's because of the epoch or rel.
1. Factor out check_epoch() and check_pkgrel() from lint_epoch() and
lint_pkgrel(), similarly to check_pkgver().
2. Add a check_fullpkgver() that takes a full [epoch:]ver[-rel] string and
splits it in to epoch/ver/rel, and calls the appropriate check_ function
on each.
3. Use check_fullpkgver() in the {,check,make,opt}depends, conflicts, and
provides linters.
---
v2:
- Don't skip lint_pkgrel if (( PKGVERFUNC ))
- check_fullpkgver: Don't let rel strip ver down to nothing
scripts/Makefile.am | 1 +
.../lint_pkgbuild/checkdepends.sh.in | 10 ++--
.../libmakepkg/lint_pkgbuild/conflicts.sh.in | 10 ++--
.../libmakepkg/lint_pkgbuild/depends.sh.in| 14 ++---
scripts/libmakepkg/lint_pkgbuild/epoch.sh.in | 10 +++-
.../libmakepkg/lint_pkgbuild/fullpkgver.sh.in | 58 +++
.../lint_pkgbuild/makedepends.sh.in | 10 ++--
.../libmakepkg/lint_pkgbuild/optdepends.sh.in | 10 ++--
scripts/libmakepkg/lint_pkgbuild/pkgrel.sh.in | 15 +++--
.../libmakepkg/lint_pkgbuild/provides.sh.in | 10 ++--
10 files changed, 105 insertions(+), 43 deletions(-)
create mode 100644 scripts/libmakepkg/lint_pkgbuild/fullpkgver.sh.in
diff --git a/scripts/Makefile.am b/scripts/Makefile.am
index f759e149..7cf8bed0 100644
--- a/scripts/Makefile.am
+++ b/scripts/Makefile.am
@@ -73,6 +73,7 @@ LIBMAKEPKG_IN = \
libmakepkg/lint_pkgbuild/conflicts.sh \
libmakepkg/lint_pkgbuild/depends.sh \
libmakepkg/lint_pkgbuild/epoch.sh \
+ libmakepkg/lint_pkgbuild/fullpkgver.sh \
libmakepkg/lint_pkgbuild/install.sh \
libmakepkg/lint_pkgbuild/makedepends.sh \
libmakepkg/lint_pkgbuild/optdepends.sh \
diff --git a/scripts/libmakepkg/lint_pkgbuild/checkdepends.sh.in
b/scripts/libmakepkg/lint_pkgbuild/checkdepends.sh.in
index d5648bd4..0a9ddf67 100644
--- a/scripts/libmakepkg/lint_pkgbuild/checkdepends.sh.in
+++ b/scripts/libmakepkg/lint_pkgbuild/checkdepends.sh.in
@@ -23,8 +23,8 @@ LIBMAKEPKG_LINT_PKGBUILD_CHECKDEPENDS_SH=1
LIBRARY=${LIBRARY:-'@libmakepkgdir@'}
+source "$LIBRARY/lint_pkgbuild/fullpkgver.sh"
source "$LIBRARY/lint_pkgbuild/pkgname.sh"
-source "$LIBRARY/lint_pkgbuild/pkgver.sh"
source "$LIBRARY/util/message.sh"
source "$LIBRARY/util/pkgbuild.sh"
@@ -43,12 +43,10 @@ lint_checkdepends() {
for checkdepend in "${checkdepends_list[@]}"; do
name=${checkdepend%%@(<|>|=|>=|<=)*}
- # remove optional epoch in version specifier
- ver=${checkdepend##$name@(<|>|=|>=|<=)?(+([0-9]):)}
lint_one_pkgname checkdepends "$name" || ret=1
- if [[ $ver != $checkdepend ]]; then
- # remove optional pkgrel in version specifier
- check_pkgver "${ver%-+([0-9])?(.+([0-9]))}"
checkdepends || ret=1
+ if [[ $name != $checkdepend ]]; then
+ ver=${checkdepend##$name@(<|>|=|>=|<=)}
+ check_fullpkgver "$ver" checkdepends || ret=1
fi
done
diff --git a/scripts/libmakepkg/lint_pkgbuild/conflicts.sh.in
b/scripts/libmakepkg/lint_pkgbuild/conflicts.sh.in
index a18c25fa..b61459e1 100644
--- a/scripts/libmakepkg/lint_pkgbuild/conflicts.sh.in
+++ b/scripts/libmakepkg/lint_pkgbuild/conflicts.sh.in
@@ -23,8 +23,8 @@ LIBMAKEPKG_LINT_PKGBUILD_CONFLICTS_SH=1
LIBRARY=${LIBRARY:-'@libmakepkgdir@'}
+source "$LIBRARY/lint_pkgbuild/fullpkgver.sh"
source "$LIBRARY/lint_pkgbuild/pkgname.sh"
-source "$LIBRARY/lint_pkgbuild/pkgver.sh"
source "$LIBRARY/util/message.sh"
source "$LIBRARY/util/pkgbuild.sh"
@@ -43,12 +43,10 @@ lint_conflicts() {
for conflict in "${conflicts_list[@]}"; do
name=${conflict%%@(<|>|=|>=|<=)*}
- # remove optional epoch in version specifier
- ver=${conflict##$name@(<|>|=|>=|<=)?(+([0-9]):)}
lint_one_pkgname conflicts "$name" || ret=1
- if [[ $ver
[pacman-dev] [PATCH v2 2/3] makepkg: check_pkgrel: Don't say "decimal" in the error message
From: Luke Shumaker
If you have a malformed pkgrel, the error message says that it must be a
"decimal". That isn't quite true, as that would mean that `1.1 == 1.10`.
---
scripts/libmakepkg/lint_pkgbuild/pkgrel.sh.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/libmakepkg/lint_pkgbuild/pkgrel.sh.in
b/scripts/libmakepkg/lint_pkgbuild/pkgrel.sh.in
index 5dc9d3a7..cc2c0f40 100644
--- a/scripts/libmakepkg/lint_pkgbuild/pkgrel.sh.in
+++ b/scripts/libmakepkg/lint_pkgbuild/pkgrel.sh.in
@@ -37,7 +37,7 @@ check_pkgrel() {
fi
if [[ $rel != +([0-9])?(.+([0-9])) ]]; then
- error "$(gettext "%s must be a decimal, not %s.")"
"pkgrel${type:+ in $type}" "$rel"
+ error "$(gettext "%s must be of the form 'integer[.integer]',
not %s.")" "pkgrel${type:+ in $type}" "$rel"
return 1
fi
}
--
2.18.0
[pacman-dev] [PATCH v2 3/3] makepkg: check_pkgver: Report what the bad pkgver is
From: Luke Shumaker
For consistency with check_epoch and check_pkgrel.
I think that this is important because if there are multiple
provides/depends/whatever that include a version, and one of them is
malformed, including the bad version in the error message identified
which one is the problem.
---
scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in
b/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in
index c105212b..65216b64 100644
--- a/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in
+++ b/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in
@@ -38,7 +38,7 @@ check_pkgver() {
fi
if [[ $ver = *[[:space:]/:-]* ]]; then
- error "$(gettext "%s is not allowed to contain colons, forward
slashes, hyphens or whitespace.")" "pkgver${type:+ in $type}"
+ error "$(gettext "%s is not allowed to contain colons, forward
slashes, hyphens or whitespace; got %s.")" "pkgver${type:+ in $type}" "$ver"
return 1
fi
}
--
2.18.0
[pacman-dev] [PATCH] makepkg: lint_pkgver: Run even if PKGVERFUNC
From: Luke Shumaker
lint_pkgver returns 0 if PKGVERFUNC, since it's likely that update_pkgver()
will change the value of pkgver anyway, and there's no point in linting the
old value. update_pkgver() will call check_pkgver() itself to validate the
new value.
However, that "optimization" only holds if we're definitely going to call
update_pkgver() later; and that's way more complicated than
if (( PKGVERFUNC )); then
it's more like:
if (( !GENINTEG && !PACKAGELIST && !PRINTSRCINFO && !SOURCEONLY && !REPKG
&& PKGVERFUNC )); then
Which is to say: If I have a PKGBUILD with pkgver():
* if I run `makepkg -g` I expect it to lint pkgver, but it won't
* if I run `makepkg -R` I expect it to lint pkgver, but it won't
* ...
So let's fix that.
Rather than try to keep a huge list of conditions in sync with the flow of
makepkg.sh.in, let's just drop it. As far as I can tell, the only thing
that skipping lint_pkgver() really enables is letting the PKGBUILD author
write `pkgver=` in the initial version, and letting pkgver() fill it in.
They can just start writing `pkgver=0` for that workflow.
---
scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in | 5 -
1 file changed, 5 deletions(-)
diff --git a/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in
b/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in
index 65216b64..e9b1566f 100644
--- a/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in
+++ b/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in
@@ -44,10 +44,5 @@ check_pkgver() {
}
lint_pkgver() {
- if (( PKGVERFUNC )); then
- # defer check to after getting version from pkgver function
- return 0
- fi
-
check_pkgver "$pkgver"
}
--
2.18.0
[pacman-dev] [PATCH] makepkg: Handle errors when canonicalizing paths
From: Luke Shumaker
If you don't have adequate permission to `cd` to one of the DEST
directories, it's clear that that something's not doing error handling
right:
$ install -d -m srcdest
$ SRCDEST=$PWD/srcdest makepkg
/usr/share/makepkg/util/util.sh: line 75: cd: /.../srcdest: Permission
denied
==> ERROR: Failed to change to directory /.../srcdest
Aborting...
==> ERROR: Failed to create the directory $SRCDEST ().
Aborting...
The first error comes from cd_safe() inside of canonicalize_path().
Because that bit runs in a subshell, it doesn't actually abort right there.
Then, makepkg.sh doesn't check for failures from canonicalize_path. So
then the variable gets set to an empty string (which makes the next error
message from ensure_writable_dir() look funny), and keeps going.
As an additional "fun" quirk, canonicalize_path doesn't canonicalize it if
ensure_writable_dir has any work to do. That's probably not intended, but
I'm not sure it is wrong either (see below).
1. Combine canonicalize_path() and ensure_writable_dir() in to
canonicalize_and_ensure_writable_dir(). Both are only ever called on
exactly the same variables (except that ensure_writable_dir() might
skip PKGDEST, SRCPKGDEST, or LOGDEST; depending on the flags).
2. Use plain `cd` and check the status ourselves, rather than `cd_safe`.
We can come up with a better (context-aware) error message than
`cd_safe`, and we avoid duplicating the "Aborting..." message.
This has a few consequences:
a. We no longer canonicalize dirpaths that we don't ensure are writable.
That's probably safe. This means that makepkg might now error in fewer
cases; for example:
$ install -d -m pkgdest
$ PKGDEST=$PWD/pkgdest makepkg -g
There's no reason for it to care that it can't `cd` to PKGDEST; it
won't be using it.
b. We now canonicalize paths that we create that didn't exist before.
That said, I'm not sure why we ever need to canonicalize the paths; I'm not
sure why c0f58ea was necessary even after 960c2cd was already applied.
---
scripts/libmakepkg/util/util.sh.in | 31 +++---
scripts/makepkg.sh.in | 14 +++---
2 files changed, 18 insertions(+), 27 deletions(-)
diff --git a/scripts/libmakepkg/util/util.sh.in
b/scripts/libmakepkg/util/util.sh.in
index e1ca5cb7..1dfe6da9 100644
--- a/scripts/libmakepkg/util/util.sh.in
+++ b/scripts/libmakepkg/util/util.sh.in
@@ -49,20 +49,6 @@ is_array() {
return $ret
}
-# Canonicalize a directory path if it exists
-canonicalize_path() {
- local path="$1";
-
- if [[ -d $path ]]; then
- (
- cd_safe "$path"
- pwd -P
- )
- else
- printf "%s\n" "$path"
- fi
-}
-
dir_is_empty() {
(
shopt -s dotglob nullglob
@@ -79,10 +65,10 @@ cd_safe() {
fi
}
-# Try to create directory if one does not yet exist. Fails if the directory
-# exists but has no write permissions, or if there is an existing file with
-# the same name.
-ensure_writable_dir() {
+# Try to create directory if one does not yet exist, and prints the
+# canonical path to the directory. Fails if the directory exists but has no
+# write permissions, or if there is an existing file with the same name.
+canonicalize_and_ensure_writable_dir() {
local dirtype="$1" dirpath="$2"
if ! mkdir -p "$dirpath" 2>/dev/null; then
@@ -92,6 +78,11 @@ ensure_writable_dir() {
error "$(gettext "You do not have write permission for the
directory \$%s (%s).")" "$dirtype" "$dirpath"
return 1
fi
-
- return 0
+ (
+ if ! cd "$dirpath"; then
+ error "$(gettext "Could not canonicalize the directory
\$%s (%s).")" "$dirtype" "$dirpath"
+ return 1
+ fi
+ pwd -P
+ )
}
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 8ee0f5c5..62c2db80 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -1353,9 +1353,9 @@ if (( ${#extra_environment[*]} )); then
export "${extra_environment[@]}"
fi
-# canonicalize paths and provide defaults if anything is still undefined
+# provide defaults if anything is still undefined
for var in PKGDEST SRCDEST SRCPKGDEST LOGDEST BUILDDIR; do
- printf -v "$var" "$(canonicalize_path "${!var:-$startdir}")"
+ printf -v "$var" "${!var:-$startdir}"
done
unset var
PACKAGER=${PACKAGER:-"Unknown Packager"}
@@ -1378,23 +1378,23 @@ lint_
Re: [pacman-dev] [PATCH 3/3] makepkg: fix pkgver() function not aborting on errors
On Mon, 13 Aug 2018 21:20:58 -0400,
Eli Schwartz wrote:
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index bb24c633..1ab2ea3c 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -188,6 +188,9 @@ enter_fakeroot() {
> # Re-sources the PKGBUILD afterwards to allow for other variables that use
> $pkgver
> update_pkgver() {
> newpkgver=$(run_function_safe pkgver)
> + if (( $? != 0 )); then
> + error_function pkgver
> + fi
Why bring $? in to it, why not:
if ! newpkgver=$(run_function_safe pkgver); then
error_function pkgver
fi
--
Happy hacking,
~ Luke Shumaker
Re: [pacman-dev] [PATCH] makepkg: use builtin globbing to print files in package
On Tue, 21 Aug 2018 10:15:12 -0400,
Eli Schwartz wrote:
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index ae1ef01b..1325b019 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -711,10 +711,14 @@ write_buildinfo() {
> # database files are placed at the beginning of the package regardless of
> # sorting
> list_package_files() {
> - (find . -path './.*' \! -name '.'; find . \! -path './.*' \! -name '.'
> | LC_ALL=C sort) |
> - sed -e 's|^\./||' | tr '\n' '\0'
> + (
> + export LC_COLLATE=C
> + shopt -s dotglob globstar
> + printf '%s\0' **
> + )
Since globbing is done in the same process, it should be sufficient to
set LC_COLLATE; no need to export it.
--
Happy hacking,
~ Luke Shumaker
[pacman-dev] [PATCH] makepkg: Fix whirlpoolsums support
From: Luke Shumaker
Commit 9cdfd187 introduced support for whirlpool checksums in v5.0.0.
However, it was sloppy and missed several places where the list of
checksums is used. So fix that. In several places, we can take advantage
of the 'known_hash_algos' variable to simplify things a bit.
Commit 57770125 switched from using OpenSSL to GNU coreutils for doing the
checksums in v5.1.0. This broke the whirlpool support, as coreutils does
not implement a 'whirlpoolsum' program. So go back to using openssl for
whirlpool sums only.
---
I'm not particularly attached to whirlpool support, and if your
reaction is "let's formally drop whirlpool", I wouldn't be upset by
that.
A handful (15) of Parabola's PKGBUILDs use whirlpoolsums, which makes
sense, because the author if the original whirlpoolsums commit is a
Parabola contributor. But, if you want to drop whirlpool, I have no
problem saying that those packages need to migrate to a different
checksum algorithm at their next update.
doc/PKGBUILD.5.asciidoc | 2 +-
etc/makepkg.conf.in | 2 +-
.../integrity/generate_checksum.sh.in | 12 +--
.../integrity/verify_checksum.sh.in | 13 ++--
.../libmakepkg/lint_pkgbuild/variable.sh.in | 4 ++--
scripts/libmakepkg/srcinfo.sh.in | 4 ++--
scripts/makepkg.sh.in | 20 +--
7 files changed, 41 insertions(+), 16 deletions(-)
diff --git a/doc/PKGBUILD.5.asciidoc b/doc/PKGBUILD.5.asciidoc
index f12effde..5715c7aa 100644
--- a/doc/PKGBUILD.5.asciidoc
+++ b/doc/PKGBUILD.5.asciidoc
@@ -154,7 +154,7 @@ contain whitespace characters.
be skipped. To easily generate md5sums, run ``makepkg -g >> PKGBUILD''.
If desired, move the md5sums line to an appropriate location.
-*sha1sums, sha224sums, sha256sums, sha384sums, sha512sums (arrays)*::
+*sha1sums, sha224sums, sha256sums, sha384sums, sha512sums, whirlpoolsums
(arrays)*::
Alternative integrity checks that makepkg supports; these all behave
similar to the md5sums option described above. To enable use and
generation
of these checksums, be sure to set up the `INTEGRITY_CHECK` option in
diff --git a/etc/makepkg.conf.in b/etc/makepkg.conf.in
index b0fdea9e..22d298f7 100644
--- a/etc/makepkg.conf.in
+++ b/etc/makepkg.conf.in
@@ -87,7 +87,7 @@ BUILDENV=(!distcc color !ccache check !sign)
#
OPTIONS=(strip docs libtool staticlibs emptydirs zipman purge !debug)
-#-- File integrity checks to use. Valid: md5, sha1, sha224, sha256, sha384,
sha512
+#-- File integrity checks to use. Valid: md5, sha1, sha224, sha256, sha384,
sha512, whirlpool
INTEGRITY_CHECK=(md5)
#-- Options to be used when stripping binaries. See `man strip' for details.
STRIP_BINARIES="@STRIP_BINARIES@"
diff --git a/scripts/libmakepkg/integrity/generate_checksum.sh.in
b/scripts/libmakepkg/integrity/generate_checksum.sh.in
index eb9b74fc..7480c0ee 100644
--- a/scripts/libmakepkg/integrity/generate_checksum.sh.in
+++ b/scripts/libmakepkg/integrity/generate_checksum.sh.in
@@ -59,8 +59,16 @@ generate_one_checksum() {
if [[ ${netfile%%::*} != *.@(sig?(n)|asc) ]];
then
local file
file="$(get_filepath "$netfile")" ||
missing_source_file "$netfile"
- sum="$("${integ}sum" "$file")"
- sum=${sum%% *}
+ case "$integ" in
+
md5|sha1|sha224|sha256|sha384|sha512)
+ sum="$("${integ}sum"
"$file")"
+ sum=${sum%% *}
+ ;;
+ whirlpool)
+ sum="$(openssl dgst
-${integ} "$file")"
+ sum=${sum##* }
+ ;;
+ esac
else
sum="SKIP"
fi
diff --git a/scripts/libmakepkg/integrity/verify_checksum.sh.in
b/scripts/libmakepkg/integrity/verify_checksum.sh.in
index 532e0693..6cd30ab4 100644
--- a/scripts/libmakepkg/integrity/verify_checksum.sh.in
+++ b/scripts/libmakepkg/integrity/verify_checksum.sh.in
@@ -82,8 +82,17 @@ verify_integrity_one() {
return 1
fi
- local realsum="$("${integ}sum" "$file")&
Re: [pacman-dev] [PATCH] makepkg: Fix whirlpoolsums support
On Mon, 27 Aug 2018 16:17:31 -0400, Eli Schwartz wrote: > Huh, and we never documented that we supported it in the first place. :/ It did show up in the NEWS file, but no where else. > No wonder we didn't notice that this would break, and, equally, no > wonder users didn't hit this in the 2.5 years since 5.0.0 was tagged... > > But, if we're going to support whirlpool then that means, going against > the original intent of the patch which broke this, that we now need the > openssl command-line tool even if built --with-crypto=nettle, because it > doesn't look like nettle supports whirlpool any more than base64. It should only need the openssl command-line tool if it's actually going to use whirlpool for a given PKGBUILD (i.e., and optdepend at best). -- Happy hacking, ~ Luke Shumaker
[pacman-dev] [PATCH] makepkg: Pass --stream to `hg clone` when creating the working copy
From: Luke Shumaker
Without --stream, `hg clone` reencodes+recompresses the entire repository,
to the storage settings of the host. But download_hg() already did that
on the initial network clone, and it is 100% pointless duplicated work for
the local clone.
The work that this saves is CPU-bound (not disk-bound), and is restricted
to a single core.
The --stream flag has only existed since Mercurial 4.4 (2017-11-01). Prior
to that, it was named --uncompressed. --uncompressed still exists as a
compatibility alias for --stream, and marked deprecated, though there is
currently no schedule for its removal.
---
scripts/libmakepkg/source/hg.sh.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/libmakepkg/source/hg.sh.in
b/scripts/libmakepkg/source/hg.sh.in
index ae9aed3b..7346e1e3 100644
--- a/scripts/libmakepkg/source/hg.sh.in
+++ b/scripts/libmakepkg/source/hg.sh.in
@@ -94,7 +94,7 @@ extract_hg() {
plain "$(gettext "Aborting...")"
exit 1
fi
- elif ! hg clone -u "$ref" "$dir" "${dir##*/}"; then
+ elif ! hg clone -u "$ref" --stream "$dir" "${dir##*/}"; then
error "$(gettext "Failure while creating working copy of %s %s
repo")" "${repo}" "hg"
plain "$(gettext "Aborting...")"
exit 1
--
2.18.0
Re: [pacman-dev] [PATCH] makepkg: Pass --stream to `hg clone` when creating the working copy
On Sat, 08 Sep 2018 16:31:16 -0400, Luke Shumaker wrote: > From: Luke Shumaker > > Without --stream, `hg clone` reencodes+recompresses the entire repository, > to the storage settings of the host. But download_hg() already did that > on the initial network clone, and it is 100% pointless duplicated work for > the local clone. > > The work that this saves is CPU-bound (not disk-bound), and is restricted > to a single core. After more testing, this didn't have the speed-up that I expected. Consider the patch withdrawn. -- Happy hacking, ~ Luke Shumaker
