Support RFC5322 subject header for zed_notify_email()

[heeplr]

Motivation and Context

Setting e-mail subject via ZED_EMAIL_OPTS doesn't always work. Small/lightweight MUAs (e.g. ssmtp) need a "Subject: ..." header in the mail body.

Description

If no @SUBJECT@ placeholder is found in ZED_EMAIL_OPTS, the notification file will be prepended with a subject header.

How Has This Been Tested?

#!/bin/sh

. "./zed-functions.sh"
. "./zed.rc"

echo "foo" > /tmp/test
zed_notify_email "Test notification" /tmp/test

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[tonyhutter]

Could you squash your two commits and modify the commit message?:

zed: Support subject as header in zed_notify_email()

Some minimal MUAs don't support passing the subjects as cmdline option.
This commit checks if "@subject@" is missing in ZED_EMAIL_OPTS and then
prepends a subject header to the notification message.

Signed-off-by: Daniel Hiepler <d-git@coderdu.de>

[tonyhutter]

One of the lines in your commit message is over 80 chars long. Would you mind breaking it into two lines? (see my earlier commit message example) We typically don't have > 80 chars in a commit line unless there's a compelling reason to.

Other than that, it looks good to go!

[nabijaczleweli]

In-placing /dev/null ($# -eq 1) fails, and editing the message in-place breaks expectations (and possibly mangles the message repeatedly forever); this is better served as

    if [ "${ZED_EMAIL_OPTS%@SUBJECT@*}" = "${ZED_EMAIL_OPTS}" ]; then
        [ -s "${pathname}" ] && sed -i "1iSubject: ${subject}
" "${pathname}" || echo "Subject: ${subject}"
    else
        cat "${pathname}"
    fi | eval ${ZED_EMAIL_PROG} ${ZED_EMAIL_OPTS} >/dev/null 2>&1

or, well,

    {
        [ "${ZED_EMAIL_OPTS%@SUBJECT@*}" = "${ZED_EMAIL_OPTS}" ] && echo "Subject: ${subject}"
        cat "${pathname}"
    } | eval ${ZED_EMAIL_PROG} ${ZED_EMAIL_OPTS} >/dev/null 2>&1

[heeplr]

@nabijaczleweli thanks, good catch! I adressed those issues.

Also I've added a default subject, so mail notifications don't get discarded if the subject is empty. @tonyhutter Should I squash those commits? (I didn't since I'd see them as different changes but can do if that's better.)

I tested with:

#!/bin/sh

. "./zed-functions.sh"
. "./zed.rc"

echo "foo" > /tmp/test

zed_notify_email "Test notification" /tmp/test
zed_notify_email "Test no body" /dev/null
zed_notify_email "" /tmp/test

rm /tmp/test

[nabijaczleweli]

Yeah, code-wise that's fine. I'm not sure what the point of the comments is, though, since they're all, almost literally

# echo subject header
echo "Subject: $subject"

and they make up 90% of the diff

[heeplr]

@nabijaczleweli the point of the comments is to serve as alternative or addition to the actual code to describe what's going on to everybody, even those who don't instantly recognize what the code is doing.

The section you cited was already changed in the current diff. I agree that this was a superflous comment.

and they make up 90% of the diff

I could optimize the diff for size but I'd abstain from that as they don't impose a lot of cost. Although I'd welcome anyone to remove them if they don't fit the general style and wouldn't insist on them.

[tonyhutter]

@heeplr yes, please squash the commits.