Lint all manpages at default level

[nabijaczleweli]

Motivation and Context

#12125 (comment)

Description

On top of #12125 right now, but uhh see first commit for the linter impl, rest are generated; it might be preferable to squash them (all? except for a few?)

Also general cleanup and/or style alignment of the manpages I touch anyway and flattening single-variant subcommands

Posting this because I'll forget otherwise and my wrist is killing me so I won't forseeably finish this in this sitting

How Has This Been Tested?

Looked at it

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.

[nabijaczleweli]

Okay, that's all of them. Open questions for someone with a fresh pair of eyes and/or understanding:

1. zfs-jail was kinda soupy, I desoupified it, but it could still be wet?
2. zfs-project was as well, to a lesser extent, and what triggered Set mode was unclear to me: I assumed it's zfs project -p id with no other flags – is that right?
3. The one .br subcommand in zfs send --redact redaction_bookmark [-DLPcenpv] [-i snapshot|bookmark] snapshot (here and here): is it just line-broken (which is what I assumed) or is it two commands with the same description?

[nabijaczleweli]

For later reference, these are the pages untouched by this and #12125:

• man/man5/spl-module-parameters.5
• man/man5/zfs-module-parameters.5
• man/man5/zpool-features.5
• man/man8/zfs-mount-generator.8.in
• man/man8/zfs-wait.8
• man/man8/zpool-destroy.8
• man/man8/zpool-export.8
• man/man8/zpool-history.8
• man/man8/zpool-labelclear.8
• man/man8/zpool-reguid.8

[behlendorf]

@nabijaczleweli this can now be rebased on master which includes the modernization patches.

[nabijaczleweli]

Rebased, tentatively undrafting, but #12129 (comment) still stands

[nabijaczleweli]

Rebased

[nabijaczleweli]

Well, man/man?/Makefile.ams all replace .Os with .Os Linux in the install hook (which isn't necessarily right, but I have something staged for later for this) for this reason exactly, and it's a mandoc lint to set .Os FreeBSD:

       operating system explicitly specified
       (mdoc, OpenBSD, NetBSD) The Os macro has an argument.  In the base system, it is conventionally left blank.

nabijaczleweli@tarta:~/store/code/zfs$ make mancheck
./scripts/mancheck.sh ./man ./tests/test-runner/man
mandoc: ./man/man8/zfs-jail.8:41:5: STYLE: operating system explicitly specified: Os FreeBSD (NetBSD)
make: *** [Makefile:1520: mancheck] Error 1

Plus, making it just .Os is consistent with every other manpage we ship.

[rlaager]

In general, this looks good. It's a pretty big changeset in terms of diff size, so I admit I skimmed a bit. But I did skim all of the commits and I did quickly view all of the man pages.

I left a few comments. A bunch were about making the pluralization of the summary match the behavior of the command. For example, if a command acts on one pool, it should say "... a ZFS storage pool" and if it can act on multiple pools, it should say "... ZFS storage pools".

I tried to mark things "maybe" where I wasn't sure.

@nabijaczleweli Can you make one pass through to check the pluralization and merge anything else that seems sane? Then (barring objections), we should be able to get this merged.

[nabijaczleweli]

In rereading some pages, it turned out I just delinted some of them without fixing the glaring holes – this has been addressed, alongside some titles, as seen above.

I've amended all pages to where they should be, but here's the interdiff from the version you reviewed: inter.diff.gz

[rlaager]

Skimming the interdiff, that looks fine to me. I see you've replaced ... with an actual ellipse character, but that's probably fine. After all, it's 2021 and Unicode support should be reasonably prevalent. I'm not sure that we need to cater to anything non-UTF-8.

[nabijaczleweli]

Yeah, I've already done that intermittently but decided to enforce it on all arguments for consistency (and because it looks infinitely better, and isn't three full cells wide)

[tonynguien]

It's a big change :) Thank you.

I confirmed mandoc -Tlint no longer complains about these files though I only skimmed through the diffs.

Can you squash the commits keeping only those worth preserving? A single a commit would be good too.

[nabijaczleweli]

Squashed all manpage commits into one big one

[rlaager]

With the caveat that I don't agree with dropping articles ("a", "an", "the") from the command summaries (both because it reads better and because I think it subtly provides useful information about whether the command acts on a single or multiple objects), I think this is good to merge.

[behlendorf]

zfs-jail was kinda soupy, I desoupified it, but it could still be wet?

The updated version looks right to me. My only thought is it wouldn't be bad to make it very clear this functionality only applies to FreeBSD. Of course, it's probably not needed since it's right there in the name, "zfs-jail — attach or detach ZFS filesystem from FreeBSD jail".

zfs-project was as well, to a lesser extent, and what triggered Set mode was unclear to me: I assumed it's zfs project -p id with no other flags – is that right?

That's right, The updated page looks good too, a little less wordy in places which is nice.

The one .br subcommand in zfs send --redact redaction_bookmark [-DLPcenpv] [-i snapshot|bookmark] snapshot (here and here): is it just line-broken (which is what I assumed) or is it two commands with the same description?

You got it right. It's just one command.

I skimmed over the rest of the pages and they all looked reasonable to me. Definitely a big step in the right direction, thanks!

[nabijaczleweli]

Point; added the "Jails are a .Fx feature and are not relevant on other platforms." blurb from zfsprops(8) to the last paragraph after "See .Xr jail 8 for more information on managing jails."

[behlendorf]

@nabijaczleweli when testing this on Ubuntu 20.04 I was able to consistently reproduce the following warning. It looks like for newer versions of awk we get this warning because the quote doesn't need to be escaped. If you concur, I'll include this one character change when merging this which fixes the warning.

diff --git a/scripts/mancheck.sh b/scripts/mancheck.sh
index 65f32a78b..a5b8b0d0a 100755
--- a/scripts/mancheck.sh
+++ b/scripts/mancheck.sh
@@ -29,7 +29,7 @@ IFS="
 files="$(find "$@" -type f -name '*[1-9]*' ! -name '*module-param*' ! -name 'zpool>
 
 add_excl="$(awk '
-    /^.\\\" lint-ok:/ {
+    /^.\\" lint-ok:/ {
         print "-e"
         $1 = "mandoc:"
         $2 = FILENAME ":[[:digit:]]+:[[:digit:]]+:"

> make mancheck
./scripts/mancheck.sh ./man ./tests/test-runner/man
awk: cmd. line:2: warning: regexp escape sequence `\"' is not a known regexp operator

[nabijaczleweli]

Yeah, that looks good! Thanks for catching this