t/porting/podcheck.t flagging POD in Windows generated HTML

jkahrman commented 2 years ago

Module: Pod::Html

Description t/porting/podcheck.t is failing example POD snippets in HTML generated from the POD files by Pod::Html via the installhtml script (used on Windows as part of the default 'install' targets). I believe prior to #18031, these example POD snippets were not detected by the test because the POD directives were not at the beginning of lines.

Steps to Reproduce gmake installhtml .\perl -Ilib t\porting\podcheck.t

Expected behavior t/porting/podcheck.t passes, either because it doesn't detect POD in the generated HTML, or it doesn't look in the generated HTML files

Test failures

...
#   "Pod NAME already used" 
#     'win32/html/lib/Pod/Usage.html' also has NAME 'sample' near line ??? of win32/html/lib/Getopt/Long.html
#   "Pod NAME already used" 
#     'win32/html/lib/Getopt/Long.html' also has NAME 'sample' near line ??? of win32/html/lib/Pod/Usage.html
...
#     '=item' '=over' near line 434 of win32/html/pod/perldocstyle.html
...
#     '=item' '=over' near line 264 of win32/html/pod/perlpod.html
...
porting/podcheck.t ........ 
Failed 4/1463 subtests

Perl configuration

Summary of my perl5 (revision 5 version 34 subversion 1) configuration:

  Platform:
    osname=MSWin32
    osvers=6.1.7601
    archname=MSWin32-x64-multi-thread
    uname=''
    config_args='undef'
    hint=recommended
    useposix=true
    d_sigaction=undef
    useithreads=define
    usemultiplicity=define
    use64bitint=define
    use64bitall=undef
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=undef
  Compiler:
    cc='cl'
    ccflags ='-nologo -GF -W3 -MD -DWIN32 -D_CONSOLE -DNO_STRICT -DWIN64 -D_CRT_SECURE_NO_DEPRECATE -D_CRT_NONSTDC_NO_DEPRECATE -D_WINSOCK_DEPRECATED_NO_WARNINGS -DWIN32 -D_CRT_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_DEPRECATE -D_CRT_NONSTDC_NO_DEPRECATE -D_SECURE_SCL=0 /D_WINDOWS /D_MBCS -D_WIN32_WINNT=0x0600 -DWINVER=0x0600 -D_WIN32_IE=0x0700 -D__SSE__ -D__SSE2__ -FC -MD -nologo -GS- -DPERL_TEXTMODE_SCRIPTS -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -DUSE_PERLIO'
    optimize='-O1 -Zi -GL -fp:precise'
    cppflags='-DWIN32'
    ccversion='19.00.24215.1'
    gccversion=''
    gccosandvers=''
    intsize=4
    longsize=4
    ptrsize=8
    doublesize=8
    byteorder=12345678
    doublekind=3
    d_longlong=undef
    longlongsize=8
    d_longdbl=define
    longdblsize=8
    longdblkind=0
    ivtype='__int64'
    ivsize=8
    nvtype='double'
    nvsize=8
    Off_t='__int64'
    lseeksize=8
    alignbytes=8
    prototype=define
  Linker and Libraries:
    ld='link'
    ldflags ='-incremental:NO -map -nologo -debug -largeaddressaware -errorReport:prompt -dynamicbase:no  -retryonfileopenfailure'
    libpth=\lib\amd64
    libs=oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib comctl32.lib msvcrt.lib vcruntime.lib ucrt.lib
    perllibs=oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib comctl32.lib msvcrt.lib vcruntime.lib ucrt.lib
    libc=ucrt.lib
    so=dll
    useshrplib=true
    libperl=perl534.lib
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_win32.xs
    dlext=dll
    d_dlsymun=undef
    ccdlflags=' '
    cccdlflags=' '
    lddlflags='-dll -incremental:NO -map -nologo -debug -largeaddressaware -errorReport:prompt -dynamicbase:no  -retryonfileopenfailure'

Characteristics of this binary (from libperl):
  Compile-time options:
    HAS_TIMES
    HAVE_INTERP_INTERN
    MULTIPLICITY
    PERLIO_LAYERS
    PERL_COPY_ON_WRITE
    PERL_DONT_CREATE_GVSV
    PERL_IMPLICIT_CONTEXT
    PERL_IMPLICIT_SYS
    PERL_MALLOC_WRAP
    PERL_OP_PARENT
    PERL_PRESERVE_IVUV
    USE_64_BIT_INT
    USE_ITHREADS
    USE_LARGE_FILES
    USE_LOCALE
    USE_LOCALE_COLLATE
    USE_LOCALE_CTYPE
    USE_LOCALE_NUMERIC
    USE_LOCALE_TIME
    USE_PERLIO
    USE_PERL_ATOF
    USE_THREAD_SAFE_LOCALE
  Built under MSWin32
  Compiled at Mar 26 2022 05:02:39
  @INC:
    B:/3p/derived/win64/perl/lib
    .

jkahrman commented 2 years ago

Modified podcheck.t to skip the generated windows HTML (win32/html)

podcheck_no_html.patch.txt

sisyphus commented 2 years ago

FWIW, I'm not seeing this issue with my Visual Studio 2019 or mingw-w64 builds. (All tests successful.)

But @jkahrman's build includes '.' in code>@INC</code, whereas my builds don't. I'm guessing that might well account for the difference.

jkahrman commented 2 years ago

Based on my reading of t/porting/podcheck.t, I don't see how those could be related. The test overwrites the default @INC to "../lib", and searches for files in the root build directory, not the INC path. @sisyphus, is your build generating the HTML before running the t/porting/podcheck.t test or after?

sisyphus commented 2 years ago

They may be unrelated ... it was just a guess. I'll try building perl again, but with '.' included in code>@INC</code, and see if I then hit the same problem.

In the meantime, @khw has asked me to offer the following thoughts, as github is preventing him from commenting on this Issue:

It is a real error to have more than one file share the same NAME.  That
means a link to a pod won't know which one to use.

It's not clear from the description if these snippets are parts of tests
or they actually are to be installed.  If win32/html contains pod that
sticks around for users to look at, then skipping it doesn't sound to me
like the right solution.  If these are just tests, shouldn't they go in
a temp directory?

jkahrman commented 2 years ago

It is a real error to have more than one file share the same NAME. That means a link to a pod won't know which one to use.

Agreed, but the POD the test is flagging are examples on how to write POD embedded in other doc.

It's not clear from the description if these snippets are parts of tests or they actually are to be installed. ... If these are just tests, shouldn't they go in a temp directory?

The win32/html directory contains HTML pages generated from all the POD in the Perl release (via Pod::Html), and it is installed along with Perl. The HTML isn't generated for the purpose of testing; it's part of the "install" make target.

If win32/html contains pod that sticks around for users to look at, then skipping it doesn't sound to me like the right solution.

All of the HTML doc under win32/html is generated from POD files that t/porting/podcheck.t is already checking.
t/porting/podcheck.t doesn't appear to be designed to validate HTML, just POD, so it doesn't seem right for this test to look in these files.

jkeenan commented 2 years ago

[snip]

If win32/html contains pod that sticks around for users to look at, then skipping it doesn't sound to me like the right solution.
1. All of the HTML doc under win32/html is generated from POD files that t/porting/podcheck.t is already checking.

2. t/porting/podcheck.t doesn't appear to be designed to validate HTML, just POD, so it doesn't seem right for this test to look in these files.

I suspect you are correct, and I therefore also suspect your p.r. should be applied.

However, let me share some thoughts -- based on having done a lot of work last year refactoring ext/Pod-Html/ -- on why this problem occurs.

AFAICT, the only thing inside the core distribution that uses the pod2html executable program (ext/Pod-Html/bin/pod2html) is the executable program installhtml.

Many years ago I was told that installhtml is "how Perl gets installed on Windows." I myself don't have access to Windows.

Now, I suspect that, on Windows like on all nix platforms, perl mostly gets installed via vendor packages -- not via installhtml. But, for the sake of argument (and this ticket), let's assume that one uses installhtml for this purpose. It follows that only people building and testing Perl on Windows are likely to notice complications that show up, as you are reporting, in the `t/porting/.t` part of the test suite.

So let's hope that we can have people who do work extensively on Windows to comment here.

@sisyphus

sisyphus commented 2 years ago

On looking more carefully at @jkahrman's original post, I can see that there's quite a few things being done with which I am unfamiliar.

Firstly, although I build perl on windows regularly and often, I've never run gmake installhtml. I don't what that's supposed to do. I guess it installs some html somewhere.

I would have expected that, when using Microsoft tools, one would instead run nmake installhtml but, FAIK, there could be something about installhtml that specifically needs gmake.

I then wonder whether perl was built using nmake (and win32/Makefile) or gmake (and win32/GNUmakefile). I believe that, with Microsoft compilers, it could be either - though I only ever use nmake with them. AFAICT, the standard perl -V output fails to indicate which flavour of make was used. Perhaps this doesn't matter ... I don't know.

And then I noticed that the command that causes the tests to fail invokes the perl debugger. (I have no experience with the perl debugger.) And, of course, I subsequently got around to noticing that @jkahrman's build of perl is (or at least looks to me to be) a debugging build.

I haven't built a debugging perl in years,

There might also be other configuration options that I haven't yet picked up on.

My approach to building and testing perl is always pretty standard: (g|n)make [args] test (g|n)make [args] install (g|n)make distclean

where "(g|n)make" means " gmake or nmake, as the case might be".

I used to edit the Makefile/GNUmakefile (in the win32 source folder) and provide no command line args. But for the last year or two, I've been using command line args so that I don't have to touch the Makefile/GNUmakefile.

In summary, I feel that even if I did reproduce this Issue, then I wouldn't know what to do about it anyway.

Maybe @xenu or Steve Hay (whose github handle I don't know) could provide something helpful here.

Cheers, Rob

PS I see there's an @steve-m-hay, but I'm not sure that's the "Steve Hay" that I have in mind.

jkahrman commented 2 years ago

And then I noticed that the command that causes the tests to fail invokes the perl debugger.

Sorry, that was a copy paste error in my original post. I've removed the debugger flag from the command I used to run the test.

My approach to building and testing perl is always pretty standard: (g|n)make [args] test (g|n)make [args] install (g|n)make distclean

... Since you are running the tests before the install step and cleaning after the install, you are likely never running with the win32/html directory present. I've been running the tests after the install step.

sisyphus commented 2 years ago

Aaah ... now I see.

The ./win32/html directory doesn't exist until 'make install' has been run. It therefore seems reasonable to me that files in this folder (should the folder exist) be excluded from podcheck.t scrutiny, as per your patch.

Cheers, Rob

jkeenan commented 2 years ago

@jkahrman , can you turn the patch you submitted in this ticket into a pull request so that we can more properly consider it? Thanks.

Perl / perl5

t/porting/podcheck.t flagging POD in Windows generated HTML #19565