Bug 245283

Summary: sysutils/checkrestart: improve rc script
Product: Ports & Packages Reporter: Fernando Apesteguía <fernape>
Component: Individual Port(s)Assignee: freebsd-ports-bugs (Nobody) <ports-bugs>
Status: Closed FIXED    
Severity: Affects Only Me CC: tom
Priority: --- Keywords: buildisok
Version: LatestFlags: bugzilla: maintainer-feedback? (tom)
Hardware: Any   
OS: Any   
Attachments:
Description Flags
Patch to 0.5.0_1
tom: maintainer-approval+
Alternate patch to 0.5.0_1 with security periodic script tom: maintainer-approval+

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.