Re: [Openvpn-devel] [PATCH applied] Re: doc/man: Replace old man page with generated man page
Hi,
On Fri, Jul 17, 2020 at 12:04:30PM +0200, David Sommerseth wrote:
> On 17/07/2020 10:02, Gert Doering wrote:
> > Acked-by: Gert Doering
> >
> > I have not tested the actual docutils / openvpn.8 generation (Samuli will
> > complain loudly if tarball making doesn't work anymore, so that *will*
> > see testing). Generally it looks sane.
> >
> > This condition looks a bit fishy, though...
> >
> > +AM_CONDITIONAL([HAVE_PYDOCUTILS], [test "${RST2MAN}" -a "${RST2HTML}"])
> >
> > not sure what that will do in a POSIX shell.
>
> Hmm ... whoops. That should probably have been
>
> test -n "${RST2MAN}" -a -n "${RST2HTML}"
>
> Not sure how that passed during my own tests. I tested it on a various of
> boxes, but I only have Linux distros easily available.
OK, there is something else - the buildbots are now failing with
Missing python-docutils - skipping man/html page generation
Missing python-docutils - skipping man page generation
cp: cannot stat './openvpn.8.html': No such file or directory
Makefile:686: recipe for target 'distdir' failed
"if it knows that it's not doing these, it shouldn't try to copy it
somewhere"
Can you have a look please, while fixing the "test" thing above?
> > Maybe this shouldn't be conditional either
> >
> > +if HAVE_PYDOCUTILS
> > dist_noinst_DATA += openvpn.8
> >
> > because it will lead to "tarballs randomly contain openvpn.8 or not,
> > depending on whether docutils are around" - "make dist" should behave
> > consistently, and if there are no docutils, I think it should fail, not
> > silently leave out files.
>
> The intention is that the tarball contains prebuilt openvpn.8 and openvpn.html
> files, which is generated by "make {dist,distcheck}".
Yes. In full agreement.
What I worry is that if you run a "make dist" on a host that has no
docutils, configure will notice this, and not add "openvpn.8" to
"dist_noinst_DATA" - so it won't be built, not included in the tarball,
and the tarball is incomplete.
> If these files exists,
> they will not be rebuilt unless explicitly removed. So most users building
> from the source tarball should not notice any difference from prioer OpenVPN
> releases. This is what the additional dist-hook rule in doc/Makefile.am does;
> this is run right before the copied source tree is put into a tarball.
>
> The challenge is that it must be a conditional to actually pass ./configure -
> even when built from source tarballs.
Not sure I understand this. If openvpn.8 is there, because it was tarball'ed,
why would this line above create a new dependency?
gert
--
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany g...@greenie.muc.de
signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH applied] Re: doc/man: Replace old man page with generated man page
On 17/07/2020 10:02, Gert Doering wrote:
> Acked-by: Gert Doering
>
> I have not tested the actual docutils / openvpn.8 generation (Samuli will
> complain loudly if tarball making doesn't work anymore, so that *will*
> see testing). Generally it looks sane.
>
> This condition looks a bit fishy, though...
>
> +AM_CONDITIONAL([HAVE_PYDOCUTILS], [test "${RST2MAN}" -a "${RST2HTML}"])
>
> not sure what that will do in a POSIX shell.
Hmm ... whoops. That should probably have been
test -n "${RST2MAN}" -a -n "${RST2HTML}"
Not sure how that passed during my own tests. I tested it on a various of
boxes, but I only have Linux distros easily available.
> Maybe this shouldn't be conditional either
>
> +if HAVE_PYDOCUTILS
> dist_noinst_DATA += openvpn.8
>
> because it will lead to "tarballs randomly contain openvpn.8 or not,
> depending on whether docutils are around" - "make dist" should behave
> consistently, and if there are no docutils, I think it should fail, not
> silently leave out files.
The intention is that the tarball contains prebuilt openvpn.8 and openvpn.html
files, which is generated by "make {dist,distcheck}". If these files exists,
they will not be rebuilt unless explicitly removed. So most users building
from the source tarball should not notice any difference from prioer OpenVPN
releases. This is what the additional dist-hook rule in doc/Makefile.am does;
this is run right before the copied source tree is put into a tarball.
The challenge is that it must be a conditional to actually pass ./configure -
even when built from source tarballs. Otherwise python-docutils will be a
build dependency. We discussed this at the Trento Hackathon and you where
skeptic to require a Python stack to be installed to build OpenVPN (as that
stack is not a common system dependency in the non-Linux world). So we agreed
we will ship pre-built man/html files in the source tarballs.
However, to do the "make {dist,distcheck}" from the *git repo*,
python-docutils need to be a mandatory dependency - because we don't check in
the prebuilt openvpn.8 and openvpn.html files into the git repo.
This logic could probably contains some flaws and can be further improved, but
I figured we need to get this tested on a broader set of use cases and
OS/distros to better see which annoyances which hits us.
--
kind regards,
David Sommerseth
OpenVPN Inc
signature.asc
Description: OpenPGP digital signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: doc/man: Replace old man page with generated man page
Acked-by: Gert Doering
I have not tested the actual docutils / openvpn.8 generation (Samuli will
complain loudly if tarball making doesn't work anymore, so that *will*
see testing). Generally it looks sane.
This condition looks a bit fishy, though...
+AM_CONDITIONAL([HAVE_PYDOCUTILS], [test "${RST2MAN}" -a "${RST2HTML}"])
not sure what that will do in a POSIX shell.
Maybe this shouldn't be conditional either
+if HAVE_PYDOCUTILS
dist_noinst_DATA += openvpn.8
because it will lead to "tarballs randomly contain openvpn.8 or not,
depending on whether docutils are around" - "make dist" should behave
consistently, and if there are no docutils, I think it should fail, not
silently leave out files.
URL pointed to SF again due to patch size.
Your patch has been applied to the master branch.
commit 51f2e4b0a4e09382dbc1b10b6a8b08febc8d293f
Author: David Sommerseth
Date: Fri Jul 17 00:53:32 2020 +0200
doc/man: Replace old man page with generated man page
Signed-off-by: David Sommerseth
Acked-by: Gert Doering
Message-Id: <20200716225338.611-3-dav...@openvpn.net>
URL: https://sourceforge.net/p/openvpn/mailman/message/37063373/
Signed-off-by: Gert Doering
--
kind regards,
Gert Doering
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
