[pacman-dev] [PATCH] makepkg: make source tarballs work with per-package files containing '$pkgname'
Extracting function variables containing arbitrarily scoped variables of arbitrary nature is a disaster, but let's at least cover the common case of using the actual '$pkgname' in an install/changelog file. It's the odd case of actually being basically justified use of disambiguating between the same variable used in multiple different split packages and is even recommended in the PKGBUILD(5) documentation... ... and also, --printsrcinfo already uses and overwrites the variable 'pkgname' in pkgbuild_extract_to_srcinfo, so this "works" in .SRCINFO but doesn't work in .src.tar.gz Fixes FS#64932 Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- scripts/makepkg.sh.in | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index ca3e7459..674e0d87 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -784,13 +784,14 @@ create_srcpackage() { fi done - local i + local pkgname_backup=(${pkgname[@]}) + local i pkgname for i in 'changelog' 'install'; do local file files [[ ${!i} ]] && files+=("${!i}") - for name in "${pkgname[@]}"; do - if extract_function_variable "package_$name" "$i" 0 file; then + for pkgname in "${pkgname_backup[@]}"; do + if extract_function_variable "package_$pkgname" "$i" 0 file; then files+=("$file") fi done @@ -802,6 +803,7 @@ create_srcpackage() { fi done done + pkgname=(${pkgname_backup[@]}) local fullver=$(get_full_version) local pkg_file="$SRCPKGDEST/${pkgbase}-${fullver}${SRCEXT}" -- 2.24.1
On 23/12/19 1:57 am, Eli Schwartz wrote:
Extracting function variables containing arbitrarily scoped variables of arbitrary nature is a disaster, but let's at least cover the common case of using the actual '$pkgname' in an install/changelog file. It's the odd case of actually being basically justified use of disambiguating between the same variable used in multiple different split packages and is even recommended in the PKGBUILD(5) documentation...
... and also, --printsrcinfo already uses and overwrites the variable 'pkgname' in pkgbuild_extract_to_srcinfo, so this "works" in .SRCINFO but doesn't work in .src.tar.gz
Fixes FS#64932
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- scripts/makepkg.sh.in | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index ca3e7459..674e0d87 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -784,13 +784,14 @@ create_srcpackage() { fi done
- local i + local pkgname_backup=(${pkgname[@]}) + local i pkgname for i in 'changelog' 'install'; do local file files
[[ ${!i} ]] && files+=("${!i}") - for name in "${pkgname[@]}"; do - if extract_function_variable "package_$name" "$i" 0 file; then + for pkgname in "${pkgname_backup[@]}"; do + if extract_function_variable "package_$pkgname" "$i" 0 file; then
It took me a while to see what this change did... We need $pkgname set right for the eval in extract_function_variable to do its thing. Can we get a brief comment in that regard above the place where you backup ${pkgname[@]}? Also note, this problem extends into checking files are present early in makepkg. e.g. for a PKGBUILD with pkgname=('a' 'b'), and using $pkgname.install in both package_*() functions we get: $ makepkg ==> ERROR: install file (a.install) does not exist or is not a regular file. ==> ERROR: install file (a.install) does not exist or is not a regular file.
files+=("$file") fi done @@ -802,6 +803,7 @@ create_srcpackage() { fi done done + pkgname=(${pkgname_backup[@]})
local fullver=$(get_full_version) local pkg_file="$SRCPKGDEST/${pkgbase}-${fullver}${SRCEXT}"
Extracting function variables containing arbitrarily scoped variables of arbitrary nature is a disaster, but let's at least cover the common case of using the actual '$pkgname' in an install/changelog file. It's the odd case of actually being basically justified use of disambiguating between the same variable used in multiple different split packages... and also, --printsrcinfo already uses and overwrites the variable 'pkgname' in pkgbuild_extract_to_srcinfo, so this "works" in .SRCINFO but doesn't work in .src.tar.gz It doesn't work in lint_pkgbuild either, but in that case the problem is being too permissive, not too restrictive -- we might end up checking the same file twice, and printing that it is missing twice. Fixes FS#64932 Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- v2: add explanatory comments, and handle lint_pkgbuild as well scripts/libmakepkg/lint_pkgbuild/changelog.sh.in | 10 +++++++--- scripts/libmakepkg/lint_pkgbuild/install.sh.in | 10 +++++++--- scripts/makepkg.sh.in | 10 +++++++--- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/scripts/libmakepkg/lint_pkgbuild/changelog.sh.in b/scripts/libmakepkg/lint_pkgbuild/changelog.sh.in index 114298f9..be6501ef 100644 --- a/scripts/libmakepkg/lint_pkgbuild/changelog.sh.in +++ b/scripts/libmakepkg/lint_pkgbuild/changelog.sh.in @@ -32,11 +32,15 @@ lint_pkgbuild_functions+=('lint_changelog') lint_changelog() { - local name file changelog_list + local file changelog_list changelog_list=("${changelog[@]}") - for name in "${pkgname[@]}"; do - if extract_function_variable "package_$name" changelog 0 file; then + # set pkgname the same way we do for running package(), this way we get + # the right value in extract_function_variable + local pkgname_backup=(${pkgname[@]}) + local pkgname + for pkgname in "${pkgname_backup[@]}"; do + if extract_function_variable "package_$pkgname" changelog 0 file; then changelog_list+=("$file") fi done diff --git a/scripts/libmakepkg/lint_pkgbuild/install.sh.in b/scripts/libmakepkg/lint_pkgbuild/install.sh.in index 95076692..1bf5681d 100644 --- a/scripts/libmakepkg/lint_pkgbuild/install.sh.in +++ b/scripts/libmakepkg/lint_pkgbuild/install.sh.in @@ -32,11 +32,15 @@ lint_pkgbuild_functions+=('lint_install') lint_install() { - local list file name install_list ret=0 + local list file install_list ret=0 install_list=("${install[@]}") - for name in "${pkgname[@]}"; do - extract_function_variable "package_$name" install 0 file + # set pkgname the same way we do for running package(), this way we get + # the right value in extract_function_variable + local pkgname_backup=(${pkgname[@]}) + local pkgname + for pkgname in "${pkgname_backup[@]}"; do + extract_function_variable "package_$pkgname" install 0 file install_list+=("$file") done diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 9f641d97..2e54b8aa 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -800,13 +800,16 @@ create_srcpackage() { fi done - local i + # set pkgname the same way we do for running package(), this way we get + # the right value in extract_function_variable + local pkgname_backup=(${pkgname[@]}) + local i pkgname for i in 'changelog' 'install'; do local file files [[ ${!i} ]] && files+=("${!i}") - for name in "${pkgname[@]}"; do - if extract_function_variable "package_$name" "$i" 0 file; then + for pkgname in "${pkgname_backup[@]}"; do + if extract_function_variable "package_$pkgname" "$i" 0 file; then files+=("$file") fi done @@ -818,6 +821,7 @@ create_srcpackage() { fi done done + pkgname=(${pkgname_backup[@]}) local fullver=$(get_full_version) local pkg_file="$SRCPKGDEST/${pkgbase}-${fullver}${SRCEXT}" -- 2.25.0
On 17/1/20 3:27 am, Eli Schwartz wrote:
Extracting function variables containing arbitrarily scoped variables of arbitrary nature is a disaster, but let's at least cover the common case of using the actual '$pkgname' in an install/changelog file. It's the odd case of actually being basically justified use of disambiguating between the same variable used in multiple different split packages... and also, --printsrcinfo already uses and overwrites the variable 'pkgname' in pkgbuild_extract_to_srcinfo, so this "works" in .SRCINFO but doesn't work in .src.tar.gz
It doesn't work in lint_pkgbuild either, but in that case the problem is being too permissive, not too restrictive -- we might end up checking the same file twice, and printing that it is missing twice.
Fixes FS#64932
Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> ---
v2: add explanatory comments, and handle lint_pkgbuild as well
Great - patch looks good. Allan
participants (2)
-
Allan McRae
-
Eli Schwartz