Adjust workaround for Windows precompilation on Julia 1.8.2+ #4451
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
My goal here is to still precompile on normal computers, but only skip it if there's a problem. Providing a warning will also allow the issue to be monitored, and if someone ever encounters it outside of the CI system, maybe we can learn more about the issue.
Codecov ReportBase: 80.60% // Head: 79.61% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #4451 +/- ##
==========================================
- Coverage 80.60% 79.61% -0.99%
==========================================
Files 30 41 +11
Lines 7466 8218 +752
==========================================
+ Hits 6018 6543 +525
- Misses 1448 1675 +227
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
This works to avoid the CI error but the warning doesn't show up... |
|
I'm a bit reluctant to use Can you check that this has no impact on For this |
|
Will do, though my expectation is that try/catch has 0 cost unless an exception occurs. Also will see if there's a way to get the warning to show up... |
|
Hopefully Pkg will get updated to print warnings... but in the meantime I'm trying to split out the precompilation step. No meaningful changes with your test: This PR: Master: |
|
Hmm. Tried a few things and no way so far to get precompilation to print anything unless there's an error thrown? Inquiring here: JuliaLang/Pkg.jl#3224 |
t-bltg
reviewed
Oct 11, 2022
Comment on lines
+28
to
+34
| # During precompilation on the Windows GitHub CI in Julia 1.8.2+, | ||
| # can fail here due to GR potentially failing to write out the | ||
| # temporary file. Pending a fix, swallow the error and continue. | ||
| $func() | ||
| catch | ||
| @debug "Plots: Failed a trial save during precompilation" | ||
| end |
There was a problem hiding this comment.
Suggested change
| # During precompilation on the Windows GitHub CI in Julia 1.8.2+, | |
| # can fail here due to GR potentially failing to write out the | |
| # temporary file. Pending a fix, swallow the error and continue. | |
| $func() | |
| catch | |
| @debug "Plots: Failed a trial save during precompilation" | |
| end | |
| $func() | |
| catch e | |
| if e isa SystemError && Sys.iswindows() && get(ENV, "CI", "false") == "true" | |
| # During precompilation on the Windows GitHub CI in Julia 1.8.2+, | |
| # can fail here due to GR potentially failing to write out the | |
| # temporary file. Pending a fix, swallow the error and continue. | |
| @debug "Plots: Failed a trial save during precompilation" e | |
| else | |
| rethrow() | |
| end | |
| end |
If the warning doesn't work as expected, then maybe silently ignore precompilation SystemEror on windows ci, else throw for hard error.
There was a problem hiding this comment.
That's fair. We probably want rethrow() though?
There was a problem hiding this comment.
Ha, yes I took rethrow(e) note in https://docs.julialang.org/en/v1/base/base/#Base.rethrow too seriously.
t-bltg
reviewed
Oct 11, 2022
Comment on lines
+52
to
+62
| env: | ||
| JULIA_PKG_PRECOMPILE_AUTO: 0 | ||
|
|
||
| # surface errors during precompilation | ||
| # auto-precompile in build step suppresses errors currently. | ||
| - name: Precompile | ||
| env: | ||
| JULIA_DEBUG: Plots | ||
| shell: julia --project=@. --color=yes {0} | ||
| run: | | ||
| using Pkg; Pkg.precompile() |
There was a problem hiding this comment.
Suggested change
| env: | |
| JULIA_PKG_PRECOMPILE_AUTO: 0 | |
| # surface errors during precompilation | |
| # auto-precompile in build step suppresses errors currently. | |
| - name: Precompile | |
| env: | |
| JULIA_DEBUG: Plots | |
| shell: julia --project=@. --color=yes {0} | |
| run: | | |
| using Pkg; Pkg.precompile() |
That does not seem to work as intended right ? No warning appear on windows 1.8.2.
There was a problem hiding this comment.
Yes, at the moment it does not. Was hoping for another workaround from the Pkg issue.
|
Superseded by #4617. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
My goal here is to still precompile on normal computers, but only skip it if there's a problem in CI
Providing a warning will also allow the issue to be monitored, and if someone ever encounters it outside of the CI system, maybe we can learn more about the issue.
Adjustment to #4406, based on work in #4445