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.

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

Merged
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
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

Copy link

codecov bot commented Apr 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.24%. Comparing base (b2405e9) to head (ed3865c).
Report is 22 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.

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

@abmusse
Copy link
Contributor Author

abmusse commented Apr 15, 2025

@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:

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

@abmusse
Copy link
Contributor Author

abmusse commented Apr 16, 2025

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!

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

@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
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++. 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
Development

Successfully merging this pull request may close these issues.

ibmi: execve issues
6 participants