Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PATCH] Make installman die on POD errors #14656

Closed
p5pRT opened this issue Apr 18, 2015 · 8 comments
Closed

[PATCH] Make installman die on POD errors #14656

p5pRT opened this issue Apr 18, 2015 · 8 comments

Comments

@p5pRT
Copy link

p5pRT commented Apr 18, 2015

Migrated from rt.perl.org#124332 (status was 'open')

Searchable as RT124332$

@p5pRT
Copy link
Author

p5pRT commented Apr 18, 2015

From @ntyni

Since podlators 2.5.0 (perl 5.17.8, Pod​::Man 2.27), Pod​::Man has supported
three different ways of handling POD errors. The old way of advertising
them in the generated manual page (in a POD ERRORS section) is still the
default for Pod​::Man, as the others are not really suitable for libraries,
but the pod2man script now defaults to dying on POD errors. The third
option is to output warnings on stderr during the conversion.

Quoting Russ Allbery in <https://rt.cpan.org/Public/Bug/Display.html?id=39007>:

  The primary argument for doing this is that POD errors are primarily
  of interest to the author of the POD, not to thte end-user, but the POD
  ERRORS section in an installed man page tells the user about things they
  can't do anything about.

There was some resulting discussion on the perl5-porters list at
http​://www.nntp.perl.org/group/perl.perl5.porters/2008/09/msg139898.html.

Now, the perl core build uses a pod2man replacement in the 'installman'
script, which is still using the default Pod​::Man behaviour. The above
rationale applies to that one as well. The attached proposed patch makes
it match the pod2man behaviour by dying on POD errors.

Current bleadperl has two files with POD errors, so those have to be
fixed first, as per the other two attached patches. However, they are in
cpan/, so possibly it's better to wait for them to be fixed upstream. The
Test-Simple one is already fixed in CPAN development versions, but the
Memoize one has a ticket from 2013 with no action so far at
https://rt.cpan.org/Public/Bug/Display.html?id=89441

Note that ExtUtils-MakeMaker uses its own pod2man implementation,
and a similar change to that is being discussed in
Perl-Toolchain-Gang/ExtUtils-MakeMaker#216

Thanks for your work,
--
Niko Tyni ntyni@​debian.org

@p5pRT
Copy link
Author

p5pRT commented Apr 18, 2015

From @ntyni

0001-Fix-POD-errors-in-Memoize.pm.patch
From b55b260ce5b797fbb08fbe736a44010f1753b743 Mon Sep 17 00:00:00 2001
From: Niko Tyni <ntyni@debian.org>
Date: Sat, 18 Apr 2015 16:18:40 +0300
Subject: [PATCH 1/3] Fix POD errors in Memoize.pm

Bug: https://rt.cpan.org/Public/Bug/Display.html?id=89441
---
 cpan/Memoize/Memoize.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cpan/Memoize/Memoize.pm b/cpan/Memoize/Memoize.pm
index 9a58c4a..96bd50c 100644
--- a/cpan/Memoize/Memoize.pm
+++ b/cpan/Memoize/Memoize.pm
@@ -748,6 +748,8 @@ cache that was populated by the first call.  C<complicated> ends up
 being called only once, and both subsequent calls return C<3> from the
 cache, regardless of the calling context.
 
+=back
+
 =head3 List values in scalar context
 
 Consider this function:
@@ -797,8 +799,6 @@ This normalizer function will store scalar context return values in
 the disk file under keys that begin with C<S:>, and list context
 return values under keys that begin with C<L:>.
 
-=back
-
 =head1 OTHER FACILITIES
 
 =head2 C<unmemoize>
-- 
2.1.4

@p5pRT
Copy link
Author

p5pRT commented Apr 18, 2015

From @ntyni

0002-Fix-POD-errors-in-ok.pm.patch
From 61584493e4b3ba6f1947a3e3ac2ec2fa0b416627 Mon Sep 17 00:00:00 2001
From: Niko Tyni <ntyni@debian.org>
Date: Sat, 18 Apr 2015 16:19:56 +0300
Subject: [PATCH 2/3] Fix POD errors in ok.pm

Already fixed in CPAN development versions with
 https://github.com/Test-More/test-more/commit/a0fcd3e465888d5e610657ea44e4180bc326071e#diff-ee1ceb573f530c93e1869b3f8917b1de
---
 cpan/Test-Simple/lib/ok.pm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/cpan/Test-Simple/lib/ok.pm b/cpan/Test-Simple/lib/ok.pm
index 02726ac..49e4eb0 100644
--- a/cpan/Test-Simple/lib/ok.pm
+++ b/cpan/Test-Simple/lib/ok.pm
@@ -20,6 +20,8 @@ sub import {
 
 __END__
 
+=encoding UTF-8
+
 =head1 NAME
 
 ok - Alternative to Test::More::use_ok
-- 
2.1.4

@p5pRT
Copy link
Author

p5pRT commented Apr 18, 2015

From @ntyni

0003-Make-installman-die-on-POD-errors.patch
From 2129d3e87177a6eec72d7b68549f098697358d5d Mon Sep 17 00:00:00 2001
From: Niko Tyni <ntyni@debian.org>
Date: Fri, 17 Apr 2015 22:55:48 +0300
Subject: [PATCH 3/3] Make installman die on POD errors

Since podlators 2.5.0 (perl 5.17.8), the pod2man script has defaulted
to dying on POD errors instead of advertising them in the generated
manual page. Quoting Russ Allbery in [rt.cpan.org #39007], also
cc'd to the perl5-porters list:

> The primary argument for doing this is that POD errors are primarily
> of interest to the author of the POD, not to thte end-user, but the POD
> ERRORS section in an installed man page tells the user about things they
> can't do anything about.

The Pod::Man module still defaults to including the errors in the
POD page, as libraries aren't generally in a position to die
or complain on stderr when used.

However, the above pod2man rationale applies to the core 'installman'
script too, so change it to match pod2man behaviour and die on errors.
---
 installman | 1 +
 1 file changed, 1 insertion(+)

diff --git a/installman b/installman
index ce4b7f3..4c20af6 100755
--- a/installman
+++ b/installman
@@ -154,6 +154,7 @@ sub pod2man {
 
         my $parser = Pod::Man->new( section => $manext,
                                     official=> 1,
+                                    errors  => 'die',
                                     center  => 'Perl Programmers Reference Guide'
                                   );
 	my $xmanpage = $manpage;
-- 
2.1.4

@p5pRT
Copy link
Author

p5pRT commented Apr 18, 2015

From @jkeenan

On Sat Apr 18 08​:06​:21 2015, ntyni@​debian.org wrote​:

Since podlators 2.5.0 (perl 5.17.8, Pod​::Man 2.27), Pod​::Man has
supported
three different ways of handling POD errors. The old way of
advertising
them in the generated manual page (in a POD ERRORS section) is still
the
default for Pod​::Man, as the others are not really suitable for
libraries,
but the pod2man script now defaults to dying on POD errors. The third
option is to output warnings on stderr during the conversion.

Quoting Russ Allbery in
<https://rt.cpan.org/Public/Bug/Display.html?id=39007>:

The primary argument for doing this is that POD errors are primarily
of interest to the author of the POD, not to thte end-user, but the
POD
ERRORS section in an installed man page tells the user about things
they
can't do anything about.

There was some resulting discussion on the perl5-porters list at
http​://www.nntp.perl.org/group/perl.perl5.porters/2008/09/msg139898.html.

Now, the perl core build uses a pod2man replacement in the
'installman'
script, which is still using the default Pod​::Man behaviour. The above
rationale applies to that one as well. The attached proposed patch
makes
it match the pod2man behaviour by dying on POD errors.

Current bleadperl has two files with POD errors, so those have to be
fixed first, as per the other two attached patches. However, they are
in
cpan/, so possibly it's better to wait for them to be fixed upstream.
The
Test-Simple one is already fixed in CPAN development versions, but the
Memoize one has a ticket from 2013 with no action so far at
https://rt.cpan.org/Public/Bug/Display.html?id=89441

I have filed a comment in Memoize's bug queue requesting action on that problem.

Note that ExtUtils-MakeMaker uses its own pod2man implementation,
and a similar change to that is being discussed in
Perl-Toolchain-Gang/ExtUtils-MakeMaker#216

Thanks for your work,

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Apr 18, 2015

The RT System itself - Status changed from 'new' to 'open'

@jkeenan
Copy link
Contributor

jkeenan commented Oct 29, 2023

From @ntyni

[snip]
The attached proposed patch makes [installman] match the pod2man behaviour by dying on POD errors.

@ntyni, this appears to be the only remaining issue in this ticket opened in 2015. If you would like to pursue it further, could you open a pull request?

Thank you very much.

@jkeenan
Copy link
Contributor

jkeenan commented Nov 16, 2023

From @ntyni

[snip]
The attached proposed patch makes [installman] match the pod2man behaviour by dying on POD errors.

@ntyni, this appears to be the only remaining issue in this ticket opened in 2015. If you would like to pursue it further, could you open a pull request?

No subsequent update; closing.

@jkeenan jkeenan closed this as completed Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants