cifs-utils: support rst2man-3
diff mbox series

Message ID 20180717052550.GA11106@onega.vda.li
State New
Headers show
Series
  • cifs-utils: support rst2man-3
Related show

Commit Message

Alexander Bokovoy July 17, 2018, 5:25 a.m. UTC
Hi,

Python 3 version of rst2man utility is called rst2man-3. On systems
without python 2 bits rst2man is not found. This patch adds support for
rst2man-3 so that man pages can still be created.

Comments

Aurélien Aptel July 17, 2018, 8:06 a.m. UTC | #1
Alexander Bokovoy <ab@samba.org> writes:

> Hi,
>
> Python 3 version of rst2man utility is called rst2man-3. On systems
> without python 2 bits rst2man is not found. This patch adds support for
> rst2man-3 so that man pages can still be created.

It's called rst2man-3.4 here, you probably want to check for that too.

>  # if docs are not disabled, check if rst2man is available
>  if test $enable_man != "no"; then
> -	AC_CHECK_PROG(have_rst2man, rst2man, yes, no)
> +	AC_CHECK_PROGS(have_rst2man, rst2man-3 rst2man, no)
>  	if test $have_rst2man = "no"; then
>  		if test $enable_man = "yes"; then
>  			AC_MSG_ERROR([rst2man not found: cannot generate man pages, consider installing perl.])
                                                                                     ^^^^^^^^^^^^^^^^^^^^^^^

Should be "python" and/or even python(3)-docutils, which doesn't look
like a base package.

Cheers,
Alexander Bokovoy July 17, 2018, 8:27 a.m. UTC | #2
On ti, 17 heinä 2018, Aurélien Aptel wrote:
> Alexander Bokovoy <ab@samba.org> writes:
> 
> > Hi,
> >
> > Python 3 version of rst2man utility is called rst2man-3. On systems
> > without python 2 bits rst2man is not found. This patch adds support for
> > rst2man-3 so that man pages can still be created.
> 
> It's called rst2man-3.4 here, you probably want to check for that too.
Ok. Added rst2man-{3,3.4,3.6} for completeness.

> >  # if docs are not disabled, check if rst2man is available
> >  if test $enable_man != "no"; then
> > -	AC_CHECK_PROG(have_rst2man, rst2man, yes, no)
> > +	AC_CHECK_PROGS(have_rst2man, rst2man-3 rst2man, no)
> >  	if test $have_rst2man = "no"; then
> >  		if test $enable_man = "yes"; then
> >  			AC_MSG_ERROR([rst2man not found: cannot generate man pages, consider installing perl.])
>                                                                                      ^^^^^^^^^^^^^^^^^^^^^^^
> 
> Should be "python" and/or even python(3)-docutils, which doesn't look
> like a base package.
Changed this to python{2,3}-docutils as both py2 and py3 version should
be working.
Aurélien Aptel July 17, 2018, 10:05 a.m. UTC | #3
Alexander Bokovoy <ab@samba.org> writes:
>> It's called rst2man-3.4 here, you probably want to check for that too.
> Ok. Added rst2man-{3,3.4,3.6} for completeness.

Good, thanks!

> / Alexander Bokovoy
> From 6f7af37a5b4b450c5e326d20f2a71423aa55606d Mon Sep 17 00:00:00 2001
> From: Alexander Bokovoy <ab@samba.org>
> Date: Tue, 17 Jul 2018 08:22:23 +0300
> Subject: [PATCH] configure.ac: support using both rst2man and rst2man-3
>
> Python3 version of rst2man is called rst2man-3.
> Add few more variants to cover Fedora and OpenSUSE.
>
> Signed-off-by: Alexander Bokovoy <ab@samba.org>
> ---
>  configure.ac | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index b0bc2b9..8e3d6ce 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -252,12 +252,12 @@ fi
>  
>  # if docs are not disabled, check if rst2man is available
>  if test $enable_man != "no"; then
> -	AC_CHECK_PROG(have_rst2man, rst2man, yes, no)
> +	AC_CHECK_PROGS(have_rst2man, rst2man-3.6 rst2man-3.4 rst2man-3 rst2man, no)

You need to use the variable in the Makefile to actually use the prog
found. It should just be a matter of replacing "rst2man" by
"$(have_rst2man)" since autoconf already calls AC_SUBST on the var
according to AC_CHECK_PROGS documentation.

Cheers,
Alexander Bokovoy July 17, 2018, 10:12 a.m. UTC | #4
On ti, 17 heinä 2018, Aurélien Aptel wrote:
> >  # if docs are not disabled, check if rst2man is available
> >  if test $enable_man != "no"; then
> > -	AC_CHECK_PROG(have_rst2man, rst2man, yes, no)
> > +	AC_CHECK_PROGS(have_rst2man, rst2man-3.6 rst2man-3.4 rst2man-3 rst2man, no)
> 
> You need to use the variable in the Makefile to actually use the prog
> found. It should just be a matter of replacing "rst2man" by
> "$(have_rst2man)" since autoconf already calls AC_SUBST on the var
> according to AC_CHECK_PROGS documentation.
Right. I replaced rst2man in RST2MAN definition in Makefile.am by
$(have_rst2man) and given that we only call for that when CONFIG_MAN is
set, that should complete the change.

New patch is attached.
Aurélien Aptel July 17, 2018, 11:21 a.m. UTC | #5
Perfect :)

Reviewed-by: Aurelien Aptel <aaptel@suse.com>
Pavel Shilovsky Aug. 17, 2018, 6:04 p.m. UTC | #6
вт, 17 июл. 2018 г. в 4:21, Aurélien Aptel <aaptel@samba.org>:
>
> Perfect :)
>
> Reviewed-by: Aurelien Aptel <aaptel@suse.com>
>
> --
> Aurélien Aptel / SUSE Labs Samba Team
> GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
> SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Merged into next.

--
Best regards,
Pavel Shilovsky
Hank Leininger Feb. 12, 2019, 1:17 a.m. UTC | #7
On 2018-07-17, Alexander Bokovoy said:
>
> Right. I replaced rst2man in RST2MAN definition in Makefile.am by
> $(have_rst2man) and given that we only call for that when CONFIG_MAN
> is set, that should complete the change.
...
>+ AC_CHECK_PROGS(have_rst2man, rst2man-3.6 rst2man-3.4 rst2man-3 rst2man, no)

Some OS's (Gentoo Linux, possibly OSX?) install rst2man as rst2man.py.

Please do:

- AC_CHECK_PROGS(have_rst2man, rst2man-3.6 rst2man-3.4 rst2man-3 rst2man, no)
+ AC_CHECK_PROGS(have_rst2man, rst2man-3.6 rst2man-3.4 rst2man-3 rst2man.py rst2man, no)

* I couldn't find a git repo where the above had actually landed?
  https://wiki.samba.org/index.php/LinuxCIFS points to
  https://wiki.samba.org/index.php/LinuxCIFS_utils, which points to
  git://git.samba.org/cifs-utils.git, which has not gotten a commit
  since 2018-07-10 (master) or 2018-07-15 (next), unless I am just dumb.

Thanks,
Pavel Shilovsky Feb. 12, 2019, 1:36 a.m. UTC | #8
пн, 11 февр. 2019 г. в 17:22, Hank Leininger <hlein@korelogic.com>:
>
> On 2018-07-17, Alexander Bokovoy said:
> >
> > Right. I replaced rst2man in RST2MAN definition in Makefile.am by
> > $(have_rst2man) and given that we only call for that when CONFIG_MAN
> > is set, that should complete the change.
> ...
> >+ AC_CHECK_PROGS(have_rst2man, rst2man-3.6 rst2man-3.4 rst2man-3 rst2man, no)
>
> Some OS's (Gentoo Linux, possibly OSX?) install rst2man as rst2man.py.
>
> Please do:
>
> - AC_CHECK_PROGS(have_rst2man, rst2man-3.6 rst2man-3.4 rst2man-3 rst2man, no)
> + AC_CHECK_PROGS(have_rst2man, rst2man-3.6 rst2man-3.4 rst2man-3 rst2man.py rst2man, no)
>
> * I couldn't find a git repo where the above had actually landed?
>   https://wiki.samba.org/index.php/LinuxCIFS points to
>   https://wiki.samba.org/index.php/LinuxCIFS_utils, which points to
>   git://git.samba.org/cifs-utils.git, which has not gotten a commit
>   since 2018-07-10 (master) or 2018-07-15 (next), unless I am just dumb.
>

Hi Hank,

The repository is currently here:

https://github.com/piastry/cifs-utils/commits/next

The master branch on git.samba.org will be updated once I make a
release. Feel free to submit a patch on top of the "next" branch.

--
Best regards,
Pavel Shilovsky
Hank Leininger Feb. 12, 2019, 1:46 a.m. UTC | #9
On Mon, Feb 11, 2019 at 05:36:13PM -0800, Pavel Shilovsky wrote:
> ????, 11 ????????. 2019 ??. ?? 17:22, Hank Leininger <hlein@korelogic.com>:
> > On 2018-07-17, Alexander Bokovoy said:
> > >+ AC_CHECK_PROGS(have_rst2man, rst2man-3.6 rst2man-3.4 rst2man-3 rst2man, no)
> >
> > Some OS's (Gentoo Linux, possibly OSX?) install rst2man as rst2man.py.
> >
> > Please do:
> >
> > - AC_CHECK_PROGS(have_rst2man, rst2man-3.6 rst2man-3.4 rst2man-3 rst2man, no)
> > + AC_CHECK_PROGS(have_rst2man, rst2man-3.6 rst2man-3.4 rst2man-3 rst2man.py rst2man, no)
> >
> > * I couldn't find a git repo where the above had actually landed?

> The repository is currently here:
> 
> https://github.com/piastry/cifs-utils/commits/next

Thanks!  Just submitted a PR there.

Hank
Aurelien Aptel Feb. 12, 2019, 10:42 a.m. UTC | #10
Hi Pavel,

Pavel Shilovsky <piastryyy@gmail.com> writes:
> The repository is currently here:
>
> https://github.com/piastry/cifs-utils/commits/next
>
> The master branch on git.samba.org will be updated once I make a
> release. Feel free to submit a patch on top of the "next" branch.

Do you prefer email patches here or github PR? I have sent a cifs-utils
patch some ago to correct the kernel release version in the smbinfo man
page.

https://patchwork.kernel.org/patch/10779659/

Cheers,
Pavel Shilovsky Feb. 15, 2019, 1:07 a.m. UTC | #11
вт, 12 февр. 2019 г. в 02:42, Aurélien Aptel <aaptel@suse.com>:
>
> Hi Pavel,
>
> Pavel Shilovsky <piastryyy@gmail.com> writes:
> > The repository is currently here:
> >
> > https://github.com/piastry/cifs-utils/commits/next
> >
> > The master branch on git.samba.org will be updated once I make a
> > release. Feel free to submit a patch on top of the "next" branch.
>
> Do you prefer email patches here or github PR? I have sent a cifs-utils
> patch some ago to correct the kernel release version in the smbinfo man
> page.
>
> https://patchwork.kernel.org/patch/10779659/

The patch has been merged together with other pending ones. Feel free
to to use github PRs in future but patches are ok too.

--
Best regards,
Pavel Shilovsky

Patch
diff mbox series

diff --git a/configure.ac b/configure.ac
index b0bc2b9..26d60ea 100644
--- a/configure.ac
+++ b/configure.ac
@@ -252,7 +252,7 @@  fi
 
 # if docs are not disabled, check if rst2man is available
 if test $enable_man != "no"; then
-	AC_CHECK_PROG(have_rst2man, rst2man, yes, no)
+	AC_CHECK_PROGS(have_rst2man, rst2man-3 rst2man, no)
 	if test $have_rst2man = "no"; then
 		if test $enable_man = "yes"; then
 			AC_MSG_ERROR([rst2man not found: cannot generate man pages, consider installing perl.])