FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

build: remove AIX/ppc (32bit) dead code by refack · Pull Request #25523 · nodejs/node · GitHub

/ node Public

build: remove AIX/ppc (32bit) dead code - #25523

Merged
refack merged 1 commit into
nodejs:masterfrom
refack:aix-gyp-cleanup
Jan 23, 2019
ConversationCommits (1)ChecksFiles changed
Merged

build: remove AIX/ppc (32bit) dead code#25523
refack merged 1 commit into
nodejs:masterfrom
refack:aix-gyp-cleanup

Conversation

refack commented Jan 15, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor
  • also refactor OS400 detection

Refs: https://github.com/nodejs/node/pull/25447/files/36839defcfaf7c46435e16fb1f0da006f3ebe8ac#r247378894

/CC @nodejs/build-files @nodejs/platform-aix

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

nodejs-github-bot commented Jan 15, 2019

Copy link
Copy Markdown
Collaborator

@refack build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/2257/pipeline

nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Jan 15, 2019

refack commented Jan 15, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

AIX CI: https://ci.nodejs.org/job/node-test-commit-aix/20427/
AIX CI: https://ci.nodejs.org/job/node-test-commit-aix/20430/

refack added the aix Issues and PRs related to the AIX platform. label Jan 15, 2019
phillipj mentioned this pull request Jan 15, 2019
Closed

mhdawson commented Jan 17, 2019

Copy link
Copy Markdown
Member

@gireeshpunathil
@ThePrez

Can you two take a look and comment if you have any concerns.

gireeshpunathil commented Jan 17, 2019

Copy link
Copy Markdown
Member

the configure error
21:24:54 gyp: Undefined variable aix_variant_name in /home/iojs/build/workspace/node-test-commit-aix/nodes/aix61-ppc64/node.gyp while trying to load

looks to be relevant here, as we moved a section containing aix_variant_name

ThePrez commented Jan 17, 2019

Copy link
Copy Markdown
Contributor

LGTM, though I haven't tested with the changes. @dmabupt, can you please sanity-check?

refack commented Jan 17, 2019

Copy link
Copy Markdown
Contributor Author

https://ci.nodejs.org/job/node-test-commit-aix/20476/
Latest change gets code to build, PTAL.

richardlau commented Jan 17, 2019

Copy link
Copy Markdown
Member

There's similar code in node.gyp:

node/node.gyp

Lines 973 to 1000 in 098c2cc

'variables': {'real_os_name': '<!(uname -s)',},
'targets': [
{
'target_name': 'node_aix_shared',
'type': 'shared_library',
'product_name': '<(node_core_target_name)',
'ldflags': [ '--shared' ],
'product_extension': '<(shlib_suffix)',
'conditions': [
['target_arch=="ppc64"', {
'ldflags': [
'-Wl,-blibpath:/usr/lib:/lib:'
'/opt/freeware/lib/pthread/ppc64'
],
}],
['target_arch=="ppc"', {
'ldflags': [
'-Wl,-blibpath:/usr/lib:/lib:/opt/freeware/lib/pthread'
],
}],
['"<(real_os_name)"=="OS400"', {
'ldflags': [
'-Wl,-blibpath:/QOpenSys/pkgs/lib:/QOpenSys/usr/lib',
'-Wl,-bbigtoc',
'-Wl,-brtl',
],
}],
],

richardlau reviewed Jan 18, 2019
View reviewed changes
Comment thread common.gypi Outdated
refack self-assigned this Jan 19, 2019

refack commented Jan 19, 2019

Copy link
Copy Markdown
Contributor Author

CI: https://ci.nodejs.org/job/node-test-commit-aix/20503/

dmabupt commented Jan 22, 2019

Copy link
Copy Markdown
Contributor

This line is not needed anymore --> https://github.com/nodejs/node/pull/25523/files#diff-259e627be3fafed9e9c10a7fcb26879eR970

dmabupt reviewed Jan 22, 2019
View reviewed changes
Comment thread common.gypi Outdated
dmabupt reviewed Jan 22, 2019
View reviewed changes
Comment thread node.gyp Outdated

refack commented Jan 22, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

New AIX CI: https://ci.nodejs.org/job/node-test-commit-aix/20560/
New AIX CI: https://ci.nodejs.org/job/node-test-commit-aix/20561/ (✔️)
AIX + shared: https://ci.nodejs.org/job/node-test-commit-aix-shared-lib/256/ (⚠️ stable)

@dmabupt, can you verify this builds on IBM i (also with ./configure --shared)?

richardlau reviewed Jan 22, 2019
View reviewed changes
Comment thread common.gypi Outdated

refack commented Jan 22, 2019

Copy link
Copy Markdown
Contributor Author

It'll probably be evident in the CI, but we need to pass -maix64 to the linker as well as the compiler.

FTR, this is a sample error message from ld:

ld: 0711-736 ERROR: Input file /home/iojs/build/workspace/node-test-commit-aix/nodes/aix61-ppc64/out/Release/obj.host/genccode/deps/icu-small/source/tools/genccode/genccode.o:
 	XCOFF64 object files are not allowed in 32-bit mode.

dmabupt commented Jan 23, 2019

Copy link
Copy Markdown
Contributor

New AIX CI: https://ci.nodejs.org/job/node-test-commit-aix/20560/
New AIX CI: https://ci.nodejs.org/job/node-test-commit-aix/20561/ ()
AIX + shared: https://ci.nodejs.org/job/node-test-commit-aix-shared-lib/256/ ( stable)

@dmabupt, can you verify this builds on IBM i (also with ./configure --shared)?

I have built Node.js on IBM i with / without the --shared option. Both builds passed.

refack commented Jan 23, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

CI: https://ci.nodejs.org/job/node-test-pull-request/20291/
Resume: https://ci.nodejs.org/job/node-test-pull-request/20293/

build: remove AIX/ppc (32bit) dead code
b1a4e41 
* also dedup OS400 detection

PR-URL: nodejs#25523
Refs: https://github.com/nodejs/node/pull/25447/files/36839defcfaf7c46435e16fb1f0da006f3ebe8ac#r247378894
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
refack merged commit b1a4e41 into nodejs:master Jan 23, 2019
refack deleted the aix-gyp-cleanup branch January 23, 2019 22:58
targos pushed a commit that referenced this pull request Jan 24, 2019
build: remove AIX/ppc (32bit) dead code
5fab92c 
* also dedup OS400 detection

PR-URL: #25523
Refs: https://github.com/nodejs/node/pull/25447/files/36839defcfaf7c46435e16fb1f0da006f3ebe8ac#r247378894
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
MylesBorins mentioned this pull request Jan 24, 2019
Merged
refack removed their assignment Mar 11, 2019
BethGriggs pushed a commit that referenced this pull request Apr 29, 2019
build: remove AIX/ppc (32bit) dead code
f449608 
* also dedup OS400 detection

PR-URL: #25523
Refs: https://github.com/nodejs/node/pull/25447/files/36839defcfaf7c46435e16fb1f0da006f3ebe8ac#r247378894
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
BethGriggs mentioned this pull request May 1, 2019
Merged
BethGriggs pushed a commit that referenced this pull request May 10, 2019
build: remove AIX/ppc (32bit) dead code
a4da17a 
* also dedup OS400 detection

PR-URL: #25523
Refs: https://github.com/nodejs/node/pull/25447/files/36839defcfaf7c46435e16fb1f0da006f3ebe8ac#r247378894
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
build: remove AIX/ppc (32bit) dead code
de26866 
* also dedup OS400 detection

PR-URL: #25523
Refs: https://github.com/nodejs/node/pull/25447/files/36839defcfaf7c46435e16fb1f0da006f3ebe8ac#r247378894
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
This was referenced May 29, 2019
Closed
Closed
Closed
Closed
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
richardlau richardlau approved these changes
BethGriggs BethGriggs approved these changes
+1 more reviewer
dmabupt dmabupt left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

aix Issues and PRs related to the AIX platform. build Issues and PRs related to build files or the CI.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

Back | FazBrowse Home | New Git URL