Skip to content

Conversation

@Flarna
Copy link
Member

@FlarnaFlarna commented Nov 29, 2024

This reverts commit 7f68e54.

fixes: #56002
Refs: #55623
Reopens: #54025

Additionally add a test to verify enumeration of windows named pipes works.


Seems normalization of path names of devices requires the trailing \ at least in some cases like debugger.

Not sure if revert is the best way to go, maybe there is a better fix. Happy to close this revert PR in case of a better proposal.

fyi @huseyinacacak-janea

Add a test to verify named pipe enumeration in windows works.
@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. path Issues and PRs related to the path subsystem. labels Nov 29, 2024
@codecov
Copy link

codecovbot commented Nov 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.99%. Comparing base (4ee87b8) to head (ccd6228).
Report is 33 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #56088 +/- ## ========================================== - Coverage 88.00% 87.99% -0.01%  ========================================== Files 656 656 Lines 189000 188989 -11 Branches 35995 35983 -12 ========================================== - Hits 166320 166301 -19 - Misses 15840 15853 +13 + Partials 6840 6835 -5 
Files with missing linesCoverage Δ
lib/path.js95.19% <100.00%> (-0.04%)⬇️
src/path.cc69.04% <ø> (ø)

... and 27 files with indirect coverage changes

@ShenHongFei
Copy link
Contributor

After testing locally, I can use vscode to debug normally after merging.

@lpinca
Copy link
Member

lpinca commented Nov 30, 2024

If/when this lands, #54025 should be reopened.

@aduh95aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. labels Nov 30, 2024
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 30, 2024
@nodejs-github-bot
Copy link
Collaborator

@huseyinacacak-janea
Copy link
Contributor

huseyinacacak-janea commented Dec 2, 2024

@Flarna thank you for tagging me. I'll open a PR to fix #56002 today or tomorrow. If the new PR lands, this revert will not be necessary.

@Flarna
Copy link
MemberAuthor

@huseyinacacak-janea fine for me. Please include the test I added here (second commit) or a similar one in your PR.

@FlarnaFlarna added the revert PRs that revert previously landed PRs. label Dec 2, 2024
@Flarna
Copy link
MemberAuthor

closing in favor of #56110

@FlarnaFlarna closed this Dec 5, 2024
@FlarnaFlarna deleted the revert-win-path-resolve branch December 5, 2024 17:38
nodejs-github-bot pushed a commit that referenced this pull request Dec 10, 2024
PR-URL: #56110Fixes: #56002 Refs: #55623 Refs: #56088 Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
aduh95 pushed a commit that referenced this pull request Dec 10, 2024
PR-URL: #56110Fixes: #56002 Refs: #55623 Refs: #56088 Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
ruyadorno pushed a commit that referenced this pull request Dec 20, 2024
PR-URL: #56110Fixes: #56002 Refs: #55623 Refs: #56088 Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
ruyadorno pushed a commit that referenced this pull request Jan 5, 2025
PR-URL: #56110Fixes: #56002 Refs: #55623 Refs: #56088 Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.c++Issues and PRs that require attention from people who are familiar with C++.commit-queue-rebaseAdd this label to allow the Commit Queue to land a PR in several commits.needs-ciPRs that need a full CI run.pathIssues and PRs related to the path subsystem.revertPRs that revert previously landed PRs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Node 23.2 fs lost the ability to enumerate named pipes on Windows

7 participants

@Flarna@ShenHongFei@lpinca@nodejs-github-bot@huseyinacacak-janea@juanarbol@aduh95