Skip to content

Conversation

@gabylb
Copy link
Contributor

@gabylb gabylb commented Sep 30, 2022

Checklist
Description of change

Open XL C/C++ for z/OS (C invocation: clang, ibm-clang, or ibm-clang64) is LLVM-based, and doesn't support XL C/C++'s -q flags. Environment variable CC is set starting with Node.js v18 for z/OS which requires clang or ibm-clang64 compiler, but CC may not be set in older versions (which required njsc or xlclang), hence default to njsc to support older versions.

Also fix defines of _ALL_SOURCE and _UNIX03_SOURCE to match how they're used.

Accompanying change to gyp/pylib/gyp/generator/make.py now deleted from this PR per #2742 (review) and now in nodejs/gyp-next#178.

Open XL C/C++ for z/OS (C invocation: clang, ibm-clang, or ibm-clang64)
is LLVM-based, and doesn't support XL C/C++'s -q flags. Environment
variable CC is set starting with Node.js v18 for z/OS which requires
clang or ibm-clang64 compiler, but CC may not be set in older versions
(which required njsc or xlclang), hence default to njsc to support older
versions.

Also fix defines of _ALL_SOURCE and _UNIX03_SOURCE to match how they're
used.
@gabylb
Copy link
Contributor Author

gabylb commented Oct 3, 2022

Hi Richard @richardlau please review when you have a chance.

Copy link
Member

@richardlau richardlau left a comment

Choose a reason for hiding this comment

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

Looks okay but the changes under gyp should be upstreamed to https://github.com/nodejs/gyp-next.

@gabylb
Copy link
Contributor Author

gabylb commented Oct 3, 2022

Looks okay but the changes under gyp should be upstreamed to https://github.com/nodejs/gyp-next.

Done, please see nodejs/gyp-next#178.

@gabylb gabylb requested a review from richardlau October 3, 2022 17:21
@gabylb
Copy link
Contributor Author

gabylb commented Oct 3, 2022

I deleted make.py rather than reverting its change; will resolve in another PR.

@gabylb gabylb closed this Oct 3, 2022
@richardlau richardlau mentioned this pull request Oct 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants