Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve brp-compress performance. JB#63057 #8

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

direc85
Copy link
Contributor

@direc85 direc85 commented Jan 17, 2025

brp-compress executed find twice for every folder in the man structure, which meant for filesystem package it was executed 30100 times (!!) without a single manpage to be found. I reworked the script so that it calls find two times, but with a long list of folders as parameters instead. This took the overall build time of filesystem on my laptop from ~185 seconds to ~60 seconds, dropping brp-compress execution time from two minutes to roughly a second.

I tested the docs build result with rpm-doc package, which resulted in same size and same contents with the old and new script.

While at it, I fixed the messy whitespace usage in brp-compress, cleaned up the code a little, removed uses of egrep and fixed the project to compile with Platform SDK too.

./usr/share/doc/*/man/man* ./usr/lib/*/man/man*
do
[ -d $d ] || continue
for f in `find $d -type f`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can replace this with one find |read loop like this:

MANDIR=...

find $MANDIRS -type f  | while read -r  manfile

...

done 

Using find with read eliminates both issues no need to deal with globbing issues in the ls and find statement or the need to call exit.

If no manuals are found the while loop won't run at all and the script exists.


# Compress man pages
COMPRESS="gzip -9 -n"
COMPRESS_EXT=.gz

for d in ./usr/man/man* ./usr/man/*/man* ./usr/info \
MANDIRS=$(ls -d ./usr/man/man* ./usr/man/*/man* ./usr/info \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need this you can skip this as mentioned earlier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants