Skip to content

Commit b7bea84

Browse files
fglockcodex
andcommitted
fix: complete CPANPLUS runtime support
Fix the remaining CPANPLUS blockers after Archive::Extract by handling tied lexical scalar cleanup, strict bareword coderef arrow calls, version numification, and stale blib/lib sources in PerlOnJava MakeMaker reruns. Add focused regressions and update the CPANPLUS progress note with the passing test result and follow-up warning. Generated with [Codex](https://openai.com/codex) Co-Authored-By: Codex <codex@openai.com>
1 parent 58277c7 commit b7bea84

12 files changed

Lines changed: 269 additions & 31 deletions

File tree

dev/modules/cpanplus.md

Lines changed: 36 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,21 @@
22

33
## Current Status
44

5-
`CPANPLUS::Config` now loads under PerlOnJava after fixing loop-control parsing for bare labels that share a name with an imported constant. This unblocks the upstream `Makefile.PL` path that declares CPANPLUS' dynamic prerequisites.
5+
`CPANPLUS::Config` loads under PerlOnJava after fixing loop-control parsing for bare labels that share a name with an imported constant. This unblocks the upstream `Makefile.PL` path that declares CPANPLUS' dynamic prerequisites.
66

7-
The current `./jcpan -t CPANPLUS` run now gets through dependency discovery and the `Archive::Extract` dependency. It is blocked by `Object::Accessor`:
7+
`./jcpan -t CPANPLUS` now passes with CPANPLUS 0.9916:
88

99
```text
10-
t/03_Object-Accessor-local.t line 49
11-
got: 't/03_Object-Accessor-local.t'
12-
expected: '<process pid>'
10+
Files=20, Tests=1751, Result: PASS
11+
EXIT: 0
1312
```
1413

15-
Because `Object::Accessor` fails its dependency test, it is not installed, and CPANPLUS' own test files then fail at compile time with:
14+
The run also verifies the dependency chain that previously blocked CPANPLUS: `Archive::Extract`, `Object::Accessor`, `File::Fetch`, `Log::Message`, `Module::Loaded`, `Package::Constants`, `Log::Message::Simple`, and `Term::UI`.
15+
16+
The only observed remaining issue is a non-fatal warning during CPANPLUS' own suite:
1617

1718
```text
18-
Base class package "Object::Accessor" is empty.
19+
Use of uninitialized value in addition (+) at jar:PERL5LIB/File/Copy.pm line 303.
1920
```
2021

2122
## Symptom
@@ -39,7 +40,7 @@ WriteMakefile(
3940

4041
That fallback had no `PREREQ_PM`, so `Log::Message` was never scheduled.
4142

42-
## Root Cause Fixed
43+
## Root Causes Fixed
4344

4445
`CPANPLUS::Config` imports constants from `CPANPLUS::Internals::Constants`, including:
4546

@@ -59,6 +60,16 @@ Perl treats bare `last BIN` as a literal loop label, even when a constant sub na
5960

6061
The fix keeps a standalone bare identifier immediately after `last`, `next`, or `redo` as a literal loop label. Parenthesized and expression labels still go through the dynamic-label path.
6162

63+
`Archive::Extract` then failed because PerlOnJava's bundled `Archive::Zip` member objects exposed `_name` but CPAN `Archive::Extract` expects the compatibility hash key `fileName`, and it passes member objects back into `extractMember`. The bundled module now exposes both names and accepts member objects.
64+
65+
`Object::Accessor` then exposed tied lexical cleanup differences. A tied lexical scalar whose tie object should be destroyed at scope exit was being cleaned through the generic scalar path, so `DESTROY` did not fire at the Perl-compatible time. Tied scalar cleanup is now explicit and idempotent, including the closure-capture case.
66+
67+
CPANPLUS' tests also use strict bareword coderef calls in forms such as `BUILD_PL->(...)`, `-e BUILD_PL->(...)`, and `stat MAKEFILE->(...)`. PerlOnJava now treats a bareword to the left of `->(...)` as a sub call returning a coderef, and reassociates filetest and `stat`/`lstat` unary operators so those expressions match Perl's parse.
68+
69+
Version checks then exposed decimal vs dotted-version numification differences. `version->parse("v1.5")->numify` now returns `1.005000`, while decimal versions such as `1.5` and `1.2345` retain decimal-padding semantics.
70+
71+
The final CPANPLUS blocker was in the generated Makefile for a dummy `Foo-Bar` distribution. When `Makefile.PL` was rerun after `blib/lib` already existed, PerlOnJava's MakeMaker treated staged `blib/lib/*.pm` files as source files. Its generated `pm_to_blib` target could delete `blib/lib/Foo/Bar.pm` and then try to copy that same path back to itself. MakeMaker now prefers real `lib/` sources over stale `blib/` entries and does not stage already-staged files back into `blib`.
72+
6273
## Completed Work
6374

6475
- Fixed loop-control parsing in [`OperatorParser.java`](../../src/main/java/org/perlonjava/frontend/parser/OperatorParser.java).
@@ -67,30 +78,37 @@ The fix keeps a standalone bare identifier immediately after `last`, `next`, or
6778
- Fixed the `Archive::Extract` dependency failure by making PerlOnJava's bundled `Archive::Zip` expose the CPAN-compatible member hash field `fileName` and accept member objects in `extractMember`.
6879
- Added regression coverage in [`archive_zip_members_matching_qr.t`](../../src/test/resources/unit/archive_zip_members_matching_qr.t) for direct member hash access and object extraction.
6980
- Verified `Archive::Extract` 0.88 upstream suite passes: `Files=1, Tests=1795, Result: PASS`.
70-
- Verified `./jcpan -t CPANPLUS` now gets past `Archive::Extract`; `File::Fetch`, `Log::Message`, `Module::Loaded`, `Package::Constants`, `Log::Message::Simple`, and `Term::UI` pass in the observed run.
81+
- Fixed tied scalar scope cleanup so `Object::Accessor` local attribute restore passes.
82+
- Added regression coverage in [`tie_scalar.t`](../../src/test/resources/unit/tie_scalar.t) for tied lexical `DESTROY` at scope exit and deferred destruction while a tie object is still referenced.
83+
- Fixed strict bareword coderef arrow parsing and filetest/stat reassociation for CPANPLUS' `BUILD_PL->(...)` and `MAKEFILE->(...)` patterns.
84+
- Added regression coverage in [`subroutine.t`](../../src/test/resources/unit/subroutine.t) for strict bareword coderef calls and unary-operator reassociation.
85+
- Fixed `version` numification/normalization for dotted `v` versions and decimal versions.
86+
- Added regression coverage in [`version_numify.t`](../../src/test/resources/unit/version_numify.t).
87+
- Fixed PerlOnJava MakeMaker reruns after `blib/lib` exists so stale staged files are not copied onto themselves.
88+
- Added regression coverage in [`makemaker_stale_blib_source.t`](../../src/test/resources/unit/makemaker_stale_blib_source.t).
89+
- Verified `Object::Accessor` upstream suite passes: `Files=7, Tests=155, Result: PASS`.
90+
- Verified `./jcpan -t CPANPLUS` passes: `Files=20, Tests=1751, Result: PASS`.
7191
- Verified `make` passes.
7292

7393
## Acceptance
7494

7595
```bash
7696
timeout 60 ./jperl src/test/resources/unit/loop_label_bareword_constant.t
7797
timeout 60 ./jperl -I$CPANPLUS_DIR/inc/bundle -I$CPANPLUS_DIR/lib -e 'require CPANPLUS::Config; print "ok\n"'
78-
timeout 600 ./jcpan -t CPANPLUS
98+
timeout 1200 ./jcpan -t CPANPLUS
7999
make
80100
```
81101

82102
Before running the full `jcpan -t CPANPLUS` acceptance, make sure no local CPANPLUS distropref is masking the dependency path. A previous investigation generated `/Users/fglock/.perlonjava/cpan/prefs/CPANPLUS.yml`; move it aside or use an isolated CPAN home before judging dependency discovery.
83103

84104
## Next Steps
85105

86-
1. Reduce `Object::Accessor`'s `t/03_Object-Accessor-local.t` failure. The failing construct is scoped attribute restore via `$Object->$Acc( $0 => \my $temp )`; PerlOnJava restores `$0` instead of the previous `$$` value when `$temp` leaves scope.
87-
2. Compare the reduced `Object::Accessor` case with system Perl and both PerlOnJava backends. Likely areas are `local`/scope cleanup, weak/DESTROY-like cleanup behavior around temporary references, or assignment to lexical references passed as arguments.
88-
3. Add a minimal unit regression for the runtime behavior before changing CPAN module prefs or overlays.
89-
4. Re-run `timeout 600 ./jcpan -t Object::Accessor` after the runtime fix, then re-run `timeout 1200 ./jcpan -t CPANPLUS`.
90-
5. Inspect the generated `Makefile`, `MYMETA.yml`, and CPAN log to verify `PREREQ_PM` includes the CPANPLUS runtime dependency set from `CPANPLUS::Selfupdate`.
91-
6. When CPANPLUS tests are passing or have documented non-runtime blockers, update this document with the final test count and remaining skips/failures.
106+
1. Reduce the non-fatal `File::Copy.pm line 303` warning from CPANPLUS' own suite. It appears to come from numeric conversion of `$!` or `$^E` after a failed move fallback, but it does not currently fail CPANPLUS.
107+
2. Re-run `timeout 1200 ./jcpan -t CPANPLUS` from a fresh or isolated CPAN home before merging if cache independence is required.
108+
3. Audit whether MakeMaker still needs to discover installable files from `blib/lib`; if it does, keep the new no-self-staging behavior as the regression guard.
109+
4. Keep CPANPLUS as a regression target when touching `Archive::Extract`, `Object::Accessor`, `ExtUtils::MakeMaker`, parser precedence, or `version`.
92110

93111
## Open Questions
94112

95-
- Is the `Object::Accessor` failure a generic PerlOnJava scope-cleanup bug or specific to the module's accessor implementation?
96-
- After `Object::Accessor` installs, do CPANPLUS' own tests expose runtime failures beyond dependency loading?
113+
- Does the `File::Copy` warning reveal a generic `$!` / `$^E` numeric conversion difference when the error variables are unset?
114+
- Are there CPAN distributions that intentionally rely on MakeMaker installing files that only exist under `blib/lib` after configure/build, and should that path be modeled more explicitly?

src/main/java/org/perlonjava/frontend/parser/ParseInfix.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,15 @@ public static Node parseInfixOperation(Parser parser, Node left, int precedence)
191191
case "(":
192192
TokenUtils.consume(parser);
193193
right = new ListNode(ListParser.parseList(parser, ")", 0), parser.tokenIndex);
194+
if (left instanceof OperatorNode op && isArrowReassociatingUnaryOperator(op.operator)) {
195+
Node arrowLeft = coderefArrowLeft(op.operand);
196+
BinaryOperatorNode arrowCall = new BinaryOperatorNode(token.text,
197+
arrowLeft,
198+
right,
199+
parser.tokenIndex);
200+
return new OperatorNode(op.operator, arrowCall, op.getIndex());
201+
}
202+
left = coderefArrowLeft(left);
194203
return new BinaryOperatorNode(token.text, left, right, parser.tokenIndex);
195204
case "**":
196205
// Postfix GLOB dereference: $ref->**
@@ -630,4 +639,23 @@ private static void checkMyInFalseConditional(String operator, Node left, Node r
630639
}
631640
}
632641
}
642+
643+
private static Node coderefArrowLeft(Node left) {
644+
if (left instanceof IdentifierNode) {
645+
OperatorNode subRef = new OperatorNode("&", left, left.getIndex());
646+
return new BinaryOperatorNode("(",
647+
subRef,
648+
new ListNode(left.getIndex()),
649+
left.getIndex());
650+
}
651+
return left;
652+
}
653+
654+
private static boolean isArrowReassociatingUnaryOperator(String operator) {
655+
if (operator == null) return false;
656+
if (operator.equals("stat") || operator.equals("lstat")) return true;
657+
return operator.length() == 2
658+
&& operator.charAt(0) == '-'
659+
&& "rwxoRWXOezsfdlpSbctugkTBMAC".indexOf(operator.charAt(1)) >= 0;
660+
}
633661
}

src/main/java/org/perlonjava/runtime/operators/VersionHelper.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,9 @@ public static String normalizeVersion(RuntimeScalar wantVersion) {
336336
if (minor.length() > 3) {
337337
minor = minor.substring(0, 3);
338338
}
339+
while (patch.length() < 3) {
340+
patch = patch + "0";
341+
}
339342
if (patch.length() > 3) {
340343
patch = patch.substring(0, 3);
341344
}

src/main/java/org/perlonjava/runtime/perlmodule/Version.java

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -173,12 +173,6 @@ else if (versionStr.type == VSTRING) {
173173
// Perl 5 treats these as v-strings with is_qv=true
174174
isVString = true;
175175
version = "v" + version;
176-
} else if (dotCount == 1 && version.length() < 4) {
177-
// If exactly one dot and short, prepend "v" for internal processing
178-
// but keep the original for stringify() and qv flag
179-
version = "v" + version;
180-
// Note: originalVersionStr stays as the user's input (e.g., "1.0")
181-
// Note: isVString remains false - this is a decimal version
182176
}
183177
}
184178

@@ -192,7 +186,7 @@ else if (versionStr.type == VSTRING) {
192186
versionObj.put("qv", getScalarBoolean(isVString));
193187

194188
// Parse components
195-
String normalized = VersionHelper.normalizeVersion(new RuntimeScalar(version));
189+
String normalized = normalizeDottedVersion(version);
196190
versionObj.put("version", new RuntimeScalar(normalized));
197191
} else {
198192
// Decimal format
@@ -216,6 +210,18 @@ else if (versionStr.type == VSTRING) {
216210
return blessed.getList();
217211
}
218212

213+
private static String normalizeDottedVersion(String version) {
214+
String normalized = version.startsWith("v") ? version.substring(1) : version;
215+
normalized = normalized.replace("_", "");
216+
String[] parts = normalized.split("\\.");
217+
StringBuilder dotted = new StringBuilder();
218+
for (int i = 0; i < parts.length; i++) {
219+
if (i > 0) dotted.append(".");
220+
dotted.append(Integer.parseInt(parts[i]));
221+
}
222+
return dotted.toString();
223+
}
224+
219225
/**
220226
* Creates a dotted-decimal version object.
221227
* This is a method that expects to be called as version->declare()
@@ -263,6 +269,11 @@ public static RuntimeList numify(RuntimeArray args, int ctx) {
263269
RuntimeHash versionObj = self.hashDeref();
264270

265271
String version = versionObj.get("version").toString();
272+
if (!versionObj.get("qv").getBoolean()) {
273+
String original = versionObj.get("original").toString().replace("_", "");
274+
return new RuntimeScalar(numifyDecimalVersion(original)).getList();
275+
}
276+
266277
String[] parts = version.split("\\.");
267278

268279
if (parts.length == 0) {
@@ -285,6 +296,32 @@ public static RuntimeList numify(RuntimeArray args, int ctx) {
285296
return new RuntimeScalar(numified.toString()).getList();
286297
}
287298

299+
private static String numifyDecimalVersion(String version) {
300+
String clean = version.trim();
301+
if (clean.startsWith("v")) {
302+
clean = clean.substring(1);
303+
}
304+
int dot = clean.indexOf('.');
305+
if (dot < 0) {
306+
return clean + ".000";
307+
}
308+
309+
String major = clean.substring(0, dot);
310+
String decimal = clean.substring(dot + 1);
311+
if (major.isEmpty()) {
312+
major = "0";
313+
}
314+
if (decimal.isEmpty()) {
315+
decimal = "0";
316+
}
317+
int width = ((decimal.length() + 2) / 3) * 3;
318+
StringBuilder padded = new StringBuilder(decimal);
319+
while (padded.length() < width) {
320+
padded.append("0");
321+
}
322+
return major + "." + padded;
323+
}
324+
288325
/**
289326
* Returns the normalized dotted-decimal form with leading v.
290327
*/

src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeCode.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,11 @@ public void releaseCaptures() {
746746
// Sub::Defer/Moo's %DEFERRED and %QUOTED weak ref tables.
747747
// The JVM GC handles truly-dead unblessed containers eventually.
748748
if (s.scopeExited) {
749+
if (s.type == RuntimeScalarType.TIED_SCALAR
750+
&& s.value instanceof TiedVariableBase tiedVariable) {
751+
tiedVariable.releaseTiedObject();
752+
continue;
753+
}
749754
if ((s.type & RuntimeScalarType.REFERENCE_BIT) != 0
750755
&& s.value instanceof RuntimeBase rb
751756
&& rb.blessId != 0) {

src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeScalar.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2712,7 +2712,8 @@ public static void scopeExitCleanup(RuntimeScalar scalar) {
27122712
// - captureCount=0 → capture handling branch not taken
27132713
// - ioOwner=false → IO fd recycling branch not taken
27142714
if (!scalar.refCountOwned && scalar.captureCount == 0 && !scalar.ioOwner
2715-
&& !scalar.ownsScalarReferenceContents) {
2715+
&& !scalar.ownsScalarReferenceContents
2716+
&& scalar.type != RuntimeScalarType.TIED_SCALAR) {
27162717
// Special case: CODE refs with unreleased captures that were never
27172718
// stored via set() (e.g., anonymous subs passed directly as arguments).
27182719
// These have refCount=0 (from makeCodeObject) and refCountOwned=false
@@ -2812,6 +2813,12 @@ public static void scopeExitCleanup(RuntimeScalar scalar) {
28122813
// Captures are properly released when the CODE ref is overwritten
28132814
// (via setLarge) or undef'd (via undefine).
28142815

2816+
if (scalar.type == RuntimeScalarType.TIED_SCALAR
2817+
&& scalar.value instanceof TiedVariableBase tiedVariable) {
2818+
tiedVariable.releaseTiedObject();
2819+
return;
2820+
}
2821+
28152822
// Existing: IO fd recycling for anonymous filehandle globs
28162823
if (scalar.ioOwner && scalar.type == GLOBREFERENCE
28172824
&& scalar.value instanceof RuntimeGlob glob

src/main/java/org/perlonjava/runtime/runtimetypes/TiedVariableBase.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ public abstract class TiedVariableBase extends RuntimeBaseProxy {
2525
*/
2626
protected final String tiedPackage;
2727

28+
private boolean tiedObjectReleased = false;
29+
2830
/**
2931
* Creates a new TiedVariableBase instance.
3032
*
@@ -207,6 +209,8 @@ public String getTiedPackage() {
207209
* Called by untie() after UNTIE has been dispatched.
208210
*/
209211
public void releaseTiedObject() {
212+
if (tiedObjectReleased) return;
213+
tiedObjectReleased = true;
210214
if ((self.type & RuntimeScalarType.REFERENCE_BIT) != 0
211215
&& self.value instanceof RuntimeBase base) {
212216
if (base.refCount > 0 && --base.refCount == 0) {
@@ -216,4 +220,3 @@ public void releaseTiedObject() {
216220
}
217221
}
218222
}
219-

src/main/perl/lib/ExtUtils/MakeMaker.pm

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -282,26 +282,32 @@ sub _install_pure_perl {
282282
# Some modules like Image::ExifTool use .pl files loaded via require,
283283
# .dat files for data (e.g., Geolocation.dat), and Mozilla::CA uses .pem
284284
my $installable_re = qr/\.(?:pm|pl|pod|dat|json|ya?ml|xml|txt|cfg|conf|ini|pem)$/i;
285+
my %pm_rel_seen;
285286
if (-d 'lib') {
286287
find({
287288
wanted => sub {
288289
return unless -f && /$installable_re/;
289290
my $src = $File::Find::name;
290291
(my $rel = $src) =~ s{^lib/}{};
291292
$pm{$src} = File::Spec->catfile($INSTALL_BASE, $rel);
293+
$pm_rel_seen{$rel} = 1;
292294
},
293295
no_chdir => 1,
294296
}, 'lib');
295297
}
296298

297-
# Also check for blib/lib (after a build)
299+
# Also check for blib/lib (after a build). Prefer real sources
300+
# under lib/ when both exist; rerunning Makefile.PL after a build
301+
# must not turn blib/lib files into self-copying pm_to_blib sources.
298302
if (-d 'blib/lib') {
299303
find({
300304
wanted => sub {
301305
return unless -f && /$installable_re/;
302306
my $src = $File::Find::name;
303307
(my $rel = $src) =~ s{^blib/lib/}{};
308+
return if $pm_rel_seen{$rel};
304309
$pm{$src} = File::Spec->catfile($INSTALL_BASE, $rel);
310+
$pm_rel_seen{$rel} = 1;
305311
},
306312
no_chdir => 1,
307313
}, 'blib/lib');
@@ -670,7 +676,7 @@ sub _create_install_makefile {
670676
# Flat layout: compute from dest path relative to INSTALL_BASE
671677
($blib_rel = $dest) =~ s{^\Q$INSTALL_BASE\E/?}{};
672678
}
673-
if ($blib_rel) {
679+
if ($blib_rel && $src !~ m{^blib/lib/}) {
674680
my $blib_dest = "\$(INST_LIB)/$blib_rel";
675681
my $blib_dir = dirname($blib_dest);
676682
unless ($blib_dirs_seen{$blib_dir}++) {
@@ -716,7 +722,7 @@ sub _create_install_makefile {
716722
my $blib_script_cmds_str = join("\n", @blib_script_cmds) || "\t\@true";
717723
my $file_count = scalar(keys %$pm) + scalar(keys %$scripts);
718724

719-
my $pm_deps_str = join(' ', sort keys %$pm);
725+
my $pm_deps_str = join(' ', sort grep { $_ !~ m{^blib/lib/} } keys %$pm);
720726
$pm_deps_str = " $pm_deps_str" if length $pm_deps_str;
721727

722728
# Make pm_to_blib target conditional - if no .pm files, make it a no-op

0 commit comments

Comments
 (0)