incremental precompile warning turned into error: breaking?

[aplavin]

Tried out 1.10rc for a bit recently, and got this error a few times:
ERROR: LoadError: Method overwriting is not permitted during Module precompile.
As I understand, this error was added deliberately, which really surprised me: it's basically the definition of a breaking change, explicitly making code that worked fine before to error now.

In previous versions, this was a warning that only appeared during precompilation (so users rarely even saw it), and it didn't really indicate that the code uses some internals and could break after updating Julia:

│  WARNING: Method definition <...> overwritten in module <...>.
│    ** incremental compilation may be fatally broken for this module **

I even found a discourse thread (https://discourse.julialang.org/t/what-is-incremental-compilation-and-what-does-it-mean-for-it-to-be-broken/100956) where Tim Holy directly states that everything is supposed to work with this warning, just with slower compilation/loading/invalidations.

So, should this breaking change be reverted?
And, probably, the warning text made cleaner: I saw it several times, but never had an idea what "incremental compilation" means and why would I be worried about it. Looking at discussions, I'm not alone in this.

See a recent slack thread https://julialang.slack.com/archives/C67910KEH/p1699434379137429 for more discussion and examples.

[PallHaraldsson]

where Tim Holy directly states that everything is supposed to work with this warning

Does he? It seems to me he's saying something is actually "fatally broken"? I.e. not good for the ecosystem if you change a method from another package (maybe not fatal in itself, until the ecosystem grow more, or more packages composed), but depending on the order of how you do it, for different packages trying to modify the same, is "broken".

I suppose if you do redefine in a certain order it COULD be ok, if you're ok with that, and the order is correct, something you're ok with. And changing this to an ERROR is then a breaking change. But I'm not sure most users are ok with this, or know the issue? E.g. in his example if only B or C is used, then ok, but would users for sure test the ecosystem for B AND C, or the other order C AND B, and would be ok with both the different conclusion? Must not either be wrong, and you might even have a combinatorial explosion of possible orders, and only 1 (or few?) ok. I suppose it best to let this error.

It seems to me the WARNING SHOULD have been a sign you or the ecosystem needed to change, and now the ERROR is forcing it. You always have the option of using the older Julia version where this "worked" in some sense, maybe with wrong answer?! or I mean at least not what you though would happen?

[vtjnash]

Warnings are printed when something should not be relied upon working, as it is known to be broken. Ignoring warnings is buyer-beware. But it was pointed out that this lacks a NEWS entry, since changing warning to error usually doesn't seem to merit one, but it might in this case

Also worth pointing out in that NEWS that this is only a problem if you want to incrementally compile code. If you avoid using module & the precompilation system (e.g. __precompile__(false)), then this is not a problem.

[aplavin]

where Tim Holy directly states that everything is supposed to work with this warning

Does he?

Yes, surely:

Even if it’s broken, it would never stop our program from running, right? For example, <...>

Correct. But order in which you load packages is not something that we want mattering <...>

[aplavin]

Also worth pointing out in that NEWS that this is only a problem if you want to incrementally compile code.

As a user, I typically don't care how the code is compiled. So, how to tell Julia that I don't explicitly want "incremental" compilation for installed packages that throw this error?
Adding such a solution do NEWS/docs would totally resolve my issue, but for now I don't see how a user (not package developer) can control this.

[aplavin]

Warnings are printed when something should not be relied upon working, as it is known to be broken.

This is quite an unusual take on warnings, IMO. Does this mean that deprecated functions can just be removed in the next minor Julia version?

[PallHaraldsson]

He does write:

You can only overwrite a method if you’re committing type piracy [..]. Hence this warning only happens if you’re committing type piracy.

So if you don't, nor your packages, then you're ok... yes, it's not unheard of in the ecosystem, but it IS bad?!

He's saying any order will compile to something, but not the same thing. So it's deterministic, and COULD be allowed. I'm just not sure most users realize. When I have e.g. a Python package, implemented in C, I think I can import them in any order, and all will work the same, since there's no possibility of the C code being changed after/when importing.

For Python code, or the wrapper code of that C code, that might actually not be true, since Python is very dynamic. Though I'm not sure.

Is there some good non-breaking way out of this? I'm also thinking, one package may need a certain version, e.g. latest, and another might not be able to use that version, so preventing installing two packages at the same time. Might we want to allow that different packages can depend on the same package, just different versions? If we allow that, then we make this headache even more complex, OS should people be able to change and get new versions, and all valid? It might still be a warning if you do something such unusual...

[aplavin]

Well, showing a warning "the state of your packages may depend on the loading order" is totally fine, and would be much easier to understand than the current warning text. Note that this warning doesn't imply that there's order dependency, it's just a possibility.

[vtjnash]

Does this mean that deprecated functions can just be removed in the next minor Julia version

If warnings have been present in all minor releases, yes, since there hasn't been a version in which it was part of the API. Otherwise, semver says it would be a major version change.

The workaround is __precompile__(false)

[BioTurboNick]

I concur with @aplavin - it may make sense to prevent a package developer from doing this going forward, but it doesn't make sense to prevent an end user from relying on an already existing package/version from being able to use that package going forward.

From both perspectives, I never knew what the error actually meant, what impact I was really being warned about. Everything seemed to work fine. What specifically was broken that I might care about? I couldn't tell you.

I would highly recommend first making the meaning of the warning more clear before taking a further step. And when that next step is taken, it should make sure that older packages don't need to be updated to continue to be used on future Julia versions. Maybe it's just added as a check for new General registrations. Maybe there's a switch for an end user to turn off.

[vtjnash]

What might happen is that it might segfault occasionally, which is what it started doing very often when we decided to change the warning to an error

[aplavin]

The workaround is precompile(false)

Where do I put this as a user, without changing loaded packages code?

[aplavin]

On a more general note, I consider myself pretty familiar with Julia and packages, and this is the first time I heard this perspective on warnings. I think going forwards this is important to highlight wherever semver/compatibility is promised, that as soon as you see any warnings – no backwards compatibility for you! I doubt many people realize this.

[PallHaraldsson]

We shouldn't warn (or error) where there's no actual problem.

It seems to me (incremental) compilation is a red herring. It's the redefinition that's the problem?! This would also be a problem with the interpreter, or without precompilation? At some point you must compile (or interpret) and if you precompile, then potentially you recompile. The only way out of that is not allowing redefinition, i.e. type piracy? But it's also very useful in an interactive system if you know what you're doing... or annoying if you couldn't redefine e.g. functions or structs... It seems there will always be a tension, and that for packages you may want to disallow more changes.

I typically don't care how the code is compiled. So, how to tell Julia that I don't explicitly want "incremental" compilation for installed packages that throw this error?

You mean get similar to what C packages in Python give you? They are not generic. I think we could disallow recompilation for existing types (like there), but allow adding new types, and they would work with those packages, note that's addition (and implied compiled code), not a change, thus not type piracy, still more compilation. Recompilation would be the wrong word for that.

[aplavin]

Ok, I guess putting concrete examples here would help. They are in the slack thread, but not here.
There are two packages broken in current rc that I noticed in my (very brief) tryout of 1.10rc:

• My PlutoStyles.jl whose whole purpose it to override a method from Pluto at https://gitlab.com/aplavin/PlutoStyles.jl/-/blob/master/src/PlutoStyles.jl?ref_type=heads#L99.
• Another is ConstructionBaseExtras, basically any environment that uses it would lead to this error. This is more of a historical thing: for Julia before 1.9, ConstructionBaseExtras was needed – but for 1.9+ the same methods are included as extensions in ConstructionBase itself. So it's present in code that is intended to support both, and/or was developed even before 1.9 and not changed after that.

In both cases, potential order dependency is not an actual issue at all.

[KristofferC]

You can do the method overwrite in __init__. Doing it at precompilation time doesn't make sense I think.

[aplavin]

Doing it at precompilation time doesn't make sense I think.

I'm not arguing with this at all, just please let me know if there's any workaround on the user side (without changing package code). This would greatly help dealing with the breakage.

[aplavin]

Does this mean that deprecated functions can just be removed in the next minor Julia version

If warnings have been present in all minor releases, yes

Actually, looking at it again – why is the policy different for different warnings? The incremental compilation one doesn't appear in earlier 1.x versions:

❯ cat M/src/M.jl        
module M
       Base.:+(::Bool, ::Bool) = 1
end

❯ rm -rf ~/.julia/compiled/v1.6  

❯ julia +1.6 --project=M         
┌ Warning: Couldn't load Revise
└ @ Main ~/.julia/config/startup.jl:12
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.6.7 (2022-07-19)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

(M) pkg> precompile
Precompiling project...
  1 dependency successfully precompiled in 2 seconds

julia> using M

or

(@v1.6) pkg> activate --temp
  Activating new environment at `/var/folders/2j/9vtd991d201dbkh9dnx3wvy00000gq/T/jl_YAInvl/Project.toml`

(jl_YAInvl) pkg> add ConstructionBaseExtras
   Resolving package versions...
    Updating `/private/var/folders/2j/9vtd991d201dbkh9dnx3wvy00000gq/T/jl_YAInvl/Project.toml`
  [914cd950] + ConstructionBaseExtras v0.1.1
    Updating `/private/var/folders/2j/9vtd991d201dbkh9dnx3wvy00000gq/T/jl_YAInvl/Manifest.toml`
  [187b0558] + ConstructionBase v1.5.4
  [914cd950] + ConstructionBaseExtras v0.1.1
  [8197267c] + IntervalSets v0.7.8
  [1e83bf80] + StaticArraysCore v1.4.2
  [ade2ca70] + Dates
  [8f399da3] + Libdl
  [37e2e46d] + LinearAlgebra
  [de0858da] + Printf
  [9a3f8284] + Random
  [9e88b42a] + Serialization
  [2f01184e] + SparseArrays
  [10745b16] + Statistics
  [4ec0a83e] + Unicode
Precompiling project...
  4 dependencies successfully precompiled in 3 seconds

julia> using ConstructionBaseExtras

but the exact same packages & code errors in 1.10rc. So if someone didn't use 1.9 for some of their code, they would see the change from "no warnings, everything works" on 1.6, to a strange error on 1.10(rc).

Also, reverted title to a correct one again: the main question asked and discussed here is not whether NEWS entry is needed, but whether this change is breaking. Better to have title and content consistent...

[KristofferC]

The incremental compilation one doesn't appear in earlier 1.x versions:

I think that is because Pkg.precompile used to swallow output before (which now has been corrected). If you precompile it with using it should be visible.

[KristofferC]

I'm not arguing with this at all, just please let me know if there's any workaround on the user side (without changing package code).

I think --compiled-modules=no would work around it.

[aplavin]

I think that is because Pkg.precompile used to swallow output before

So in this scenario the user-facing behavior is: no warnings, perfectly working code on 1.6 – throws an error on 1.10rc.

Also, the case with ConstructionBaseExtra doesn't throw any warning before 1.9 at all, no matter how you precompile it.