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

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

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 •

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 •

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
- 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>
Signed-off-by: Gaby Baghdadi <baghdadi@ca.ibm.com>
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;

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Please see https://www.ibm.com/docs/en/zos/3.1.0?topic=functions-sigaction-examine-change-signal-action.
For SA_SIGINFO, sa_sigaction is specified for the sigaction() call here. For SA_ONSTACK, it's to use the alternate signal stack created initially in zosStart(), to not interfere with the z/OS Language Environment runtime environment.

Would you like such comments added in the code, or in the commit's message?

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

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

Development

Successfully merging this pull request may close these issues.

4 participants