fix(makemaker): mirror upstream prereq check (no false "have 0")#575
Merged
fix(makemaker): mirror upstream prereq check (no false "have 0")#575
Conversation
The PerlOnJava ExtUtils::MakeMaker stub used `eval "require $module"` to verify PREREQ_PM, then read $Module::VERSION. That fails open in the common CPAN scenario where PERL_USE_UNSAFE_INC=1 puts `.` on @inc and the build directory contains a similarly-named .pm with a different package (e.g. ACME-Error-Coy ships `Coy.pm` whose package is `ACME::Error::Coy`). `require Coy` succeeds against that file, but $Coy::VERSION is undef, so the check reported the misleading Missing dependencies: - Coy (>= 0.01, have 0) Please install these modules first. even though the next line said "Configured! 1 files will be installed". Real ExtUtils::MakeMaker (5.42 / 7.76, lines ~579-647 of the system copy) uses `MM->_installed_file_for_module` (file-path search of @inc) plus `MM->parse_version` (static parse of `$VERSION = ...`), never `require`. With the same Makefile.PL system Perl silently considers the prereq satisfied; CPAN.pm independently resolves the real Coy via its own metadata. This commit aligns _check_prereqs with upstream: - File-path search via _installed_file_for_module (skipping @inc code/array/blessed hooks). - parse_version on the found file instead of require + $VERSION. - Per-module `Warning: prerequisite Foo 1.0 not found.` / `... not found. We have X.` warnings via warn(), matching the upstream wording. - Drops the unconditional "Missing dependencies / Please install these modules first" banner that contradicted the subsequent "Configured!" message. - Adds PREREQ_FATAL handling that dies with the upstream message ("MakeMaker FATAL: prerequisites not found. ... Please install these modules first and rerun 'perl Makefile.PL'."). - Aggregates BUILD_REQUIRES / CONFIGURE_REQUIRES / TEST_REQUIRES the same way upstream does. After: `jcpan -t ACME::Error::Coy` configures cleanly with no false warning, tests still pass, and the CPAN.pm-driven Coy resolution is unaffected. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a misleading dependency-check report in PerlOnJava's
ExtUtils::MakeMakershim and aligns the prereq logic with upstream MakeMaker (Perl 5.42 / MakeMaker 7.76).Problem
jcpan -t ACME::Error::Coywas printing a self-contradictory configure block:Root cause:
_check_prereqsdideval "require Coy; 1"and then read$Coy::VERSION. CPAN.pm setsPERL_USE_UNSAFE_INC=1so.is on@INC. The cwd at configure time is theACME-Error-Coybuild directory, which contains a file literally namedCoy.pm— but it declarespackage ACME::Error::Coy.require Coyhappily loads the wrong file,%INCrecordsCoy.pm, but$Coy::VERSIONstays undef, so the check reportshave 0. Coy is in fact not installed at all.What real Perl does
System Perl's
ExtUtils::MakeMaker(5.42, MakeMaker 7.76, lines ~579-647) does notrequireprereqs. It uses:MM->_installed_file_for_module($prereq)— file-path search of@INCforFoo/Bar.pm.MM->parse_version($installed_file)— static parse of$VERSION = ...from the file.Reporting:
warn "Warning: prerequisite Foo 1.0 not found.\n"and continue.warn "Warning: prerequisite Foo 1.0 not found. We have 0.5.\n"and continue.MakeMaker FATAL: prerequisites not found.\n... Please install these modules first and rerun 'perl Makefile.PL'.— only onPREREQ_FATAL.I confirmed system Perl on the same build dir is silent (parse_version returns
0.01from the misnamed file, so the check looks satisfied; CPAN.pm independently catches the real missing dep via its own metadata).Fix
src/main/perl/lib/ExtUtils/MakeMaker.pm:_installed_file_for_modulemirroring upstream (skipping@INCcode/array/blessed hooks)._check_prereqsto use file-path search +MM->parse_version, neverrequire.Warning: prerequisite Foo X not found./... We have Y.to STDERR with the upstream wording.Missing dependencies / Please install these modules firstbanner that contradicted the subsequentConfigured!line.PREREQ_FATALand die with the upstream message.BUILD_REQUIRES,CONFIGURE_REQUIRES,TEST_REQUIRESthe same way upstream does.After
CPAN.pm's own dependency resolution (the
---- Unsatisfied dependencies detected ----mechanism) still drives the actual Coy build.Test plan
make(build + unit tests): greenjcpan -t ACME::Error::Coyend-to-end on a freshly cleaned~/.cpan/build: configure clean, no falsehave 0, both modules tested, exit 0Makefile.PLwith a non-existent prereq: printsWarning: prerequisite NoSuchModuleZZZ 1.0 not found.and continuesPREREQ_FATAL => 1: prints the warning and dies with the upstreamMakeMaker FATAL: ... Please install these modules first and rerun 'perl Makefile.PL'.messageperl Makefile.PLon the same tree — output now matchesmake test-bundled-modulesshows 2 pre-existing failures (Net-SSLeay/t/local/33_x509_create_cert.t,Text-CSV/t/55_combi.t) due to missing test helpers (./t/util.pl,Test::Net::SSLeay); unrelated to this change.Generated with Devin