Conversation
| end | ||
|
|
||
| remaining_libs = Set(strip_prefix_suffix.(copy(all_libraries), Sys.isunix() ? "linux" : Sys.isapple() ? "mac" : "windows")) | ||
| remaining_libs = Set(strip_prefix_suffix.(copy(all_libraries), Sys.isapple() ? "mac" : Sys.isunix() ? "linux" : "windows")) |
There was a problem hiding this comment.
Note the change of order here. Since Sys.isunix() is true on macOS, the Sys.isapple() path is never executed, so we had been setting this argument to "linux" on macOS. That seemed unintentional, so I changed it (even though no edits to this line were otherwise needed within the scope of the PR) but I can change it back.
|
Ah, I just saw #1080. @KristofferC, should we close this in favor of that or vice versa? |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1109 +/- ##
==========================================
- Coverage 85.45% 85.43% -0.02%
==========================================
Files 3 3
Lines 832 831 -1
==========================================
- Hits 711 710 -1
Misses 121 121 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
What should I check specifically? |
|
My thought had just been to verify that this package's functionality works as expected, but I was actually able to do that myself (I think—see below). I realized that when I had tried A journey of self-discovery (click to expand, if interested)While I was able to successfully compile an incremental system image, actually using it as described in the documentation behaved a bit strangely. Granted, this is my first time using PackageCompiler (at least post v1.0), so perhaps this is user error/misunderstanding, and it's also worth noting that I'm using a local build of Julia EDIT: EDIT 2: TIL REPL and InteractiveUtils are not in the default sysimage, so perhaps it makes sense that they're not included in the custom built one, thus triggering additional precompilation. So maybe all is well? EDIT 3: Yeah I think it's fine |
|
@topolarity, looks like you approved #1080, but that has merge conflicts. Can you take a look at this one? Should be ready to go, and we can close #1080. |
topolarity
left a comment
There was a problem hiding this comment.
Changes look good to me
Thanks @ararslan
|
Any chance we can tag a new version too, or are there other things you wanted to get in before the next version? |
The changes are pretty trivial since we can reuse the Linux code paths.
I was trying this out locally and it got OOM killed, but it seemed to have been working up to that point. @giordano, you have access to a beefier FreeBSD system than I do, would you be willing to try out this branch?The example from the documentation works as expected.Fixes #1079. Closes #1080.