[pacman-dev] [PATCH 00/11] Support VCS URLs.
At the request of Dave, here are the updated versions of patches sent to the list earlier. Support for the following needs readded: CVS, Mercurial, Darcs and Bazaar Known issue: the pkgver check needs adjusted when using an auto-updating pkgver Allan McRae (11): makepkg: remove VCS package support makepkg: reorder source handling functions makepkg: add function to return download protocol makepkg: generalize download_sources makepkg: skip integrity checking early makepkg: allow using GIT source URLs makepkg: fix checksum generation with VCS sources makepkg: modify get_filename to handle VCS sources makepkg: modify get_filepath to handle VCS sources makepkg: provide mechanism for auto-updating pkgver makepkg: add support for SVN urls doc/PKGBUILD.5.txt | 64 ----- doc/makepkg.8.txt | 10 - scripts/makepkg.sh.in | 682 +++++++++++++++++++++++++++++--------------------- 3 files changed, 394 insertions(+), 362 deletions(-) -- 1.7.11.1
The current VCS packaging support is really, really, really bad.
It is best to strip it out completely before rewriting it.
Signed-off-by: Allan McRae
On Tue, Jun 26, 2012 at 5:58 PM, Allan McRae
The current VCS packaging support is really, really, really bad. It is best to strip it out completely before rewriting it.
Signed-off-by: Allan McRae
--- doc/PKGBUILD.5.txt | 64 ---------------------- doc/makepkg.8.txt | 10 ---- scripts/makepkg.sh.in | 145 +++----------------------------------------------- 3 files changed, 6 insertions(+), 213 deletions(-) +1 as far as this approach goes.
There is no actual code change here, but these related functions
were all over the place which makes this code difficult to adjust.
Signed-off-by: Allan McRae
Extract the download protocol from a source entry. Returns "local"
for local source files.
Signed-off-by: Allan McRae
On Wed, Jun 27, 2012 at 08:58:09AM +1000, Allan McRae wrote:
Extract the download protocol from a source entry. Returns "local" for local source files.
Signed-off-by: Allan McRae
--- scripts/makepkg.sh.in | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 4299816..0d87cba 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -222,6 +222,17 @@ get_url() { printf "%s\n" "${1#*::}" }
+# extract the protocol from a source entry - return "local" for local sources +get_protocol() { + if [[ $1 = *://* ]]; then + # strip leading filename + local proto="${1##*::}" + printf "%s\n" "${proto%%://*}"
While we're here, should we catch file:// protos and return 'local' as well? It would save the end user from having to define file:// as a DLAGENT.
+ else + printf "%s\n" local + fi +} + get_downloadclient() { # $1 = URL with valid protocol prefix local url=$1 -- 1.7.11.1
On Tue, Jun 26, 2012 at 6:51 PM, Dave Reisner
On Wed, Jun 27, 2012 at 08:58:09AM +1000, Allan McRae wrote:
Extract the download protocol from a source entry. Returns "local" for local source files.
Signed-off-by: Allan McRae
--- scripts/makepkg.sh.in | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 4299816..0d87cba 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -222,6 +222,17 @@ get_url() { printf "%s\n" "${1#*::}" }
+# extract the protocol from a source entry - return "local" for local sources +get_protocol() { + if [[ $1 = *://* ]]; then + # strip leading filename + local proto="${1##*::}" + printf "%s\n" "${proto%%://*}"
While we're here, should we catch file:// protos and return 'local' as well? It would save the end user from having to define file:// as a DLAGENT.
Shouldn't we just return "file" instead of make up a "local" protocol, while we're on this train of thought? -Dan
On 27/06/12 11:14, Dan McGee wrote:
On Tue, Jun 26, 2012 at 6:51 PM, Dave Reisner
wrote: On Wed, Jun 27, 2012 at 08:58:09AM +1000, Allan McRae wrote:
Extract the download protocol from a source entry. Returns "local" for local source files.
Signed-off-by: Allan McRae
--- scripts/makepkg.sh.in | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 4299816..0d87cba 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -222,6 +222,17 @@ get_url() { printf "%s\n" "${1#*::}" }
+# extract the protocol from a source entry - return "local" for local sources +get_protocol() { + if [[ $1 = *://* ]]; then + # strip leading filename + local proto="${1##*::}" + printf "%s\n" "${proto%%://*}"
While we're here, should we catch file:// protos and return 'local' as well? It would save the end user from having to define file:// as a DLAGENT.
Shouldn't we just return "file" instead of make up a "local" protocol, while we're on this train of thought?
Hmm.... this could be interesting... local assumes that the file is in $startdir. I guess file:// could be used to specify files anywhere on your system, but that really does not make a portable PKGBUILD. I'm tempted to say all local files must be in $startdir and so no support of file:// is needed. Allan
Am 27.06.2012 04:18, schrieb Allan McRae:
Hmm.... this could be interesting...
local assumes that the file is in $startdir. I guess file:// could be used to specify files anywhere on your system, but that really does not make a portable PKGBUILD.
I'm tempted to say all local files must be in $startdir and so no support of file:// is needed.
Imagine you have a bunch of PKGBUILDs with mirror=... in them and use $mirror a lot in source=(). Then you get a local mirror and mass-replace mirror= with a file:// URL in all of them. I say this is useful.
In order to treat all VCS sources as URLs, we need to be able to
deal with more protocols. Rewrite download_sources to use a case
statement so additional protocols are easily added.
Also fix the use of scp to not pass the protocol in the URL
(noticed by William J. Bowman
If "SKIP" is provided for an integrity check, abort checking as soon
as possible.
Signed-off-by: Allan McRae
On Wed, Jun 27, 2012 at 08:58:11AM +1000, Allan McRae wrote:
If "SKIP" is provided for an integrity check, abort checking as soon as possible.
Signed-off-by: Allan McRae
--- scripts/makepkg.sh.in | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 2a8866f..985bbbd 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -736,6 +736,12 @@ check_checksums() { file="$(get_filename "$file")" printf "%s" " $file ... " >&2
+ if [[ ${integrity_sums[$idx]} = 'SKIP' ]]; then + echo "$(gettext "Skipped")" >&2 + idx=$((idx + 1)) + continue + fi + if ! file="$(get_filepath "$file")"; then printf -- "$(gettext "NOT FOUND")\n" >&2 errors=1 @@ -743,18 +749,14 @@ check_checksums() { fi
if (( $found )) ; then - if [[ ${integrity_sums[$idx]} = 'SKIP' ]]; then - echo "$(gettext "Skipped")" >&2 + local expectedsum=$(tr '[:upper:]' '[:lower:]' <<< "${integrity_sums[$idx]}")
While you're touching this, we're bash4 capable now, so we could skip the fork and use a PE instead: "${integrity_sums[idx],,}"
+ local realsum="$(openssl dgst -${integ} "$file")" + realsum="${realsum##* }" + if [[ $expectedsum = "$realsum" ]]; then + printf -- "$(gettext "Passed")\n" >&2 else - local expectedsum=$(tr '[:upper:]' '[:lower:]' <<< "${integrity_sums[$idx]}") - local realsum="$(openssl dgst -${integ} "$file")" - realsum="${realsum##* }" - if [[ $expectedsum = "$realsum" ]]; then - printf -- "$(gettext "Passed")\n" >&2 - else - printf -- "$(gettext "FAILED")\n" >&2 - errors=1 - fi + printf -- "$(gettext "FAILED")\n" >&2 + errors=1 fi fi
-- 1.7.11.1
Allow specifing GIT sources using the following syntax
source=('<folder>::<repo>#<fragment>')
This will download the git repo <repo> into <folder> (into $SRCDIR
if set, otherwise $startdir). <repo> must start with "git", but
non-git protocols are handled using (e.g.) "git+http://...".
The <fragment> can be used to specify a branch, tag, or commit to
build from. e.g. branch=maint.
Checksum entries for git sources should be "SKIP".
Signed-off-by: Allan McRae
On Wed, Jun 27, 2012 at 08:58:12AM +1000, Allan McRae wrote:
Allow specifing GIT sources using the following syntax
source=('<folder>::<repo>#<fragment>')
This will download the git repo <repo> into <folder> (into $SRCDIR if set, otherwise $startdir). <repo> must start with "git", but non-git protocols are handled using (e.g.) "git+http://...".
The <fragment> can be used to specify a branch, tag, or commit to build from. e.g. branch=maint.
Checksum entries for git sources should be "SKIP".
Signed-off-by: Allan McRae
--- scripts/makepkg.sh.in | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 985bbbd..2b29759 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -341,6 +341,88 @@ download_file() { ln -s "$SRCDEST/$filename" "$srcdir/" }
+download_git() { + local netfile=$1 + + local fragment=${netfile##*#} + if [[ $fragment = "$netfile" ]]; then + unset fragment + fi + + local folder=${netfile%%::*}
This makes my OCD twitch -- dir instead of folder?
+ local repo=${netfile##*/} + repo=${repo%%#*} + repo=${repo%%.git*} + + if [[ $folder = "$netfile" ]]; then + folder="${repo}" + fi + + if [[ ! -d "$startdir"/$folder && -d "$SRCDEST"/$folder ]]; then + folder="$SRCDEST"/$folder + else + folder="$startdir"/$folder + fi
None of the quoting in this block is strictly necessary.
+ + local url=$(get_url "$netfile") + url=${url##*git+} + url=${url%%#*} + + if [[ ! -d $folder ]]; then + msg2 "$(gettext "Cloning %s %s repo...")" "${repo}" "git" + if ! git clone --mirror "$url" "$folder"; then + error "$(gettext "Failure while downloading %s %s repo")" "${repo}" "git" + plain "$(gettext "Aborting...")" + exit 1 + fi + else + msg2 "$(gettext "Updating %s %s repo...")" "${repo}" "git" + cd_safe "$folder" + if ! git fetch --all -p; then + # only warn on failure to allow offline builds + warning "$(gettext "Failure while updating %s %s repo")" "${repo}" "git" + fi + fi + + msg2 "$(gettext "Creating working copy of %s %s repo...")" "${repo}" "git" + pushd "$srcdir" &>/dev/null + rm -rf ${folder##*/} + + if ! git clone $folder; then
Your spare quotes from above can go here.
+ error "$(gettext "Failure while creating working copy of %s %s repo")" "${repo}" "git" + plain "$(gettext "Aborting...")" + exit 1 + fi + + cd_safe ${folder##*/}
Quotes here too, please.
+ + local ref + if [[ -n $fragment ]]; then + case $fragment in + commit=*|tag*)
tag=* rather than tag* ? You might want to trim $fragment right in the switch to avoid all the glob matching, i.e. case ${fragmen%%=*} in commit|tag) .... branch) ....
+ ref=${fragment##*=} + ;; + branch=*) + ref=origin/${fragment##*=} + ;; + *) + error "$(gettext "Unrecognized reference: %s")" "${fragment}" + plain "$(gettext "Aborting...")" + exit 1 + esac + fi + + if [[ -n $ref ]]; then + if ! git checkout -b makepkg $ref; then
Is that branch name really intentional here? Why not name it the same as $ref?
+ error "$(gettext "Failure while creating working copy of %s %s repo")" "${repo}" "git" + plain "$(gettext "Aborting...")" + exit 1 + fi + fi + + popd &>/dev/null +} + download_sources() { msg "$(gettext "Retrieving Sources...")"
@@ -357,6 +439,9 @@ download_sources() { ftp|http|https|rsync|scp) download_file "$nethe tfile" ;; + git*) + download_git "$netfile" + ;; *) error "$(gettext "Unknown download protocol: %s")" "$proto" plain "$(gettext "Aborting...")" -- 1.7.11.1
Am 27.06.2012 00:58, schrieb Allan McRae:
+ if [[ ! -d "$startdir"/$folder && -d "$SRCDEST"/$folder ]]; then + folder="$SRCDEST"/$folder + else + folder="$startdir"/$folder + fi
I don't understand this logic: Does this mean that you put the git clone to $startdir/$folder by default? If $SRCDEST is set, I would always expect it to go to $SRCDEST.
On 27/06/12 18:54, Thomas Bächler wrote:
Am 27.06.2012 00:58, schrieb Allan McRae:
+ if [[ ! -d "$startdir"/$folder && -d "$SRCDEST"/$folder ]]; then + folder="$SRCDEST"/$folder + else + folder="$startdir"/$folder + fi
I don't understand this logic: Does this mean that you put the git clone to $startdir/$folder by default? If $SRCDEST is set, I would always expect it to go to $SRCDEST.
Yeah, that was an oversight there - and probably completely broken. But it is entirely fixed in patch 09/11. Allan
Am 27.06.2012 11:14, schrieb Allan McRae:
On 27/06/12 18:54, Thomas Bächler wrote:
Am 27.06.2012 00:58, schrieb Allan McRae:
+ if [[ ! -d "$startdir"/$folder && -d "$SRCDEST"/$folder ]]; then + folder="$SRCDEST"/$folder + else + folder="$startdir"/$folder + fi
I don't understand this logic: Does this mean that you put the git clone to $startdir/$folder by default? If $SRCDEST is set, I would always expect it to go to $SRCDEST.
Yeah, that was an oversight there - and probably completely broken. But it is entirely fixed in patch 09/11.
Didn't see that, but yeah, seems to make more sense there.
VCS sources should have "SKIP" for their checksum value
Signed-off-by: Allan McRae
Modify get_filename to return the name of the folder with VCS sources.
This fixes output issues in checksum checking.
Signed-off-by: Allan McRae
With VCS sources, get_filepath should return the directory of the
checkout. This allows backing up of the VCS checkout when using
--allsource. Fixes FS#21098.
Signed-off-by: Allan McRae
Am 27.06.2012 00:58, schrieb Allan McRae:
At the request of Dave, here are the updated versions of patches sent to the list earlier.
Support for the following needs readded: CVS, Mercurial, Darcs and Bazaar
Known issue: the pkgver check needs adjusted when using an auto-updating pkgver
I've used the code from your vcs branch for a bit and it worked quite well for me with git sources. It is also incredibly useful compared to the old implementation. One weird bug: I used a pkgver function like this: pkgver() { local myver ... } This failed on the 'myver' variable name, although it is local.
On Thu, Jul 12, 2012 at 05:04:52PM +0200, Thomas Bächler wrote:
Am 27.06.2012 00:58, schrieb Allan McRae:
At the request of Dave, here are the updated versions of patches sent to the list earlier.
Support for the following needs readded: CVS, Mercurial, Darcs and Bazaar
Known issue: the pkgver check needs adjusted when using an auto-updating pkgver
I've used the code from your vcs branch for a bit and it worked quite well for me with git sources. It is also incredibly useful compared to the old implementation.
One weird bug: I used a pkgver function like this:
pkgver() { local myver
... }
This failed on the 'myver' variable name, although it is local.
Just as I told you in IRC, the debug log showed that you're trying to stomp on a read-only var. bash will not let you do this even if its scoped to a function. We could probably change the name of this var, e.g. to "makepkg_version", instead of something so foolishly generic. d
Am 12.07.2012 17:15, schrieb Dave Reisner:
Just as I told you in IRC, the debug log showed that you're trying to stomp on a read-only var. bash will not let you do this even if its scoped to a function.
Which is stupid and defies the purpose of scoping.
We could probably change the name of this var, e.g. to "makepkg_version", instead of something so foolishly generic.
Exactly. Why not give ALL variables in makepkg an ugly prefix, so nobody accidently uses them. This wouldn't hurt the eyes too much when editing makepkg.in.
participants (4)
-
Allan McRae
-
Dan McGee
-
Dave Reisner
-
Thomas Bächler