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

Man page scan results for rear #1892

Closed
gdha opened this issue Aug 6, 2018 · 17 comments
Closed

Man page scan results for rear #1892

gdha opened this issue Aug 6, 2018 · 17 comments

Comments

@gdha
Copy link
Member

gdha commented Aug 6, 2018

In order to improve usability of packages in Fedora, project Man Page Scan was created and its task is to provide consistency of man pages (and documentation in general). The results are now available for package maintainers to fix documentation issues.

If you need to re-run the check yourself, here is the simple process of man page check:

  1. Download man-page-day from:

    https://pagure.io/ManualPageScan/blob/master/f/man-page-day.sh

  2. Run the script:

$ /home/gdha/Downloads/man-page-day.sh rear
Current version of package:
rear-2.4-26.git.0.9a6f640.unknown.el7.x86_64

checking whether binary files have man pages:

[ OK ] binary /usr/sbin/rear has man page /usr/share/man/man8/rear.8.gz
[ Warn ] man page is in section 8. Only commands requiring admin privileges, or deamons should be in this section, the rest should be in section 1

checking whether config files have man pages:

[ Error ] config file /etc/cron.d/rear has no man page! Check it's content for proper documentation
[ Error ] config file /etc/rear/local.conf has no man page! Check it's content for proper documentation

checking for unused man pages:

[ OK ] no man pages left

now checking individual man pages:

checking /usr/share/man/man8/rear.8.gz:
[ OK ] man page parsing with lexgrog succeeded
[ OK ] man page doesn't contain any repeated word

Summary: 4x OK, 1 warnings, 2 errors
@gdha gdha added the enhancement Adaptions and new features label Aug 6, 2018
@gdha gdha self-assigned this Aug 6, 2018
@jsmeix
Copy link
Member

jsmeix commented Aug 7, 2018

@gdha
whether or not /etc/cron.d/rear exists depends on the particular RPM package.

We do not have any 'cron' file in the ReaR upstream source files.
But we do some cron setup in the RPM spec file
https://github.com/rear/rear/blob/master/packaging/rpm/rear.spec
which seems to be used for Fedora but I cannot use it for SUSE.

In particular my SUSE and openSUSE rear* RPM packages
(i.e. those rear* RPM packages where I make the RPM spec file
cf. #1855 (comment) )
do not provide any cron file because I am against any kind of automatically
activated "service-like-thingies" in RPM packages.
With "service-like-thingy" I mean anything that runs automatically
without having been explicitly activated by the user, i.e. real services
(like the cupsd) but also things like cron jobs and advanced udev rules
(when a udev rule does more than the basic hardware enablement).
Furthermore automatically activated services are in general forbidden
by the SUSE and openSUSE guidelines how to make RPM packages
for SUSE and openSUSE.

@gdha
Copy link
Member Author

gdha commented Jan 19, 2019

@jsmeix in our upstrean (#1892 (comment)) we do have a cron entry enabled. Which leads to the following question: do we still want this in upcoming releases? I know from end-users it generates ISO images without rear being properly configured, and therefore, it would perhaps be better we disable the cron entry altogether in rear. If we describe it in our documentation should be sufficient, no?

@schlomo
Copy link
Member

schlomo commented Jan 19, 2019

What is the RHEL policy? Do they shop a cron job? If not then I would also move it to the docs or examples area so that the users have a more consistent RPM installation experience. This is especially important if users upgrade a distro RPM with our upstream RPM. In that scenario the perceived behavior should not change.

@gdha
Copy link
Member Author

gdha commented Jan 20, 2019

@rmetrich What are your thoughts on this topic?
@schabrolles @jsmeix @gozora And, what are your opinions?

@jsmeix
Copy link
Member

jsmeix commented Jan 21, 2019

@gdha
we may add such a 'cron' file to the upstream sources as an example file
or provide it only as a piece of text in a documentation.

The latter is already there in
https://github.com/rear/rear/blob/master/doc/rear-presentation.adoc

Use case: Centralized images /2
...

*  Relax-and-Recover cron-job at /etc/cron.d/rear:

30 0 1 * * root /usr/sbin/rear mkrescue
30 1 * * * root /usr/sbin/rear checklayout || /usr/sbin/rear mkrescue

I think we must not install such a 'cron' file into /etc/cron.d/
on an end-user system because I think we are not allowed
to silently add an automatically activated "service-like-thingy"
on the end-user's system.

I think our current documentation is sufficient
i.e. I think an example 'cron' file is not needed.

I think the current example RPM spec file
https://github.com/rear/rear/blob/master/packaging/rpm/rear.spec
should be improved by removing therein what installs
such a 'cron' file into /etc/cron.d/ on the user's system.

Alternatively we may have several example RPM spec files
as needed for different Linux distributions, for example
rename packaging/rpm/ into packaging/fedora/ and
add a packaging/suse/ for different RPM spec files
for Fedora and SUSE based Linux distributions.

@rmetrich
Copy link
Contributor

This automatic cron is broken and leads to having broken ReaR ISOs at the end, in my opinion, we should remove this file and provide a systemd service + timer instead, which wouldn't be enabled by default.

Example:

  • rear-rescue-iso.timer
[Unit]
Description=ReaR ISO Creation Timer Task
Documentation=man:rear(8)
After=network.target

[Timer]
OnCalendar=daily
RandomizedDelaySec=14400

[Install]
WantedBy=multi-user.target
  • rear-rescue-iso.service
[Unit]
Description=ReaR ISO Creation
Documentation=man:rear(8)
After=network.target

[Service]
Type=simple
ExecStart=/bin/sh -c '/usr/sbin/rear checklayout || /usr/sbin/rear mkrescue'
Restart=no
WatchdogSec=600
BlockIOWeight=100

@jsmeix
Copy link
Member

jsmeix commented Jan 21, 2019

@rmetrich
could you describe why calling rear mkrescue via cron
is broken and leads to having broken ReaR ISOs at the end
in contrast to calling the same rear mkrescue via systemd?
(Not all systems have systemd, e.g. SLES11 has not.)

What I know what cannot work reliably is calling rear mkrescue
via ReaR's 'udev' workflow that gets triggered via a udev rule in
etc/udev/rules.d/62-rear-usb.rules see
#840

@rmetrich
Copy link
Contributor

The cron entry doesn't catch errors and the admin will not notice that, unless he configures automatic email for example.
With the systemd service, this is more easily catchable.

@jsmeix
Copy link
Member

jsmeix commented May 6, 2019

@gdha
can we also remove /etc/cron.d/rear/ and its related things
from rear.spec for ReaR 2.5 ?

Cf.
#2135 (comment)

What I did there to remove /etc/cron.d/rear/ related things is
https://build.opensuse.org/package/rdiff/home:jsmeix:branches:Archiving:Backup:Rear:Snapshot/rear?linkrev=base&rev=5

--- rear.spec (revision 4)
+++ rear.spec (revision 5)
@@ -132,21 +132,16 @@
 %prep
 %setup -q -n rear-2.4-git.0.3930248.unknown
 
-echo "30 1 * * * root /usr/sbin/rear checklayout || /usr/sbin/rear mkrescue" >rear.cron
-
 %build
 
 %install
 %{__rm} -rf %{buildroot}
 %{__make} install DESTDIR="%{buildroot}"
-%{__install} -Dp -m0644 rear.cron %{buildroot}%{_sysconfdir}/cron.d/rear
-
 
 %files
 %defattr(-, root, root, 0755)
 %doc MAINTAINERS COPYING README.adoc doc/*.txt
 %doc %{_mandir}/man8/rear.8*
-%config(noreplace) %{_sysconfdir}/cron.d/rear
 %config(noreplace) %{_sysconfdir}/rear/
 %config(noreplace) %{_sysconfdir}/rear/cert/
 %{_datadir}/rear/

@gdha
Copy link
Member Author

gdha commented May 7, 2019

@jsmeix I agree that we better remove the cron.d entry
@rmetrich Indeed we better replace it with an inactive systemd service, however, the ISO in the name is a bit misleading as it depends what the user defined at the end, no? I would drop the ISO in the name.

@gdha gdha added this to the ReaR v2.5 milestone May 7, 2019
@rmetrich
Copy link
Contributor

rmetrich commented May 7, 2019

@gdha Sure, drop ISO :)

jsmeix added a commit that referenced this issue May 7, 2019
Removed the /etc/cron.d/rear/ related things in packaging/rpm/rear.spec
see #1892
@jsmeix
Copy link
Member

jsmeix commented May 7, 2019

I removed the /etc/cron.d/rear/ related things via
89a8f18

I noticed in packaging/rpm/rear.spec there is still

Requires: crontabs

Does that also belong to the /etc/cron.d/rear/ related things
that can be removed or is that needed in general?

@gdha
Copy link
Member Author

gdha commented May 7, 2019

@jsmeix Yes Requires: crontabs can be removed as well now

jsmeix added a commit that referenced this issue May 7, 2019
Removed "Requires: crontabs" in packaging/rpm/rear.spec
see #1892
@jsmeix
Copy link
Member

jsmeix commented May 7, 2019

Removed "Requires: crontabs" via
f98a187

@jsmeix
Copy link
Member

jsmeix commented May 7, 2019

Now the one error part of this issue

[ Error ] config file /etc/cron.d/rear has no man page!

is fixed.

The remaining error part

[ Error ] config file /etc/rear/local.conf has no man page!

is not yet fixed.

I think we do not need and should not have a "man local.conf" man page
in particular not because of its confusing/meaningless name local.conf.

I think we should close this issue as "fixed as far as reasonable".

@jsmeix jsmeix added the cleanup label May 7, 2019
@gdha
Copy link
Member Author

gdha commented May 7, 2019

systemd related changes in the spec file differs from:

guess we better post-pone this to release 2.6?

@jsmeix
Copy link
Member

jsmeix commented May 9, 2019

I created the new issue #2139
to provide a systemd service and timer to run

/usr/sbin/rear checklayout || /usr/sbin/rear mkrescue

in ReaR 2.6 so that this issue which is actually about
"Man page scan results for rear" can be closed for ReaR 2.5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants