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.

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

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

Merged
merged 1 commit into from
Jan 23, 2019
Merged

Conversation

refack
Copy link
Contributor

@refack refack commented Jan 15, 2019

  • 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 nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Jan 15, 2019
@refack
Copy link
Contributor Author

refack commented Jan 15, 2019

@refack refack added the aix Issues and PRs related to the AIX platform. label Jan 15, 2019
@mhdawson
Copy link
Member

@gireeshpunathil
@ThePrez

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

@gireeshpunathil
Copy link
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
Contributor

ThePrez commented Jan 17, 2019

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

@refack
Copy link
Contributor Author

refack commented Jan 17, 2019

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

@richardlau
Copy link
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',
],
}],
],

common.gypi Outdated Show resolved Hide resolved
@refack refack self-assigned this Jan 19, 2019
@refack
Copy link
Contributor Author

refack commented Jan 19, 2019

@dmabupt
Copy link
Contributor

dmabupt commented Jan 22, 2019

common.gypi Outdated Show resolved Hide resolved
node.gyp Outdated Show resolved Hide resolved
@refack
Copy link
Contributor Author

refack commented Jan 22, 2019

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

@refack
Copy link
Contributor Author

refack commented Jan 22, 2019

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
Contributor

dmabupt commented Jan 23, 2019

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
Copy link
Contributor Author

refack commented Jan 23, 2019

* 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:master Jan 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>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2019
@refack refack 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>
@BethGriggs BethGriggs 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 free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aix Issues and PRs related to the AIX platform. build Issues and PRs related to build files or the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants