[pacman-dev] [PATCH 3/3] makepkg: Empty/create only $pkgdir's relevant to current PKGBUILD (unless CLEANUP is set)
--- scripts/makepkg.sh.in | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 6ca678e..af8a606 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -2822,6 +2822,11 @@ run_split_packaging() { local pkgname_backup=${pkgname[@]} for pkgname in ${pkgname_backup[@]}; do pkgdir="$pkgdirbase/$pkgname" + # clean existing pkg directory + if [[ -d $pkgdir ]]; then + msg "$(gettext "Removing existing %s directory...")" "\$pkgdir/" + rm -rf "$pkgdir" + fi mkdir "$pkgdir" backup_package_variables run_package $pkgname @@ -3276,6 +3281,11 @@ if (( INFAKEROOT )); then chmod 755 "$pkgdirbase" if (( ! SPLITPKG )); then pkgdir="$pkgdirbase/$pkgname" + # clean existing pkg directory + if [[ -d $pkgdir ]]; then + msg "$(gettext "Removing existing %s directory...")" "\$pkgdir/" + rm -rf "$pkgdir" + fi mkdir "$pkgdir" if (( PKGFUNC )); then run_package @@ -3396,11 +3406,6 @@ else cd_safe "$startdir" fi - # clean existing pkg directory - if [[ -d $pkgdirbase ]]; then - msg "$(gettext "Removing existing %s directory...")" "\$pkgdir/" - rm -rf "$pkgdirbase" - fi mkdir -p "$pkgdirbase" chmod a-srwx "$pkgdirbase" -- 1.9.4.msysgit.2 -- David Macek
On 18/03/15 03:19, David Macek wrote:
---
Provide a commit message explaining why this change is being made.
scripts/makepkg.sh.in | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 6ca678e..af8a606 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -2822,6 +2822,11 @@ run_split_packaging() { local pkgname_backup=${pkgname[@]} for pkgname in ${pkgname_backup[@]}; do pkgdir="$pkgdirbase/$pkgname" + # clean existing pkg directory + if [[ -d $pkgdir ]]; then + msg "$(gettext "Removing existing %s directory...")" "\$pkgdir/" + rm -rf "$pkgdir" + fi mkdir "$pkgdir" backup_package_variables run_package $pkgname @@ -3276,6 +3281,11 @@ if (( INFAKEROOT )); then chmod 755 "$pkgdirbase" if (( ! SPLITPKG )); then pkgdir="$pkgdirbase/$pkgname" + # clean existing pkg directory + if [[ -d $pkgdir ]]; then + msg "$(gettext "Removing existing %s directory...")" "\$pkgdir/" + rm -rf "$pkgdir" + fi mkdir "$pkgdir" if (( PKGFUNC )); then run_package @@ -3396,11 +3406,6 @@ else cd_safe "$startdir" fi
- # clean existing pkg directory - if [[ -d $pkgdirbase ]]; then
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 ...
- msg "$(gettext "Removing existing %s directory...")" "\$pkgdir/" - rm -rf "$pkgdirbase" - fi mkdir -p "$pkgdirbase" chmod a-srwx "$pkgdirbase"
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. -- David Macek
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
On 25. 3. 2015 6:13, Allan McRae wrote:
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?
Hmm. You're right -- if some $pkgdir is important to me, running makepkg that needs to overwrite it would be silly. Shall I make patch v2? -- David Macek
On 25/03/15 18:09, David Macek wrote:
On 25. 3. 2015 6:13, Allan McRae wrote:
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?
Hmm. You're right -- if some $pkgdir is important to me, running makepkg that needs to overwrite it would be silly.
Shall I make patch v2?
That would be great thanks! A
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. When CLEANUP is set, the behavior is unchanged. Discussed in: https://lists.archlinux.org/pipermail/pacman-dev/2015-February/019939.html --- scripts/makepkg.sh.in | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 2e6a0cc..e89d0ff 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -3413,10 +3413,12 @@ if (( NOBUILD )); then msg "$(gettext "Sources are ready.")" exit 0 #E_OK else - # clean existing pkg directory + # clean existing pkg directories if [[ -d $pkgdirbase ]]; then msg "$(gettext "Removing existing %s directory...")" "\$pkgdir/" - rm -rf "$pkgdirbase" + for pkg in $pkgname; do + rm -rf "$pkgdirbase/$pkg" + done fi mkdir -p "$pkgdirbase" chmod a-srwx "$pkgdirbase" -- 1.9.4.msysgit.2 -- David Macek
On Thu, Mar 26, 2015 at 02:48:33PM +0100, David Macek wrote:
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.
When CLEANUP is set, the behavior is unchanged.
Discussed in: https://lists.archlinux.org/pipermail/pacman-dev/2015-February/019939.html --- scripts/makepkg.sh.in | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 2e6a0cc..e89d0ff 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -3413,10 +3413,12 @@ if (( NOBUILD )); then msg "$(gettext "Sources are ready.")" exit 0 #E_OK else - # clean existing pkg directory + # clean existing pkg directories if [[ -d $pkgdirbase ]]; then msg "$(gettext "Removing existing %s directory...")" "\$pkgdir/" - rm -rf "$pkgdirbase" + for pkg in $pkgname; do
This can't be right -- you wanted "${pkgname[@]}" (with quotes).
+ rm -rf "$pkgdirbase/$pkg" + done fi mkdir -p "$pkgdirbase" chmod a-srwx "$pkgdirbase" -- 1.9.4.msysgit.2
-- David Macek
On 26. 3. 2015 14:53, Dave Reisner wrote:
+ for pkg in $pkgname; do
This can't be right -- you wanted "${pkgname[@]}" (with quotes).
+ rm -rf "$pkgdirbase/$pkg" + done
I wasn't completely sure about it, but I don't think pkgnames can contain whitespace and it is what Allan suggested. Maybe I wasn't supposed to copy it verbatim. Am I missing `local pkg` there somewhere? -- David Macek
On Thu, Mar 26, 2015 at 02:59:31PM +0100, David Macek wrote:
On 26. 3. 2015 14:53, Dave Reisner wrote:
+ for pkg in $pkgname; do
This can't be right -- you wanted "${pkgname[@]}" (with quotes).
+ rm -rf "$pkgdirbase/$pkg" + done
I wasn't completely sure about it, but I don't think pkgnames can contain whitespace and it is what Allan suggested. Maybe I wasn't supposed to copy it verbatim.
No, pkgname cannot contain quotes, but that's a minor issue. "$pkgname" will only expand to the equivalent of "${pkgname[0]}", leaving you with directories still in "$pkgbasedir" after the loop terminates.
Am I missing `local pkg` there somewhere?
Yes.
-- David Macek
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. When CLEANUP is set, the behavior is unchanged. Discussed in: https://lists.archlinux.org/pipermail/pacman-dev/2015-February/019939.html --- v3 -- fixed "${pkgname[@]}" and added unset pkg. scripts/makepkg.sh.in | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 2e6a0cc..fb1ef20 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -3413,10 +3413,13 @@ if (( NOBUILD )); then msg "$(gettext "Sources are ready.")" exit 0 #E_OK else - # clean existing pkg directory + # clean existing pkg directories if [[ -d $pkgdirbase ]]; then msg "$(gettext "Removing existing %s directory...")" "\$pkgdir/" - rm -rf "$pkgdirbase" + for pkg in "${pkgname[@]}"; do + rm -rf "$pkgdirbase/$pkg" + done + unset pkg fi mkdir -p "$pkgdirbase" chmod a-srwx "$pkgdirbase" -- 1.9.4.msysgit.2 -- David Macek
participants (3)
-
Allan McRae
-
Dave Reisner
-
David Macek