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: fix IBM i build with Python 3.9 #48056

Closed
wants to merge 2 commits into from
Closed

Conversation

richardlau
Copy link
Member

Python 3.9 on IBM i returns "os400" for sys.platform.

Refs: #46739
Refs: nodejs/build#3358


cc @nodejs/platform-ibmi

@richardlau richardlau added the ibm i Issues and PRs related to the IBM i platform. label May 18, 2023
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. openssl Issues and PRs related to the OpenSSL dependency. labels May 18, 2023
@richardlau
Copy link
Member Author

Copy link
Member

@MoLow MoLow left a comment

Choose a reason for hiding this comment

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

Assuming build passes

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@richardlau
Copy link
Member Author

Assuming build passes

Unfortunately it did not (after just shy of 8 hours)😞.

Errors look like this
https://ci.nodejs.org/job/node-test-commit-ibmi/1163/nodes=ibmi73-ppc64/consoleFull

20:51:27 Assembler:
20:51:27 /home/IOJS/build/cc49avPB.s: line 11: 1252-016 The specified opcode or pseudo-op is not valid.
20:51:27 	Use supported instructions or pseudo-ops only.
20:51:27 /home/IOJS/build/cc49avPB.s: line 12: 1252-016 The specified opcode or pseudo-op is not valid.
20:51:27 	Use supported instructions or pseudo-ops only.
...

I'm not 100% sure but it looks like this might be an intermediate/generated file during v8 (v8_snapshot?) -- the openssl bits that this PR was fixing were much earlier in the log. @abmusse Could you possibly take a look?

@abmusse
Copy link
Contributor

abmusse commented May 18, 2023

Just took a peek at the log had scroll a bunch of The specified opcode or pseudo-op is not valid. like Richard noted above.

Looks like the command just before the first opcode error was:

15:51:26   ccache \
gcc-10 \
-o \
/home/IOJS/build/workspace/node-test-commit-ibmi/nodes/ibmi73-ppc64/out/Release/obj.target/v8_snapshot/geni/embedded.o \
/home/IOJS/build/workspace/node-test-commit-ibmi/nodes/ibmi73-ppc64/out/Release/obj.target/v8_snapshot/geni/embedded.S \
'-D_GLIBCXX_USE_CXX11_ABI=1' \
'-DNODE_OPENSSL_CONF_NAME=nodejs_conf' \
'-DNODE_OPENSSL_HAS_QUIC' \
'-DICU_NO_USER_DATA_OVERRIDE' \
'-DV8_GYP_BUILD' \
'-DV8_TYPED_ARRAY_MAX_SIZE_IN_HEAP=64' \
'-D__STDC_FORMAT_MACROS' \
'-DOPENSSL_NO_PINSHARED' \
'-DOPENSSL_THREADS' \
'-DV8_TARGET_ARCH_PPC64' \
'-DV8_TARGET_ARCH_PPC_BE' \
'-D_LINUX_SOURCE_COMPAT=1' \
'-D_ALL_SOURCE=1' \
'-DV8_EMBEDDER_STRING="-node.9"' \
'-DENABLE_DISASSEMBLER' \
'-DV8_PROMISE_INTERNAL_FIELD_COUNT=1' \
'-DOBJECT_PRINT' \
'-DV8_INTL_SUPPORT' \
'-DV8_ATOMIC_OBJECT_FIELD_WRITES' \
'-DV8_ENABLE_LAZY_SOURCE_POSITIONS' \
'-DV8_USE_SIPHASH' \
'-DV8_SHARED_RO_HEAP' \
'-DV8_WIN64_UNWINDING_INFO' \
'-DV8_ENABLE_REGEXP_INTERPRETER_THREADED_DISPATCH' \
'-DV8_USE_ZLIB' \
'-DV8_ENABLE_TURBOFAN' \
'-DV8_ENABLE_WEBASSEMBLY' \
'-DV8_ENABLE_JAVASCRIPT_PROMISE_HOOKS' \
'-DV8_ALLOCATION_FOLDING' \
'-DV8_ALLOCATION_SITE_TRACKING' \
'-DV8_SCRIPTORMODULE_LEGACY_LIFETIME' \
'-DV8_ADVANCED_BIGINT_ALGORITHMS' \
'-DUCONFIG_NO_SERVICE=1' \
'-DU_ENABLE_DYLOAD=0' \
'-DU_STATIC_IMPLEMENTATION=1' \
'-DU_HAVE_STD_STRING=1' \
'-DUCONFIG_NO_BREAK_ITERATION=0' \
-I../deps/v8 \
-I../deps/v8/include \
-I/home/IOJS/build/workspace/node-test-commit-ibmi/nodes/ibmi73-ppc64/out/Release/obj/gen/generate-bytecode-output-root \
-I/home/IOJS/build/workspace/node-test-commit-ibmi/nodes/ibmi73-ppc64/out/Release/obj/gen \
-I../deps/icu-small/source/i18n \
-I../deps/icu-small/source/common \
-pthread \
-Wno-unused-parameter \
-maix64 \
-Wno-return-type \
-ffp-contract=off \
-mcpu=power5+ \
-mfprnd \
-mno-popcntb \
-fno-strict-aliasing \
-maix64 \
-fdollars-in-identifiers \
-fno-extern-tls-init \
-O3 \
-fno-omit-frame-pointer \
-fdata-sections \
-ffunction-sections \
-O3 \
-MMD \
-MF \
/home/IOJS/build/workspace/node-test-commit-ibmi/nodes/ibmi73-ppc64/out/Release/.deps//home/IOJS/build/workspace/node-test-commit-ibmi/nodes/ibmi73-ppc64/out/Release/obj.target/v8_snapshot/geni/embedded.o.d.raw \
-c
19:51:42 gmake[2]: *** [Makefile:272: /home/IOJS/build/workspace/node-test-commit-ibmi/nodes/ibmi73-ppc64/out/Release/obj.target/v8_snapshot/geni/embedded.o] Error 1

@abmusse
Copy link
Contributor

abmusse commented May 18, 2023

@richardlau

Does v8 link with openssl? I'm not sure how your changes to the embedded openssl results in v8 compile failure.

FYI for our IBM i releases of node we build with --shared-openssl and link with our release openssl3.
When using --shared-openssl do we need these changes to the embedded openssl?

Is it possible for us to do the same in the CI, i.e. Install our openssl3 and build with --shared-openssl?

@richardlau
Copy link
Member Author

richardlau commented May 19, 2023

@richardlau

Does v8 link with openssl? I'm not sure how your changes to the embedded openssl results in v8 compile failure.

FYI for our IBM i releases of node we build with --shared-openssl and link with our release openssl3. When using --shared-openssl do we need these changes to the embedded openssl?

Is it possible for us to do the same in the CI, i.e. Install our openssl3 and build with --shared-openssl?

I tried passing --shared-openssl in CONFIG_FLAGS in https://ci.nodejs.org/job/node-test-commit-ibmi/1164/ and we still have the same failures. From https://ci.nodejs.org/job/node-test-commit-ibmi/1164/nodes=ibmi73-ppc64/consoleFull you can see it has set:

23:13:27 { 'target_defaults': { 'cflags': [],
23:13:27                        'default_configuration': 'Release',
23:13:27                        'defines': [ 'NODE_OPENSSL_CONF_NAME=nodejs_conf',
23:13:27                                     'ICU_NO_USER_DATA_OVERRIDE'],
23:13:27                        'include_dirs': ['/QOpenSys/pkgs/include'],
23:13:27                        'libraries': [ '-L/QOpenSys/pkgs/lib',
23:13:27                                       '-lcrypto',
23:13:27                                       '-lssl']},
...
23:13:31                  'node_shared_openssl': 'true',

That run was on main, without the changes in this PR. My suspicion is that something isn't being set correctly now that we are using Python 3.9.

@abmusse
Copy link
Contributor

abmusse commented May 19, 2023

Ah yes, I suspect we still need these changes:

https://chromium-review.googlesource.com/c/v8/v8/+/4259330

This is the only patch we apply in our Node 20 build as v8 has not merged this yet.

Should we make a PR with these changes?

@richardlau
Copy link
Member Author

Ah yes, I suspect we still need these changes:

https://chromium-review.googlesource.com/c/v8/v8/+/4259330

This is the only patch we apply in our Node 20 build as v8 has not merged this yet.

Should we make a PR with these changes?

We generally avoid floating patches on top of V8 as it complicates updating V8 versions in Node.js. We need to get https://chromium-review.googlesource.com/c/v8/v8/+/4259330 merged upstream in V8 first and then we can cherry-pick or update V8 in Node.js. Perhaps @miladfarca could help advise how to get that CL merged into V8.

Refs: https://github.com/nodejs/node/blob/main/doc/contributing/maintaining/maintaining-V8.md

In the meantime I'll make node-test-commit-ibmi run with Python 3.6 until we have the V8 patch merged.

@miladfarca
Copy link
Contributor

We generally avoid floating patches on top of V8 as it complicates updating V8 versions in Node.js. We need to get https://chromium-review.googlesource.com/c/v8/v8/+/4259330 merged upstream in V8 first and then we can cherry-pick or update V8 in Node.js. Perhaps @miladfarca could help advise how to get that CL merged into V8.

If contributing for the first time make sure you have signed a CLA: https://v8.dev/docs/contribute#sign-the-cla
To start the review process click on Reply > Suggest Owners and choose form the list. Above changes could have verwaest and machenbach as reviewers.

@kadler
Copy link

kadler commented Aug 1, 2023

Since Meng Xu is no longer working on IBM i, I think we'll have to take over his PR. @abmusse let's work on getting you set up to take the upstream PR over.

@abmusse
Copy link
Contributor

abmusse commented Aug 17, 2023

We generally avoid floating patches on top of V8 as it complicates updating V8 versions in Node.js. We need to get https://chromium-review.googlesource.com/c/v8/v8/+/4259330 merged upstream in V8 first and then we can cherry-pick or update V8 in Node.js. Perhaps @miladfarca could help advise how to get that CL merged into V8.

If contributing for the first time make sure you have signed a CLA: https://v8.dev/docs/contribute#sign-the-cla To start the review process click on Reply > Suggest Owners and choose form the list. Above changes could have verwaest and machenbach as reviewers.

@miladfarca

I've gained approval to contribute to v8 through the IBM process. Should be good with signing the CLA.
I've added verwaest and machenbach as reviewers as you suggested and commented on the PR.
Hopefully we can get the ball rolling on the next steps with moving forward with the PR!

@abmusse
Copy link
Contributor

abmusse commented Sep 6, 2023

Got setup and created a CL with the minimal amount of changes needed

https://chromium-review.googlesource.com/c/v8/v8/+/4846413

@miladfarca
Copy link
Contributor

miladfarca commented Sep 6, 2023

@abmusse Click on "Reply > Suggest Owners > show all owners" and select at least one owner to review your CL.

@abmusse
Copy link
Contributor

abmusse commented Sep 6, 2023

@abmusse Click on "Reply > Suggest Owners > show all owners" and select at least one owner to review your CL.

Thanks, I've added the reviewers!

@abmusse
Copy link
Contributor

abmusse commented Sep 14, 2023

Got setup and created a CL with the minimal amount of changes needed

https://chromium-review.googlesource.com/c/v8/v8/+/4846413

CC @richardlau

UPDATE:

The above CL got merged 🎉

@richardlau
Copy link
Member Author

Got setup and created a CL with the minimal amount of changes needed
https://chromium-review.googlesource.com/c/v8/v8/+/4846413

CC @richardlau

UPDATE:

The above CL got merged 🎉

@abmusse Could you open a V8 backport PR?
See https://github.com/nodejs/node/blob/main/doc/contributing/maintaining/maintaining-V8.md#backporting-to-abandoned-branches

@richardlau richardlau added the blocked PRs that are blocked by other issues or PRs. label Sep 15, 2023
@abmusse
Copy link
Contributor

abmusse commented Sep 25, 2023

Got setup and created a CL with the minimal amount of changes needed
https://chromium-review.googlesource.com/c/v8/v8/+/4846413

CC @richardlau
UPDATE:
The above CL got merged 🎉

@abmusse Could you open a V8 backport PR? See https://github.com/nodejs/node/blob/main/doc/contributing/maintaining/maintaining-V8.md#backporting-to-abandoned-branches

Opened backport PR: #49862

GeoffreyBooth pushed a commit to GeoffreyBooth/node that referenced this pull request Oct 1, 2023
When built with Python 3.9 on IBM i, `process.platform` will return
`os400` instead of `aix`. In preparation for this, make `common.isAIX`
only return true for AIX and update the tests to add checks for
`common.isIBMi` where they were missing.

PR-URL: nodejs#48056
Refs: nodejs#46739
Refs: nodejs/build#3358
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
GeoffreyBooth pushed a commit to GeoffreyBooth/node that referenced this pull request Oct 1, 2023
Python 3.9 on IBM i returns "os400" for `sys.platform`.

PR-URL: nodejs#48056
Refs: nodejs#46739
Refs: nodejs/build#3358
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
targos added a commit to targos/node that referenced this pull request Oct 5, 2023
Original commit message:

    fix: EmbeddedTargetOs on IBM i with Python 3.9

    For some context, Python 3.9 on IBM i returns "os400" for sys.platform
    instead of "aix". We used to build with Python 3.6 which returned "aix"
    as the platform

    When attempting to build Node.js with python 3.9 on IBM i we run into a
    build error.

    Ref: nodejs#48056
    Ref: nodejs#48056 (comment)

    I'm not quite sure where target_os is being passed down to the function ToEmbeddedTargetOs.
    It seems as though target_os is being generated from sys.platform or
    similar call from python as we started running into this issue after
    building with Python 3.9.

    This PR supersedes initial changes proposed in:
    https://chromium-review.googlesource.com/c/v8/v8/+/4259330

    This PR contains the minimal changes to successfully build Node.js (builds v8 as an internal dep)
    on IBM i with Python 3.9.

    Change-Id: I32d43197bce994a72a0d85091e91f80eeea4482d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4846413
    Commit-Queue: Jakob Linke <[email protected]>
    Reviewed-by: Michael Achenbach <[email protected]>
    Reviewed-by: Jakob Linke <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#89981}

Refs: v8/v8@8ec2651
targos added a commit to targos/node that referenced this pull request Oct 7, 2023
Original commit message:

    fix: EmbeddedTargetOs on IBM i with Python 3.9

    For some context, Python 3.9 on IBM i returns "os400" for sys.platform
    instead of "aix". We used to build with Python 3.6 which returned "aix"
    as the platform

    When attempting to build Node.js with python 3.9 on IBM i we run into a
    build error.

    Ref: nodejs#48056
    Ref: nodejs#48056 (comment)

    I'm not quite sure where target_os is being passed down to the function ToEmbeddedTargetOs.
    It seems as though target_os is being generated from sys.platform or
    similar call from python as we started running into this issue after
    building with Python 3.9.

    This PR supersedes initial changes proposed in:
    https://chromium-review.googlesource.com/c/v8/v8/+/4259330

    This PR contains the minimal changes to successfully build Node.js (builds v8 as an internal dep)
    on IBM i with Python 3.9.

    Change-Id: I32d43197bce994a72a0d85091e91f80eeea4482d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4846413
    Commit-Queue: Jakob Linke <[email protected]>
    Reviewed-by: Michael Achenbach <[email protected]>
    Reviewed-by: Jakob Linke <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#89981}

Refs: v8/v8@8ec2651
targos added a commit to targos/node that referenced this pull request Oct 10, 2023
Original commit message:

    fix: EmbeddedTargetOs on IBM i with Python 3.9

    For some context, Python 3.9 on IBM i returns "os400" for sys.platform
    instead of "aix". We used to build with Python 3.6 which returned "aix"
    as the platform

    When attempting to build Node.js with python 3.9 on IBM i we run into a
    build error.

    Ref: nodejs#48056
    Ref: nodejs#48056 (comment)

    I'm not quite sure where target_os is being passed down to the function ToEmbeddedTargetOs.
    It seems as though target_os is being generated from sys.platform or
    similar call from python as we started running into this issue after
    building with Python 3.9.

    This PR supersedes initial changes proposed in:
    https://chromium-review.googlesource.com/c/v8/v8/+/4259330

    This PR contains the minimal changes to successfully build Node.js (builds v8 as an internal dep)
    on IBM i with Python 3.9.

    Change-Id: I32d43197bce994a72a0d85091e91f80eeea4482d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4846413
    Commit-Queue: Jakob Linke <[email protected]>
    Reviewed-by: Michael Achenbach <[email protected]>
    Reviewed-by: Jakob Linke <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#89981}

Refs: v8/v8@8ec2651
targos added a commit that referenced this pull request Oct 10, 2023
Original commit message:

    fix: EmbeddedTargetOs on IBM i with Python 3.9

    For some context, Python 3.9 on IBM i returns "os400" for sys.platform
    instead of "aix". We used to build with Python 3.6 which returned "aix"
    as the platform

    When attempting to build Node.js with python 3.9 on IBM i we run into a
    build error.

    Ref: #48056
    Ref: #48056 (comment)

    I'm not quite sure where target_os is being passed down to the function ToEmbeddedTargetOs.
    It seems as though target_os is being generated from sys.platform or
    similar call from python as we started running into this issue after
    building with Python 3.9.

    This PR supersedes initial changes proposed in:
    https://chromium-review.googlesource.com/c/v8/v8/+/4259330

    This PR contains the minimal changes to successfully build Node.js (builds v8 as an internal dep)
    on IBM i with Python 3.9.

    Change-Id: I32d43197bce994a72a0d85091e91f80eeea4482d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4846413
    Commit-Queue: Jakob Linke <[email protected]>
    Reviewed-by: Michael Achenbach <[email protected]>
    Reviewed-by: Jakob Linke <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#89981}

Refs: v8/v8@8ec2651
PR-URL: #49639
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
targos pushed a commit that referenced this pull request Oct 28, 2023
When built with Python 3.9 on IBM i, `process.platform` will return
`os400` instead of `aix`. In preparation for this, make `common.isAIX`
only return true for AIX and update the tests to add checks for
`common.isIBMi` where they were missing.

PR-URL: #48056
Refs: #46739
Refs: nodejs/build#3358
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
targos pushed a commit that referenced this pull request Oct 28, 2023
Python 3.9 on IBM i returns "os400" for `sys.platform`.

PR-URL: #48056
Refs: #46739
Refs: nodejs/build#3358
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
Original commit message:

    fix: EmbeddedTargetOs on IBM i with Python 3.9

    For some context, Python 3.9 on IBM i returns "os400" for sys.platform
    instead of "aix". We used to build with Python 3.6 which returned "aix"
    as the platform

    When attempting to build Node.js with python 3.9 on IBM i we run into a
    build error.

    Ref: nodejs#48056
    Ref: nodejs#48056 (comment)

    I'm not quite sure where target_os is being passed down to the function ToEmbeddedTargetOs.
    It seems as though target_os is being generated from sys.platform or
    similar call from python as we started running into this issue after
    building with Python 3.9.

    This PR supersedes initial changes proposed in:
    https://chromium-review.googlesource.com/c/v8/v8/+/4259330

    This PR contains the minimal changes to successfully build Node.js (builds v8 as an internal dep)
    on IBM i with Python 3.9.

    Change-Id: I32d43197bce994a72a0d85091e91f80eeea4482d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4846413
    Commit-Queue: Jakob Linke <[email protected]>
    Reviewed-by: Michael Achenbach <[email protected]>
    Reviewed-by: Jakob Linke <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#89981}

Refs: v8/v8@8ec2651
PR-URL: nodejs#49862
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
When built with Python 3.9 on IBM i, `process.platform` will return
`os400` instead of `aix`. In preparation for this, make `common.isAIX`
only return true for AIX and update the tests to add checks for
`common.isIBMi` where they were missing.

PR-URL: nodejs#48056
Refs: nodejs#46739
Refs: nodejs/build#3358
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
Python 3.9 on IBM i returns "os400" for `sys.platform`.

PR-URL: nodejs#48056
Refs: nodejs#46739
Refs: nodejs/build#3358
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
Original commit message:

    fix: EmbeddedTargetOs on IBM i with Python 3.9

    For some context, Python 3.9 on IBM i returns "os400" for sys.platform
    instead of "aix". We used to build with Python 3.6 which returned "aix"
    as the platform

    When attempting to build Node.js with python 3.9 on IBM i we run into a
    build error.

    Ref: nodejs#48056
    Ref: nodejs#48056 (comment)

    I'm not quite sure where target_os is being passed down to the function ToEmbeddedTargetOs.
    It seems as though target_os is being generated from sys.platform or
    similar call from python as we started running into this issue after
    building with Python 3.9.

    This PR supersedes initial changes proposed in:
    https://chromium-review.googlesource.com/c/v8/v8/+/4259330

    This PR contains the minimal changes to successfully build Node.js (builds v8 as an internal dep)
    on IBM i with Python 3.9.

    Change-Id: I32d43197bce994a72a0d85091e91f80eeea4482d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4846413
    Commit-Queue: Jakob Linke <[email protected]>
    Reviewed-by: Michael Achenbach <[email protected]>
    Reviewed-by: Jakob Linke <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#89981}

Refs: v8/v8@8ec2651
PR-URL: nodejs#49639
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
targos pushed a commit that referenced this pull request Nov 11, 2023
Original commit message:

    fix: EmbeddedTargetOs on IBM i with Python 3.9

    For some context, Python 3.9 on IBM i returns "os400" for sys.platform
    instead of "aix". We used to build with Python 3.6 which returned "aix"
    as the platform

    When attempting to build Node.js with python 3.9 on IBM i we run into a
    build error.

    Ref: #48056
    Ref: #48056 (comment)

    I'm not quite sure where target_os is being passed down to the function ToEmbeddedTargetOs.
    It seems as though target_os is being generated from sys.platform or
    similar call from python as we started running into this issue after
    building with Python 3.9.

    This PR supersedes initial changes proposed in:
    https://chromium-review.googlesource.com/c/v8/v8/+/4259330

    This PR contains the minimal changes to successfully build Node.js (builds v8 as an internal dep)
    on IBM i with Python 3.9.

    Change-Id: I32d43197bce994a72a0d85091e91f80eeea4482d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4846413
    Commit-Queue: Jakob Linke <[email protected]>
    Reviewed-by: Michael Achenbach <[email protected]>
    Reviewed-by: Jakob Linke <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#89981}

Refs: v8/v8@8ec2651
PR-URL: #49862
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
targos pushed a commit that referenced this pull request Nov 11, 2023
When built with Python 3.9 on IBM i, `process.platform` will return
`os400` instead of `aix`. In preparation for this, make `common.isAIX`
only return true for AIX and update the tests to add checks for
`common.isIBMi` where they were missing.

PR-URL: #48056
Refs: #46739
Refs: nodejs/build#3358
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
targos pushed a commit that referenced this pull request Nov 11, 2023
Python 3.9 on IBM i returns "os400" for `sys.platform`.

PR-URL: #48056
Refs: #46739
Refs: nodejs/build#3358
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
debadree25 pushed a commit to debadree25/node that referenced this pull request Apr 15, 2024
Original commit message:

    fix: EmbeddedTargetOs on IBM i with Python 3.9

    For some context, Python 3.9 on IBM i returns "os400" for sys.platform
    instead of "aix". We used to build with Python 3.6 which returned "aix"
    as the platform

    When attempting to build Node.js with python 3.9 on IBM i we run into a
    build error.

    Ref: nodejs#48056
    Ref: nodejs#48056 (comment)

    I'm not quite sure where target_os is being passed down to the function ToEmbeddedTargetOs.
    It seems as though target_os is being generated from sys.platform or
    similar call from python as we started running into this issue after
    building with Python 3.9.

    This PR supersedes initial changes proposed in:
    https://chromium-review.googlesource.com/c/v8/v8/+/4259330

    This PR contains the minimal changes to successfully build Node.js (builds v8 as an internal dep)
    on IBM i with Python 3.9.

    Change-Id: I32d43197bce994a72a0d85091e91f80eeea4482d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4846413
    Commit-Queue: Jakob Linke <[email protected]>
    Reviewed-by: Michael Achenbach <[email protected]>
    Reviewed-by: Jakob Linke <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#89981}

Refs: v8/v8@8ec2651
PR-URL: nodejs#49862
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
debadree25 pushed a commit to debadree25/node that referenced this pull request Apr 15, 2024
When built with Python 3.9 on IBM i, `process.platform` will return
`os400` instead of `aix`. In preparation for this, make `common.isAIX`
only return true for AIX and update the tests to add checks for
`common.isIBMi` where they were missing.

PR-URL: nodejs#48056
Refs: nodejs#46739
Refs: nodejs/build#3358
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
debadree25 pushed a commit to debadree25/node that referenced this pull request Apr 15, 2024
Python 3.9 on IBM i returns "os400" for `sys.platform`.

PR-URL: nodejs#48056
Refs: nodejs#46739
Refs: nodejs/build#3358
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
debadree25 pushed a commit to debadree25/node that referenced this pull request Apr 15, 2024
Original commit message:

    fix: EmbeddedTargetOs on IBM i with Python 3.9

    For some context, Python 3.9 on IBM i returns "os400" for sys.platform
    instead of "aix". We used to build with Python 3.6 which returned "aix"
    as the platform

    When attempting to build Node.js with python 3.9 on IBM i we run into a
    build error.

    Ref: nodejs#48056
    Ref: nodejs#48056 (comment)

    I'm not quite sure where target_os is being passed down to the function ToEmbeddedTargetOs.
    It seems as though target_os is being generated from sys.platform or
    similar call from python as we started running into this issue after
    building with Python 3.9.

    This PR supersedes initial changes proposed in:
    https://chromium-review.googlesource.com/c/v8/v8/+/4259330

    This PR contains the minimal changes to successfully build Node.js (builds v8 as an internal dep)
    on IBM i with Python 3.9.

    Change-Id: I32d43197bce994a72a0d85091e91f80eeea4482d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4846413
    Commit-Queue: Jakob Linke <[email protected]>
    Reviewed-by: Michael Achenbach <[email protected]>
    Reviewed-by: Jakob Linke <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#89981}

Refs: v8/v8@8ec2651
PR-URL: nodejs#49639
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
When built with Python 3.9 on IBM i, `process.platform` will return
`os400` instead of `aix`. In preparation for this, make `common.isAIX`
only return true for AIX and update the tests to add checks for
`common.isIBMi` where they were missing.

PR-URL: nodejs/node#48056
Refs: nodejs/node#46739
Refs: nodejs/build#3358
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
Python 3.9 on IBM i returns "os400" for `sys.platform`.

PR-URL: nodejs/node#48056
Refs: nodejs/node#46739
Refs: nodejs/build#3358
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
When built with Python 3.9 on IBM i, `process.platform` will return
`os400` instead of `aix`. In preparation for this, make `common.isAIX`
only return true for AIX and update the tests to add checks for
`common.isIBMi` where they were missing.

PR-URL: nodejs/node#48056
Refs: nodejs/node#46739
Refs: nodejs/build#3358
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
Python 3.9 on IBM i returns "os400" for `sys.platform`.

PR-URL: nodejs/node#48056
Refs: nodejs/node#46739
Refs: nodejs/build#3358
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. dependencies Pull requests that update a dependency file. ibm i Issues and PRs related to the IBM i platform. needs-ci PRs that need a full CI run. openssl Issues and PRs related to the OpenSSL dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants