Skip to content

fix: warn on shallow-clone fallback at MAX_DEPTH#2

Draft
Koan-Bot wants to merge 9 commits into
atoomic:masterfrom
Koan-Bot:koan.atoomic/warn-shallow-clone-fallback
Draft

fix: warn on shallow-clone fallback at MAX_DEPTH#2
Koan-Bot wants to merge 9 commits into
atoomic:masterfrom
Koan-Bot:koan.atoomic/warn-shallow-clone-fallback

Conversation

@Koan-Bot

Copy link
Copy Markdown
Collaborator

What

Emit a Perl warning when Clone falls back to a shallow copy for non-array types that exceed the internal recursion depth limit.

Why

The rdepth > MAX_DEPTH fallback for non-AV/non-RV-to-AV types silently returns SvREFCNT_inc(ref) — a shared reference, not a clone. Users have no way to detect that their deeply nested hashrefs/objects are incompletely cloned, which can cause subtle mutation bugs.

How

  • Added Perl_warn() before the SvREFCNT_inc return in the MAX_DEPTH guard (Clone.xs line 272)
  • Updated LIMITATIONS section in Clone.pm with accurate documentation of the platform-dependent limits, the iterative arrayref path, and the shallow-copy fallback behavior
  • Added 3 tests to t/10-deep_recursion.t that build a deep hashref chain exceeding MAX_DEPTH and verify the warning is emitted

Testing

  • Full test suite passes (269 tests, 20 files) on macOS with Perl 5.42.0
  • New tests specifically exercise the hashref shallow-copy path and capture the warning via $SIG{__WARN__}

🤖 Generated with Claude Code

Koan-Bot and others added 9 commits March 24, 2026 20:12
Set COPYFILE_DISABLE=1 before tar in the dist target so that macOS
extended attributes (com.apple.provenance, com.apple.lastuseddate, etc.)
are not embedded in the CPAN tarball. These attributes cause tar errors
on non-Apple systems (NetBSD, Linux) that do not understand them.

COPYFILE_DISABLE=1 is macOS-specific and is a no-op on Linux/Windows.

Fixes garu#85

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add AI_POLICY.md describing how AI tools are used in the project's
development workflow. The policy establishes the principle that AI
assists but humans decide — maintainers remain responsible for every
review, merge, and release.

Also update MANIFEST to include the new file and MANIFEST.SKIP to
exclude dist tarballs (Clone-*.gz) and the local/ directory.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fix garu#82: Cloned DBI handles lack XS-level magic, causing DBI's
DESTROY to dump pages of SV internals to STDERR. Circular refs
inside the cloned handle tree also cause SEGVs during global
destruction. The previous test 16 ($cloned->prepare) segfaulted
on most platforms.

Fix by running DBI clone tests in forked subprocesses:
- Child redirects STDERR to /dev/null (suppresses all DBI noise)
- Child uses POSIX::_exit(0) to skip destructors (avoids SEGV)
- Child communicates results via pipe as key=value pairs
- Parent relays results to Test::More assertions

Also replace test 16's unsafe $cloned->prepare() call with a safe
Scalar::Util::reftype structural check, and add $sth->finish
before $dbh->disconnect to eliminate the "disconnect invalidates
active statement handle" warning.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Refs garu#82: The DBI clone tests use fork() + pipe to isolate
cloned DBI handle destruction (which causes SEGVs and STDERR
noise). Strawberry Perl on Windows emulates fork() via ithreads
which does not reliably support pipe+fork, causing "planned 18,
ran 12" failures in CI.

Add _can_fork() guard that returns false on MSWin32 and skips
the 6 DBI tests with a clear message instead of dying with a
bad plan count.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The file defines its own ok() sub (line 58) that returns undef
instead of the test result. This caused the `or warn` on line 133
to always fire, printing "(?^:abcdefg) vs (?^:abcdefg)" to STDERR
even when the test passed. Remove the dead diagnostic.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Update version timestamps in Changes to match the actual git tag
commit times for v0.32+. Normalize formatting to consistent
single-space separators, remove double spaces and tab indentation,
and convert old non-ISO dates (0.06, 0.01) to YYYY-MM-DD format.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
COPYFILE_DISABLE=1 (added for issue garu#85) prevents macOS tar from
embedding ._AppleDouble sidecar files, but does not prevent it from
writing PAX extended headers for xattrs that already exist on the
files themselves (e.g. com.apple.provenance, which macOS sets on
every file in a git-cloned repo).

Add a PREOP step that runs 'xattr -cr $(DISTVNAME)' to strip all
extended attributes from the distribution directory before tar is
invoked. xattr(1) is macOS-specific; the '|| true' makes it a
silent no-op on Linux and Windows.

Fixes garu#91

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When rdepth exceeds MAX_DEPTH, Clone has an iterative fallback for
arrayrefs but silently returns a shallow copy (SvREFCNT_inc) for all
other SV types. This makes incomplete clones invisible to users.

Add a Perl warn() so the shallow-copy fallback is detectable. Update
the LIMITATIONS section in Clone.pm to document this behavior and
suggest workarounds. Add tests for deep hashref chains that verify
the warning is emitted.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

2 participants