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

v10 compiler lock file no longer cleaned up after SIGINT #5590

Closed
TheSpyder opened this issue Jul 24, 2022 · 10 comments · Fixed by #5591
Closed

v10 compiler lock file no longer cleaned up after SIGINT #5590

TheSpyder opened this issue Jul 24, 2022 · 10 comments · Fixed by #5591
Milestone

Comments

@TheSpyder
Copy link
Contributor

Occasionally I need to use ctrl+c while rescript is running (during the phase where it says rescript: [340/2391] and counts up while compiling files). It's not a normal thing to do but it happened today.

Using 9.1.4, when I do this the .bsb.lock file is cleaned up. This doesn't happen with v10, I'm using 10.0.0-rc.1. The lock file is left behind and the compiler refuses to run until it's manually deleted.

@cristianoc
Copy link
Collaborator

Thanks for the report.

This is unlikely to receive attention until everyone is back from vacation.
Feel free to open a PR in the meanwhile.

@cristianoc cristianoc added this to the v10.1 milestone Jul 24, 2022
@TheSpyder
Copy link
Contributor Author

Can you give me any clues about where to start looking? There’s a lot of changes since the last v9 release 🤔

@cristianoc
Copy link
Collaborator

cristianoc commented Jul 24, 2022

This is the build system. Which is pretty much self-contained.
You can find it in jscomp/bsb, plus whatever is driving it (ninja).
I have never looked deeply into that, so it might be a good chance to document what's there and add to contribution guides. That alone would be a great contribution, separately from the bug fix.

@cristianoc
Copy link
Collaborator

A probably related issue, that is biting people more frequently, is this: #5206

@cristianoc
Copy link
Collaborator

"related" as in: the same code is likely going to be involved
Though notice that #5206 is not a regression, it's just a design choice that is affecting a bunch of people negatively

@TheSpyder
Copy link
Contributor Author

TheSpyder commented Jul 24, 2022

Looks like lockfile management moved into the rescript node script. I wonder what might've changed there between v9 and v10 🤔

That also suggests it's not related to 5206?

@TheSpyder
Copy link
Contributor Author

Looks like it's been a flaw since the nodejs code was written. What changed is you updated it to actually create the lockfile 😆
c32cc34

@TheSpyder
Copy link
Contributor Author

TheSpyder commented Jul 24, 2022

This explains why I've had occasional issues with build conflicts since the switch from bsb to rescript 🤔 there was no locking at all.

TheSpyder added a commit to TheSpyder/rescript-compiler that referenced this issue Jul 24, 2022
TheSpyder added a commit to TheSpyder/rescript-compiler that referenced this issue Jul 24, 2022
@cristianoc
Copy link
Collaborator

Looks like it's been a flaw since the nodejs code was written. What changed is you updated it to actually create the lockfile 😆

c32cc34

That was a superficial fix to at least restore the intent of the code.
I was looking for other issues at the moment. To do with tests never terminating, and was wondering whether lock files were implicated (they were not).

@TheSpyder
Copy link
Contributor Author

I've nearly cracked it. The only management of SIGINT is done in watch mode. I just need to move the code around so it also monitors for interrupts in one-shot mode.

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

Successfully merging a pull request may close this issue.

2 participants