fix(MakeMaker): use :: rules so Alien::Build postambles work#534
Merged
fix(MakeMaker): use :: rules so Alien::Build postambles work#534
Conversation
Generated Makefiles used single-colon rules for all, pm_to_blib, pure_all, pl_files, install_scripts, test, install, clean, realclean and distclean. Postambles from Alien::Build (and File::ShareDir::Install) add :: rules for the same targets, which GNU make rejects with "target file 'X' has both : and :: entries. Stop.". This broke `make` for Alien::m4, Alien::GMP and anything depending on them (e.g. Math::GMP, which blocks Net::SSH::Perl). Real ExtUtils::MakeMaker uses :: for these targets for exactly this reason. Switching our template to match fixes the build for those distributions and unblocks downstream tests. Also refresh dev/cpan-reports/ with the latest random-tester results (1695 -> 2541 modules tested, pass rate 22.0% -> 24.7%). Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…osures Named subs at file/package scope capture outer `my` lexicals via a "BEGIN-package alias": SubroutineParser.handleNamedSub registers the shared RuntimeHash/Array/Scalar under `PerlOnJava::_BEGIN_<id>::<name>` at compile time, and the owning `my` declaration later calls retrieveBeginHash (etc.) to take ownership of the same object at runtime. evalStringHelper was also populating that same alias map at entry (to let BEGIN blocks inside the eval see outer lexicals) and then unconditionally removing the entry in its `finally` block. When a sibling sub ran `eval STRING` BEFORE the outer `my` assignment had been executed, the finally-block removed SubroutineParser's compile-time alias. The later `my %WM = (...)` then found nothing in the map, created a fresh RuntimeHash, and the named sub's closure was left pointing at an empty, orphaned object. Fix: only schedule an alias for cleanup in evalAliasKeys if this invocation was the one that actually installed it (putIfAbsent returns null). If a compile-time alias was already present, leave it alone and let its owner (`my %WM` at runtime, or an enclosing eval) clean it up. Both the JVM path and the BytecodeInterpreter path in evalStringHelper had the same bug and get the same fix. Concrete impact: Net::SSH::Perl's modulino Makefile.PL failed with "NAME is required" whenever Digest::BubbleBabble was already installed, because that made have_module() run `eval "use ..."` before the file-scope `my %WriteMakefile = (...)` assignment. Added regression test: src/test/resources/unit/closure_eval_string.t covers hash, array and scalar closures with the exact shape from Net::SSH::Perl's Makefile.PL. Verified the test behaves identically on system perl 5.42 and on jperl. Generated with [Devin](https://devin.ai) 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 two distinct bugs exposed by
jcpan -t Net::SSH::Perl.1. Mixed
:/::rules in generated MakefilemakeforAlien::m4andAlien::GMP(deps ofMath::GMP, requiredby
Net::SSH::Perl) died with:Root cause: our PerlOnJava MakeMaker template emitted
clean:,realclean:,distclean:,all:,pm_to_blib:,pure_all:,test:,install:with single-colon rules. Postambles fromAlien::Build(
MY::postamble) andFile::ShareDir::Installextend the same targetswith double-colon rules — which GNU make rejects when mixed. Real
ExtUtils::MakeMakeruses::on these targets for exactly thisreason. Switched our template to match.
2. Closure-capture bug with
eval STRINGin a sibling subMinimal reproducer — both system perl 5.42 and jperl should print
keys=NAME,X:Before this PR jperl printed
keys=(empty). This causedNet::SSH::Perl's modulinoMakefile.PLto die withNAME is requiredwhenever
Digest::BubbleBabblewas already installed — thehave_module()path then runseval "use ..."BEFORE the file-scopemy %WriteMakefile = (...)assignment.Mechanism. Named/package subs capture outer
mylexicals via a"BEGIN-package alias":
SubroutineParser.handleNamedSubregisters the sharedRuntimeHash/Array/ScalarunderPerlOnJava::_BEGIN_<id>::<name>at compile time, and passes it as a constructor arg to the named sub's
generated class.
handleMyOperatorcallsPersistentVariable.retrieveBeginHashwhich takes ownership of the same object out of the alias map — so
the
myvariable and the named sub's capture point to the sameRuntimeHash.RuntimeCode.evalStringHelperalso populates that same alias map atentry (so BEGIN blocks inside the eval can see outer lexicals) and
unconditionally removes the entries in its
finallyblock. When asibling sub ran
eval STRINGbefore the outermyhad executed, thefinally block removed SubroutineParser's compile-time alias. The later
my %WM = (...)then found nothing in the map, created a freshRuntimeHash, and the named sub's closure was left pointing at anempty, orphaned object.
Fix. Only record an alias for cleanup in
evalAliasKeysif thisinvocation was the one that actually installed it (
putIfAbsentreturns null). If a compile-time alias was already there, leave it
alone; its owner — the outer
myat runtime, or an enclosing eval —will clean it up. Both the JVM path and the BytecodeInterpreter path in
evalStringHelperhad the same bug and get the same fix.Regression test added at
src/test/resources/unit/closure_eval_string.tcovering hash, array and scalar closures with the exact shape from
Net::SSH::Perl'sMakefile.PL. Behaviour verified identical onsystem perl 5.42.0 and jperl.
Additional, not in this PR
Also visible in the same
jcpan -t Net::SSH::Perlrun:ExtUtils::CBuilder/Alien::Buildinvokesjavacas a C compilerbecause
Config{cc} = 'javac'. Produces nonsense probe commands likejavac -Iperlonjava/.../CORE -c -DSILENT_NO_TAINT_SUPPORT ... mytest.c.We should either set
ccto something obviously absent so probesfail cleanly, or short-circuit
CBuilderprobes.Net::SSH::Perl's hard deps (CryptX,Crypt::Curve25519,Crypt::IDEA,String::CRC32) have no pure-Perl fallback, soinstalling the
.pmfiles "anyway" just produces crypticCan't load loadable object for module CryptXat test time. Worthfailing
Makefile.PLwhen hard runtime deps lack both an XS Javabridge and a pure-Perl fallback.
Test plan
makepasses (full unit-test suite including the newclosure_eval_string.t)../jcpan -t Alien::m4progresses past the Makefile parse error.Net::SSH::Perl'sMakefile.PLnow completes with"Configured! 75 files will be installed".
keys=NAME,Xon both systemperl 5.42 and jperl.
dev/cpan-reports/with the latest random-tester run.