Visitar URL original
src: update node.cc and related for z/OS by gabylb · Pull Request #66569 · nodejs/node · GitHub
Skip to content

src: update node.cc and related for z/OS - #66569

Open
gabylb wants to merge 2 commits into
nodejs:mainfrom
gabylb:zos-src
Open

src: update node.cc and related for z/OS#66569
gabylb wants to merge 2 commits into
nodejs:mainfrom
gabylb:zos-src

Conversation

@gabylb

@gabylb gabylb commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Updates here apply to z/OS only.

  • Add new src/node_zos.h and src/node_zos.cc, used by src/node.cc.
  • Add new src/zos_setlibpath.cc and link its .o directly into node so the __setlibpath object's constructor finds libnode.so and add its path to LIBPATH, so it can be loaded.
  • Pass '-Wl,-bedit=no' when linking libnode.so to reduce its size.
  • Use z/OS specific signal handling logic, as signals on z/OS must be handled in a dedicated thread.
  • Force V8 flag --nohard_abort due to absence of XPLINK headers in JS generated code.
  • Include display of zoslib build version.

@nodejs-github-bot

nodejs-github-bot commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Oct 7, 2026
@inoway46

inoway46 commented Oct 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for working on this! The commit and lint checks are currently failing. Have you also verified that this builds successfully locally?

Please follow the guide below:
https://github.com/nodejs/node/blob/main/doc/contributing/pull-requests.md

@gabylb
gabylb marked this pull request as draft October 7, 2026 15:19
@codecov

codecov Bot commented Oct 7, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.43%. Comparing base (60c7958) to head (36e2894).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #66569      +/-   ##
==========================================
- Coverage   92.77%   90.43%   -2.34%     
==========================================
  Files         422      791     +369     
  Lines      193594   276487   +82893     
  Branches    29857    53088   +23231     
==========================================
+ Hits       179604   250053   +70449     
- Misses      13662    16847    +3185     
- Partials      328     9587    +9259
Files with missing lines Coverage Δ
src/node.cc 79.24% <ø> (ø)

... and 498 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gabylb

gabylb commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for working on this! The commit and lint checks are currently failing. Have you also verified that this builds successfully locally?

Please follow the guide below: https://github.com/nodejs/node/blob/main/doc/contributing/pull-requests.md

Hello Yuya. This PR is part of a number of PRs to be opened for the z/OS port; in our environment, which includes all the port, node builds successfully.

@gabylb
gabylb marked this pull request as ready for review October 7, 2026 19:05
@gabylb
src: update node.cc and related for z/OS
f5c1ac1 
- Add new src/node_zos.h and src/node_zos.cc, used by src/node.cc.
- Add new src/zos_setlibpath.cc and link its .o directly into node
  so the __setlibpath object's constructor finds libnode.so
  and add its path to LIBPATH, so it can be loaded.
- Pass '-Wl,-bedit=no' when linking libnode.so to reduce its size.
- Use z/OS specific signal handling logic, as signals on z/OS must be
  handled in a dedicated thread.
- Force V8 flag --nohard_abort due to absence of XPLINK headers in JS
  generated code.
- Include display of zoslib build version.

Signed-off-by: Gaby Baghdadi <baghdadi@ca.ibm.com>
@gabylb
src: address review comments
36e2894 
Signed-off-by: Gaby Baghdadi <baghdadi@ca.ibm.com>
@richardlau richardlau mentioned this pull request Oct 8, 2026
Open
addaleax
addaleax reviewed Oct 8, 2026
View reviewed changes
Comment thread src/node.cc
sa.sa_sigaction = handler;
sa.sa_flags = reset_handler ? SA_RESETHAND : 0;
#ifdef __MVS__
sa.sa_flags |= SA_ONSTACK | SA_SIGINFO;

@addaleax addaleax Oct 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A lot of these changes could use comments that explain why specifically z/OS needs these and other OSes don't – e.g. SA_ONSTACK and SA_SIGINFO aren't z/OS-specific per se, so it's not obvious why the conditional is here (I might guess because of SIGABRT being listed below as a handled signal – but let's not have this be a silent assumption?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@addaleax addaleax addaleax left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gabylb @nodejs-github-bot @inoway46 @addaleax