[Rpm-maint] [rpm-software-management/rpm] scripts/brp-strip: Rectify parallel stripping logic (PR #2149)

[Shreenidhi Shedi]

@sshedi commented on this pull request.

Having 3 distinct steps seems a bit costly but it's needed.

Assume we first 32 lines in find command results are individual binaries but 33rd entry is a hard linked file. And sort command will act on batch of 32 lines, so there is a chance that in the sort will give same binary to strip command at same time. So, sort has to work on the whole list of files at once.

And using a named pipe is an excellent idea and huge performance gain compared to using a temp file. Thanks for the input 👍🏻  

> +# Create a tmp file for storing file paths
+# Below is the explanation of commands in the order of their appearance
+# Ignore /usr/lib/debug entries
+# Ignore all go(guile objects & golang) files
+# Run the file command to find relevant non-stripped binaries, with bundle size of 32
+# Ignore all 'no machine' files
+# Only operate on non-stripped binaries
+# Get inode & file names to help eliminate hard links (%i %n)
+tmp_fn="$(mktemp)"
+find "$RPM_BUILD_ROOT" -type f \
+  ! -regex "${RPM_BUILD_ROOT}/?usr/lib/debug/.*" \
+  ! -name "*.go" -print0 | \
+  xargs -0 -r -P${NCPUS} -n${MAX_ARGS} sh -c "file \"\$@\" | \
+  sed -n -e 's/^\(.*\):[[:space:]]*ELF.*, not stripped.*/\1/p' | \
+  grep -v 'no machine' | \
+  xargs -r stat -c '%i %n'" ARG0 > "${tmp_fn}"

Yeah, I got this thought. Surprisingly it doesn't and I tested this with a random find command with xargs like the way it's used here. Anyway, I will do this change.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2149#pullrequestreview-1084211151
You are receiving this because you are subscribed to this thread.

Message ID: <rpm-software-management/rpm/pull/2149/review/1084211151 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20220824/0c4bddc7/attachment-0001.html>