Skip to content

vfs: support renaming implicit ZIP directories - #65752

Open
trivikr wants to merge 4 commits into
nodejs:mainfrom
trivikr:vfs-zip-provider-rename
Open

vfs: support renaming implicit ZIP directories#65752
trivikr wants to merge 4 commits into
nodejs:mainfrom
trivikr:vfs-zip-provider-rename

Conversation

@trivikr

@trivikr trivikr commented Sep 3, 2026

Copy link
Copy Markdown
Member

Fixes: #65751

Treat archive entry prefixes as directories when renaming with ZipProvider. Move all descendant entries to the new prefix for both asynchronous and synchronous operations.


Assisted-by: codex:gpt-5.6-sol

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. vfs Issues and PRs related to the virtual filesystem subsystem. labels Sep 3, 2026
@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 3, 2026
@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.61039% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.09%. Comparing base (2384897) to head (8e7df26).
⚠️ Report is 28 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/vfs/providers/ziparchive.js 89.61% 4 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65752      +/-   ##
==========================================
+ Coverage   90.07%   90.09%   +0.01%     
==========================================
  Files         769      769              
  Lines      261410   261461      +51     
  Branches    49627    49687      +60     
==========================================
+ Hits       235474   235562      +88     
+ Misses      16980    16928      -52     
- Partials     8956     8971      +15     
Files with missing lines Coverage Δ
lib/internal/vfs/providers/ziparchive.js 96.83% <89.61%> (-1.11%) ⬇️

... and 30 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jasnell

jasnell commented Sep 3, 2026

Copy link
Copy Markdown
Member

@pipobscure ... just want to get your review on this as well.

@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 3, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@trivikr trivikr added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Sep 4, 2026
Comment thread lib/internal/vfs/providers/ziparchive.js
@pipobscure

pipobscure commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

LGTM definitely a step in the right direction. Made me think of another edgecase where the provider could be better in a write/rename situation.

@pipobscure

Copy link
Copy Markdown
Contributor

In this context, we (probably I) should also take a look at #65373 to make sure the bahaviour is fully correct for ZipArchives.

Decision Needed: merge this and then fix the issues as this is already a significant improvement, or fix the latent rename issues in one go.

My €0.02 is to merge and continue improving, that's after all what --experimental is for.

@trivikr trivikr added the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 4, 2026
Treat archive entry prefixes as directories when renaming with
ZipProvider. Move all descendant entries to the new prefix for both
asynchronous and synchronous operations.

Signed-off-by: Kamat, Trivikram <[email protected]>
Assisted-by: codex:gpt-5.6-sol
@trivikr
trivikr force-pushed the vfs-zip-provider-rename branch from 1bb560a to 8e7df26 Compare September 4, 2026 13:18
@trivikr trivikr removed commit-queue PRs queued for automated landing through the Commit Queue. author ready PRs with CI started, the required approvals, and no outstanding review comments. labels Sep 4, 2026
@trivikr
trivikr requested a review from jasnell September 4, 2026 13:19
@trivikr

trivikr commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

Re-requested a review from @jasnell as a regression test was added based on feedback in #65752 (comment)

@trivikr trivikr added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs with CI started, the required approvals, and no outstanding review comments. labels Sep 5, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 5, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@trivikr
trivikr requested a review from pipobscure September 5, 2026 04:48

@pipobscure pipobscure left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. needs-ci PRs that need a full CI run. vfs Issues and PRs related to the virtual filesystem subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vfs: ZipProvider.rename() fails with ENOENT for implicit directories

4 participants