Skip to content

fix(MakeMaker): use :: rules so Alien::Build postambles work#534

Merged
fglock merged 2 commits intomasterfrom
fix/makemaker-double-colon-rules
Apr 22, 2026
Merged

fix(MakeMaker): use :: rules so Alien::Build postambles work#534
fglock merged 2 commits intomasterfrom
fix/makemaker-double-colon-rules

Conversation

@fglock
Copy link
Copy Markdown
Owner

@fglock fglock commented Apr 22, 2026

Summary

Fixes two distinct bugs exposed by jcpan -t Net::SSH::Perl.

1. Mixed : / :: rules in generated Makefile

make for Alien::m4 and Alien::GMP (deps of Math::GMP, required
by Net::SSH::Perl) died with:

Makefile:60: *** target file `realclean' has both : and :: entries.  Stop.

Root cause: our PerlOnJava MakeMaker template emitted clean:,
realclean:, distclean:, all:, pm_to_blib:, pure_all:, test:,
install: with single-colon rules. Postambles from Alien::Build
(MY::postamble) and File::ShareDir::Install extend the same targets
with double-colon rules — which GNU make rejects when mixed. Real
ExtUtils::MakeMaker uses :: on these targets for exactly this
reason. Switched our template to match.

2. Closure-capture bug with eval STRING in a sibling sub

Minimal reproducer — both system perl 5.42 and jperl should print
keys=NAME,X:

use strict; use warnings;
foo();                                  # forward call, does eval STRING
my %WM = ( NAME => 'Foo', X => 'Y' );   # declared AFTER the call
do_it();
sub do_it { print "keys=", join(",", sort keys %WM), "\n"; }
sub foo   { eval("1;"); }

Before this PR jperl printed keys= (empty). This caused
Net::SSH::Perl's modulino Makefile.PL to die with NAME is required
whenever Digest::BubbleBabble was already installed — the
have_module() path then runs eval "use ..." BEFORE the file-scope
my %WriteMakefile = (...) assignment.

Mechanism. Named/package subs 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 passes it as a constructor arg to the named sub's
    generated class.
  • At runtime, handleMyOperator calls PersistentVariable.retrieveBeginHash
    which takes ownership of the same object out of the alias map — so
    the my variable and the named sub's capture point to the same
    RuntimeHash.

RuntimeCode.evalStringHelper also populates that same alias map at
entry (so BEGIN blocks inside the eval can see outer lexicals) and
unconditionally removes the entries in its finally block. When a
sibling sub ran eval STRING before the outer my had 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 record 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 there, leave it
alone; its owner — the outer my at runtime, or an enclosing eval —
will clean it up. Both the JVM path and the BytecodeInterpreter path in
evalStringHelper had the same bug and get the same fix.

Regression test added at src/test/resources/unit/closure_eval_string.t
covering hash, array and scalar closures with the exact shape from
Net::SSH::Perl's Makefile.PL. Behaviour verified identical on
system perl 5.42.0 and jperl.

Additional, not in this PR

Also visible in the same jcpan -t Net::SSH::Perl run:

  • ExtUtils::CBuilder / Alien::Build invokes javac as a C compiler
    because Config{cc} = 'javac'. Produces nonsense probe commands like
    javac -Iperlonjava/.../CORE -c -DSILENT_NO_TAINT_SUPPORT ... mytest.c.
    We should either set cc to something obviously absent so probes
    fail cleanly, or short-circuit CBuilder probes.
  • XS-only hard deps reported "missing" but install continues.
    Net::SSH::Perl's hard deps (CryptX, Crypt::Curve25519,
    Crypt::IDEA, String::CRC32) have no pure-Perl fallback, so
    installing the .pm files "anyway" just produces cryptic
    Can't load loadable object for module CryptX at test time. Worth
    failing Makefile.PL when hard runtime deps lack both an XS Java
    bridge and a pure-Perl fallback.

Test plan

  • make passes (full unit-test suite including the new
    closure_eval_string.t).
  • ./jcpan -t Alien::m4 progresses past the Makefile parse error.
  • Net::SSH::Perl's Makefile.PL now completes with
    "Configured! 75 files will be installed".
  • Minimal closure repros produce keys=NAME,X on both system
    perl 5.42 and jperl.
  • Refreshed dev/cpan-reports/ with the latest random-tester run.

fglock and others added 2 commits April 22, 2026 09:55
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>
@fglock fglock merged commit e228c25 into master Apr 22, 2026
2 checks passed
@fglock fglock deleted the fix/makemaker-double-colon-rules branch April 22, 2026 09:19
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.

1 participant