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

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

eserte
Copy link
Contributor

@eserte eserte commented Feb 4, 2024

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

…PAN/PAUSE" hack

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)
@eserte
Copy link
Contributor Author

eserte commented Feb 4, 2024

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
Copy link
Member

haarg commented Feb 4, 2024

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
Copy link
Contributor Author

eserte commented Feb 5, 2024

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
Copy link
Contributor

Grinnz commented Feb 5, 2024

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
Copy link
Member

haarg commented Feb 5, 2024

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

@haarg
Copy link
Member

haarg commented Feb 5, 2024

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
Copy link
Member

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
Copy link
Contributor Author

eserte commented Feb 6, 2024

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 #450 (comment) . The version comparison is done using parse_version.

@karenetheridge
Copy link
Member

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.

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

Successfully merging this pull request may close these issues.

4 participants