Skip to content

fix: Fixed potential bug in check-emfile-handling.js #19975

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 1 commit into from
Jul 29, 2025

Conversation

suwakei
Copy link
Contributor

@suwakei suwakei commented Jul 28, 2025

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[x] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

Problem:
On Linux and macOS, the command ulimit -n checks the maximum number of files that can be opened at one time. However, if this setting is set to "unlimited," the script will try to convert the string "unlimited" to a number and will fail (resulting in NaN) and will not correctly calculate the number of files to use in the test.

Fix:
If the result of ulimit -n cannot be treated as a number, modify it to use the predetermined default number of files (15,000) without forcing the calculation.

// if the platform isn't windows, get the ulimit to see what the actual limit is
if (os.platform() !== "win32") {
	try {
		--- FILE_COUNT = parseInt(execSync("ulimit -n").toString().trim(), 10) + 1;
		+++ const limit = execSync("ulimit -n").toString().trim();
		+++ const parsedLimit = parseInt(limit, 10);

		+++ // "unlimited" will result in NaN, in which case use the default value
		+++ if (!isNaN(parsedLimit)) {
			+++ FILE_COUNT = parsedLimit + 1;
		+++ }

		console.log(`Detected Linux file limit of ${FILE_COUNT}.`);

@suwakei suwakei requested a review from a team as a code owner July 28, 2025 22:52
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Jul 28, 2025
@eslint-github-bot eslint-github-bot bot added the bug ESLint is working incorrectly label Jul 28, 2025
Copy link

linux-foundation-easycla bot commented Jul 28, 2025

CLA Signed


The committers listed above are authorized under a signed CLA.

Copy link

netlify bot commented Jul 28, 2025

Deploy Preview for docs-eslint ready!

Name Link
🔨 Latest commit
🔍 Latest deploy log https://app.netlify.com/projects/docs-eslint/deploys/6888db4395a6d60c95076d9c
😎 Deploy Preview https://deploy-preview-19975--docs-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@lumirlumir
Copy link
Member

Hi, @suwakei. Thanks for opening the PR.

It looks like the CI is currently failing. Could you take a look at it when you have some time?

@suwakei
Copy link
Contributor Author

suwakei commented Jul 29, 2025

@lumirlumir
Thank you for your comment
I'll check it !

@suwakei
Copy link
Contributor Author

suwakei commented Jul 29, 2025

The CI failure seems to be caused by a 503 error when trying to install Python 3.13.5 via mise.
Switching to a stable version like 3.12.3 or retrying to build later when the GitHub release server is available again seems to sometimes resolve the issue.

@nzakas
Copy link
Member

nzakas commented Jul 29, 2025

The CI failure is unrelated to this PR. It looks like a momentary glitch in Netlify.

@suwakei
Copy link
Contributor Author

suwakei commented Jul 29, 2025

Thanks for confirming! Let me know if I should re-run the CI or if it's good to go as-is.

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

@nzakas
Copy link
Member

nzakas commented Jul 29, 2025

I did re-run and everything passed. It just didn't update the display.

@nzakas nzakas merged commit e8a6914 into eslint:main Jul 29, 2025
26 of 30 checks passed
@github-project-automation github-project-automation bot moved this from Needs Triage to Complete in Triage Jul 29, 2025
@suwakei suwakei deleted the fix/check-emfile-handling.js branch August 7, 2025 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug ESLint is working incorrectly
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

3 participants