FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

process: disable building execve on IBM i by abmusse · Pull Request #57883 · nodejs/node · GitHub

/ node Public

process: disable building execve on IBM i - #57883

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
abmusse:ibmi-execve
Apr 18, 2025
ConversationCommits (2)ChecksFiles changed
Merged

process: disable building execve on IBM i#57883
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
abmusse:ibmi-execve

Conversation

abmusse commented Apr 15, 2025

Copy link
Copy Markdown
Contributor

The execve syscall does exist on IBM i but
it has caveats that make it not usable in Node.js context.

These changes disable building with execve like Windows does.

Fixes #57882

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Apr 15, 2025
process: disable building execve on IBM i
c8ffb7d 
The `execve` syscall does exist on IBM i but
it has caveats that make it not usable in Node.js context.

These changes disable building with `execve` like Windows does.
abmusse changed the title refactor: disable building execve on IBM i process: disable building execve on IBM i Apr 15, 2025

abmusse commented Apr 15, 2025

Copy link
Copy Markdown
Contributor Author

CC

@nodejs/platform-ibmi

codecov Bot commented Apr 15, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.24%. Comparing base (b2405e9) to head (ed3865c).
Report is 431 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #57883      +/-   ##
==========================================
+ Coverage   90.23%   90.24%   +0.01%     
==========================================
  Files         630      630              
  Lines      185688   185687       -1     
  Branches    36405    36410       +5     
==========================================
+ Hits       167559   167581      +22     
- Misses      11000    11005       +5     
+ Partials     7129     7101      -28
Files with missing lines Coverage Δ
lib/internal/process/per_thread.js 99.45% <100.00%> (ø)
src/node_process_methods.cc 87.80% <ø> (ø)

... and 24 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.

mhdawson approved these changes Apr 15, 2025
View reviewed changes

mhdawson left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 15, 2025
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 15, 2025

nodejs-github-bot commented Apr 15, 2025

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/66284/

abmusse commented Apr 15, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@mhdawson

I ran test build on IBM i and looks like this test failed parallel.test-process-execve-validation

https://ci.nodejs.org/job/node-test-commit-ibmi/nodes=ibmi74-ppc64/1897/testReport/(root)/parallel/test_process_execve_validation/

I think I need to add to this condition for IBM i here:

node/lib/internal/process/per_thread.js

Lines 289 to 291 in 91d8a52

} else if (process.platform === 'win32') {
throw new ERR_FEATURE_UNAVAILABLE_ON_PLATFORM('process.execve');
}

abmusse commented Apr 16, 2025

Copy link
Copy Markdown
Contributor Author

After adding ed3865c build is passing again

https://ci.nodejs.org/job/node-test-commit-ibmi/nodes=ibmi74-ppc64/1899/console

ShogunPanda approved these changes Apr 16, 2025
View reviewed changes

ShogunPanda left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM!

mhdawson approved these changes Apr 16, 2025
View reviewed changes

mhdawson left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM again after update

mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 16, 2025
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 16, 2025

nodejs-github-bot commented Apr 16, 2025

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/66320/

lpinca added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Apr 18, 2025
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 18, 2025
nodejs-github-bot merged commit 963b24e into nodejs:main Apr 18, 2025

nodejs-github-bot commented Apr 18, 2025

Copy link
Copy Markdown
Collaborator

Landed in 963b24e

abmusse deleted the ibmi-execve branch April 19, 2025 00:31
RafaelGSS pushed a commit that referenced this pull request May 1, 2025
process: disable building execve on IBM i
93f5b46 
The `execve` syscall does exist on IBM i but
it has caveats that make it not usable in Node.js context.

These changes disable building with `execve` like Windows does.

PR-URL: #57883
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
RafaelGSS mentioned this pull request May 1, 2025
Merged
RafaelGSS pushed a commit that referenced this pull request May 2, 2025
process: disable building execve on IBM i
8b83ab3 
The `execve` syscall does exist on IBM i but
it has caveats that make it not usable in Node.js context.

These changes disable building with `execve` like Windows does.

PR-URL: #57883
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
aduh95 pushed a commit that referenced this pull request May 6, 2025
process: disable building execve on IBM i
0f87f03 
The `execve` syscall does exist on IBM i but
it has caveats that make it not usable in Node.js context.

These changes disable building with `execve` like Windows does.

PR-URL: #57883
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
aduh95 pushed a commit that referenced this pull request May 6, 2025
process: disable building execve on IBM i
c72aa1f 
The `execve` syscall does exist on IBM i but
it has caveats that make it not usable in Node.js context.

These changes disable building with `execve` like Windows does.

PR-URL: #57883
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
RafaelGSS pushed a commit that referenced this pull request May 14, 2025
process: disable building execve on IBM i
8eb5139 
The `execve` syscall does exist on IBM i but
it has caveats that make it not usable in Node.js context.

These changes disable building with `execve` like Windows does.

PR-URL: #57883
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
aduh95 pushed a commit that referenced this pull request May 16, 2025
process: disable building execve on IBM i
0d24823 
The `execve` syscall does exist on IBM i but
it has caveats that make it not usable in Node.js context.

These changes disable building with `execve` like Windows does.

PR-URL: #57883
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
aduh95 pushed a commit that referenced this pull request May 17, 2025
process: disable building execve on IBM i
660d023 
The `execve` syscall does exist on IBM i but
it has caveats that make it not usable in Node.js context.

These changes disable building with `execve` like Windows does.

PR-URL: #57883
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
aduh95 pushed a commit that referenced this pull request May 18, 2025
process: disable building execve on IBM i
19a9613 
The `execve` syscall does exist on IBM i but
it has caveats that make it not usable in Node.js context.

These changes disable building with `execve` like Windows does.

PR-URL: #57883
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
github-actions Bot mentioned this pull request May 19, 2025
Merged
aduh95 pushed a commit that referenced this pull request May 19, 2025
process: disable building execve on IBM i
248c938 
The `execve` syscall does exist on IBM i but
it has caveats that make it not usable in Node.js context.

These changes disable building with `execve` like Windows does.

PR-URL: #57883
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
ghost mentioned this pull request Jun 8, 2025
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

ShogunPanda ShogunPanda approved these changes
lpinca lpinca approved these changes
richardlau Awaiting requested review from richardlau
+2 more reviewers
mhdawson mhdawson approved these changes
bjohansebas bjohansebas approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. 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.

ibmi: execve issues

7 participants

Back | FazBrowse Home | New Git URL