-
Notifications
You must be signed in to change notification settings - Fork 4
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
Fix Julia sysimage creation #152
Conversation
Creating new Julia sysimages was temporarily disabled because it wasn't working. This restores the functionality. - Remove the early return from _create_sysimage() to enable the creation. - Fix missing job id when creating Spine Engine Worker. There's a corresponding Toolbox side fix as well - Fix Windows paths embedded in Julia code: we must escape '\' in Julia's strings. - Don't supply Julia with the --project keyword if we don't have a project (the project is actually @.). There's a corresponding Engine side fix as well. - Code beautification. Re spine-tools/Spine-Toolbox#1225
@manuelma I think I got the sysimage stuff working but I would appreciate it a lot if you could review the changes and perhaps test if everything is working as expected. Especially, the Julia code in |
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## master #152 +/- ##
=======================================
Coverage 55.10% 55.11%
=======================================
Files 192 192
Lines 17371 17376 +5
Branches 2849 2850 +1
=======================================
+ Hits 9572 9576 +4
- Misses 7304 7305 +1
Partials 495 495
☔ View full report in Codecov by Sentry. |
Looks good @soininen - I inspected the code and made a couple of comments - only one of them is a suggestion for a change. Maybe with more time I can test it and see how it works in reality, but my feeling is it should be close enough. |
@manuelma Thanks a lot for the review! I think we're good to go with this PR. |
This PR resurrects Julia sysimage creation.
Accompanying PRs:
spine-tools/spine-engine#111
spine-tools/Spine-Toolbox#2246
Fixes spine-tools/Spine-Toolbox#1225
Checklist before merging