suppress the output of rd %config% by liudonghua123 · Pull Request #51437 · nodejs/node · GitHub

I noticed the confusing error like message The system cannot find the file specified. in the CIs and on my first build locally. It's really annoying.

And after some debugging, I found the root case of this error message.

Then this simple pr comes for it!

See also #40749 (comment).

cc @nodejs/build-files

cc @nodejs/platform-windows

CI: https://ci.nodejs.org/job/node-test-pull-request/56756/

Thanks for the PR!

CI: https://ci.nodejs.org/job/node-test-pull-request/56760/

@H4ad Hi, could we merge the pull request.

I noticed some warnings/errors as follows in https://ci.nodejs.org/job/node-test-commit/67906/console, but I couldn't help to fix it.

20:26:32 git@github.com: Permission denied (publickey).
20:26:32 fatal: Could not read from remote repository.

Would something like this work instead to not suppress other errors?

Suggested change

      
            :: Suppress "The system cannot find the file specified." on every clean build.
          
            rd %config% > nul 2> nul
          
            if exists %config% rd %config%

The correct syntax should be
"IF EXIST file command". However, check whether the file exists doesn't help a lot here because it's safe to remove the link even the link doesn't exists. Both of them works, and RD directly is just more simpler IMO.

On the other hand, if %config% doesn't exist, it's not necessary to invoke rd, and when it does exist, this approach doesn't silently ignore the output of rd.

Yes, I updated this pr. Thanks! 😄

CI: https://ci.nodejs.org/job/node-test-pull-request/56774/

CI: https://ci.nodejs.org/job/node-test-pull-request/56776/

Note @ whoever decides to merge this: the commit queue would incorrectly use the first commit message as the final commit message, so either land this manually or force-push to the PR branch with an amended commit message (but that requires a fresh CI).

Note @ whoever decides to merge this: the commit queue would incorrectly use the first commit message as the final commit message, so either land this manually or force-push to the PR branch with an amended commit message (but that requires a fresh CI).

@tniessen so what should I do? Amend the two commits into one, change the commit message then force-push?

That option would be the easiest solution for us, then we only need to re-run CI on the squashed commit :)

@tniessen Thanks, now I reset the last two commits and force pushed the amended log message commit.

Thank you @liudonghua123!

CI: https://ci.nodejs.org/job/node-test-pull-request/56794/

CI: https://ci.nodejs.org/job/node-test-pull-request/56808/

CI: https://ci.nodejs.org/job/node-test-pull-request/56870/

Landed in b9a4279

PR-URL: nodejs#51437 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

PR-URL: #51437 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

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 11, 2024

anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 12, 2024

anonrig requested review from H4ad and targos January 12, 2024 01:07

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 12, 2024

H4ad reviewed Jan 12, 2024

View reviewed changes

Comment thread vcbuild.bat Outdated

H4ad added the review wanted PRs that need reviews. label Jan 12, 2024

H4ad approved these changes Jan 12, 2024

View reviewed changes

tniessen reviewed Jan 13, 2024

View reviewed changes

tniessen approved these changes Jan 14, 2024

View reviewed changes

Comment thread vcbuild.bat Outdated

H4ad added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 14, 2024

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 14, 2024

liudonghua123 force-pushed the fix_rd_confusing_error_like_message branch from 62c3e0a to 79efff7 Compare January 14, 2024 12:50

H4ad approved these changes Jan 14, 2024

View reviewed changes

H4ad added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 14, 2024

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 14, 2024

build: check before removing %config% link

2f328c0

liudonghua123 force-pushed the fix_rd_confusing_error_like_message branch from 79efff7 to 2f328c0 Compare January 15, 2024 12:45

tniessen added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 15, 2024

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 15, 2024

lpinca approved these changes Jan 21, 2024

View reviewed changes

lpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 25, 2024

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 25, 2024

nodejs-github-bot merged commit b9a4279 into nodejs:main Jan 25, 2024

liudonghua123 deleted the fix_rd_confusing_error_like_message branch January 26, 2024 00:08

marco-ippolito mentioned this pull request Mar 1, 2024

v21.7.0 proposal #51932

Merged

richardlau mentioned this pull request Mar 25, 2024

v20.12.0 proposal #52212

Merged

	:: Suppress "The system cannot find the file specified." on every clean build.
	rd %config% > nul 2> nul
	if exists %config% rd %config%

suppress the output of rd %config% by liudonghua123 · Pull Request #51437 · nodejs/node · GitHub

Uh oh!

Uh oh!

suppress the output of rd %config% - #51437

suppress the output of rd %config%#51437nodejs-github-bot merged 1 commit intonodejs:mainnodejs/node:mainfrom liudonghua123:fix_rd_confusing_error_like_messageliudonghua123/node:fix_rd_confusing_error_like_messageCopy head branch name to clipboard

Conversation

liudonghua123 commented Jan 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marco-ippolito commented Jan 11, 2024

Uh oh!

tniessen commented Jan 11, 2024

Uh oh!

nodejs-github-bot commented Jan 12, 2024

Uh oh!

Uh oh!

H4ad left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Jan 12, 2024

Uh oh!

liudonghua123 commented Jan 13, 2024

Uh oh!

tniessen Jan 13, 2024

Choose a reason for hiding this comment

Uh oh!

liudonghua123 Jan 13, 2024

Choose a reason for hiding this comment

Uh oh!

tniessen Jan 13, 2024

Choose a reason for hiding this comment

Uh oh!

liudonghua123 Jan 14, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nodejs-github-bot commented Jan 14, 2024

Uh oh!

nodejs-github-bot commented Jan 14, 2024

Uh oh!

tniessen commented Jan 14, 2024

Uh oh!

liudonghua123 commented Jan 15, 2024

Uh oh!

tniessen commented Jan 15, 2024

Uh oh!

liudonghua123 commented Jan 15, 2024

Uh oh!

tniessen commented Jan 15, 2024

Uh oh!

nodejs-github-bot commented Jan 15, 2024

Uh oh!

nodejs-github-bot commented Jan 16, 2024

Uh oh!

nodejs-github-bot commented Jan 21, 2024

Uh oh!

nodejs-github-bot commented Jan 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

suppress the output of rd %config%#51437
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
liudonghua123:fix_rd_confusing_error_like_message

liudonghua123 commented Jan 11, 2024 •
edited
Loading