Bug 245283 - sysutils/checkrestart: improve rc script
Summary: sysutils/checkrestart: improve rc script
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-ports-bugs mailing list
URL:
Keywords: buildisok
Depends on:
Blocks:
 
Reported: 2020-04-02 14:05 UTC by Fernando Apesteguía
Modified: 2020-04-04 12:07 UTC (History)
1 user (show)

See Also:
bugzilla: maintainer-feedback? (tom)


Attachments
Patch to 0.5.0_1 (883 bytes, patch)
2020-04-02 15:38 UTC, Thomas Hurst
tom: maintainer-approval+
Details | Diff
Alternate patch to 0.5.0_1 with security periodic script (5.06 KB, patch)
2020-04-04 03:36 UTC, Thomas Hurst
tom: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fernando Apesteguía freebsd_committer 2020-04-02 14:05:24 UTC
Hi Thomas,

Would change the hand-rolled Yes-no check to "checkyesno" from rc.subr(8)?

https://www.freebsd.org/doc/en/books/porters-handbook/rc-scripts.html

Also /usr/local in files/checkrestart.in should be changed to %%PREFIX%%.

Cheers!
Comment 1 Thomas Hurst 2020-04-02 15:36:22 UTC
Whoops, could have sworn I changed it to %%PREFIX%% before submission.

It's a periodic(8) script, so sourcing rc.subr and using checkyesno seems inappropriate - literally no other periodic script, neither in base nor ports, does so.  The case statement is the de-facto standard way to do it.

If there's to be something like this it should probably go in /etc/defaults/periodic.conf alongside check_yesno_period.
Comment 2 Thomas Hurst 2020-04-02 15:38:20 UTC
Created attachment 212982 [details]
Patch to 0.5.0_1

Use %%PREFIX%% in checkrestart.in and bump PORTREVISION in case a user has modified LOCALBASE.
Comment 3 Automation User 2020-04-02 15:55:41 UTC
Build info is available at https://gitlab.com/swills/freebsd-ports/pipelines/132283479
Comment 4 Fernando Apesteguía freebsd_committer 2020-04-03 08:31:09 UTC
(In reply to Thomas Hurst from comment #1)
Tom, I think you're right.

Could you please change the script to use check_yesno_periodic similarly as it is done in security/base-audit?

Thanks for your understanding.
Comment 5 Thomas Hurst 2020-04-04 03:36:50 UTC
Created attachment 213047 [details]
Alternate patch to 0.5.0_1 with security periodic script

This patch switches to check_yesno_period - and since doing so only makes sense in the context of a security periodic(8) script, that has been changed as well, along with an upgrade message notifying users of the change.
Comment 6 commit-hook freebsd_committer 2020-04-04 12:06:09 UTC
A commit references this bug:

Author: fernape
Date: Sat Apr  4 11:58:04 UTC 2020
New revision: 530648
URL: https://svnweb.freebsd.org/changeset/ports/530648

Log:
  sysutils/checkrestart: improve rc script

  * Use %%PREFIX%% in checkrestart.in
  * Use check_yesno_period instead of handmade solution

  PR:	245283
  Submitted by:	tom@hur.st (maintainer)
  Reported by:	0mp@

Changes:
  head/sysutils/checkrestart/Makefile
  head/sysutils/checkrestart/files/checkrestart.in
  head/sysutils/checkrestart/pkg-message
Comment 7 Fernando Apesteguía freebsd_committer 2020-04-04 12:07:42 UTC
Committed,

Thank you very much.