| Message ID | 20170410121230.16029-3-stephen@that.guru |
|---|---|
| State | Superseded |
| Headers | show |
Thank you! rST is much more readable than nroff. I have some comments below. On Mon, Apr 10, 2017 at 01:12:28PM +0100, Stephen Finucane wrote: > Let's start with a simple one that lets us focus on setting up most of > the required "infrastructure" for building man pages using Sphinx. > > There are a couple of things worth noting here: > > - The 'check-htmldocs' target becomes 'check-docs' as its now > responsible for building man page docs too. > > - The outputted file will always have a '.1' suffix. This is Sphinx's > decision, and likely stems from the man page guidelines [1] which > state that "the name of the man page's source file...is the name of > the command, function or file name, followed by a dot, followed by the > section character". It looks to me like the last element of the tuples inside man_pages in conf.py controls the section. When I changed 1 to 8 there, it switched the manpage to section 8. So I made that change in conf.py and I removed the above paragraph from the commit message. > Other than that, hurrah for (mostly) legible syntaxes. > > [1] http://www.tldp.org/HOWTO/Man-Page/q2.html > > Signed-off-by: Stephen Finucane <stephen@that.guru> > --- > I don't know if this is correctly integrated into the docs build system > or not. I need someone to double check this for me. In particular, I > think I need to integrate the 'dh_sphinxdoc' package [2] into the Debian > build but I've no idea how to. > > [2] http://manpages.ubuntu.com/manpages/zesty/man1/dh_sphinxdoc.1.html I spent a couple of hours working on the build and install system here. My first thought was to add rules to allow Automake to find and install the generated manpages. This turned out to be a huge mess that required tons of nasty GNU Make specific crap in the makefiles, and I didn't like it. My second approach was better. I gave up on integrating with the builtin Automake rules for manpages. All those really do anyway is handle install and uninstall, so I wrote some Make rules to do that. They're ugly because they're make+shell, but readable enough if you squint. The main question for install and uninstall is how to choose the right section. The easiest way seems to be to give the .rst files names that embed the section, like "ovs-vlan-test.8.rst". This is also handy to distinguish manpages with the same name but in different sections, which is sometimes a reasonable thing to have, so that's what I did. The patch didn't delete the old manpage, so I did that too. I noticed that the "Synposis" section of the new ovs-vlan-test.rst did not use bold and italic in the canonical way for manpages, so I added * and ** in the right places.
On Thu, 2017-04-13 at 21:27 -0700, Ben Pfaff wrote: > Thank you! rST is much more readable than nroff. I have some > comments > below. > > On Mon, Apr 10, 2017 at 01:12:28PM +0100, Stephen Finucane wrote: > > Let's start with a simple one that lets us focus on setting up most > > of the required "infrastructure" for building man pages using > > Sphinx. > > > > There are a couple of things worth noting here: > > > > - The 'check-htmldocs' target becomes 'check-docs' as its now > > responsible for building man page docs too. > > > > - The outputted file will always have a '.1' suffix. This is > > Sphinx's > > decision, and likely stems from the man page guidelines [1] which > > state that "the name of the man page's source file...is the name > > of > > the command, function or file name, followed by a dot, followed > > by the > > section character". > > It looks to me like the last element of the tuples inside man_pages > in conf.py controls the section. When I changed 1 to 8 there, it > switched the manpage to section 8. So I made that change in conf.py > and I removed the above paragraph from the commit message. > > > Other than that, hurrah for (mostly) legible syntaxes. > > > > [1] http://www.tldp.org/HOWTO/Man-Page/q2.html > > > > Signed-off-by: Stephen Finucane <stephen@that.guru> > > --- > > I don't know if this is correctly integrated into the docs build > > system or not. I need someone to double check this for me. In > > particular, I think I need to integrate the 'dh_sphinxdoc' package > > [2] into the Debian build but I've no idea how to. > > > > [2] http://manpages.ubuntu.com/manpages/zesty/man1/dh_sphinxdoc.1.h > > tml > > I spent a couple of hours working on the build and install system > here. > > My first thought was to add rules to allow Automake to find and > install the generated manpages. This turned out to be a huge mess > that required tons of nasty GNU Make specific crap in the makefiles, > and I didn't like it. > > My second approach was better. I gave up on integrating with the > builtin Automake rules for manpages. All those really do anyway is > handle install and uninstall, so I wrote some Make rules to do that. > They're ugly because they're make+shell, but readable enough if you > squint. > > The main question for install and uninstall is how to choose the > right section. The easiest way seems to be to give the .rst files > names that embed the section, like "ovs-vlan-test.8.rst". This is > also handy to distinguish manpages with the same name but in > different sections, which is sometimes a reasonable thing to have, so > that's what I did. I still need to have a look into the patch (anything to do with autotools is generally worth avoiding, heh), but I had a crazy idea over the weekend: couldn't we just include the compiled man pages in the source? The only issue with this would be the '.TH' line, but there are solutions for avoiding this [1][2]. If we have man pages included in the source, would that be adequate to use the standard autotools setup for manpages? > The patch didn't delete the old manpage, so I did that too. > > I noticed that the "Synposis" section of the new ovs-vlan-test.rst > did not use bold and italic in the canonical way for manpages, so I > added * and ** in the right places. ++ Stephen [1] https://gitlab.labs.nic.cz/labs/knot/blob/bab62de/doc/Makefile.am#L 149-158 [2] https://gitlab.labs.nic.cz/labs/knot/blob/bab62de/doc/man/kdig.1in
On Fri, Apr 14, 2017 at 06:55:38PM +0100, Stephen Finucane wrote: > On Thu, 2017-04-13 at 21:27 -0700, Ben Pfaff wrote: > > Thank you! rST is much more readable than nroff. I have some > > comments > > below. > > > > On Mon, Apr 10, 2017 at 01:12:28PM +0100, Stephen Finucane wrote: > > > Let's start with a simple one that lets us focus on setting up most > > > of the required "infrastructure" for building man pages using > > > Sphinx. > > > > > > There are a couple of things worth noting here: > > > > > > - The 'check-htmldocs' target becomes 'check-docs' as its now > > > responsible for building man page docs too. > > > > > > - The outputted file will always have a '.1' suffix. This is > > > Sphinx's > > > decision, and likely stems from the man page guidelines [1] which > > > state that "the name of the man page's source file...is the name > > > of > > > the command, function or file name, followed by a dot, followed > > > by the > > > section character". > > > > It looks to me like the last element of the tuples inside man_pages > > in conf.py controls the section. When I changed 1 to 8 there, it > > switched the manpage to section 8. So I made that change in conf.py > > and I removed the above paragraph from the commit message. > > > > > Other than that, hurrah for (mostly) legible syntaxes. > > > > > > [1] http://www.tldp.org/HOWTO/Man-Page/q2.html > > > > > > Signed-off-by: Stephen Finucane <stephen@that.guru> > > > --- > > > I don't know if this is correctly integrated into the docs build > > > system or not. I need someone to double check this for me. In > > > particular, I think I need to integrate the 'dh_sphinxdoc' package > > > [2] into the Debian build but I've no idea how to. > > > > > > [2] http://manpages.ubuntu.com/manpages/zesty/man1/dh_sphinxdoc.1.h > > > tml > > > > I spent a couple of hours working on the build and install system > > here. > > > > My first thought was to add rules to allow Automake to find and > > install the generated manpages. This turned out to be a huge mess > > that required tons of nasty GNU Make specific crap in the makefiles, > > and I didn't like it. > > > > My second approach was better. I gave up on integrating with the > > builtin Automake rules for manpages. All those really do anyway is > > handle install and uninstall, so I wrote some Make rules to do that. > > They're ugly because they're make+shell, but readable enough if you > > squint. > > > > The main question for install and uninstall is how to choose the > > right section. The easiest way seems to be to give the .rst files > > names that embed the section, like "ovs-vlan-test.8.rst". This is > > also handy to distinguish manpages with the same name but in > > different sections, which is sometimes a reasonable thing to have, so > > that's what I did. > > I still need to have a look into the patch (anything to do with > autotools is generally worth avoiding, heh), but I had a crazy idea > over the weekend: couldn't we just include the compiled man pages in > the source? The only issue with this would be the '.TH' line, but there > are solutions for avoiding this [1][2]. If we have man pages included > in the source, would that be adequate to use the standard autotools > setup for manpages? I might not understand this idea yet. Do you mean, to include the nroff output of sphinx in the Git repository? I don't know why this would help. I'm not sure there are any serious disadvantages to writing Make rules to install and uninstall manpages.
On Fri, 2017-04-14 at 12:49 -0700, Ben Pfaff wrote: > On Fri, Apr 14, 2017 at 06:55:38PM +0100, Stephen Finucane wrote: > > On Thu, 2017-04-13 at 21:27 -0700, Ben Pfaff wrote: > > > Thank you! rST is much more readable than nroff. I have some > > > comments > > > below. > > > > > > On Mon, Apr 10, 2017 at 01:12:28PM +0100, Stephen Finucane wrote: > > > > Let's start with a simple one that lets us focus on setting up > > > > most > > > > of the required "infrastructure" for building man pages using > > > > Sphinx. > > > > > > > > There are a couple of things worth noting here: > > > > > > > > - The 'check-htmldocs' target becomes 'check-docs' as its now > > > > responsible for building man page docs too. > > > > > > > > - The outputted file will always have a '.1' suffix. This is > > > > Sphinx's > > > > decision, and likely stems from the man page guidelines [1] > > > > which > > > > state that "the name of the man page's source file...is the > > > > name > > > > of > > > > the command, function or file name, followed by a dot, > > > > followed > > > > by the > > > > section character". > > > > > > It looks to me like the last element of the tuples inside > > > man_pages > > > in conf.py controls the section. When I changed 1 to 8 there, it > > > switched the manpage to section 8. So I made that change in > > > conf.py > > > and I removed the above paragraph from the commit message. > > > > > > > Other than that, hurrah for (mostly) legible syntaxes. > > > > > > > > [1] http://www.tldp.org/HOWTO/Man-Page/q2.html > > > > > > > > Signed-off-by: Stephen Finucane <stephen@that.guru> > > > > --- > > > > I don't know if this is correctly integrated into the docs > > > > build > > > > system or not. I need someone to double check this for me. In > > > > particular, I think I need to integrate the 'dh_sphinxdoc' > > > > package > > > > [2] into the Debian build but I've no idea how to. > > > > > > > > [2] http://manpages.ubuntu.com/manpages/zesty/man1/dh_sphinxdoc > > > > .1.h > > > > tml > > > > > > I spent a couple of hours working on the build and install system > > > here. > > > > > > My first thought was to add rules to allow Automake to find and > > > install the generated manpages. This turned out to be a huge > > > mess > > > that required tons of nasty GNU Make specific crap in the > > > makefiles, > > > and I didn't like it. > > > > > > My second approach was better. I gave up on integrating with the > > > builtin Automake rules for manpages. All those really do anyway > > > is > > > handle install and uninstall, so I wrote some Make rules to do > > > that. > > > They're ugly because they're make+shell, but readable enough if > > > you > > > squint. > > > > > > The main question for install and uninstall is how to choose the > > > right section. The easiest way seems to be to give the .rst > > > files > > > names that embed the section, like "ovs-vlan-test.8.rst". This > > > is > > > also handy to distinguish manpages with the same name but in > > > different sections, which is sometimes a reasonable thing to > > > have, so > > > that's what I did. > > > > I still need to have a look into the patch (anything to do with > > autotools is generally worth avoiding, heh), but I had a crazy idea > > over the weekend: couldn't we just include the compiled man pages > > in > > the source? The only issue with this would be the '.TH' line, but > > there > > are solutions for avoiding this [1][2]. If we have man pages > > included > > in the source, would that be adequate to use the standard autotools > > setup for manpages? > > I might not understand this idea yet. Do you mean, to include the > nroff output of sphinx in the Git repository? I don't know why this > would help. Correct. > I'm not sure there are any serious disadvantages to writing Make > rules to install and uninstall manpages. The only significant advantage would be simplicity for use in other build systems. Would the rules you've proposed work for the Debian and RHEL packaging, for example? Stephen
On Fri, Apr 14, 2017 at 09:00:49PM +0100, Stephen Finucane wrote: > On Fri, 2017-04-14 at 12:49 -0700, Ben Pfaff wrote: > > On Fri, Apr 14, 2017 at 06:55:38PM +0100, Stephen Finucane wrote: > > > On Thu, 2017-04-13 at 21:27 -0700, Ben Pfaff wrote: > > > > Thank you! rST is much more readable than nroff. I have some > > > > comments > > > > below. > > > > > > > > On Mon, Apr 10, 2017 at 01:12:28PM +0100, Stephen Finucane wrote: > > > > > Let's start with a simple one that lets us focus on setting up > > > > > most > > > > > of the required "infrastructure" for building man pages using > > > > > Sphinx. > > > > > > > > > > There are a couple of things worth noting here: > > > > > > > > > > - The 'check-htmldocs' target becomes 'check-docs' as its now > > > > > responsible for building man page docs too. > > > > > > > > > > - The outputted file will always have a '.1' suffix. This is > > > > > Sphinx's > > > > > decision, and likely stems from the man page guidelines [1] > > > > > which > > > > > state that "the name of the man page's source file...is the > > > > > name > > > > > of > > > > > the command, function or file name, followed by a dot, > > > > > followed > > > > > by the > > > > > section character". > > > > > > > > It looks to me like the last element of the tuples inside > > > > man_pages > > > > in conf.py controls the section. When I changed 1 to 8 there, it > > > > switched the manpage to section 8. So I made that change in > > > > conf.py > > > > and I removed the above paragraph from the commit message. > > > > > > > > > Other than that, hurrah for (mostly) legible syntaxes. > > > > > > > > > > [1] http://www.tldp.org/HOWTO/Man-Page/q2.html > > > > > > > > > > Signed-off-by: Stephen Finucane <stephen@that.guru> > > > > > --- > > > > > I don't know if this is correctly integrated into the docs > > > > > build > > > > > system or not. I need someone to double check this for me. In > > > > > particular, I think I need to integrate the 'dh_sphinxdoc' > > > > > package > > > > > [2] into the Debian build but I've no idea how to. > > > > > > > > > > [2] http://manpages.ubuntu.com/manpages/zesty/man1/dh_sphinxdoc > > > > > .1.h > > > > > tml > > > > > > > > I spent a couple of hours working on the build and install system > > > > here. > > > > > > > > My first thought was to add rules to allow Automake to find and > > > > install the generated manpages. This turned out to be a huge > > > > mess > > > > that required tons of nasty GNU Make specific crap in the > > > > makefiles, > > > > and I didn't like it. > > > > > > > > My second approach was better. I gave up on integrating with the > > > > builtin Automake rules for manpages. All those really do anyway > > > > is > > > > handle install and uninstall, so I wrote some Make rules to do > > > > that. > > > > They're ugly because they're make+shell, but readable enough if > > > > you > > > > squint. > > > > > > > > The main question for install and uninstall is how to choose the > > > > right section. The easiest way seems to be to give the .rst > > > > files > > > > names that embed the section, like "ovs-vlan-test.8.rst". This > > > > is > > > > also handy to distinguish manpages with the same name but in > > > > different sections, which is sometimes a reasonable thing to > > > > have, so > > > > that's what I did. > > > > > > I still need to have a look into the patch (anything to do with > > > autotools is generally worth avoiding, heh), but I had a crazy idea > > > over the weekend: couldn't we just include the compiled man pages > > > in > > > the source? The only issue with this would be the '.TH' line, but > > > there > > > are solutions for avoiding this [1][2]. If we have man pages > > > included > > > in the source, would that be adequate to use the standard autotools > > > setup for manpages? > > > > I might not understand this idea yet. Do you mean, to include the > > nroff output of sphinx in the Git repository? I don't know why this > > would help. > > Correct. > > > I'm not sure there are any serious disadvantages to writing Make > > rules to install and uninstall manpages. > > The only significant advantage would be simplicity for use in other > build systems. Would the rules you've proposed work for the Debian and > RHEL packaging, for example? I understand now. I think that this change should be invisible to the Debian and RHEL packaging, because both kinds of packaging use "make install DESTDIR=..." to install the manpages into a staging tree, then package them from that staging tree. With this change, "make install" behaves the same way as before: both before and after the change, "make install" puts nroff versions of the manpages into $(DESTDIR)/usr/share/man/man#/ (or wherever they are configured to go). (I have not actually tested either kind of packaging.)
I think this change might have broken packaging :)
I just tested, and with the removal / renaming of the man8 pages for
ovs-test and ovs-vlan-test, the RPM fails to build because of missing files
that no longer match the glob.
These two lines need to be removed from the build:
471 %{_mandir}/man8/ovs-test.8*
472 %{_mandir}/man8/ovs-vlan-test.8*
I'll submit a patch shortly.
Thanks,
Leif.
On Fri, Apr 14, 2017 at 4:18 PM, Ben Pfaff <blp@ovn.org> wrote:
> I understand now.
>
> I think that this change should be invisible to the Debian and RHEL
> packaging, because both kinds of packaging use "make install
> DESTDIR=..." to install the manpages into a staging tree, then package
> them from that staging tree. With this change, "make install" behaves
> the same way as before: both before and after the change, "make install"
> puts nroff versions of the manpages into $(DESTDIR)/usr/share/man/man#/
> (or wherever they are configured to go).
>
> (I have not actually tested either kind of packaging.)
On Mon, Apr 24, 2017 at 03:52:53PM -0400, Leif Madsen wrote: > I think this change might have broken packaging :) > > I just tested, and with the removal / renaming of the man8 pages for > ovs-test and ovs-vlan-test, the RPM fails to build because of missing files > that no longer match the glob. > > These two lines need to be removed from the build: > > 471 %{_mandir}/man8/ovs-test.8* > > > > 472 %{_mandir}/man8/ovs-vlan-test.8* > > > I'll submit a patch shortly. OK, I'm confused then. There was no removal or renaming of the installed man8 pages, only of the source files. So, when I run "make install DESTDIR=$PWD/inst", I get a file inst/usr/share/man/man8/ovs-test.8 installed, and that should be the same as before. Any idea what's going on? Thanks, Ben.
On Mon, Apr 24, 2017 at 3:57 PM, Ben Pfaff <blp@ovn.org> wrote: > On Mon, Apr 24, 2017 at 03:52:53PM -0400, Leif Madsen wrote: > > I think this change might have broken packaging :) > > > > I just tested, and with the removal / renaming of the man8 pages for > > ovs-test and ovs-vlan-test, the RPM fails to build because of missing > files > > that no longer match the glob. > > > > These two lines need to be removed from the build: > > > > 471 %{_mandir}/man8/ovs-test.8* > > 472 %{_mandir}/man8/ovs-vlan-test.8* > > > > > > I'll submit a patch shortly. > > OK, I'm confused then. There was no removal or renaming of the > installed man8 pages, only of the source files. So, when I run "make > install DESTDIR=$PWD/inst", I get a file > inst/usr/share/man/man8/ovs-test.8 installed, and that should be the > same as before. > > Any idea what's going on? > Huh, well then I'm very confused as well :) I just did a test, and things seem to build fine when I remove those two lines. Otherwise, the RPM will fail to build with the following: Processing files: openvswitch-test-2.7.90.14191.git3570f7e4-1.el7.centos.noarch error: File not found by glob: /builddir/build/BUILDROOT/openvswitch-2.7.90.14191.git3570f7e4-1.el7.centos.x86_64/usr/share/man/man8/ovs-test.8* error: File not found by glob: /builddir/build/BUILDROOT/openvswitch-2.7.90.14191.git3570f7e4-1.el7.centos.x86_64/usr/share/man/man8/ovs-vlan-test.8* RPM build errors: File not found by glob: /builddir/build/BUILDROOT/openvswitch-2.7.90.14191.git3570f7e4-1.el7.centos.x86_64/usr/share/man/man8/ovs-test.8* File not found by glob: /builddir/build/BUILDROOT/openvswitch-2.7.90.14191.git3570f7e4-1.el7.centos.x86_64/usr/share/man/man8/ovs-vlan-test.8* So I'm not entirely sure why my build isn't resulting in those files. I have everything being built from a script and done in mock, so things should be quite reproducable. I see the same failure both on my COPR repo, and in my local build environment. Thoughts? Leif.
> From: "Ben Pfaff" <blp@ovn.org> > To: "Leif Madsen" <lmadsen@redhat.com> > Cc: "ovs dev" <dev@openvswitch.org> > Sent: Monday, 24 April, 2017 5:02:28 PM > Subject: Re: [ovs-dev] [RFC 2/4] doc: Convert ovs-vlan-test to rST > > On Mon, Apr 24, 2017 at 04:37:51PM -0400, Leif Madsen wrote: > > On Mon, Apr 24, 2017 at 3:57 PM, Ben Pfaff <blp@ovn.org> wrote: > > > > > On Mon, Apr 24, 2017 at 03:52:53PM -0400, Leif Madsen wrote: > > > > I think this change might have broken packaging :) > > > > > > > > I just tested, and with the removal / renaming of the man8 pages for > > > > ovs-test and ovs-vlan-test, the RPM fails to build because of missing > > > files > > > > that no longer match the glob. > > > > > > > > These two lines need to be removed from the build: > > > > > > > > 471 %{_mandir}/man8/ovs-test.8* > > > > 472 %{_mandir}/man8/ovs-vlan-test.8* > > > > > > > > > > > > I'll submit a patch shortly. > > > > > > OK, I'm confused then. There was no removal or renaming of the > > > installed man8 pages, only of the source files. So, when I run "make > > > install DESTDIR=$PWD/inst", I get a file > > > inst/usr/share/man/man8/ovs-test.8 installed, and that should be the > > > same as before. > > > > > > Any idea what's going on? > > > > > > > Huh, well then I'm very confused as well :) > > > > I just did a test, and things seem to build fine when I remove those two > > lines. Otherwise, the RPM will fail to build with the following: > > > > Processing files: > > openvswitch-test-2.7.90.14191.git3570f7e4-1.el7.centos.noarch > > error: File not found by glob: > > /builddir/build/BUILDROOT/openvswitch-2.7.90.14191.git3570f7e4-1.el7.centos.x86_64/usr/share/man/man8/ovs-test.8* > > error: File not found by glob: > > /builddir/build/BUILDROOT/openvswitch-2.7.90.14191.git3570f7e4-1.el7.centos.x86_64/usr/share/man/man8/ovs-vlan-test.8* > > > > > > RPM build errors: > > File not found by glob: > > /builddir/build/BUILDROOT/openvswitch-2.7.90.14191.git3570f7e4-1.el7.centos.x86_64/usr/share/man/man8/ovs-test.8* > > File not found by glob: > > /builddir/build/BUILDROOT/openvswitch-2.7.90.14191.git3570f7e4-1.el7.centos.x86_64/usr/share/man/man8/ovs-vlan-test.8* > > I ran into this a little while ago. The problem in my case was that sphinx-build was not installed, so these man pages were not being built (seems odd that this would not cause a build failure...) Doing "yum install python-sphinx" let me build the rpms successfully once again. Seems we should add "BuildRequires: python-sphinx" to the spec file. Regards, Lance
This is indeed the correct approach it seems. I've validated that adding Lance's change results in a successful build with the original globs. I'll ACK his patch here in a moment. Thanks! Leif. On Tue, Apr 25, 2017 at 4:03 PM, Lance Richardson <lrichard@redhat.com> wrote: > > From: "Ben Pfaff" <blp@ovn.org> > > To: "Leif Madsen" <lmadsen@redhat.com> > > Cc: "ovs dev" <dev@openvswitch.org> > > Sent: Monday, 24 April, 2017 5:02:28 PM > > Subject: Re: [ovs-dev] [RFC 2/4] doc: Convert ovs-vlan-test to rST > > > > On Mon, Apr 24, 2017 at 04:37:51PM -0400, Leif Madsen wrote: > > > On Mon, Apr 24, 2017 at 3:57 PM, Ben Pfaff <blp@ovn.org> wrote: > > > > > > > On Mon, Apr 24, 2017 at 03:52:53PM -0400, Leif Madsen wrote: > > > > > I think this change might have broken packaging :) > > > > > > > > > > I just tested, and with the removal / renaming of the man8 pages > for > > > > > ovs-test and ovs-vlan-test, the RPM fails to build because of > missing > > > > files > > > > > that no longer match the glob. > > > > > > > > > > These two lines need to be removed from the build: > > > > > > > > > > 471 %{_mandir}/man8/ovs-test.8* > > > > > 472 %{_mandir}/man8/ovs-vlan-test.8* > > > > > > > > > > > > > > > I'll submit a patch shortly. > > > > > > > > OK, I'm confused then. There was no removal or renaming of the > > > > installed man8 pages, only of the source files. So, when I run "make > > > > install DESTDIR=$PWD/inst", I get a file > > > > inst/usr/share/man/man8/ovs-test.8 installed, and that should be the > > > > same as before. > > > > > > > > Any idea what's going on? > > > > > > > > > > Huh, well then I'm very confused as well :) > > > > > > I just did a test, and things seem to build fine when I remove those > two > > > lines. Otherwise, the RPM will fail to build with the following: > > > > > > Processing files: > > > openvswitch-test-2.7.90.14191.git3570f7e4-1.el7.centos.noarch > > > error: File not found by glob: > > > /builddir/build/BUILDROOT/openvswitch-2.7.90.14191. > git3570f7e4-1.el7.centos.x86_64/usr/share/man/man8/ovs-test.8* > > > error: File not found by glob: > > > /builddir/build/BUILDROOT/openvswitch-2.7.90.14191. > git3570f7e4-1.el7.centos.x86_64/usr/share/man/man8/ovs-vlan-test.8* > > > > > > > > > RPM build errors: > > > File not found by glob: > > > /builddir/build/BUILDROOT/openvswitch-2.7.90.14191. > git3570f7e4-1.el7.centos.x86_64/usr/share/man/man8/ovs-test.8* > > > File not found by glob: > > > /builddir/build/BUILDROOT/openvswitch-2.7.90.14191. > git3570f7e4-1.el7.centos.x86_64/usr/share/man/man8/ovs-vlan-test.8* > > > > > I ran into this a little while ago. The problem in my case was that > sphinx-build > was not installed, so these man pages were not being built (seems odd that > this > would not cause a build failure...) > > Doing "yum install python-sphinx" let me build the rpms successfully once > again. Seems we should add "BuildRequires: python-sphinx" to the spec file. > > Regards, > > Lance >
On Tue, Apr 25, 2017 at 04:03:55PM -0400, Lance Richardson wrote: > I ran into this a little while ago. The problem in my case was that sphinx-build > was not installed, so these man pages were not being built (seems odd that this > would not cause a build failure...) > > Doing "yum install python-sphinx" let me build the rpms successfully once > again. Seems we should add "BuildRequires: python-sphinx" to the spec file. Huh. Somehow I thought that we'd made Sphinx an overall build requirement. Maybe that's the right thing to do.
On Wed, Apr 26, 2017 at 10:33 AM, Ben Pfaff <blp@ovn.org> wrote: > On Tue, Apr 25, 2017 at 04:03:55PM -0400, Lance Richardson wrote: > > I ran into this a little while ago. The problem in my case was that > sphinx-build > > was not installed, so these man pages were not being built (seems odd > that this > > would not cause a build failure...) > > > > Doing "yum install python-sphinx" let me build the rpms successfully once > > again. Seems we should add "BuildRequires: python-sphinx" to the spec > file. > > Huh. Somehow I thought that we'd made Sphinx an overall build > requirement. Maybe that's the right thing to do. > Unfortunately, actually, the build failed (what I saw must have been an out of date build that succeeded). I'm going to test locally and I'll get back to you :) Leif.
> From: "Ben Pfaff" <blp@ovn.org> > To: "Lance Richardson" <lrichard@redhat.com> > Cc: "Leif Madsen" <lmadsen@redhat.com>, "ovs dev" <dev@openvswitch.org> > Sent: Wednesday, 26 April, 2017 10:33:09 AM > Subject: Re: [ovs-dev] [RFC 2/4] doc: Convert ovs-vlan-test to rST > > On Tue, Apr 25, 2017 at 04:03:55PM -0400, Lance Richardson wrote: > > I ran into this a little while ago. The problem in my case was that > > sphinx-build > > was not installed, so these man pages were not being built (seems odd that > > this > > would not cause a build failure...) > > > > Doing "yum install python-sphinx" let me build the rpms successfully once > > again. Seems we should add "BuildRequires: python-sphinx" to the spec file. > > Huh. Somehow I thought that we'd made Sphinx an overall build > requirement. Maybe that's the right thing to do. > The rules to build rst-formatted man pages in the top-level Makefile.in are predicated on HAVE_SPHINX_TRUE, which does give the impression that Sphinx is optional. Lance
On Wed, 2017-04-26 at 10:55 -0400, Lance Richardson wrote: > > From: "Ben Pfaff" <blp@ovn.org> > > To: "Lance Richardson" <lrichard@redhat.com> > > Cc: "Leif Madsen" <lmadsen@redhat.com>, "ovs dev" <dev@openvswitch. > > org> > > Sent: Wednesday, 26 April, 2017 10:33:09 AM > > Subject: Re: [ovs-dev] [RFC 2/4] doc: Convert ovs-vlan-test to rST > > > > On Tue, Apr 25, 2017 at 04:03:55PM -0400, Lance Richardson wrote: > > > I ran into this a little while ago. The problem in my case was > > > that > > > sphinx-build > > > was not installed, so these man pages were not being built (seems > > > odd that > > > this > > > would not cause a build failure...) > > > > > > Doing "yum install python-sphinx" let me build the rpms > > > successfully once > > > again. Seems we should add "BuildRequires: python-sphinx" to the > > > spec file. > > > > Huh. Somehow I thought that we'd made Sphinx an overall build > > requirement. Maybe that's the right thing to do. > > > > The rules to build rst-formatted man pages in the top-level > Makefile.in are predicated on HAVE_SPHINX_TRUE, which does give the > impression that Sphinx is optional. > > Lance Yup, it's optional since these were only used for HTML docs initially. I didn't change that as HAVE_GROFF is also a thing, afaict, and I wasn't sure if man pages were essential. Maybe they should be now. Stephen
On Wed, Apr 26, 2017 at 11:00 AM, Stephen Finucane <stephen@that.guru> wrote: > On Wed, 2017-04-26 at 10:55 -0400, Lance Richardson wrote: > > The rules to build rst-formatted man pages in the top-level > > Makefile.in are predicated on HAVE_SPHINX_TRUE, which does give the > > impression that Sphinx is optional. > > Yup, it's optional since these were only used for HTML docs initially. > I didn't change that as HAVE_GROFF is also a thing, afaict, and I > wasn't sure if man pages were essential. Maybe they should be now. Looks like it might not be optional now (at least for package building). I can confirm 100% now that the change Lance provided does result in successful builds. Leif.
> From: "Stephen Finucane" <stephen@that.guru> > To: "Lance Richardson" <lrichard@redhat.com>, "Ben Pfaff" <blp@ovn.org> > Cc: "ovs dev" <dev@openvswitch.org> > Sent: Wednesday, 26 April, 2017 11:00:55 AM > Subject: Re: [ovs-dev] [RFC 2/4] doc: Convert ovs-vlan-test to rST > > On Wed, 2017-04-26 at 10:55 -0400, Lance Richardson wrote: > > > From: "Ben Pfaff" <blp@ovn.org> > > > To: "Lance Richardson" <lrichard@redhat.com> > > > Cc: "Leif Madsen" <lmadsen@redhat.com>, "ovs dev" <dev@openvswitch. > > > org> > > > Sent: Wednesday, 26 April, 2017 10:33:09 AM > > > Subject: Re: [ovs-dev] [RFC 2/4] doc: Convert ovs-vlan-test to rST > > > > > > On Tue, Apr 25, 2017 at 04:03:55PM -0400, Lance Richardson wrote: > > > > I ran into this a little while ago. The problem in my case was > > > > that > > > > sphinx-build > > > > was not installed, so these man pages were not being built (seems > > > > odd that > > > > this > > > > would not cause a build failure...) > > > > > > > > Doing "yum install python-sphinx" let me build the rpms > > > > successfully once > > > > again. Seems we should add "BuildRequires: python-sphinx" to the > > > > spec file. > > > > > > Huh. Somehow I thought that we'd made Sphinx an overall build > > > requirement. Maybe that's the right thing to do. > > > > > > > The rules to build rst-formatted man pages in the top-level > > Makefile.in are predicated on HAVE_SPHINX_TRUE, which does give the > > impression that Sphinx is optional. > > > > Lance > > Yup, it's optional since these were only used for HTML docs initially. > I didn't change that as HAVE_GROFF is also a thing, afaict, and I > wasn't sure if man pages were essential. Maybe they should be now. > It seems fine to me to handle this by having stricter requirements for packaging than for the base build (as they need to be anyway). OTOH there would be some benefits to making it a requirement for the base build, such as catching build errors earlier. Lance > Stephen >
On Wed, Apr 26, 2017 at 04:00:55PM +0100, Stephen Finucane wrote: > On Wed, 2017-04-26 at 10:55 -0400, Lance Richardson wrote: > > > From: "Ben Pfaff" <blp@ovn.org> > > > To: "Lance Richardson" <lrichard@redhat.com> > > > Cc: "Leif Madsen" <lmadsen@redhat.com>, "ovs dev" <dev@openvswitch. > > > org> > > > Sent: Wednesday, 26 April, 2017 10:33:09 AM > > > Subject: Re: [ovs-dev] [RFC 2/4] doc: Convert ovs-vlan-test to rST > > > > > > On Tue, Apr 25, 2017 at 04:03:55PM -0400, Lance Richardson wrote: > > > > I ran into this a little while ago. The problem in my case was > > > > that > > > > sphinx-build > > > > was not installed, so these man pages were not being built (seems > > > > odd that > > > > this > > > > would not cause a build failure...) > > > > > > > > Doing "yum install python-sphinx" let me build the rpms > > > > successfully once > > > > again. Seems we should add "BuildRequires: python-sphinx" to the > > > > spec file. > > > > > > Huh. Somehow I thought that we'd made Sphinx an overall build > > > requirement. Maybe that's the right thing to do. > > > > > > > The rules to build rst-formatted man pages in the top-level > > Makefile.in are predicated on HAVE_SPHINX_TRUE, which does give the > > impression that Sphinx is optional. > > > > Lance > > Yup, it's optional since these were only used for HTML docs initially. > I didn't change that as HAVE_GROFF is also a thing, afaict, and I > wasn't sure if man pages were essential. Maybe they should be now. HAVE_GROFF might be a deceptive example, because the availability of groff only affects whether the build can do a little bit of checking of the manpages' syntax at build time. It doesn't affect whether manpages can be built; they always can be. I'm inclined to try making Sphinx a hard requirement for the build, and then relaxing that if it proves untenable in practice.
diff --git a/Documentation/automake.mk b/Documentation/automake.mk index 1fd452b..2d62a6a 100644 --- a/Documentation/automake.mk +++ b/Documentation/automake.mk @@ -59,7 +59,6 @@ DOC_SOURCE = \ Documentation/howto/vlan.png \ Documentation/howto/vlan.rst \ Documentation/howto/vtep.rst \ - Documentation/ref/index.rst \ Documentation/faq/index.rst \ Documentation/faq/configuration.rst \ Documentation/faq/contributing.rst \ @@ -90,6 +89,8 @@ DOC_SOURCE = \ Documentation/internals/contributing/documentation-style.rst \ Documentation/internals/contributing/libopenvswitch-abi.rst \ Documentation/internals/contributing/submitting-patches.rst \ + Documentation/ref/index.rst \ + Documentation/ref/ovs-vlan-test.rst \ Documentation/requirements.txt EXTRA_DIST += $(DOC_SOURCE) @@ -110,18 +111,20 @@ sphinx_verbose_ = $(sphinx_verbose_@AM_DEFAULT_V@) sphinx_verbose_0 = -q if HAVE_SPHINX -htmldocs-check: $(DOC_SOURCE) +docs-check: $(DOC_SOURCE) $(AM_V_GEN)$(SPHINXBUILD) $(sphinx_verbose) -b html $(ALLSPHINXOPTS) $(SPHINXBUILDDIR)/html && touch $@ -ALL_LOCAL += htmldocs-check -CLEANFILES += htmldocs-check + $(AM_V_GEN)$(SPHINXBUILD) $(sphinx_verbose) -b man $(ALLSPHINXOPTS) $(SPHINXBUILDDIR)/man && touch $@ +ALL_LOCAL += docs-check +CLEANFILES += docs-check + +man_MANS += \ + $(SPHINXBUILDDIR)/man/ovs-vlan-test.1 check-docs: $(SPHINXBUILD) -b linkcheck $(ALLSPHINXOPTS) $(SPHINXBUILDDIR)/linkcheck clean-docs: - rm -rf $(SPHINXBUILDDIR)/doctrees - rm -rf $(SPHINXBUILDDIR)/html - rm -rf $(SPHINXBUILDDIR)/linkcheck + rm -rf $(SPHINXBUILDDIR) CLEAN_LOCAL += clean-docs endif .PHONY: check-docs diff --git a/Documentation/conf.py b/Documentation/conf.py index 6a924b3..e40302c 100644 --- a/Documentation/conf.py +++ b/Documentation/conf.py @@ -312,7 +312,8 @@ latex_documents = [ # One entry per manual page. List of tuples # (source start file, name, description, authors, manual section). man_pages = [ - (master_doc, 'openvswitch', u'Open vSwitch Documentation', + ('ref/ovs-vlan-test', 'ovs-vlan-test', + u'Check Linux drivers for problems with vlan traffic', [author], 1) ] diff --git a/Documentation/internals/contributing/documentation-style.rst b/Documentation/internals/contributing/documentation-style.rst index 0ba5e54..fcf3b9e 100644 --- a/Documentation/internals/contributing/documentation-style.rst +++ b/Documentation/internals/contributing/documentation-style.rst @@ -347,7 +347,8 @@ In addition to the above, man pages have some specific requirements: - The man page must be included in the list of man page documents found in `conf.py`__ -Refer to existing man pages for worked examples. +Refer to existing man pages, such as :doc:`/ref/ovs-vlan-test` for a worked +example. __ http://www.sphinx-doc.org/en/stable/domains.html#directive-program __ http://www.sphinx-doc.org/en/stable/domains.html#directive-option diff --git a/Documentation/intro/install/documentation.rst b/Documentation/intro/install/documentation.rst index 94af950..0eeeab1 100644 --- a/Documentation/intro/install/documentation.rst +++ b/Documentation/intro/install/documentation.rst @@ -74,11 +74,11 @@ Building Once Sphinx installed, the documentation can be built using the provided Makefile targets:: - $ make htmldocs-check + $ make docs-check .. important:: - The ``htmldocs-check`` target will fail if there are any syntax errors. + The ``docs-check`` target will fail if there are any syntax errors. However, it won't catch more succint issues such as style or grammar issues. As a result, you should always inspect changes visually to ensure the result is as intended. diff --git a/Documentation/ref/index.rst b/Documentation/ref/index.rst index 49121b7..1e0d413 100644 --- a/Documentation/ref/index.rst +++ b/Documentation/ref/index.rst @@ -30,10 +30,18 @@ Reference Guide Man Pages --------- -.. TODO(stephenfin): Investigate some way to get the manpages into rST format. - The most viable option seems to be writing them all in rST, converting them - to roff format and storing both the rST and roff formatted docs in version - control. +.. TODO(stephenfin): Remove the below notice once everything is converted to + rST + +The following man pages are written in rST and converted to roff at compile +time: + +.. toctree:: + :maxdepth: 3 + + ovs-vlan-test + +The remainder are still in roff format can be found below: .. list-table:: diff --git a/Documentation/ref/ovs-vlan-test.rst b/Documentation/ref/ovs-vlan-test.rst new file mode 100644 index 0000000..f29835f --- /dev/null +++ b/Documentation/ref/ovs-vlan-test.rst @@ -0,0 +1,117 @@ +============= +ovs-vlan-test +============= + +Synopsis +======== + +:: + + ovs-vlan-test [-s | --server] control_ip vlan_ip + +Description +=========== + +.. TODO(stephenfin): Add the `:program:` prefixes to `ovs-test` once that doc + is converted + +The :program:`ovs-vlan-test` utility has some limitations, for example, it does +not use TCP in its tests. Also it does not take into account MTU to detect +potential edge cases. To overcome those limitations a new tool was developed - +`ovs-test`. `ovs-test` is currently supported only on Debian so, if possible, +try to use that on instead of :program:`ovs-vlan-test`. + +The :program:`ovs-vlan-test` program may be used to check for problems sending +802.1Q traffic which may occur when running Open vSwitch. These problems can +occur when Open vSwitch is used to send 802.1Q traffic through physical +interfaces running certain drivers of certain Linux kernel versions. To run a +test, configure Open vSwitch to tag traffic originating from `vlan_ip` and +forward it out the target interface. Then run the :program:`ovs-vlan-test` in +client mode connecting to an :program:`ovs-vlan-test` server. +:program:`ovs-vlan-test` will display "OK" if it did not detect problems. + +Some examples of the types of problems that may be encountered are: + +- When NICs use VLAN stripping on receive they must pass a pointer to a + `vlan_group` when reporting the stripped tag to the networking core. If no + `vlan_group` is in use then some drivers just drop the extracted tag. + Drivers are supposed to only enable stripping if a `vlan_group` is registered + but not all of them do that. + +- On receive, some drivers handle priority tagged packets specially and don't + pass the tag onto the network stack at all, so Open vSwitch never has a + chance to see it. + +- Some drivers size their receive buffers based on whether a `vlan_group` is + enabled, meaning that a maximum size packet with a VLAN tag will not fit if + no `vlan_group` is configured. + +- On transmit, some drivers expect that VLAN acceleration will be used if it is + available, which can only be done if a `vlan_group` is configured. In these + cases, the driver may fail to parse the packet and correctly setup checksum + offloading or TSO. + +Client Mode + An :program:`ovs-vlan-test` client may be run on a host to check for VLAN + connectivity problems. The client must be able to establish HTTP connections + with an :program:`ovs-vlan-test` server located at the specified `control_ip` + address. UDP traffic sourced at `vlan_ip` should be tagged and directed out + the interface whose connectivity is being tested. + +Server Mode + To conduct tests, an :program:`ovs-vlan-test` server must be running on a + host known not to have VLAN connectivity problems. The server must have a + `control_ip` on a non-VLAN network which clients can establish connectivity + with. It must also have a `vlan_ip` address on a VLAN network which clients + will use to test their VLAN connectivity. Multiple clients may test against a + single :program:`ovs-vlan-test` server concurrently. + +Options +======= + +.. program:: ovs-vlan-test + +.. option:: -s, --server + + Run in server mode. + +.. option:: -h, --help + + Prints a brief help message to the console. + +.. option:: -V, --version + + Prints version information to the console. + +Examples +======== + +Display the Linux kernel version and driver of `eth1`:: + + uname -r + ethtool -i eth1 + +Set up a bridge which forwards traffic originating from `1.2.3.4` out `eth1` +with VLAN tag 10:: + + ovs-vsctl -- add-br vlan-br \ + -- add-port vlan-br eth1 \ + -- add-port vlan-br vlan-br-tag tag=10 \ + -- set Interface vlan-br-tag type=internal + ifconfig vlan-br-tag up 1.2.3.4 + +Run an :program:`ovs-vlan-test` server listening for client control traffic on +`172.16.0.142` port `8080` and VLAN traffic on the default port of `1.2.3.3`:: + + ovs-vlan-test -s 172.16.0.142:8080 1.2.3.3 + +Run an :program:`ovs-vlan-test` client with a control server located at +`172.16.0.142` port `8080` and a local VLAN IP of `1.2.3.4`:: + + ovs-vlan-test 172.16.0.142:8080 1.2.3.4 + +See Also +======== + +`ovs-vswitchd(8)`, `ovs-ofctl(8)`, `ovs-vsctl(8)`, `ovs-test(8)`, `ethtool(8)`, +`uname(1)` diff --git a/debian/openvswitch-switch.manpages b/debian/openvswitch-switch.manpages index a3aae64..a2f661a 100644 --- a/debian/openvswitch-switch.manpages +++ b/debian/openvswitch-switch.manpages @@ -5,7 +5,6 @@ utilities/ovs-dpctl.8 utilities/ovs-pcap.1 utilities/ovs-tcpdump.8 utilities/ovs-tcpundump.1 -utilities/ovs-vlan-test.8 utilities/ovs-vsctl.8 vswitchd/ovs-vswitchd.8 vswitchd/ovs-vswitchd.conf.db.5 diff --git a/manpages.mk b/manpages.mk index 742bd66..372d06c 100644 --- a/manpages.mk +++ b/manpages.mk @@ -244,16 +244,6 @@ utilities/ovs-vlan-bug-workaround.8.in: lib/common.man: utilities/ovs-vlan-bugs.man: -utilities/ovs-vlan-test.8: \ - utilities/ovs-vlan-test.8.in \ - lib/common-syn.man \ - lib/common.man \ - utilities/ovs-vlan-bugs.man -utilities/ovs-vlan-test.8.in: -lib/common-syn.man: -lib/common.man: -utilities/ovs-vlan-bugs.man: - utilities/ovs-vsctl.8: \ utilities/ovs-vsctl.8.in \ lib/common.man \ diff --git a/utilities/automake.mk b/utilities/automake.mk index 169388f..02531f6 100644 --- a/utilities/automake.mk +++ b/utilities/automake.mk @@ -72,7 +72,6 @@ MAN_ROOTS += \ utilities/ovs-tcpundump.1.in \ utilities/ovs-vlan-bug-workaround.8.in \ utilities/ovs-test.8.in \ - utilities/ovs-vlan-test.8.in \ utilities/ovs-vsctl.8.in MAN_FRAGMENTS += utilities/ovs-vlan-bugs.man CLEANFILES += \ @@ -101,7 +100,6 @@ CLEANFILES += \ utilities/ovs-test \ utilities/ovs-test.8 \ utilities/ovs-vlan-test \ - utilities/ovs-vlan-test.8 \ utilities/ovs-vlan-bug-workaround.8 \ utilities/ovs-vsctl.8 @@ -120,7 +118,6 @@ man_MANS += \ utilities/ovs-tcpundump.1 \ utilities/ovs-vlan-bug-workaround.8 \ utilities/ovs-test.8 \ - utilities/ovs-vlan-test.8 \ utilities/ovs-vsctl.8 utilities_ovs_appctl_SOURCES = utilities/ovs-appctl.c
Let's start with a simple one that lets us focus on setting up most of the required "infrastructure" for building man pages using Sphinx. There are a couple of things worth noting here: - The 'check-htmldocs' target becomes 'check-docs' as its now responsible for building man page docs too. - The outputted file will always have a '.1' suffix. This is Sphinx's decision, and likely stems from the man page guidelines [1] which state that "the name of the man page's source file...is the name of the command, function or file name, followed by a dot, followed by the section character". Other than that, hurrah for (mostly) legible syntaxes. [1] http://www.tldp.org/HOWTO/Man-Page/q2.html Signed-off-by: Stephen Finucane <stephen@that.guru> --- I don't know if this is correctly integrated into the docs build system or not. I need someone to double check this for me. In particular, I think I need to integrate the 'dh_sphinxdoc' package [2] into the Debian build but I've no idea how to. [2] http://manpages.ubuntu.com/manpages/zesty/man1/dh_sphinxdoc.1.html --- Documentation/automake.mk | 17 +-- Documentation/conf.py | 3 +- .../internals/contributing/documentation-style.rst | 3 +- Documentation/intro/install/documentation.rst | 4 +- Documentation/ref/index.rst | 16 ++- Documentation/ref/ovs-vlan-test.rst | 117 +++++++++++++++++++++ debian/openvswitch-switch.manpages | 1 - manpages.mk | 10 -- utilities/automake.mk | 3 - 9 files changed, 145 insertions(+), 29 deletions(-) create mode 100644 Documentation/ref/ovs-vlan-test.rst