feat(YAML): vendor real YAML.pm 1.31 instead of YAML::PP shim#596
Closed
feat(YAML): vendor real YAML.pm 1.31 instead of YAML::PP shim#596
Conversation
Previously src/main/perl/lib/YAML.pm was a 12-line wrapper around
YAML::PP that pinned $VERSION = '1.31' but produced YAML 1.2 output
(`null` instead of `~`, JSON-style booleans, snakeyaml error
messages). This caused 5/12 failures in the YAML-1.31 distribution's
own t/2-scalars.t, and would never satisfy YAML.pm-1.x-specific
assertions in the rest of that test suite.
Vendor the real pure-Perl YAML.pm 1.31 distribution into
src/main/perl/lib/:
- YAML.pm (replaced)
- YAML/{Any,Dumper,Error,Loader,Marshall,Mo,Node,Tag,Types}.pm
- YAML/Loader/Base.pm
- YAML/Dumper/Base.pm
YAML::PP (Java-backed via YAMLPP.java + snakeyaml-engine) is kept
unchanged for users who specifically `use YAML::PP;` and for the
yaml_pp.t unit test. Storable.java already uses snakeyaml-engine
directly and is unaffected.
Verified on the YAML-1.31 distribution itself:
- t/000-compile-modules.t: 12/12 pass (all YAML modules load)
- t/2-scalars.t: 12/12 pass (was 7/12)
Documented in dev/modules/yaml_any_fixes.md.
Generated with [Devin](https://cli.devin.ai/docs)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Drop seven tests from the upstream YAML-1.31 distribution into src/test/resources/module/YAML/t/ so `make test-bundled-modules` exercises the freshly-vendored real YAML.pm against its own test suite: - 2-scalars.t — full scalar round-trip + error matching - dump-synopsis.t — dump example - issue-149.t — load without trailing newline - issue-69.t — inline collection trailing whitespace - numify.t — B::svref_2object numification - roundtrip.t — Test::Deep round-trip on `=` key - rt-90593.t — RT bug regression Verified via `./gradlew testModule --rerun-tasks`: all 7 YAML tests pass. The two pre-existing failures in the suite (Net-SSLeay/t/local/33_x509_create_cert.t and Text-CSV/t/55_combi.t) are unrelated to YAML and present on master. The remaining tests in the upstream YAML distribution (basic-tests.t, bugs-*, dump-*, load-*, marshall.t, etc.) all use `Test::Base` and are blocked on the Spiffy source-filter leak documented as "Bug 1" in dev/modules/yaml_any_fixes.md. They will land alongside that fix. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
7 tasks
Owner
Author
|
Subsumed by #597 — combined into a single PR per request. |
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
1 participant
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
Replace the 12-line
YAML.pmshim (which delegated toYAML::PPand impersonated$YAML::VERSION = '1.31') with the actual pure-Perl YAML.pm 1.31 distribution.YAML::PPitself (Java-backed viaYAMLPP.java+snakeyaml-engine) is kept unchanged as a separate bundled module —Storable.javaalready usessnakeyaml-enginedirectly and is unaffected.This fixes the long-standing semantic mismatches that caused
t/2-scalars.tto fail 5 of 12 sub-tests and made any test asserting YAML.pm-1.x output (~for undef, plain"true"/"false",Can't parse single/Can't parse doubleerrors) impossible to satisfy.What changed
src/main/perl/lib/YAML.pm— replaced with real YAML.pm 1.31src/main/perl/lib/YAML/{Any,Dumper,Error,Loader,Marshall,Mo,Node,Tag,Types}.pm— addedsrc/main/perl/lib/YAML/{Loader,Dumper}/Base.pm— addedsrc/test/resources/module/YAML/t/— 7 upstream tests added to bundled-module suitedev/modules/yaml_any_fixes.md— full investigation, design rationale, resultsWhat did not change
src/main/java/org/perlonjava/runtime/perlmodule/YAMLPP.java— kept (use YAML::PP;is its own bundled module with its own unit test, docs, and CPAN consumers).src/main/perl/lib/YAML/PP.pm— kept.src/main/perl/lib/Storable.pm— unaffected.Test plan
make— clean (no PerlOnJava unit-test regressions)../gradlew testModule --rerun-tasks— all 7 new YAML tests pass:module/YAML/t/2-scalars.t(12/12 sub-tests; was 7/12 with the shim)module/YAML/t/dump-synopsis.tmodule/YAML/t/issue-149.tmodule/YAML/t/issue-69.tmodule/YAML/t/numify.t(usesB)module/YAML/t/roundtrip.t(usesTest::Deep)module/YAML/t/rt-90593.t./jperl -MYAML -e '...'and./jperl -MYAML::PP -e '...'both work side-by-side.Net-SSLeay/t/local/33_x509_create_cert.t,Text-CSV/t/55_combi.t) reproduce on master — unrelated to this PR (no diff in those module dirs).Why not also include the rest of the YAML test suite?
35 of the 54 upstream
t/*.tfiles useTest::Base, which dies at compile time withsyntax error at .../Test/Base.pm line 53, near "=> [qw"because of a separate Spiffy source-filter leak inexecutePerlCode(documented as Bug 1 indev/modules/yaml_any_fixes.md). They will be added in a follow-up PR alongside that fix.Generated with Devin