openssl.pod: Move detailed text on options to new specific pages like verification.pod #13315

DDvO · 2020-11-04T13:13:12Z

While doing #13312 I noticed again that
details on certificate verification, including a rather long list of options, are part of the top-level openssl app documentation
(and that doc/man1/openssl-verify.pod.in contains a very vague reference to openssl.pod).

I felt that this is pretty imbalanced and moved those contents to a new .pod file, adapting the various references to it.

doc/man7/verification.pod

richsalz · 2020-11-04T13:47:02Z

The OpenSSL page has all the common options. Moving just this one set of options seems like it's a partial job; if this is necessary they should all be moved. I don't understand the "seems unbalanced" comment. This PR makes things unbalanced. :)

As a second issue, putting command line information into man7 is just plain bizarre.

Perhaps splitting openssl.pod into man1/openssl-options.pod and moving all the option sections should be done.

DDvO · 2020-11-04T14:21:10Z

The OpenSSL page has all the common options. Moving just this one set of options seems like it's a partial job; if this is necessary they should all be moved. I don't understand the "seems unbalanced" comment. This PR makes things unbalanced. :)

It may be unbalanced in the way you state here, but I found the original state unbalanced in a different way:
heaps of detail on cert verification and its options as part of the doc of the overall openssl app.

As a second issue, putting command line information into man7 is just plain bizarre.

Your wording 'plain bizarre' is inappropriate.
I had looked it up: man7 is for mixed contents, which IMHO may be anything.

Also note that, as I wrote above to the first review comment, most of this text basically pertains to both the CLI and lib level, though the latter is not prominent and should be mentioned more explicitly.
BTW, I see this as a further imbalance, which apparently you introduced in October 2012 in commit 21d08b9.

Perhaps splitting openssl.pod into man1/openssl-options.pod and moving all the option sections should be done.

I'd prefer to move each class of options to a separate file.

richsalz · 2020-11-04T14:25:50Z

BTW, I see this as a further imbalance, which apparently you introduced in October 2012 in commit 21d08b9.

Yes, the documentation at the time had too much cut and paste duplication,
and things were updated in one place and not all others, as that commit shows.

That's true.

But I think your comment is off, since that commit wasn't in October 2012 :)

Right, I confused the day with the year - the commit is of Sat Oct 12 17:45:56 2019

I seem to have offended you, and I am sorry if I did.

Fine now :)

I just found to my surprise that much of the CLI-vs-lib documentation redundancy is still there, at least regarding the verification flags, namely in doc/man3/X509_VERIFY_PARAM_set_flags.pod.

DDvO · 2020-11-04T15:25:10Z

Oops, @richsalz by mistake I put my responses to your last comment as an edit to your comment rather than quoting it in a new comment - sorry for that.
Luckily your original response is still there (and just appears as quoted).

DDvO · 2020-11-04T16:24:27Z

@t8m and @richsalz I meanwhile I see/agree that most of the extracted contents is still very CLI-oriented.
Therefore I've renamed doc/man7/verification.pod to doc/man1/openssl-verification.pod
and adapted the SYNOPSIS section as well as the references to the new file accordingly.

@richsalz, I agree that something similar {c,s}ould be done also for other classes of generic options,
but I do not have the time these days for further such cleanup. Feel free to help out here :)

richsalz · 2020-11-04T17:08:53Z

Glad you agree, but @DDvO it's not fair to do a part-way job and hope that the rest of the work will get done by someone else :)

DDvO · 2020-11-04T17:49:36Z

Glad you agree, but @DDvO it's not fair to do a part-way job and hope that the rest of the work will get done by someone else :)

It's pretty typical that the one who brings up an issue is the first candidate for solving it, which in this case is you ;-)

Actually there have been (and will be) loads of issues in OpenSSL (and elsewhere) where someone does an improvement while (for various reasons) does not at the same time fix further related issues.
And it's actually very natural at least in community-based projects that a change is done by someone who feels stronger than others the need for doing it.

In this case, I do not strongly feel the need for the extra changes you requested, and (as mentioned) I currently have extremely limited time, so I do what I can and need to focus on my core topic, which is certificates. If neither you nor anybody else does further improvements in the direction indicated we can open an issue such that the need for further such cleanup is not forgotten.

richsalz · 2020-11-04T19:08:07Z

A couple of months ago, when some of us in the FIPS project were asking to just finish 3.0 now, to avoid slipping the schedule yet again, @mattcaswell explained that the 3.0 release is about more than just FIPS, and that OpenSSL believes it is important to have a coherent and comprehensive release.

At this point, doing something part-way goes against that thread. It is annoying to see various "part of the way there" PR's, like this one, hoping that someone else will finish off the complete task. Actually it's more than annoying.

Sure, as volunteers I understand how time is limited, and not everything is within every volunteer's level of expertise. But this particular PR isn't fixing protocol bugs, it's moving documentation around. A similar example is the "new" deprecation stuff which now has two ways of deprecating functions in header files because the original PR author wanted others to finish the job. I am sure there are other examples.

At this point, I think the project should

Only work on what it just voted to work on
Reject any PR that doesn't finish the job or leave something inconsistent.

I really think the projec

DDvO · 2020-11-04T19:11:32Z

@richsalz, for good reason I did not indicate that this PR is targeting 3.0 - it is not really urgent.

richsalz · 2020-11-04T19:16:11Z

I think "not marking it as 3.0" isn't sufficient; there are several PR's not marked that are likely to go into 3.0 (e.g., #13316, #13318, #13319, #13320, and countless others). I'd prefer to see a hold of some sort.

DDvO · 2020-11-07T07:29:48Z

After resolving merge conflicts, Travis timeout out (unrelated to the doc changes done here) on one build,
so I'm kicking it by closing and re-opening this PR.

levitte · 2020-11-08T09:50:30Z

there are several PR's not marked that are likely to go into 3.0 (e.g., #13316, #13318, #13319, #13320, and countless others)

... except all those you mention there are marked to go into 3.0.

richsalz · 2020-11-08T16:48:05Z

... except all those you mention there are marked to go into 3.0.

I was wrong. I missed the milestone; I was looking only at the project field. (I still think many PR's are going to be part of 3.0 that aren't marked as such)

Since there is a post-3.0 milestone, I think this should get marked with that.

DDvO · 2020-11-09T07:13:35Z

I am not in the position to decide on whether this PR should go into 3.0 or not.
It's not technically urgent, but I think it would be good to include rather soon due to documentation improvements included.

DDvO · 2020-11-09T07:15:34Z

@t8m, thanks for the review you did a couple of days back.
I've addressed the comment you gave.
Do you see anything else to do/improve?

t8m · 2020-11-09T11:22:56Z

I am undecided on whether this is good change as whole or not. On one hand I agree that the openssl page is too long now and this would help making it shorter. On the other hand @richsalz has some merit in his arguments as well.

DDvO · 2020-11-09T11:59:35Z

How about the following suggestion:

I mark this PR as WIP and hand it over to my new working student later this week,
who shall move all other overly detailed/bulky sections out of openssl.pod
basically in the same way I've done it here already for the cert verification procedure and options description?

t8m · 2020-11-09T16:32:17Z

Yep, that could be a good way forward.

levitte · 2020-11-19T15:14:21Z

While I understand the desire to split things up file wise, I can see the risk for confusion when just reading the apropos output, for the very single reason that there is no command called verification. So suddenly, we have section files with split purposes, to describe specific commands, (x)or sometimes describe some common options.

This kind of restructuring needs to mature substantially...

DDvO · 2020-11-19T15:25:30Z

I'm also not too happy with this file name/location.
My first idea was doc/man7/verification.pod but I got pushback on that (see above).

Then I tried doc/man1/verification.pod but then ./util/find-doc-nits -n -l -e said:

doc/man1/verification.pod doesn't start with openssl-

So should I better use doc/man1/verification.pod and make ./util/find-doc-nits relax on this,
or is there any other (better) suggestion?

DDvO · 2020-11-30T16:49:38Z

@seardes and me have meanwhile finished moving also other bulky (likely distracting) detail on
generic options and their parameters out of openssl.pod into specific new openssl-*-options.pod files.

So I've removed the WIP: from the PR title to indicate that it is ready for review again.
Now the remaining contents of openssl.pod should be pretty balanced; let us know if you (dis)agree.

DDvO · 2020-12-03T14:24:28Z

@seardes, could you please rebase this to fix merge conflict that meanwhile came up.

seardes · 2020-12-03T14:31:01Z

@seardes, could you please rebase this to fix merge conflict that meanwhile came up.

@DDvO, Will do

…special

…on-options.pod

… and Format Options Move detailed doc to specific new files in doc/man1/openssl-*-options.pod

DDvO · 2020-12-04T07:13:17Z

@seardes, unfortunately your resolution of non-trivial merge conflicts went wrong. I've fixed this.

DDvO · 2020-12-04T07:15:26Z

As mentioned, meanwhile we have finished extending this PR.
It's hard to keep this PR current due to high potential of non-trivlal conflicts.
@t8m, are you up for continuing your review on this?

openssl-machine · 2020-12-05T10:00:18Z

This pull request is ready to merge

…special Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from #13315)

…on-options.pod Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from #13315)

… and Format Options Move detailed doc to specific new files in doc/man1/openssl-*-options.pod Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com> (Merged from #13315)

DDvO · 2020-12-05T17:07:11Z

Merged - thanks @t8m, @richsalz, and @seardes

levitte · 2020-12-07T07:51:50Z

doc/man1/openssl-namefmt-options.pod

+
+=head1 NAME
+
+openssl-namefmt-options - DN display options


"Name display options" or "Distinguished name display options"

Also, I'm wondering if these shouldn't be exactly that throughout the manuals, "name display options". That would avoid having them mixed up with the other format options

That includes the file name of this pod.

levitte · 2020-12-07T07:57:22Z

doc/man1/openssl-asn1parse.pod.in

@@ -41,7 +41,7 @@ Print out a usage message.
 =item B<-inform> B<DER>|B<PEM>

 The input format; the default is B<PEM>.
-See L<openssl(1)/Format Options> for details.
+See L<openssl-format-options(1)/Format Options> for details.


I don't see the need to include the section. L<openssl-format-options(1)> should suffice.

This applies to every such link you changed 😉

levitte · 2020-12-07T07:58:43Z

Argh, I didn't notice this was closed... sorry

DDvO · 2020-12-07T09:25:12Z

Argh, I didn't notice this was closed... sorry

Thanks anyway @levitte for your good comments.
@seardes could you please handle those in a new PR.

seardes · 2020-12-07T09:55:59Z

Argh, I didn't notice this was closed... sorry

Thanks anyway @levitte for your good comments.
@seardes could you please handle those in a new PR.

@DDvO Sure, will do!

DDvO added the approval: otc review pending This pull request needs review by an OTC member label Nov 4, 2020

t8m reviewed Nov 4, 2020

View reviewed changes

doc/man7/verification.pod Outdated Show resolved Hide resolved

DDvO force-pushed the restructure_verification_doc branch from a804273 to 5ae723a Compare November 4, 2020 16:17

DDvO changed the title ~~Move verification doc from doc/man1/openssl.pod to new doc/man7/verification.pod~~ Move verification doc from openssl.pod to new openssl-verification.pod Nov 4, 2020

richsalz mentioned this pull request Nov 4, 2020

Really deprecate the old NAMEerr() macros #13320

Closed

DDvO force-pushed the restructure_verification_doc branch from 5ae723a to 709a310 Compare November 6, 2020 10:32

DDvO closed this Nov 7, 2020

DDvO reopened this Nov 7, 2020

DDvO changed the title ~~Move verification doc from openssl.pod to new openssl-verification.pod~~ WIP: Move verification doc from openssl.pod to new openssl-verification.pod Nov 9, 2020

DDvO force-pushed the restructure_verification_doc branch from 709a310 to 1559deb Compare November 19, 2020 15:07

seardes force-pushed the restructure_verification_doc branch from f4f0a5c to 654b84d Compare November 30, 2020 15:13

DDvO force-pushed the restructure_verification_doc branch from ed06f1d to 4a4a320 Compare November 30, 2020 16:43

DDvO changed the title ~~WIP: openssl.pod: Move detailed text on options to new specific pages like verification.pod~~ openssl.pod: Move detailed text on options to new specific pages like verification.pod Nov 30, 2020

DDvO and others added 3 commits December 4, 2020 07:55

find-doc-nits: fix regexp and point out that CA.pl and tsget.pod are …

7cb082a

…special

openssl.pod: Move verification doc to new doc/man1/openssl-verificati…

f16fc5f

…on-options.pod

openssl.pod: Carve out Trusted Certificate, Pass Phrase, Name Format,…

14adc65

… and Format Options Move detailed doc to specific new files in doc/man1/openssl-*-options.pod

DDvO force-pushed the restructure_verification_doc branch from d308fe1 to 14adc65 Compare December 4, 2020 07:11

t8m approved these changes Dec 4, 2020

View reviewed changes

t8m added approval: done This pull request has the required number of approvals branch: master Merge to master branch and removed approval: otc review pending This pull request needs review by an OTC member labels Dec 4, 2020

openssl-machine removed the approval: done This pull request has the required number of approvals label Dec 5, 2020

openssl-machine added the approval: ready to merge The 24 hour grace period has passed, ready to merge label Dec 5, 2020

openssl-machine pushed a commit that referenced this pull request Dec 5, 2020

find-doc-nits: fix regexp and point out that CA.pl and tsget.pod are …

6b480ee

…special Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from #13315)

openssl-machine pushed a commit that referenced this pull request Dec 5, 2020

openssl.pod: Move verification doc to new doc/man1/openssl-verificati…

b6f18ed

…on-options.pod Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from #13315)

DDvO closed this Dec 5, 2020

levitte reviewed Dec 7, 2020

View reviewed changes

seardes mentioned this pull request Dec 9, 2020

Fixup 13315 openssl options doc #13651

Closed

1 task

levitte mentioned this pull request Jan 18, 2021

openssl(1) section "PASS PHRASE ARGUMENTS" #13883

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

openssl.pod: Move detailed text on options to new specific pages like verification.pod #13315

openssl.pod: Move detailed text on options to new specific pages like verification.pod #13315

DDvO commented Nov 4, 2020

richsalz commented Nov 4, 2020

DDvO commented Nov 4, 2020

richsalz commented Nov 4, 2020 •

edited by DDvO

DDvO commented Nov 4, 2020

DDvO commented Nov 4, 2020

richsalz commented Nov 4, 2020

DDvO commented Nov 4, 2020

richsalz commented Nov 4, 2020

DDvO commented Nov 4, 2020 •

edited

richsalz commented Nov 4, 2020

DDvO commented Nov 7, 2020

levitte commented Nov 8, 2020

richsalz commented Nov 8, 2020

DDvO commented Nov 9, 2020

DDvO commented Nov 9, 2020

t8m commented Nov 9, 2020

DDvO commented Nov 9, 2020 •

edited

t8m commented Nov 9, 2020

levitte commented Nov 19, 2020

DDvO commented Nov 19, 2020 •

edited

DDvO commented Nov 30, 2020

DDvO commented Dec 3, 2020

seardes commented Dec 3, 2020

DDvO commented Dec 4, 2020

DDvO commented Dec 4, 2020

openssl-machine commented Dec 5, 2020

DDvO commented Dec 5, 2020

levitte Dec 7, 2020

levitte Dec 7, 2020

levitte Dec 7, 2020

levitte Dec 7, 2020

levitte commented Dec 7, 2020

DDvO commented Dec 7, 2020

seardes commented Dec 7, 2020

openssl.pod: Move detailed text on options to new specific pages like verification.pod #13315

openssl.pod: Move detailed text on options to new specific pages like verification.pod #13315

Conversation

DDvO commented Nov 4, 2020

richsalz commented Nov 4, 2020

DDvO commented Nov 4, 2020

richsalz commented Nov 4, 2020 • edited by DDvO

DDvO commented Nov 4, 2020

DDvO commented Nov 4, 2020

richsalz commented Nov 4, 2020

DDvO commented Nov 4, 2020

richsalz commented Nov 4, 2020

DDvO commented Nov 4, 2020 • edited

richsalz commented Nov 4, 2020

DDvO commented Nov 7, 2020

levitte commented Nov 8, 2020

richsalz commented Nov 8, 2020

DDvO commented Nov 9, 2020

DDvO commented Nov 9, 2020

t8m commented Nov 9, 2020

DDvO commented Nov 9, 2020 • edited

t8m commented Nov 9, 2020

levitte commented Nov 19, 2020

DDvO commented Nov 19, 2020 • edited

DDvO commented Nov 30, 2020

DDvO commented Dec 3, 2020

seardes commented Dec 3, 2020

DDvO commented Dec 4, 2020

DDvO commented Dec 4, 2020

openssl-machine commented Dec 5, 2020

DDvO commented Dec 5, 2020

levitte Dec 7, 2020

Choose a reason for hiding this comment

levitte Dec 7, 2020

Choose a reason for hiding this comment

levitte Dec 7, 2020

Choose a reason for hiding this comment

levitte Dec 7, 2020

Choose a reason for hiding this comment

levitte commented Dec 7, 2020

DDvO commented Dec 7, 2020

seardes commented Dec 7, 2020

richsalz commented Nov 4, 2020 •

edited by DDvO

DDvO commented Nov 4, 2020 •

edited

DDvO commented Nov 9, 2020 •

edited

DDvO commented Nov 19, 2020 •

edited