Skip to content

Fix crash when creating a patch for a package containing a broken symlink#613

Open
KAMRONBEK wants to merge 1 commit into
ds300:masterfrom
KAMRONBEK:fix-580-broken-symlink-crash
Open

Fix crash when creating a patch for a package containing a broken symlink#613
KAMRONBEK wants to merge 1 commit into
ds300:masterfrom
KAMRONBEK:fix-580-broken-symlink-crash

Conversation

@KAMRONBEK

Copy link
Copy Markdown

Problem

Running patch-package <pkg> crashes with Error: ENOENT: no such file or directory, stat '...' when the package directory contains a symlink whose target does not exist (e.g. react-native's hermes.framework/hermes -> Versions/Current/hermes after building RN from source). --exclude cannot help because the crash happens before ignored files are filtered.

Root cause

In makePatch, the user's package is copied into the tmp repo with copySync(realpathSync(packagePath), tmpRepoPackagePath) before removeIgnoredFiles runs. On patch-package 6.x (fs-extra 7) copySync itself crashed stat'ing the dangling symlink, as reported in the issue. On current master (fs-extra 10.1) copySync copies the dangling link into the tmp tree, and then removeIgnoredFiles crashes instead, because klaw-sync calls fs.statSync on every entry, which follows the symlink to its missing target. Either way the ENOENT surfaces before include/exclude filtering can apply.

Fix

Pass a filter to that copySync call which skips entries whose target doesn't exist (existsSync follows symlinks, so a dangling symlink returns false). Broken symlinks never enter the tmp tree, which fixes both the old fs-extra copy crash and the current klaw-sync crash. Valid symlinks are unaffected: they are still copied as before, and the existing no-symbolic-links integration test (which asserts the friendly "patch-package does not yet support symlinks" error) still passes unchanged. Since patch files cannot represent symlinks anyway (mode 120000 is rejected by the patch parser), skipping broken ones loses nothing.

Tests

  • New integration test integration-tests/broken-symlink (modeled on no-symbolic-links / happy-path-yarn): installs left-pad@1.3.0, creates a dangling symlink inside node_modules/left-pad, edits index.js, and runs patch-package left-pad. Snapshots assert the patch is created successfully and contains only the index.js changes (no symlink). Without the fix this test fails with the exact ENOENT stat error from the issue (verified by reverting the fix and rebuilding).
  • Existing no-symbolic-links integration test and the full src/ unit suite pass.

Fixes #580

@KAMRONBEK

Copy link
Copy Markdown
Author

@ds300 TL;DR: patch-package <pkg> crashes with ENOENT when the package contains a symlink to a non-existent target (#580), because copySync stats the dangling link before removeIgnoredFiles ever runs. Fix adds a copy filter that skips broken symlinks (valid symlinks unaffected), with an integration test that builds a fixture package containing a dangling link.

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.

patch package fails when file contains symbolic link to non existing file

1 participant