Hi Michael,

Thanks for running through your thought process - it makes good sense,
and you've pointed out a lot of things I didn't/wouldn't think of. So I've
prepared another patch that takes these into account, this time on the
MakeMaker 6.05 source. It passes the test suite on OS X.

Putting pod2man() in ExtUtils::Command::MM was a good idea - wish
I'd known the module existed in the first place ;-). It _was_ a long command,
and if moving it avoids/fixes bugs then all the better.

But one thing's been nagging the back of my head -- we're re-inventing
the pod2man wheel here. So what about sticking the pod2man code
(which has been reduced to a mere 50 lines of mostly GetOpt work) into
it's own module and have both the binary & the Command::MM extension
use it?

That way no extra support for Date::Roman is needed - effectively we'd
be running the pod2man binary. And it's got the same syntax as what's
already used in MM, so that shouldn't be a problem.

But baby steps - I've moved pod2man() into ExtUtils::Command::MM,
and we can worry about the above later.

On =section vs. --section:
I started off with --section, but perl was trying to grab it as a cmdline
option instead of passing it along, which broke everything. I didn't
realize that at the time, but instead quickly narrowed it down, quickly
switched it to =section, and quickly forgot to update the docs (bad
spurkis! :). I've reverted to --section in this patch, now I know the fix.

Finally, the patch does *not* define POD2MAN_EXE as you said not
to worry about it (but it should be easy enough to fill in, or extract
from the previous patches, if required). It also contains a couple of
minor cleanups at the beginning of MM_Unix.pm and
Command/MM.pm, though I haven't changed any $version numbers.

Hope this helps,
-Steve

Attachment: MakeMaker-6.05-manifypods.patch
Description: Binary data


On Tuesday, November 12, 2002, at 01:15 , Michael G Schwern wrote:
On Mon, Nov 11, 2002 at 07:42:07PM +0000, Steve Purkis wrote:
On Monday, November 11, 2002, at 07:33 , Steve Purkis wrote:
...

What you've suggested above works fine. Besides - not having to
maintain
code that finds pod2man is one less thing to maintain, however small it
is.
*grumble*
In saying that, I realized that I hadn't chopped out find_pod2man_exe()
See the attached patch [again!]
I'd love to dump that code, but I'm warying of chopping out the POD2MAN_EXE
macro. Even though its not documented, I've done a scan of CPAN and its
being used. I've only found 4. 3 are by the same author (RCGI, PQEdit and
RDBAL) and what they're doing can be accomplished in better ways. They're
using the POD2MAN_EXE macro to find pod2html and then hacking manifypods to
generate HTML documentation. Definately better ways to do that. Either
way, its not critical functionality at all.

The other is Date::Roman and I can't see how to easily emulate what's being
done there, passing in its own version and date to pod2man.

Well, we could simply emulate pod2man in the POD2MAN_EXE macro. The only
extension necessary would be to emulate the -r/--release and -d/--date
switches to keep Date::Roman working. With the ExtUtils::Command::MM
solution proposed below this should be easy.

Don't worry about this problem. I'll contact the two authors and see if
they care.


+manifypods : pure_all $dependencies
+ $self->{NOECHO}\$(POD2MAN) =section 1 $man1pods
+ $self->{NOECHO}\$(POD2MAN) =section 3 $man3pods

Your seperating by section is a good idea. However...


+# This returns a pod-to-man make variable that contains a perl script
+# that is used like this:
+#
+# $(POD2MAN) [--section <man_section>]
+# <infile> <outfile> [ <infile> <outfile> ... ]

Code says '=section', docs say --section. --section at least matches
pod2man's syntax.


Hmmm... looking at this:

+ my $make_vars = <<'END_OF_PROGRAM';
+POD2MAN = $(PERL) -we ' \
+ use Pod::Man; \
+ \
+ my %args = @ARGV; \
+ my $$section = delete($$args{"=section"}) || ""; \
+ \
+ print "Manifying section ", $$section || "[unknown]", " pods:\n"; \
+ my $$parser = new Pod::Man(section => $$section); \
+ \
+ foreach my $$infile (keys %args) { \
+ my $$outfile = $$args{$$infile}; \
+ next if ((-e $$outfile) && \
+ (-M $$outfile < -M $$infile) && \
+ (-M $$outfile < -M "Makefile")); \
+ print " + $$outfile"; \
+ $$parser->parse_from_file($$infile, $$outfile) \
+ or (warn("Could not install $$outfile\n") and next); \
+ chmod(oct($(PERM_RW)), $$outfile) \
+ or (warn("chmod $(PERM_RW) $$outfile: $$!\n") and next); \
+ print "\n"; \
+ }'
+END_OF_PROGRAM
+

I'm very worried about shell command size here. The command itself as
written eats up nearly 1200 characters. Add to that a list of pm files and
man pages and you can easily blow over command limits on some shells.
IRIX is one for sure. In fact, there's an open bug about man page
generation commands being too long on IRIX.
https://rt.cpan.org/Ticket/Display.html?id=667
Once we've got your cleanups sorted out that bug will be much easier to fix.

The trailing whitespace can be stripped for a big savings. The code could
even be made more readable by formatting it normally (ie. without trailing
\) and then putting them on later with s{\n}{\\\n}g;

Alternatively, it could be made a function in ExtUtils::Command::MM and the
shell command reduced do:

POD2MAN = $(PERL) "-MExtUtils::Command::MM" -e "pod2man @ARGV"

I like that idea.


--

Michael G. Schwern <[EMAIL PROTECTED]> http://www.pobox.com/~schwern/
Perl Quality Assurance <[EMAIL PROTECTED]> Kwalitee Is Job One
If I got anything to say, I'll say it with lead.
-- Jon Wayne


Reply via email to