-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Fix lock file error with test/runner and EMCC_DEBUG mode. #26110
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
Conversation
be30cd6 to
4d48571
Compare
test/runner.py
Outdated
| """Deletes all files and directories under Emscripten | ||
| that look like they might have been created by Emscripten.""" | ||
| if shared.DEBUG or common.EMTEST_SAVE_DIR: | ||
| return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add comment here? What is the lock file in question?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
emscripten.lock from FileLock
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it would be useful to add a logging.debug('Not cleaning temp since running in debug/EM_SAVE_DIR mode') line before return?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a huge fan of auto deleting stuff at the beginning. I'd prefer tests clean up on their own at the end, so I was thinking we reverse this, where if a file is found, we error out, add an option that clears the files on start. e.g. "Test runner found emscripten temporary files. Re-run with --clear-temp-files to remove them."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A related PR which should dramatically reduce the number of emscripten-related tmp directories: #25615
4d48571 to
8cce438
Compare
|
Ooh, this might be what is causing the issue in my CI failing in a number of parallel tests, where some tests fail to link, complaining of a missing library. |
|
What kind of scenario are you seeing this happen, actually? Is this when you do a parallel test run? Or when you manually set EMCC_DEBUG before doing a parallel run? Or are there some individual tests that enable EMCC_DEBUG on their own? (I think we have/had some of those at some point) |
Running a single test with EMCC_DEBUG triggers it for me. e.g. |
`cleanup_emscripten_temp()` is run at the beginning of tests and caused the lock file that was created to be removed. Then when test was finished it attempted to delete that lock file.
8cce438 to
46759a6
Compare
cleanup_emscripten_temp()was run at the beginning of tests and caused theemscripten.lockfile that was created earlier to be removed. Then when test was finished it attempted to delete that lock file again.