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.

Sign up for GitHub

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

Jump to bottom

build: move compiling for Windows ARM64 to Tier 2 #34721

Closed
wants to merge 1 commit into from
Closed

build: move compiling for Windows ARM64 to Tier 2 #34721

wants to merge 1 commit into from
+1 −4

Conversation

joaocgreis
Copy link
Member

Support for cross-compiling for Windows ARM64 was added recently. This moves Windows ARM64 compilation to Tier 2.

We don't yet have reliable machines that we can connect to Jenkins to run CI, so it's too soon to move all Windows ARM64 to Tier 2. We already publish unofficial builds, however a regression has prevented us from publishing releases after v12.15.0. Moving to Tier 2 should prevent such regressions while we wait for reliable hardware for testing.

This depends on nodejs/build#2408

cc @nodejs/build @nodejs/platform-windows

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

@joaocgreis joaocgreis added windows Issues and PRs related to the Windows platform. build Issues and PRs related to build files or the CI. dont-land-on-v10.x labels Aug 11, 2020
@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Aug 11, 2020
@nodejs-github-bot
Copy link
Collaborator

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

@rvagg
Copy link
Member

rvagg commented Aug 11, 2020

I guess discussion here should focus on how close this comes to our Tier 2 definition:

  • Tier 2: These platforms represent smaller segments of the Node.js user
    base. The Node.js Build Working Group maintains infrastructure for full test
    coverage. Test failures on tier 2 platforms will block releases.
    Infrastructure issues may delay the release of binaries for these platforms.

Given that there's an opt-in checkbox in a Jenkins subjob that gets compilation happening does it get close enough? Do we need to extend that a bit more to make a daily test job against master to get closer? I don't imagine too many people being aware of the tickbox or thinking to regularly run these jobs outside of one or two people.

It strictly meets "The Node.js Build Working Group maintains infrastructure for full test coverage." but "Test failures on tier 2 platforms will block releases" suggests that it's tied into our CI in such a way that failures would be noticed by someone preparing for releases.

@joaocgreis
Copy link
Member Author

I agree this does not meet the definition of Tier 2 if we consider testing, but my goal is to have it only for compilation.

The Jenkins change in nodejs/build#2408 makes compilation run on every PR, so issues compiling for ARM64 will be visible well before releases. Only running the tests with the compiled binary is behind an opt-in checkbox, and tests do not need to pass for PRs nor releases.

My objective here is to keep compilation working, as it is very easy to break. If compilation works, it is likely that the produced binary works and it is much easier to investigate and fix other issues with it.

rvagg
rvagg approved these changes Aug 11, 2020
View reviewed changes
Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

The Jenkins change in nodejs/build#2408 makes compilation run on every PR

I missed that, and in that case this makes sense the way it's spelled out

@Trott
Copy link
Member

Trott commented Aug 14, 2020

Landed in c2771dc

@Trott Trott closed this Aug 14, 2020
Trott pushed a commit that referenced this pull request Aug 14, 2020
@joaocgreis @Trott
build: move compiling for Windows ARM64 to Tier 2
c2771dc 
PR-URL: #34721
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Ash Cripps <ashley.cripps@ibm.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins pushed a commit that referenced this pull request Aug 17, 2020
@joaocgreis @MylesBorins
build: move compiling for Windows ARM64 to Tier 2
2b2a87c 
PR-URL: #34721
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Ash Cripps <ashley.cripps@ibm.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@danielleadams danielleadams mentioned this pull request Aug 20, 2020
Merged
BethGriggs pushed a commit that referenced this pull request Aug 20, 2020
@joaocgreis @BethGriggs
build: move compiling for Windows ARM64 to Tier 2
d89a83c 
PR-URL: #34721
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Ash Cripps <ashley.cripps@ibm.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@dennisameling dennisameling mentioned this pull request Oct 1, 2020
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@rvagg rvagg rvagg approved these changes

@Trott Trott Trott approved these changes

@richardlau richardlau richardlau approved these changes

@AshCripps AshCripps AshCripps approved these changes

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. windows Issues and PRs related to the Windows platform.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@joaocgreis @nodejs-github-bot @rvagg @Trott @jasnell @richardlau @AshCripps