Skip to content

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

Merged
refack merged 1 commit into
nodejs:masterfrom
refack:aix-gyp-cleanup
Jan 23, 2019
Merged

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

Conversation

@refack

@refackrefack commented Jan 15, 2019

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

Copy link
Copy Markdown
Collaborator

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

refack commented Jan 15, 2019

Copy link
Copy Markdown
ContributorAuthor

@refackrefack added the aix Issues and PRs related to the AIX platform. label Jan 15, 2019
@mhdawson

Copy link
Copy Markdown
Member

@gireeshpunathil
@ThePrez

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

@gireeshpunathil

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

Copy link
Copy Markdown
Contributor

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

@refack

Copy link
Copy Markdown
ContributorAuthor

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

@richardlau

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',
],
}],
],

Comment threadcommon.gypi Outdated
@refackrefack self-assigned this Jan 19, 2019
@refack

Copy link
Copy Markdown
ContributorAuthor

@dmabupt

Copy link
Copy Markdown
Contributor

Comment threadcommon.gypi Outdated
Comment threadnode.gyp Outdated
@refack

refack commented Jan 22, 2019

Copy link
Copy Markdown
ContributorAuthor

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)?

Comment threadcommon.gypi Outdated
@refack

Copy link
Copy Markdown
ContributorAuthor

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

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

refack commented Jan 23, 2019

Copy link
Copy Markdown
ContributorAuthor

* 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
refack merged commit b1a4e41 into nodejs:masterJan 23, 2019
@refack
refack deleted the aix-gyp-cleanup branch January 23, 2019 22:58
targos pushed a commit that referenced this pull request Jan 24, 2019
* 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>
@MylesBorinsMylesBorins mentioned this pull request Jan 24, 2019
@refackrefack removed their assignment Mar 11, 2019
BethGriggs pushed a commit that referenced this pull request Apr 29, 2019
* 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>
@BethGriggsBethGriggs mentioned this pull request May 1, 2019
BethGriggs pushed a commit that referenced this pull request May 10, 2019
* 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
* 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>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aixIssues and PRs related to the AIX platform.buildIssues and PRs related to build files or the CI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@refack@nodejs-github-bot@mhdawson@gireeshpunathil@ThePrez@richardlau@dmabupt@jasnell@BethGriggs