Skip to content

s390: enable march=z196#11730

Closed
john-yan wants to merge 1 commit into
nodejs:masterfrom
john-yan:master
Closed

s390: enable march=z196#11730
john-yan wants to merge 1 commit into
nodejs:masterfrom
john-yan:master

Conversation

@john-yan

@john-yan john-yan commented Mar 7, 2017

Copy link
Copy Markdown

enable -march=z196 for s390, because it's the lowest platform we intend to support for zlinux platforms.

@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Mar 7, 2017
@addaleax

addaleax commented Mar 7, 2017

Copy link
Copy Markdown
Member

@nodejs/platform-s390

@sam-github

sam-github commented Mar 7, 2017

Copy link
Copy Markdown
Contributor

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

@gibfahn

gibfahn commented Mar 7, 2017

Copy link
Copy Markdown
Member

cc/ @joransiu @jBarz

@joransiu

joransiu commented Mar 7, 2017

Copy link
Copy Markdown
Contributor

LGTM. http://www-03.ibm.com/systems/z/os/linux/resources/testedplatforms.html shows that matches the minimum hardware requirements for the distros with right level of GCC/Glibc support (SLES 12, RHEL 7, Ubuntu 16.04 or newer).

richardlau
richardlau approved these changes Mar 7, 2017
View reviewed changes

@richardlau richardlau left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

Does this need to be noted in #8922?

@sam-github

sam-github commented Mar 7, 2017

Copy link
Copy Markdown
Contributor

Hard to believe this broke OS X, try again:

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

@jBarz

jBarz commented Mar 7, 2017

Copy link
Copy Markdown
Contributor

LGTM

@john-yan

john-yan commented Mar 7, 2017

Copy link
Copy Markdown
Author

@sam-github It doesn't seem to fail by looking at the details of ARM build.

@sam-github

sam-github commented Mar 7, 2017

Copy link
Copy Markdown
Contributor

ARM builds always fail, so ci result is a pass.

@gibfahn

gibfahn commented Mar 7, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

Rerun of CI was green (arm failure on Github is a reporting issue, you need to click on the CI link).

EDIT: what Sam said

mhdawson
mhdawson approved these changes Mar 7, 2017
View reviewed changes

@mhdawson mhdawson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@mscdex mscdex added the s390x Issues and PRs related to the s390x architecture. label Mar 7, 2017
@john-yan

john-yan commented Mar 8, 2017

Copy link
Copy Markdown
Author

any updates on this?

@sam-github

sam-github commented Mar 8, 2017

Copy link
Copy Markdown
Contributor

Requires 2 mid-week days to give people time to comment before it can be merged.

@gibfahn

gibfahn commented Mar 8, 2017

Copy link
Copy Markdown
Member

@john-yan see https://github.com/nodejs/node/blob/master/CONTRIBUTING.md

@mhdawson

mhdawson commented Mar 10, 2017

Copy link
Copy Markdown
Member

Landing since its now past the wait time.

@mhdawson

mhdawson commented Mar 10, 2017

Copy link
Copy Markdown
Member

Landed as b806e18

@mhdawson mhdawson closed this Mar 10, 2017
mhdawson pushed a commit that referenced this pull request Mar 10, 2017
@mhdawson
s390: enable march=z196
b806e18 
PR-URL: #11730
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Mar 13, 2017
s390: enable march=z196
52f0092 
PR-URL: nodejs#11730
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@italoacasas italoacasas mentioned this pull request Mar 13, 2017
Merged
jungx098 pushed a commit to jungx098/node that referenced this pull request Mar 21, 2017
@jungx098
s390: enable march=z196
9072293 
PR-URL: nodejs#11730
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins

MylesBorins commented Apr 17, 2017

Copy link
Copy Markdown
Contributor

should this be backported to v6.x?

@gibfahn

gibfahn commented Apr 18, 2017

Copy link
Copy Markdown
Member

Talked to @joransiu and @john-yan , and this probably shouldn't apply to v6.x.

cc/ @nodejs/platform-s390 , if anyone disagrees let me know.

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

@cjihrig cjihrig cjihrig approved these changes

@richardlau richardlau richardlau approved these changes

+4 more reviewers

@sam-github sam-github sam-github approved these changes

@jbajwa jbajwa jbajwa approved these changes

@mhdawson mhdawson mhdawson approved these changes

@gibfahn gibfahn gibfahn approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

build Issues and PRs related to build files or the CI. s390x Issues and PRs related to the s390x architecture.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.