Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tools: fix vcbuild test when path contain spaces #56481

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

stduhpf
Copy link

@stduhpf stduhpf commented Jan 5, 2025

No description provided.

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. windows Issues and PRs related to the Windows platform. labels Jan 5, 2025
@stduhpf stduhpf force-pushed the vcbuild-test-spaces branch from 64b16c5 to 7a4087c Compare January 5, 2025 16:58
@stduhpf stduhpf changed the title tools: fix cbuild test when path contain spaces tools: fix vcbuild test when path contain spaces Jan 5, 2025
@aduh95 aduh95 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. labels Jan 5, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 5, 2025
@nodejs-github-bot
Copy link
Collaborator

vcbuild.bat Outdated Show resolved Hide resolved
Co-authored-by: Luigi Pinca <luigipinca@gmail.com>
@lpinca lpinca added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. request-ci Add this label to start a Jenkins CI on a PR. labels Jan 5, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 5, 2025
@nodejs-github-bot
Copy link
Collaborator

@aduh95 aduh95 removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Jan 5, 2025
@aduh95
Copy link
Contributor

aduh95 commented Jan 5, 2025

Related failure on Windows:

> "Release\node.exe" deps\npm\bin\npm-cli.js ci 
The system cannot find the path specified.

Dropping/Reverting c33319c should fix the issue.

@lpinca
Copy link
Member

lpinca commented Jan 6, 2025

Adding the following patch without reverting c33319c should also fix the issue.

diff --git a/vcbuild.bat b/vcbuild.bat
index f93998d2b1..c5fd1a5d0c 100644
--- a/vcbuild.bat
+++ b/vcbuild.bat
@@ -590,9 +590,7 @@ if not defined doc if not defined build_addons (
 )
 if exist "tools\doc\node_modules\unified\package.json" goto skip-install-doctools
 SETLOCAL
-cd tools\doc
-%npm_exe% ci
-cd ..\..
+%npm_exe% --prefix tools\doc ci
 if errorlevel 1 goto exit
 ENDLOCAL
 :skip-install-doctools
@@ -735,9 +733,7 @@ goto lint-js-build
 
 :lint-js-build
 if not defined lint_js_build if not defined lint_js if not defined lint_js_fix goto lint-md-build
-cd tools\eslint
-%npm_exe% ci
-cd ..\..
+%npm_exe% --prefix tools\eslint ci
 
 :lint-js
 if not defined lint_js goto lint-js-fix
@@ -755,9 +751,7 @@ goto lint-md-build
 
 :lint-md-build
 if not defined lint_md if not defined format_md goto lint-md
-cd tools\lint-md
-%npm_exe% ci
-cd ..\..
+%npm_exe% --prefix tools\lint-md ci
 
 :lint-md
 if not defined lint_md goto format-md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants