GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — build: enable compilation for linuxOne by mhdawson · Pull Request #5941 · nodejs/node

Conversation

mhdawson commented Mar 29, 2016

Copy link

Copy Markdown

Member

Pull Request check-list

Please make sure to review and check all of these items:

Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
this change (including linting)?
Is the commit message formatted according to [CONTRIBUTING.md][0]?
If this change fixes a bug (or a performance problem), is a regression
test (or a benchmark) included?
Is a documentation update included (if this change modifies
existing APIs, or introduces new ones)?

NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.

Affected core subsystem(s)

build

Description of change

Changes to Node core in order to allow compilation for linuxOne.

The ../archs/linux32-s390x/opensslconf.h and
../archs/linux64-s390x/opensslconf.h were automatically
generated by running make linux-ppc linux-ppc64 in the
deps/openssl/config directory as per our standard
practice

After these changes we still need a version of v8
which supports linuxOne but that will be coming soon
in the 5.1 version of v8.

I tested by building a version of node.js from master
with the latest v8 tree based on the vee-eight-lkgr branch
of https://github.com/ofrobots/node and then adding
both this patch and additional patches to v8 for linuxOne.
The additional patches for v8 were required because
since it was based off of lkgr there is no guarantee
you get a tree that compiles for all platforms at
any given point in time.

The net is that it compiled but there are test
failures, many due to networking. I think those
are not related to anything needed in this patch. We
we don't see these failures in our internal environments
and I also saw a number of failures in the libuv tests
which makes me think its something to do with
the machine setup (one of my next things to look
into). This is the test job:
https://ci.nodejs.org/job/node-test-commit-linuxone-mdawson/14/nodes=rhel72-s390x/consoleFull

Getting these changes in now will make it easier
to be ready when we get v8 5.1 and to start
investigating the issues seen so far.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          build: enable compilation for linuxOne

aa1364f

Changes to Node core in order to allow compilation for linuxOne.

The ../archs/linux32-s390x/opensslconf.h and
../archs/linux64-s390x/opensslconf.h were automatically
generated by running make linux-ppc linux-ppc64 in the
deps/openssl/config directory as per our standard
practice

After these changes we still need a version of v8
which supports linuxOne but that will be coming soon
in the 5.1 version of v8.  Until then with these changes
we'll be able to create a hybrid build which pulls in
v8 from the http://github/andrewlow repo.

Fishrock123 added the build Issues and PRs related to Node.js builds or CI infrastructure. label

Mar 29, 2016

jbergstroem reviewed

Mar 29, 2016

View reviewed changes

Comment thread

Makefile Outdated

		ifeq ($(DESTCPU),s390x)
		ARCH=s390x
		else
		RCH=x86

jbergstroem Mar 29, 2016

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

ARCH

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

mhdawson Mar 29, 2016

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks, fixed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

mhdawson added 2 commits

March 29, 2016 10:21


          Address first set of review comments

0255e9f


          Address additional review comments

54d22a0

bnoordhuis commented Mar 29, 2016

Copy link

Copy Markdown

Member

LGTM

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mhdawson self-assigned this

Mar 29, 2016

mhdawson mentioned this pull request

Mar 29, 2016

Add s390/s390x Linux support to Node v6.x ibmruntimes/node#58

Closed

2 of 3 tasks

jbergstroem commented Mar 29, 2016

Copy link

Copy Markdown

Member

LGTM

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mhdawson commented Mar 29, 2016

Copy link

Copy Markdown

Member Author

Fixed configuration issue on machine, seems like it was the firewall that was too restrictive. My Frankenstein build compiled/passed the core tests https://ci.nodejs.org/job/node-test-commit-linuxone-mdawson/nodes=rhel72-s390x/15/console :)

Will land this now.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mhdawson commented Mar 29, 2016

Copy link

Copy Markdown

Member Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mhdawson added a commit that referenced this pull request

Mar 29, 2016


          build: enable compilation for linuxOne

a4c4a85

Changes to Node core in order to allow compilation for linuxOne.

The ../archs/linux32-s390x/opensslconf.h and
../archs/linux64-s390x/opensslconf.h were automatically
generated by running make linux-ppc linux-ppc64 in the
deps/openssl/config directory as per our standard
practice

After these changes we still need a version of v8
which supports linuxOne but that will be coming soon
in the 5.1 version of v8.  Until then with these changes
we'll be able to create a hybrid build which pulls in
v8 from the http://github/andrewlow repo.

PR-URL: #5941
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>

mhdawson commented Mar 29, 2016

Copy link

Copy Markdown

Member Author

CI all green, landed as a4c4a85

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mhdawson closed this

Mar 29, 2016

evanlucas pushed a commit that referenced this pull request

Mar 30, 2016


          build: enable compilation for linuxOne

6e4d9b3

Changes to Node core in order to allow compilation for linuxOne.

The ../archs/linux32-s390x/opensslconf.h and
../archs/linux64-s390x/opensslconf.h were automatically
generated by running make linux-ppc linux-ppc64 in the
deps/openssl/config directory as per our standard
practice

After these changes we still need a version of v8
which supports linuxOne but that will be coming soon
in the 5.1 version of v8.  Until then with these changes
we'll be able to create a hybrid build which pulls in
v8 from the http://github/andrewlow repo.

PR-URL: #5941
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>

MylesBorins commented Mar 30, 2016

Copy link

Copy Markdown

Contributor

@mhdawson do we want to backport this?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

MylesBorins added the lts-watch-v4.x label

Mar 30, 2016

mhdawson mentioned this pull request

Mar 30, 2016

Adding linux on Z machines to CI nodejs/build#360

Closed

10 tasks done

evanlucas mentioned this pull request

Mar 31, 2016

v5.10.0 Release Proposal #5970

Closed

evanlucas pushed a commit that referenced this pull request

Mar 31, 2016


          build: enable compilation for linuxOne

5681ffe

Changes to Node core in order to allow compilation for linuxOne.

The ../archs/linux32-s390x/opensslconf.h and
../archs/linux64-s390x/opensslconf.h were automatically
generated by running make linux-ppc linux-ppc64 in the
deps/openssl/config directory as per our standard
practice

After these changes we still need a version of v8
which supports linuxOne but that will be coming soon
in the 5.1 version of v8.  Until then with these changes
we'll be able to create a hybrid build which pulls in
v8 from the http://github/andrewlow repo.

PR-URL: #5941
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>

MylesBorins commented Jun 1, 2016

Copy link

Copy Markdown

Contributor

ping @mhdawson

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

MylesBorins pushed a commit that referenced this pull request

Jun 29, 2016


          build: enable compilation for linuxOne

48949fb

Changes to Node core in order to allow compilation for linuxOne.

The ../archs/linux32-s390x/opensslconf.h and
../archs/linux64-s390x/opensslconf.h were automatically
generated by running make linux-ppc linux-ppc64 in the
deps/openssl/config directory as per our standard
practice

After these changes we still need a version of v8
which supports linuxOne but that will be coming soon
in the 5.1 version of v8.  Until then with these changes
we'll be able to create a hybrid build which pulls in
v8 from the http://github/andrewlow repo.

PR-URL: #5941
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>

MylesBorins pushed a commit that referenced this pull request

Jul 12, 2016


          build: enable compilation for linuxOne

13d0e46

Changes to Node core in order to allow compilation for linuxOne.

The ../archs/linux32-s390x/opensslconf.h and
../archs/linux64-s390x/opensslconf.h were automatically
generated by running make linux-ppc linux-ppc64 in the
deps/openssl/config directory as per our standard
practice

After these changes we still need a version of v8
which supports linuxOne but that will be coming soon
in the 5.1 version of v8.  Until then with these changes
we'll be able to create a hybrid build which pulls in
v8 from the http://github/andrewlow repo.

PR-URL: #5941
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>

MylesBorins mentioned this pull request

Jul 12, 2016

v4.5.0 proposal #7688

Merged

MylesBorins added dont-land-on-v4.x and removed lts-watch-v4.x labels

Jul 14, 2016

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

mhdawson

Labels

build

Issues and PRs related to Node.js builds or CI infrastructure.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.