motd: Generate from template to /var/run

[cem]

Summary

Suggested by: delphij@

Test Plan

I have not tested it yet.

[cem]

• Fix a typo in rc.d/motd
• Update login(1) and motd.5

[bcr]

OK for the manpage changes.

[cem]

Thanks, Benedict!

[delphij]

libexec/rc/rc.d/motd

34

Because this is a template, it do not really need the header anymore, because we prepend it anyway. This should be replaced with:

sed '1{/^FreeBSD.*/{d;};};' "${COMPAT_MOTD}" > "${TEMPLATE}"

39

TEMPLATE permission should be set here (or after line 34).

41

41-42: ln -sF "${TARGET}" "${COMPAT_MOTD}" would take care of the pre-existing file.

45

45-47: If after the above dance the template still do not exist as a file, we should bail out (return) or just ignore.

49–50

49-52 should be deleted. It's fine when ${TARGET} do not exist, we can generate its contents.

56–59

Note that since we now store template separately, this edit should only happen once. In other words, replace this with cat "${TEMPLATE}" >> ${T}

usr.bin/login/motd.template

1

This line should be deleted.

[cem]

Sorry about the delay! I've been traveling and not very available. Thanks for the feedback, I will try to address it now.

libexec/rc/rc.d/motd

34

Thanks, will fix.

39

Will fix, thanks.

41

-F or -f? Do we have any reason to believe /etc/motd is a directory? And if it is, that it is empty?

Yes, -f takes care of the expected preexisting file. I guess ln -sf unconditionally unlinks and recreates the symlink inode every time, but we are still conditional on [ ! -f $TEMPLATE ], so probably the IO isn't a concern.

Will plan to change to -f unless you feel strongly about -F.

45

Ok

49–50

Hm, what do you think about testing ${TARGET}/.. (i.e., "/var/run") for writability instead?

if [ ! -w /var/run ] || [ -f "${TARGET}" -a ! -w "${TARGET}" ]; then
    ...

OTOH, the test is inherited from the pretty ancient script; maybe we can assume we have a writable /var/run now. I am ok to delete it if you prefer.

56–59

Will fix.

usr.bin/login/motd.template

1

Will fix, thanks.

[cem]

• Discard header from template (both from new install and compat import of existing motd)
• Explicitly set template file permission when created from legacy motd
• Simplify replacing preexisting non-symlink /etc/motd
• Correct writability test for generated TARGET
• Simplify construction of /var/run/motd from template with header discarded

[delphij]

Thanks! Please see my comment inline.

I'd perfer -F be used (see reasoning inline; but I won't insist); the writeability check should be removed as install would give more meaningful messages.

libexec/rc/rc.d/motd

41

I don't have strong opinion here but it was intentional that -F is used (reasoning below):

When -f is used, and /etc/motd was a directory, and there is nothing preventing ln from creating /etc/motd/motd, it would silently succeed with unwanted result; on the other hand with -F, ln would make its best effort (delete empty directory /etc/motd or pre-existing /etc/motd file and create the symlink), and fail when it can't do it, so overall it's slightly better than -f.

49–50

Yeah let's just remove this portion of code. The install below would emit more meaningful error messages, and this test is not really accurate anyway.

[cem]

libexec/rc/rc.d/motd

41

Ok, will change.

49–50

works for me!

[cem]

• -F
• Drop not useful TARGET writability test

[delphij]

LGTM, thanks!