Perl-Toolchain-Gang / ExtUtils-MakeMaker

Perl module to make Makefiles and build modules (what backs Makefile.PL)
https://metacpan.org/release/ExtUtils-MakeMaker
64 stars 76 forks source link

parse_version: handle "package version" syntax using the "hide from CPAN/PAUSE" hack #450

Open eserte opened 5 months ago

eserte commented 5 months ago

parse_version cannot parse a "package $package $version" if the line is broken after the "package" keyword, which is usually done for the "hide from CPAN" (or "hide from PAUSE") hack.

Sample affected module: https://metacpan.org/release/PEVANS/IO-Async-0.803/source/lib/IO/Async/Internals/Connector.pm#L6 (The previous version found in the 0.802 distribution used the traditional package keyword, and there parse_version worked)

@andk @leonerd FYI

eserte commented 5 months ago

Some additional context: if CPAN.pm is configured with allow_installing_module_downgrades=no, then the versions of the already installed module and the module about to be installed are compared, and the installation is refused if the new one has a lower version, or, as in this case, undef.

haarg commented 5 months ago

I don't think we should change this. The entire point of using a comment like that is prevent the version from being statically parsed.

This seems like a bug in the IO::Async::Internals::Connector module.

eserte commented 5 months ago

The

package # hide from CPAN
    Foo::Bar;

(the comment could also be written more correctly "hide from PAUSE") hack is widely used to prevent PAUSE to index the module, thus making it a "private" module, so the author is free to do refactoring and remove this module without notice.

The task of parsing a version is orthogonal to this, and should still be possible even when using the new package-version syntax.

Grinnz commented 5 months ago

There is a difference here between finding the package to be indexed (which the newline is meant to prevent) and parsing the version. The question is whether fixing parse_version to recognize this would affect what PAUSE chooses to index, as this was not intentionally designed this way.

haarg commented 5 months ago

If the module is internal, what is the externally parseable version number for?

haarg commented 5 months ago

As a comparison, cpanminus uses Parse::PMFile for this, which is meant to directly mirror the PAUSE behavior. Module::Metadata also does not extract this type of version. And ideally these implementations would all converge even more.

karenetheridge commented 5 months ago

If the module is internal, what is the externally parseable version number for?

This is what I would like to know too. What was the usecase where parse_version needed to know the version of IO::Async::Internals::Connector?

eserte commented 5 months ago

This is what I would like to know too. What was the usecase where parse_version needed to know the version of IO::Async::Internals::Connector?

See https://github.com/Perl-Toolchain-Gang/ExtUtils-MakeMaker/pull/450#issuecomment-1925893861 . The version comparison is done using parse_version.

karenetheridge commented 5 months ago

It seems wrong for CPAN.pm to care about the versions of unindexed modules. It really should only be concerned with the main module in the distribution.