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

build,windows: make vcbuild fail if upload fails #19833

Merged
merged 1 commit into from Apr 8, 2018

Conversation

refack
Copy link
Contributor

@refack refack commented Apr 5, 2018

Refs: nodejs/build#1211

Crood solution but effective.

Output before:

d:\code\node$ vcbuild nobuild noprojgen upload
Looking for Python 2.x
ssh: Could not resolve hostname node-www: Name or service not known
ssh: Could not resolve hostname node-www: Name or service not known
lost connection
ssh: Could not resolve hostname node-www: Name or service not known
lost connection
ssh: Could not resolve hostname node-www: Name or service not known
lost connection
ssh: Could not resolve hostname node-www: Name or service not known
lost connection
ssh: Could not resolve hostname node-www: Name or service not known
lost connection
ssh: Could not resolve hostname node-www: Name or service not known
lost connection
ssh: Could not resolve hostname node-www: Name or service not known
lost connection
ssh: Could not resolve hostname node-www: Name or service not known

Output after:

d:\code\node$ vcbuild nobuild noprojgen upload
Looking for Python 2.x
ssh: Could not resolve hostname node-www: Name or service not known
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes — manual
  • commit message follows commit guidelines

@refack refack requested review from rvagg and bzoz April 5, 2018 14:13
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform. labels Apr 5, 2018
@rvagg
Copy link
Member

rvagg commented Apr 5, 2018

nice, do you know if errorlevel will propagate outside of the script so Jenkins will mark a build as failed?

@refack
Copy link
Contributor Author

refack commented Apr 6, 2018

do you know if errorlevel will propagate

It should (since nothing sets or resets it until the batch exits), and it did in my manual test. But in the wild cmd will do whatever cmd likes...

@refack
Copy link
Contributor Author

refack commented Apr 6, 2018

BTW:

if errorlevel 1 ...

means if %ERRORLEVEL% is greater or equal to 1. Another wonderful cmdism.

@refack refack self-assigned this Apr 6, 2018
@BridgeAR
Copy link
Member

BridgeAR commented Apr 6, 2018

I guess there is no CI that makes sense to run for this?

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 6, 2018
@rvagg
Copy link
Member

rvagg commented Apr 6, 2018

Yeah, there is actually, I've kicked off a test build which should come out @ https://nodejs.org/download/test/ as v10.0.0-test201804064cc80f32f9, the .msi and win-X directories will be the interesting part, plus also we'll have to check Jenkins to make sure we got green and the logs for the Windows builds look good. Someone with access will have to look @ https://ci-release.nodejs.org/job/iojs+release/3287/, I think @refack has access to do that right? I'll be 💤 soon and will have forgotten about this tomorrow.

@rvagg
Copy link
Member

rvagg commented Apr 6, 2018

https://nodejs.org/download/test/v10.0.0-test201804064cc80f32f9/ got a success from both windows builders, I see the x86 files in there now, the x64 ones should show up within the next 15 minutes. @refack you should have a look and make sure they are all there and work properly.

@refack
Copy link
Contributor Author

refack commented Apr 8, 2018

@refack you should have a look and make sure they are all there and work properly.

All expected files are accounted for and valid (SHA256, and 7z tested).

@refack refack merged commit 53035b1 into nodejs:master Apr 8, 2018
@refack refack deleted the vbcuild-fail-with-ssh branch April 8, 2018 16:35
@joaocgreis
Copy link
Member

This is a safety net that should be present in all branches, added the lts-watch labels.

@refack refack removed their assignment Oct 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants