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

qmail-qstat: simplify the code #234

Merged
merged 2 commits into from
May 10, 2022
Merged

qmail-qstat: simplify the code #234

merged 2 commits into from
May 10, 2022

Conversation

DerDakon
Copy link
Member

@DerDakon DerDakon commented May 3, 2022

See the commits for a detailed description, but boils down to:

  • avoid needless dependencies in Makefile
  • use find -type f to just count files and avoid much complication in the script

@DerDakon
Copy link
Member Author

DerDakon commented May 3, 2022

@mbhangui your Indimail uses big-todo IIRC, could you check if this works properly there?

@mbhangui
Copy link
Contributor

mbhangui commented May 3, 2022

@mbhangui your Indimail uses big-todo IIRC, could you check if this works properly there?

This works for big-todo because it uses find. The earlier script didn't work because it expected all files to be in queue/todo directory.

Copy link

@josuah josuah left a comment

Choose a reason for hiding this comment

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

LGTM, seal of approval

Maybe if someone does something funky with symlinks, then an extra -L flag to find could be useful though?

Comment on lines +1 to +2
messfiles=`find QMAIL/queue/mess/* -type f | wc -l`
todofiles=`find QMAIL/queue/todo -type f | wc -l`
Copy link

Choose a reason for hiding this comment

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

The wc -w to wc -l make sense, find produces one result per line.
It is all based upon the absence of spaces in file names here anyway.

Maybe another case where -type f was not widely available.

Copy link
Member Author

Choose a reason for hiding this comment

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

I expect someone that does such special things to the queue to be able to hack this script as well.

@schmonz
Copy link
Member

schmonz commented May 9, 2022

find(1) is always a portability risk, but the way it's used here feels likely to work on every platform we could possibly care about, and the way this would break (if we're wrong about that) would be tolerable. Breaking this portion of the big-todo patch would usually be counter to our project goals, but I consider this an excellent exception: we're breaking it because we've made it unnecessary, and the rest of the patch is unaffected. And of course removing unused dependencies is a small but permanently cost-cutting victory.

qmail-qstat.sh Outdated Show resolved Hide resolved
First, do not chdir to the qmail directory, but instead pass the absolute path
to the find commands.

Then tell find to only output files, and count them, instead of counting
everything and then substracting the directories. If there is anything beyond
a plain file somewhere in the queue this would have been counted before and is
no longer counted now, but the presence of anything else can be considered a
bug in itself.

When someone is using the big-todo patch this would collide, but for good: the
new version can work with both the original and the big-todo queue style
without modifications.

While at it quote the arguments to echo just to be sure.

The additional "-type f" argument can already be found in the AT&T System V
user manual from 1986, so it's safe to assume it's presence.
@DerDakon DerDakon added this to the 1.09 milestone May 10, 2022
@DerDakon DerDakon merged commit 31c590b into master May 10, 2022
@DerDakon DerDakon deleted the Dakon-qstat branch May 10, 2022 14:34
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.

None yet

5 participants