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

installhtml generates possibly bogus warnings #11860

Closed
p5pRT opened this issue Jan 10, 2012 · 13 comments · Fixed by #17531
Closed

installhtml generates possibly bogus warnings #11860

p5pRT opened this issue Jan 10, 2012 · 13 comments · Fixed by #17531
Labels
Closable? We might be able to close this ticket, but we need to check with the reporter distro-All type-utilities

Comments

@p5pRT
Copy link

p5pRT commented Jan 10, 2012

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

Searchable as RT107882$

@p5pRT
Copy link
Author

p5pRT commented Jan 10, 2012

From @nwc10

installhtml now generates many seemingly troublesome warnings. For example,
36 instances of

Cannot find "IPC​::Open3" in podpath​: more than one possible replacement path to IPC​::Open3, using lib​::IPC​::Open3

It's not clear whether this is actually a genuine problem, as it doesn't look
like the installed HTML contains links to modules "named" lib​::IPC​::Open3
Either way, it needs investigating and resolving, either to silence
6773 false warnings, or if there is a real problem, to address it.

(installhtml on perl 5.14.0 generates 841 lines on STDERR, which is bad,
but nothing compared with the 7010 now generated)

This may be a fundamental problem of how installhtml is currently written
to take the directory for the "podroot" from the command line, and this is
passed in as ".", the *top level* of the distribution. Given that installhtml
is *not* installed, and hence is only used for installing from the core,
probably it should be refactored to be structurally very similar to
installman. Possibly even the two could be merged (or much more code
made common in a library), given that both are

1​: find the list of Pod files that need to be installed
2​: convert those to the correct format
3​: copy the generated files to the correct place

It might be that duplicate files in lib/ and in {ext,dist,cpan} are actually
the cause of all the "more than one possible replacement path" warnings,
hence eliminating them would naturally fall out from such a refactoring.

This might also solve bug #107880 as a side effect.

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented Jan 13, 2012

From ponguile@gmail.com

On Tue, Jan 10, 2012 at 2​:56 PM, Nicholas Clark
<perlbug-followup@​perl.org>wrote​:

# New Ticket Created by Nicholas Clark
# Please include the string​: [perl #107882]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=107882 >

installhtml now generates many seemingly troublesome warnings. For example,
36 instances of

Cannot find "IPC​::Open3" in podpath​: more than one possible replacement
path to IPC​::Open3, using lib​::IPC​::Open3

It's not clear whether this is actually a genuine problem, as it doesn't
look
like the installed HTML contains links to modules "named" lib​::IPC​::Open3
Either way, it needs investigating and resolving, either to silence
6773 false warnings, or if there is a real problem, to address it.

These warnings are issued by Pod​::Html. They are a result of Pod​::Html
trying to make cross-references between the contents of L<> formatting
codes and the file being linked to. Warnings are raised when an exact
filepath match cannot be found in the hash containing all POD files found.
E.g., trying to cross-reference L<IPC​::Open3> results in the warning you
mentioned because Pod​::Html did not find a file IPC/Open3.pod relative to
--podpath. Instead, it found a partial match, lib/IPC/Open3.pod, and it is
letting the user know of this.

A rather simple fix is to only display these warnings if --verbose is
specified, rather than the current implementation which allows them to be
silenced with --quiet, but displayed otherwise.

(installhtml on perl 5.14.0 generates 841 lines on STDERR, which is bad,
but nothing compared with the 7010 now generated)

This may be a fundamental problem of how installhtml is currently written
to take the directory for the "podroot" from the command line, and this is
passed in as ".", the *top level* of the distribution. Given that
installhtml
is *not* installed, and hence is only used for installing from the core,
probably it should be refactored to be structurally very similar to
installman. Possibly even the two could be merged (or much more code
made common in a library), given that both are

1​: find the list of Pod files that need to be installed
2​: convert those to the correct format
3​: copy the generated files to the correct place

It might be that duplicate files in lib/ and in {ext,dist,cpan} are
actually
the cause of all the "more than one possible replacement path" warnings,
hence eliminating them would naturally fall out from such a refactoring.

This might also solve bug #107880 as a side effect.

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented Jan 13, 2012

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

@p5pRT
Copy link
Author

p5pRT commented Jan 13, 2012

From @nwc10

On Thu, Jan 12, 2012 at 10​:08​:30PM -0500, Marc Green wrote​:

On Tue, Jan 10, 2012 at 2​:56 PM, Nicholas Clark

It's not clear whether this is actually a genuine problem, as it doesn't
look
like the installed HTML contains links to modules "named" lib​::IPC​::Open3
Either way, it needs investigating and resolving, either to silence
6773 false warnings, or if there is a real problem, to address it.

These warnings are issued by Pod​::Html. They are a result of Pod​::Html
trying to make cross-references between the contents of L<> formatting
codes and the file being linked to. Warnings are raised when an exact
filepath match cannot be found in the hash containing all POD files found.
E.g., trying to cross-reference L<IPC​::Open3> results in the warning you
mentioned because Pod​::Html did not find a file IPC/Open3.pod relative to
--podpath. Instead, it found a partial match, lib/IPC/Open3.pod, and it is
letting the user know of this.

Aha right. Thanks.

A rather simple fix is to only display these warnings if --verbose is
specified, rather than the current implementation which allows them to be
silenced with --quiet, but displayed otherwise.

I think that that actually might just be brushing the real problem under
the carpet. Related to RT #107880 - installhtml installs private pod
it's looking like the *actual* problem behind both tickets is that
Pod​::Html is invoked (on *nix) from ./installhtml with

  --podroot=. --podpath=. --recurse

I suspect that that should be 'lib' instead of '.'. Or maybe 'pod​:lib​:utils'
(which is almost what the Win32 Makefiles do - they have 'ext' too, which
probably should go)

Although, really, I'm still suspecting that *that* is only a medium-term
fix, and the best solution is to unify on the scanner used by installman,
and pass the explicit list of Pod files that it finds.

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented Jan 13, 2012

From @nwc10

On Thu, Jan 12, 2012 at 10​:08​:30PM -0500, Marc Green wrote​:

On Tue, Jan 10, 2012 at 2​:56 PM, Nicholas Clark
<perlbug-followup@​perl.org>wrote​:

# New Ticket Created by Nicholas Clark
# Please include the string​: [perl #107882]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=107882 >

installhtml now generates many seemingly troublesome warnings. For example,
36 instances of

Cannot find "IPC​::Open3" in podpath​: more than one possible replacement
path to IPC​::Open3, using lib​::IPC​::Open3

It's not clear whether this is actually a genuine problem, as it doesn't
look
like the installed HTML contains links to modules "named" lib​::IPC​::Open3
Either way, it needs investigating and resolving, either to silence
6773 false warnings, or if there is a real problem, to address it.

These warnings are issued by Pod​::Html. They are a result of Pod​::Html
trying to make cross-references between the contents of L<> formatting
codes and the file being linked to. Warnings are raised when an exact
filepath match cannot be found in the hash containing all POD files found.
E.g., trying to cross-reference L<IPC​::Open3> results in the warning you
mentioned because Pod​::Html did not find a file IPC/Open3.pod relative to
--podpath. Instead, it found a partial match, lib/IPC/Open3.pod, and it is
letting the user know of this.

Also, I find the text of the error message a bit troubling. I keep thinking
that it's about to make a link <a href="lib​::Pod​::Html">
However, I think that it didn't do this.

[Some of the manpage scanning code etc *has* had bugs where it's done things
like this - prefixed lib​:: to everything because it found it in a path
starting 'lib/']

If it's about a filename, is it possible to report the filename instead?

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented Jan 13, 2012

From ponguile@gmail.com

I think that that actually might just be brushing the real problem under
the carpet. Related to RT #107880 - installhtml installs private pod
it's looking like the *actual* problem behind both tickets is that
Pod​::Html is invoked (on *nix) from ./installhtml with

--podroot=. --podpath=. --recurse

I suspect that that should be 'lib' instead of '.'. Or maybe
'pod​:lib​:utils'
(which is almost what the Win32 Makefiles do - they have 'ext' too, which
probably should go)

I agree that --podpath should be limited to the directories that have POD
that we want scanned, rather than the entire source tree.

On Fri, Jan 13, 2012 at 5​:18 AM, Nicholas Clark <nick@​ccl4.org> wrote​:

On Thu, Jan 12, 2012 at 10​:08​:30PM -0500, Marc Green wrote​:

These warnings are issued by Pod​::Html. They are a result of Pod​::Html
trying to make cross-references between the contents of L<> formatting
codes and the file being linked to. Warnings are raised when an exact
filepath match cannot be found in the hash containing all POD files
found.
E.g., trying to cross-reference L<IPC​::Open3> results in the warning you
mentioned because Pod​::Html did not find a file IPC/Open3.pod relative to
--podpath. Instead, it found a partial match, lib/IPC/Open3.pod, and it
is
letting the user know of this.

Also, I find the text of the error message a bit troubling. I keep thinking
that it's about to make a link <a href="lib​::Pod​::Html">
However, I think that it didn't do this.

You think Pod​::Html linked to itself instead of the module it is
cross-referencing? Or is that a typo? Or are you emphasizing the arisdottle
(that is what the '​::' is called, right?)? If the latter, then you are
correct, it replaces those with the filesystem path separator.

I think the warning message is clear, but I am undoubtedly biased, as I
wrote it. Perhaps you can expand on what you mean by "troubling".

[Some of the manpage scanning code etc *has* had bugs where it's done
things
like this - prefixed lib​:: to everything because it found it in a path
starting 'lib/']

Is the problem the arisdottle, or the fact that 'lib/' is prefixed? I am
confused as to what you are emphasizing again.

If it's about a filename, is it possible to report the filename instead?

It's reporting about the path to the file.

@p5pRT
Copy link
Author

p5pRT commented Jan 17, 2012

From @nwc10

On Fri, Jan 13, 2012 at 11​:33​:32AM -0500, Marc Green wrote​:

On Fri, Jan 13, 2012 at 5​:18 AM, Nicholas Clark <nick@​ccl4.org> wrote​:

On Thu, Jan 12, 2012 at 10​:08​:30PM -0500, Marc Green wrote​:

These warnings are issued by Pod​::Html. They are a result of Pod​::Html
trying to make cross-references between the contents of L<> formatting
codes and the file being linked to. Warnings are raised when an exact
filepath match cannot be found in the hash containing all POD files
found.
E.g., trying to cross-reference L<IPC​::Open3> results in the warning you
mentioned because Pod​::Html did not find a file IPC/Open3.pod relative to
--podpath. Instead, it found a partial match, lib/IPC/Open3.pod, and it
is
letting the user know of this.

Also, I find the text of the error message a bit troubling. I keep thinking
that it's about to make a link <a href="lib​::Pod​::Html">
However, I think that it didn't do this.

You think Pod​::Html linked to itself instead of the module it is
cross-referencing? Or is that a typo? Or are you emphasizing the arisdottle
(that is what the '​::' is called, right?)? If the latter, then you are

<off topic>
I believe that its name is Paamayim Nekudotayim.
This is because someone was too lazy (or overworked - maybe that's worse)
to look up the real name for it. And there was no code review stage to
catch this process failure.
</off topic>

correct, it replaces those with the filesystem path separator.

I think the warning message is clear, but I am undoubtedly biased, as I
wrote it. Perhaps you can expand on what you mean by "troubling".

as below​:

[Some of the manpage scanning code etc *has* had bugs where it's done
things
like this - prefixed lib​:: to everything because it found it in a path
starting 'lib/']

Is the problem the arisdottle, or the fact that 'lib/' is prefixed? I am
confused as to what you are emphasizing again.

If it's about a filename, is it possible to report the filename instead?

It's reporting about the path to the file.

Then why is it using "​::" in the message rather than "/"?

That's my concern. If I see "​::" in a message, I assume that we're talking
about namespaces, and that everything is now in the wrong namespace because
a directory that should have been in the search path, is actually being
treated as part of the module name.

All this confusion and potential for bugs being because things like this​:

  foo/bar/baz.pm

Is that bar​::baz as found in ./foo?
baz as found in ./foo/bar?
Or foo​::bar​::baz as found in .?

If output starts reporting "You said bar​::baz, but I'm using foo​::bar​::baz"
then I assume it's picked the third of the three, and it's wrong.

Whereas if it said "You said bar​::baz, I'm using bar​::baz I found in foo/"
then I assume it got it right.

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented Jan 18, 2012

From ponguile@gmail.com

Or are you emphasizing the arisdottle
(that is what the '​::' is called, right?)? If the latter, then you are

<off topic>
I believe that its name is Paamayim Nekudotayim.
This is because someone was too lazy (or overworked - maybe that's worse)
to look up the real name for it. And there was no code review stage to
catch this process failure.

I remember stumbling across a Paamayim Nekudotayim error in PHP a few years
back, which is when I discovered that it meant "double colon" in Hebrew,
but I was under the impression no one else besides PHP folk use the term.

</off topic>

[Some of the manpage scanning code etc *has* had bugs where it's done
things
like this - prefixed lib​:: to everything because it found it in a path
starting 'lib/']

Is the problem the arisdottle, or the fact that 'lib/' is prefixed? I am
confused as to what you are emphasizing again.

If it's about a filename, is it possible to report the filename
instead?

It's reporting about the path to the file.

Then why is it using "​::" in the message rather than "/"?

Simply because the destination of some L<> formatting codes contain "​::" in
them. The warning is issued before it retrieves the path of the
destination, which has "/"s in it as expected -- it uses the key, not the
value, of the cache. It is letting the user know that an exact match for,
e.g., L<IPC​::Open3>, cannot be found (because there is no path (value)
"IPC/Open3" in the %Pages cache, but a module of the same name *is* found
in another directory, in this case below "lib/". I can see how this is
confusing -- the warning message considers namespaces and paths
interchangeable.

That's my concern. If I see "​::" in a message, I assume that we're talking
about namespaces, and that everything is now in the wrong namespace because
a directory that should have been in the search path, is actually being
treated as part of the module name.

All this confusion and potential for bugs being because things like this​:

foo/bar/baz.pm

Is that bar​::baz as found in ./foo?
baz as found in ./foo/bar?
Or foo​::bar​::baz as found in .?

If output starts reporting "You said bar​::baz, but I'm using foo​::bar​::baz"
then I assume it's picked the third of the three, and it's wrong.

Whereas if it said "You said bar​::baz, I'm using bar​::baz I found in foo/"
then I assume it got it right.

I am in agreement with you. I think the warning message should be clarified
in a way similar to the one you propose.

Also, I still think that the warning message should be issued only `if
$verbose` instead of `unless $quiet`, despite it not being the root of the
problem at hand.

@p5pRT
Copy link
Author

p5pRT commented Jan 18, 2012

From @nwc10

On Tue, Jan 17, 2012 at 08​:40​:59PM -0500, Marc Green wrote​:

Or are you emphasizing the arisdottle
(that is what the '​::' is called, right?)? If the latter, then you are

<off topic>
I believe that its name is Paamayim Nekudotayim.
This is because someone was too lazy (or overworked - maybe that's worse)
to look up the real name for it. And there was no code review stage to
catch this process failure.

I remember stumbling across a Paamayim Nekudotayim error in PHP a few years
back, which is when I discovered that it meant "double colon" in Hebrew,
but I was under the impression no one else besides PHP folk use the term.

That's somewhat my point. Maybe I'm too subtle, but I didn't stick a smiley
on it because the situation doesn't amuse me. Even *they* shouldn't be
using it. I'd argue it's clear process failure that the *wrong language*
ended up publicly exposed in an English language project.

Simply because the destination of some L<> formatting codes contain "​::" in
them. The warning is issued before it retrieves the path of the
destination, which has "/"s in it as expected -- it uses the key, not the
value, of the cache. It is letting the user know that an exact match for,
e.g., L<IPC​::Open3>, cannot be found (because there is no path (value)
"IPC/Open3" in the %Pages cache, but a module of the same name *is* found
in another directory, in this case below "lib/". I can see how this is
confusing -- the warning message considers namespaces and paths
interchangeable.

Does this mean that if I have Pod with a link that is L<Open3> it will
also guess that I meant to reference lib/IPC/Open3.pm and link from Open3
to IPC​::Open3?

I am in agreement with you. I think the warning message should be clarified
in a way similar to the one you propose.

Also, I still think that the warning message should be issued only `if
$verbose` instead of `unless $quiet`, despite it not being the root of the
problem at hand.

If the answer to my above question is yes, I suspect that it (at best)
should still warn if there is ambiguity, because it's likely that such
ambiguity is an error.

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented Jan 18, 2012

From @cpansprout

On Wed Jan 18 05​:43​:17 2012, nicholas wrote​:

On Tue, Jan 17, 2012 at 08​:40​:59PM -0500, Marc Green wrote​:

I remember stumbling across a Paamayim Nekudotayim error in PHP a
few years
back, which is when I discovered that it meant "double colon" in
Hebrew,
but I was under the impression no one else besides PHP folk use the
term.

That's somewhat my point. Maybe I'm too subtle, but I didn't stick a
smiley
on it because the situation doesn't amuse me. Even *they* shouldn't be
using it. I'd argue it's clear process failure that the *wrong
language*
ended up publicly exposed in an English language project.

What’s wrong with multilingual (or polylingual/multiglottal) projects?

From locale.t​:

/*
* A Elbereth Gilthoniel,
* silivren penna míriel
* o menel aglar elenath!
* Na-chaered palan-díriel
* o galadhremmin ennorath,
* Fanuilos, le linnathon
* nef aear, si nef aearon!
*
* [p.238 of _The Lord of the Rings_, II/i​: "Many Meetings"]
*/

Some day I’ll slip ἐνθάδε πάρεισι δράκοντες into the source. :-)

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Jan 19, 2012

From ponguile@gmail.com

Simply because the destination of some L<> formatting codes contain "​::"
in
them. The warning is issued before it retrieves the path of the
destination, which has "/"s in it as expected -- it uses the key, not the
value, of the cache. It is letting the user know that an exact match for,
e.g., L<IPC​::Open3>, cannot be found (because there is no path (value)
"IPC/Open3" in the %Pages cache, but a module of the same name *is* found
in another directory, in this case below "lib/". I can see how this is
confusing -- the warning message considers namespaces and paths
interchangeable.

Does this mean that if I have Pod with a link that is L<Open3> it will
also guess that I meant to reference lib/IPC/Open3.pm and link from Open3
to IPC​::Open3?

From the code​:

  ... if there is no exact match then ...
  # Try to find a POD that ends with $to and use that.
  # e.g., given L<XHTML>, if there is no $Podpath/XHTML in %Pages,
  # look for $Podpath/*/XHTML in %Pages, with * being any path,
  # as a substitute (e.g., $Podpath/Pod/Simple/XHTML)

It will check for an exact match, and since one does not exist, it will
expand its search to anything that ends in "Open3" within the given
parameters. If multiple possibilities are found, it will use the last one
it finds. Actually, now that I look at the code, it will *not* use the last
one it finds, but it is supposed to. As of now, it uses the last element of
the results array, but that is populated from keys(), so it is random. It
seems I forgot to sort the results before picking one. Whoops.

I am in agreement with you. I think the warning message should be
clarified
in a way similar to the one you propose.

Also, I still think that the warning message should be issued only `if
$verbose` instead of `unless $quiet`, despite it not being the root of
the
problem at hand.

If the answer to my above question is yes, I suspect that it (at best)
should still warn if there is ambiguity, because it's likely that such
ambiguity is an error.

That is a good point -- in this case it may be better to be safe than
sorry. My rationalization was that in an extreme majority of the cases
there is no ambiguity and the warnings are only an annoyance.

@p5pRT
Copy link
Author

p5pRT commented Feb 19, 2012

From ponguile@gmail.com

Although my commit does not fix the real issue at hand, I have provided marcgreen/perl@a83bce1 that needs to be smoked. However, the work in this patch was done after the work in my other patch, marcgreen/perl@2fdb0c8 -- fixing pod2html's cross referencing, and I checkout out of that branch instead of blead, so this shouldn't be smoked until that one is merged. (Unless someone feels like cherry-picking, then I think it work be OK.)

As discussed, this patch only sweeps the problem under the rug. I feel that
doing so is better than not, as long as we remember that the real problem
still exists.

@toddr
Copy link
Member

toddr commented Feb 5, 2020

It appears that marcgreen/perl@2fdb0c8 was eventually pulled into blead as 707a94f

I can't find any evidence that marcgreen/perl@a83bce1 was ever
brought in. I'm going to drop a PR and see what happens just for giggles.

@toddr toddr added the Closable? We might be able to close this ticket, but we need to check with the reporter label Feb 5, 2020
toddr pushed a commit to toddr/perl that referenced this issue Feb 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closable? We might be able to close this ticket, but we need to check with the reporter distro-All type-utilities
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants