freebsd / poudriere

Port/Package build and test system
https://github.com/freebsd/poudriere/wiki
BSD 2-Clause "Simplified" License
379 stars 161 forks source link

logclean: Fix cleanup for multiple MASTERNAMEs #1157

Open delphij opened 1 month ago

delphij commented 1 month ago

Describe the bug

When multiple jails (on my system there were neptune-amd64, saturn-amd64 and sirius-amd64) are used, the first logclean run would fail at "Removing empty build log directories" step with, e.g.:

[00:00:00] Removing empty build log directories...find: neptune-amd64-default saturn-amd64-default sirius-amd64-default/latest-per-pkg: No such file or directory
Error: (77076) /usr/local/share/poudriere/logclean.sh:422: set -e error: status = 1

How to reproduce

Steps to reproduce the behavior:

  1. Use poudriere to generate logs, this can be done by using multiple jails to perform an empty bulk build.
  2. Run poudriere logclean -ay

Expected behavior

The whole process should complete without error.

Environment

Additional context

I would propose the following change:

diff --git a/src/share/poudriere/logclean.sh b/src/share/poudriere/logclean.sh
index f28ecc44..99bcf920 100755
--- a/src/share/poudriere/logclean.sh
+++ b/src/share/poudriere/logclean.sh
@@ -370,11 +370,11 @@ if [ ${logs_deleted} -eq 1 ]; then

        msg_n "Removing empty build log directories..."
        if lock_have "logs_latest-per-pkg"; then
-               echo "${MASTERNAMES_LOCKED:?}" | sed -e 's,$,/latest-per-pkg,' | \
-                   tr '\n' '\000' | \
-                   xargs -0 -J % find -x % -mindepth 0 -maxdepth 0 -empty | \
+               for MASTERNAME in ${MASTERNAMES_LOCKED:?}; do
+                   find -x "${MASTERNAME}/latest-per-pkg" -mindepth 0 -maxdepth 0 -empty | \
                    sed -e 's,$,/..,' | xargs realpath | tr '\n' '\000' | \
                    xargs -0 rm -rf
+               done
                echo " done"
        else
                echo " skipped (locked by another process)"

Basically, MASTERNAMES_LOCKED was a set of strings separated by blank space. For all other usages, for MASTERNAME in ${MASTERNAMES_LOCKED:?}; idiom was used, and for this one case the sed seems to be both redundant and incorrect (because it adds /latest-per-pkg to the end of the list, and the code seems to be expecting the list to be \n separated and it is not).

I think it makes more sense to simply avoid these sed's and manipulate the string in shell.