loading: make __precompile__ the default #26991
Conversation
|
Bump? |
|
There should be a warning that people aren't allowed to write too big numbers in their code. |
|
Or maybe just fix that issue? |
|
So this makes things less reliable and slower for new users (whose first modules are typically small and frequently reloaded as they develop), while not affecting load times for large packages (which inevitably turn precompilation on when they begin to mature). Great? But I agree that this simplifies the situation for precompiled modules loading modules lacking a |
base/loading.jl
Outdated
| throw(PrecompilableError(isprecompilable)) | ||
| @noinline function __precompile__(isprecompilable::Bool=false) | ||
| if isprecompilable | ||
| depwarn("__precompile__() is now the default", :__precompile__) |
There was a problem hiding this comment.
This deprecation seems painful: it will require an explicit VERSION check for code that wants to continue to be precompiled in 0.6 without the warning, since __precompile__() lives outside the module (before Compat is loaded).
There was a problem hiding this comment.
FWIW, such version checks were the norm when precompilation was introduced. So instead of VERSION >= v"0.4.0-something" && __precompile__() it would be a check against 0.7. Not saying it's an ideal situation, just that it isn't without precedent.
There was a problem hiding this comment.
I don't see why, in general, changing the default value for something should require changes when the desired value is explicitly chosen by the package developer.
This required every package that didn't use __precompile__(false) to make additional changes for 0.7-rc2, while I think it should have just been limited to those packages that didn't have an explicit __precompile__()/__precompile__(true)/__precompile__(false) statement. Alternatively, automated PRs could have been opened against all packages to change this syntax.
Just my two cents.
|
I'd be much more comfortable with this if the BigInt issue can be fixed first. |
|
It doesn't feel like this decision needs to block 0.7 alpha. Moving to 1.0. |
2586846
to
d21962a
Compare
4f56ec4
to
72f9e67
Compare
|
Hm, seems like this is trigger an issue with the Pkg tests that has snuck in since I last rebased this. There appears to be a minor misunderstanding in the way that Pkg is attempting to use |
|
It seems like you're uniquely positioned to understand and fix that breakage. Let me know if there's any assistance I can provide. |
|
From triage: @JeffBezanson is going to look into addressing the |
72f9e67
to
c76bf28
Compare
|
Rebased to fix merge conflicts. |
|
Bump. This is unblocked now. Would be reeeeeally nice to get this in today if it's in 0.7. |
|
I think the outstanding issue is the Pkg thing Jameson mentioned. @vtjnash, do you know what needs to happen to fix that? |
c76bf28
to
eba6f03
Compare
|
@vtjnash Is this good to merge? |
|
xref #28384 |
Adapt to JuliaLang/julia#26991.
Adapt to JuliaLang/julia#26991.
|
What's the best way to stay compatible with 0.6 in our packages, as having |
|
Conditionally execute it based on |
|
I've used |
|
0.7 is not supposed to complain about that anymore either unless that change didn't get merged. |
|
Ah yes, I guess it was just the 0.7 release candidates. |
Julia packages are precompilable by default since 2018: <JuliaLang/julia#26991>. See also: `?__precompile__`.
- Remove unnecessary `__precompile__` statement. Julia packages are precompilable by default since 2018: <JuliaLang/julia#26991>. - Update `import` statements. The `import ... as` syntax has been introduced in Julia 1.6. - Update comments.
This is nice, because it simplifies the
requirecode. That should also make error handling more understandable and predictable. And the problems resulting from non-precompile-declaring code is used from__precompile__code simply won't exist anymore.fix #26282