Re: [pacman-dev] [PATCH v3 1/2] Make get_pkg_arch treat arch as an array
On 25 Mar 2015, at 2:39 pm +1000, Allan McRae wrote: > On 21/03/15 10:19, joyfulg...@archlinux.us wrote: > > From: Ivy Foster > This took me a whole lot of reviewing for a single character change! Yeah, I was afraid it might. I did some grepping of my own to make sure nothing in the repos was using a singleton $arch, but I'm reassured that you found the same thing. > tl;dr - Ack. I'll expand the commit message. Thanks! Ivy
Re: [pacman-dev] [PATCH v3 2/2] Add makepkg option --packagelist; fix bug #42150
On 25 Mar 2015, at 3:09 pm +1000, Allan McRae wrote: > On 21/03/15 10:19, joyfulg...@archlinux.us wrote: > > From: Ivy Foster > Delete the bug number from the subject line, it can be > added as a note to the commit message. Thanks, I'll keep that in mind in the future. > Also - BAD! file permission change. Whoops! Sorry about that. I set up my editor to mark shell scripts as executable on save, and then promptly forgot I'd done that. > I have added some changes below. This allows two things > 1) we can run --packagelist on any PKGBUILD no matter the architecture > we are running and the architecture required to build the PKGBUILD > 2) it lists all package products - so both foo-1-1-i686 and > foo-1-1-x86_64. It is easy to filter the unwanted ones away. > I have made the changes on my patchqueue branch. Let me know if you > approve and I can push the patch. > https://projects.archlinux.org/users/allan/pacman.git/log/?h=patchqueue Looks good to me! Thanks for the feedback. Anything else I need to do for this to go in? Ivy
[pacman-dev] [PATCH] Update PKGBUILD-split.proto allowed overrides
Commit 8a02abcf194 disallowed overridding pkgver/pkgrel/epoch. Update the
split package prototype to refelct this change.
Signed-off-by: Allan McRae
---
proto/PKGBUILD-split.proto | 3 ---
1 file changed, 3 deletions(-)
diff --git a/proto/PKGBUILD-split.proto b/proto/PKGBUILD-split.proto
index 6dae704..9898ef8 100644
--- a/proto/PKGBUILD-split.proto
+++ b/proto/PKGBUILD-split.proto
@@ -49,9 +49,6 @@ check() {
package_pkg1() {
# options and directives that can be overridden
- pkgver=
- pkgrel=
- epoch=
pkgdesc=""
arch=()
url=""
--
2.3.4
Re: [pacman-dev] [PATCH 3/3] makepkg: Empty/create only $pkgdir's relevant to current PKGBUILD (unless CLEANUP is set)
On 20/03/15 07:18, David Macek wrote: > On 19. 3. 2015 3:08, Allan McRae wrote: >> Provide a commit message explaining why this change is being made. > > Currently makepkg clears the whole $pkgbasedir which is needless. Moreover, > in the obscure case of multiple makepkg runs (with different $pkgname) that > share a $pkgdirbase, only $pkgdir's from the last run will remain. Since I > consider the contents of $pkgdir an important artifact, this commit restricts > the deletion to individual $pkgdir's and defers it until each $pkgdir is > actually needed. > > Discussed in > https://lists.archlinux.org/pipermail/pacman-dev/2015-February/019939.html > >> To avoid the duplication, why not loop over pkgname here? (note pkgname >> contains only the packages being built if --pkg is used). >> >> msg "$(gettext "Cleaning existing %s directory...")" "\$pkgdir/" >> for pkg in $pkgname; do >> ... > > I've thought about it, and I like more the version that defers deleting > $pkgdir's until it is really needed. Clearing all $pkgdir's at once would > produce different results in case a failure occurs before all packaging > function are called. > If packaging files, you could end up with two $pkgdir from the new package (one broken) and one from the old package - I do not like that inconsistency. Why would you want to keep the old package $pkgdir if you are intending to replace it? Allan
Re: [pacman-dev] [PATCH v3 2/2] Add makepkg option --packagelist; fix bug #42150
On 21/03/15 10:19, joyfulg...@archlinux.us wrote:
> From: Ivy Foster
>
> makepkg --packagelist prints the name of each package that would
> normally be produced, minus $PKGEXT, and exits.
>
> Signed-off-by: Ivy Foster
> ---
Delete the bug number from the subject line, it can be added as a note
to the commit message.
I have added some changes below. This allows two things
1) we can run --packagelist on any PKGBUILD no matter the architecture
we are running and the architecture required to build the PKGBUILD
2) it lists all package products - so both foo-1-1-i686 and
foo-1-1-x86_64. It is easy to filter the unwanted ones away.
I have made the changes on my patchqueue branch. Let me know if you
approve and I can push the patch.
https://projects.archlinux.org/users/allan/pacman.git/log/?h=patchqueue
Thanks,
Allan
> v3: Separate change to get_pkg_arch into another commit
> doc/makepkg.8.txt | 3 +++
> scripts/makepkg.sh.in | 26 +++---
> 2 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt
> index 31a2ef7..ce4364b 100644
> --- a/doc/makepkg.8.txt
> +++ b/doc/makepkg.8.txt
> @@ -203,6 +203,9 @@ Options
> (Passed to pacman) Prevent pacman from displaying a progress bar;
> useful if you are redirecting makepkg output to file.
>
> +*\--packagelist*::
> + Instead of building, list packages that would be produced. Listed
List the packages that would be produced without building.
> + package names do not include PKGEXT.
>
> Additional Features
> ---
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 5b3bffd..3c2e381 100755
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -2834,6 +2834,19 @@ run_split_packaging() {
> pkgname=${pkgname_backup[@]}
> }
>
> +print_all_package_names() {
> + local version=$(get_full_version)
> + local architecture pkg opts
local architecture pkg opts a
> + for pkg in ${pkgname[@]}; do
> + architecture=$(get_pkg_arch $pkg)
pkgbuild_get_attribute "$pkg" 'arch' 1 architectures
> + pkgbuild_get_attribute "$pkg" 'options' 1 opts
for a in ${architectures[@]}; do
> + printf "%s-%s-%s\n" "$pkg" "$version" "$architecture"
> + if in_opt_array "debug" ${opts[@]} && in_opt_array "strip"
> ${opts[@]}; then
> + printf "%s-%s-%s-%s\n" "$pkg" "@DEBUGSUFFIX@" "$pkgver"
> "$architecture"
> + fi
done
> + done
> +}
> +
> # Canonicalize a directory path if it exists
> canonicalize_path() {
> local path="$1";
> @@ -2893,6 +2906,7 @@ usage() {
> printf -- "$(gettext " --nocheckDo not run the %s function in
> the %s")\n" "check()" "$BUILDSCRIPT"
> printf -- "$(gettext " --noprepare Do not run the %s function in
> the %s")\n" "prepare()" "$BUILDSCRIPT"
> printf -- "$(gettext " --nosign Do not create a signature for
> the package")\n"
> + printf -- "$(gettext " --packagelistOnly list packages that would
> be produced, without PKGEXT")\n"
> printf -- "$(gettext " --pkg Only build listed packages
> from a split package")\n"
> printf -- "$(gettext " --sign Sign the resulting package
> with %s")\n" "gpg"
> printf -- "$(gettext " --skipchecksums Do not verify checksums of the
> source files")\n"
> @@ -2938,9 +2952,9 @@ ARGLIST=("$@")
> OPT_SHORT="AcCdefFghiLmop:rRsSV"
> OPT_LONG=('allsource' 'check' 'clean' 'cleanbuild' 'config:' 'force'
> 'geninteg'
>'help' 'holdver' 'ignorearch' 'install' 'key:' 'log' 'noarchive'
> 'nobuild'
> - 'nocolor' 'nocheck' 'nodeps' 'noextract' 'noprepare' 'nosign'
> 'pkg:' 'repackage'
> - 'rmdeps' 'sign' 'skipchecksums' 'skipinteg' 'skippgpcheck'
> 'source' 'syncdeps'
> - 'verifysource' 'version')
> + 'nocolor' 'nocheck' 'nodeps' 'noextract' 'noprepare' 'nosign'
> 'packagelist'
> + 'pkg:' 'repackage' 'rmdeps' 'sign' 'skipchecksums' 'skipinteg'
> + 'skippgpcheck' 'source' 'syncdeps' 'verifysource' 'version')
>
> # Pacman Options
> OPT_LONG+=('asdeps' 'noconfirm' 'needed' 'noprogressbar')
> @@ -2983,6 +2997,7 @@ while true; do
> -o|--nobuild) NOBUILD=1 ;;
> -p) shift; BUILDFILE=$1 ;;
> --pkg)shift; IFS=, read -ra p <<<"$1";
> PKGLIST+=("${p[@]}"); unset p ;;
> + --packagelist)PACKAGELIST=1 ;;
--packagelist)PACKAGELIST=1 IGNOREARCH=1 ;;
> -r|--rmdeps) RMDEPS=1 ;;
> -R|--repackage) REPKG=1 ;;
> --sign) SIGNPKG='y' ;;
> @@ -3264,6 +3279,11 @@ if { [[ -z $SIGNPKG ]] && check_buildenv "sign" "y"; }
> || [[ $SIGNPKG == 'y' ]];
> fi
> fi
>
> +if (( PACKAGELIST )); then
> + print_all_package_names
> + exit 0
> +fi
> +
> if (( ! PKGVERFUNC )); then
> check_build_status
>
Re: [pacman-dev] [PATCH v3 1/2] Make get_pkg_arch treat arch as an array
On 25/03/15 14:39, Allan McRae wrote:
> On 21/03/15 10:19, joyfulg...@archlinux.us wrote:
>> From: Ivy Foster
>>
>> Signed-off-by: Ivy Foster
>> ---
>> v3: Separate change to get_pkg_arch into separate commit
>> scripts/makepkg.sh.in | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> mode change 100644 => 100755 scripts/makepkg.sh.in
Also - BAD! file permission change.
>>
>
> This took me a whole lot of reviewing for a single character change! I
> only knew that this could potentially break something on the basis of
> skimming IRC chat a while ago. So I tested...
>
> pkgname=f
> arch=('any')
>
> package_f() {
> arch='x86_64' #1
> arch=('x86_64') #2
> arch=('i686 'x86_64') #3
>
> }
>
> for the --packagelist patch, currently #1 works and #2 and #3 fail.
> After the patch, #2 and #3 succeed and #1 fails.
>
> All our documentation says that arch should be an array, so the patch is
> correct. In fact, the .SRCINFO generating code uses the array version
> of pkgbuild_get_attribute, and so "fails" when the arch variable is not
> an array.
>
> This issue was never exposed because we don't use get_pkg_arch with a
> parameter very often, and I guess no-one has looked at a .SRCINFO file
> from a bad PKGBUILD before.
>
> TODO: We should probably add a PKGBUILD lint function that confirms the
> fields we expect to be arrays are in fact arrays. And that those that
> are not arrays are not arrays.
>
> tl;dr - Ack. I'll expand the commit message.
>
>
>> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
>> old mode 100644
>> new mode 100755
>> index 168f334..5b3bffd
>> --- a/scripts/makepkg.sh.in
>> +++ b/scripts/makepkg.sh.in
>> @@ -879,7 +879,7 @@ get_pkg_arch() {
>> fi
>> else
>> local arch_override
>> -pkgbuild_get_attribute "$1" arch 0 arch_override
>> +pkgbuild_get_attribute "$1" arch 1 arch_override
>> (( ${#arch_override[@]} == 0 )) && arch_override=("${arch[@]}")
>> if [[ $arch_override = "any" ]]; then
>> printf "%s\n" "any"
>>
>
>
Re: [pacman-dev] [PATCH v3 1/2] Make get_pkg_arch treat arch as an array
On 21/03/15 10:19, joyfulg...@archlinux.us wrote:
> From: Ivy Foster
>
> Signed-off-by: Ivy Foster
> ---
> v3: Separate change to get_pkg_arch into separate commit
> scripts/makepkg.sh.in | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> mode change 100644 => 100755 scripts/makepkg.sh.in
>
This took me a whole lot of reviewing for a single character change! I
only knew that this could potentially break something on the basis of
skimming IRC chat a while ago. So I tested...
pkgname=f
arch=('any')
package_f() {
arch='x86_64' #1
arch=('x86_64') #2
arch=('i686 'x86_64') #3
}
for the --packagelist patch, currently #1 works and #2 and #3 fail.
After the patch, #2 and #3 succeed and #1 fails.
All our documentation says that arch should be an array, so the patch is
correct. In fact, the .SRCINFO generating code uses the array version
of pkgbuild_get_attribute, and so "fails" when the arch variable is not
an array.
This issue was never exposed because we don't use get_pkg_arch with a
parameter very often, and I guess no-one has looked at a .SRCINFO file
from a bad PKGBUILD before.
TODO: We should probably add a PKGBUILD lint function that confirms the
fields we expect to be arrays are in fact arrays. And that those that
are not arrays are not arrays.
tl;dr - Ack. I'll expand the commit message.
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> old mode 100644
> new mode 100755
> index 168f334..5b3bffd
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -879,7 +879,7 @@ get_pkg_arch() {
> fi
> else
> local arch_override
> - pkgbuild_get_attribute "$1" arch 0 arch_override
> + pkgbuild_get_attribute "$1" arch 1 arch_override
> (( ${#arch_override[@]} == 0 )) && arch_override=("${arch[@]}")
> if [[ $arch_override = "any" ]]; then
> printf "%s\n" "any"
>
