Skip to content

process: disable building execve on IBM i #57883

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 18, 2025
Merged

process: disable building execve on IBM i #57883

merged 2 commits into from
Apr 18, 2025

Conversation

abmusse
Copy link
Contributor

@abmusse abmusse commented Apr 15, 2025

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 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
@abmusse
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 abmusse changed the title refactor: disable building execve on IBM i process: disable building execve on IBM i Apr 15, 2025
@abmusse
Copy link
Contributor Author

abmusse commented Apr 15, 2025

CC

@nodejs/platform-ibmi

@codecov Codecov
Copy link

codecov bot commented Apr 15, 2025 •
edited
Loading

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
mhdawson approved these changes Apr 15, 2025
View reviewed changes
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

@mhdawson mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 15, 2025
@github-actions 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
Copy link
Collaborator

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

@abmusse
Copy link
Contributor Author

abmusse commented Apr 15, 2025 •
edited
Loading

@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
Copy link
Contributor Author

abmusse commented Apr 16, 2025

After adding ed3865c build is passing again

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

ShogunPanda
ShogunPanda approved these changes Apr 16, 2025
View reviewed changes
Copy link
Contributor

@ShogunPanda ShogunPanda left a comment

Choose a reason for hiding this comment

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

LGTM!

mhdawson
mhdawson approved these changes Apr 16, 2025
View reviewed changes
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 again after update

@mhdawson mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 16, 2025
@github-actions 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
Copy link
Collaborator

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

@lpinca 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 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 nodejs-github-bot merged commit 963b24e into nodejs:main Apr 18, 2025
63 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 963b24e

@abmusse abmusse deleted the ibmi-execve branch April 19, 2025 00:31
RafaelGSS pushed a commit that referenced this pull request May 1, 2025
@abmusse @RafaelGSS
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 <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request May 1, 2025
Merged
RafaelGSS pushed a commit that referenced this pull request May 2, 2025
@abmusse @RafaelGSS
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 <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
aduh95 pushed a commit that referenced this pull request May 6, 2025
@abmusse @aduh95
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 <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
aduh95 pushed a commit that referenced this pull request May 6, 2025
@abmusse @aduh95
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 <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
RafaelGSS pushed a commit that referenced this pull request May 14, 2025
@abmusse @RafaelGSS
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 <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
aduh95 pushed a commit that referenced this pull request May 16, 2025
@abmusse @aduh95
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 <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
aduh95 pushed a commit that referenced this pull request May 17, 2025
@abmusse @aduh95
process: disable building execve on IBM i
41468be 
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 <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
aduh95 pushed a commit that referenced this pull request May 17, 2025
@abmusse @aduh95
process: disable building execve on IBM i
9ccd9af 
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 <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
aduh95 pushed a commit that referenced this pull request May 17, 2025
@abmusse @aduh95
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 <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
aduh95 pushed a commit that referenced this pull request May 18, 2025
@abmusse @aduh95
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 <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
@github-actions github-actions bot mentioned this pull request May 19, 2025
Merged
aduh95 pushed a commit that referenced this pull request May 19, 2025
@abmusse @aduh95
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 <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
@marco-ippolito marco-ippolito added the dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. label Jun 5, 2025
@ghost ghost mentioned this pull request Jun 8, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ShogunPanda ShogunPanda ShogunPanda approved these changes

@lpinca lpinca lpinca approved these changes

@mhdawson mhdawson mhdawson approved these changes

@richardlau richardlau Awaiting requested review from richardlau

+1 more reviewer

@bjohansebas 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. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. 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
@abmusse @nodejs-github-bot @ShogunPanda @lpinca @mhdawson @bjohansebas @marco-ippolito