Skip to content
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

Freezing when too many files are present with "rescript format -all" command #6685

Closed
JonoPrest opened this issue Mar 18, 2024 · 3 comments · Fixed by #7081
Closed

Freezing when too many files are present with "rescript format -all" command #6685

JonoPrest opened this issue Mar 18, 2024 · 3 comments · Fixed by #7081
Assignees
Labels
Milestone

Comments

@JonoPrest
Copy link
Contributor

How does the issue come about?

We have a codegenerator tool that outputs rescript files unformatted. It then runs rescript format -all without running rescript build which happens right at the end.

We have been able to reproduce it reliably on one of our devs machines using a simple test repo:
https://github.com/enviodev/rescript-format-too-many-files

On his machine (M2 macbook), it seems to happen reliably when there are roughly > 65 .res files or so.

What we've found is that some user's shell environments will completely freeze. Even if you exit the the process, you can no longer run any other npx commands. (Even from a new shell).

Rescript versions tested:
10.1.4, 11.0.1 and 11.1.0-rc.4

Environment:
So far we've only seen this reproduce with 2 users on M1 and M2 apple silicon macbooks.

Steps to reproduce:

  1. clone the repo
  2. install node modules
  3. run rescript format -all WITHOUT building

There is a shell script in the repo for generating more files if need be for testing.

If you build the code before running rescript format -all everything works as expected.

@cknitt
Copy link
Member

cknitt commented May 29, 2024

@JonoPrest It seems rescript format runs the format for all files in parallel, see https://github.com/rescript-lang/rescript-compiler/blob/c7dedbbfdf89f9cfa01205f2850f82fbb77cb651/scripts/rescript_format.js#L79

This should be easy to solve by doing it in batches instead.

However, the question is this shouldn't rather go into rewatch. /cc @jfrolich @rolandpeelen

@JonoPrest
Copy link
Contributor Author

@JonoPrest It seems rescript format runs the format for all files in parallel, see

https://github.com/rescript-lang/rescript-compiler/blob/c7dedbbfdf89f9cfa01205f2850f82fbb77cb651/scripts/rescript_format.js#L79

This should be easy to solve by doing it in batches instead.

However, the question is this shouldn't rather go into rewatch. /cc @jfrolich @rolandpeelen
Happy to patch this if the script will still be used but I assume rewatch will handle it?

@fhammerschmidt
Copy link
Member

Well, I think we could even have the fix in v11.1.2, so adapting the script might still be worthwhile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants