[PATCH] makepkg: use the `declare` builtin when backing up variables to eval

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] makepkg: use the `declare` builtin when backing up variables to eval

Eli Schwartz-2
Rather than manually crafting foo_backup in a loop and eval'ing them
with a complicated escape pattern, store every splitpkg_overrides
element into a single variable via the eval-friendly `declare` builtin.

An alternative to eval would be using `printf -v` but this does not work
for arrays.

This has the additional benefit of reducing the number of
variables/arrays floating around in the environment.

Signed-off-by: Eli Schwartz <[hidden email]>
---
 scripts/makepkg.sh.in | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index f0703d41..d6cb00da 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -1111,34 +1111,22 @@ check_build_status() {
 backup_package_variables() {
  local var
  for var in ${splitpkg_overrides[@]}; do
- local indirect="${var}_backup"
- eval "${indirect}=(\"\${$var[@]}\")"
- done
-}
-
-restore_package_variables() {
- local var
- for var in ${splitpkg_overrides[@]}; do
- local indirect="${var}_backup"
- if [[ -n ${!indirect} ]]; then
- eval "${var}=(\"\${$indirect[@]}\")"
- else
- unset ${var}
- fi
+ declare -p $var || printf '%s\n'  "unset $var"
  done
 }
 
 run_split_packaging() {
  local pkgname_backup=("${pkgname[@]}")
+ local restore_package_variables
  for pkgname in ${pkgname_backup[@]}; do
  pkgdir="$pkgdirbase/$pkgname"
  mkdir "$pkgdir"
- backup_package_variables
+ restore_package_variables="$(backup_package_variables)"
  run_package $pkgname
  tidy_install
  lint_package || exit $E_PACKAGE_FAILED
  create_package
- restore_package_variables
+ eval "$restore_package_variables"
  done
  pkgname=("${pkgname_backup[@]}")
  create_debug_package
--
2.16.2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] makepkg: use the `declare` builtin when backing up variables to eval

Allan McRae
On 15/03/18 10:56, Eli Schwartz wrote:

> Rather than manually crafting foo_backup in a loop and eval'ing them
> with a complicated escape pattern, store every splitpkg_overrides
> element into a single variable via the eval-friendly `declare` builtin.
>
> An alternative to eval would be using `printf -v` but this does not work
> for arrays.
>
> This has the additional benefit of reducing the number of
> variables/arrays floating around in the environment.
>

Stop submitting patches without even basic testing.


==> Entering fakeroot environment...
/home/allan/arch/code/pacman/scripts/.lib/makepkg: line 1118: declare:
pkgdesc: not found
/home/allan/arch/code/pacman/scripts/.lib/makepkg: line 1118: declare:
url: not found
/home/allan/arch/code/pacman/scripts/.lib/makepkg: line 1118: declare:
license: not found
/home/allan/arch/code/pacman/scripts/.lib/makepkg: line 1118: declare:
groups: not found
/home/allan/arch/code/pacman/scripts/.lib/makepkg: line 1118: declare:
backup: not found
/home/allan/arch/code/pacman/scripts/.lib/makepkg: line 1118: declare:
options: not found
/home/allan/arch/code/pacman/scripts/.lib/makepkg: line 1118: declare:
install: not found
/home/allan/arch/code/pacman/scripts/.lib/makepkg: line 1118: declare:
changelog: not found
Reply | Threaded
Open this post in threaded view
|

[PATCH v2] makepkg: use the `declare` builtin when backing up variables to eval

Eli Schwartz-2
In reply to this post by Eli Schwartz-2
Rather than manually crafting foo_backup in a loop and eval'ing them
with a complicated escape pattern, store every splitpkg_overrides
element into a single variable via the eval-friendly `declare` builtin.

An alternative to eval would be using `printf -v` but this does not work
for arrays.

This has the additional benefit of reducing the number of
variables/arrays floating around in the environment.

Signed-off-by: Eli Schwartz <[hidden email]>
---

v2: squelch error messages.

My apologies -- that was inexcusable. This was originally inspired by
the patch "makepkg: reorganize the restoration of settings by precedence"
which did the same thing correctly...

 scripts/makepkg.sh.in | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 96a96483..bc2d0061 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -1115,34 +1115,22 @@ check_build_status() {
 backup_package_variables() {
  local var
  for var in ${splitpkg_overrides[@]}; do
- local indirect="${var}_backup"
- eval "${indirect}=(\"\${$var[@]}\")"
- done
-}
-
-restore_package_variables() {
- local var
- for var in ${splitpkg_overrides[@]}; do
- local indirect="${var}_backup"
- if [[ -n ${!indirect} ]]; then
- eval "${var}=(\"\${$indirect[@]}\")"
- else
- unset ${var}
- fi
+ declare -p $var 2>/dev/null || printf '%s\n'  "unset $var"
  done
 }
 
 run_split_packaging() {
  local pkgname_backup=("${pkgname[@]}")
+ local restore_package_variables
  for pkgname in ${pkgname_backup[@]}; do
  pkgdir="$pkgdirbase/$pkgname"
  mkdir "$pkgdir"
- backup_package_variables
+ restore_package_variables="$(backup_package_variables)"
  run_package $pkgname
  tidy_install
  lint_package || exit $E_PACKAGE_FAILED
  create_package
- restore_package_variables
+ eval "$restore_package_variables"
  done
  pkgname=("${pkgname_backup[@]}")
  create_debug_package
--
2.16.2