prevent loading other extensions when precompiling an extension
The current way of loading extensions when precompiling an extension very easily leads to cycles. For example, if you have more than one extension and you happen to transitively depend on the triggers of one of your extensions you will immediately hit a cycle where the extensions will try to load each other indefinitely. This is an issue because you cannot directly influence your transitive dependency graph so from this p.o.v the current system of loading extension is "unsound".
The test added here checks this scenario and we can now precompile and load it without any warnings or issues.
Would have made https://github.com/JuliaLang/julia/issues/55517 a non issue.
Fixes https://github.com/JuliaLang/julia/issues/55557
@nanosoldier runtests()
The package evaluation job you requested has completed - possible new issues were detected. The full report is available.
@dlfivefifty, you seem to be the only one using this feature (at least in PkgEval) in e.g. https://github.com/JuliaArrays/LazyArrays.jl/blob/7777232394f9e42a7685cbf15001bf4b3bcba696/ext/LazyArraysBlockBandedMatricesExt.jl#L25.
To check, would it be possible and how hard would it be to provide the same functionality without requiring this. The reason is that extensions loading extensions (as it is done now) is quite brittle and easily lead to cycles and bad experience. There might be a slightly more "relaxed" version of this that could be implemented that could maybe still allow the use case in LazyArrays but I want to check with you if the simple thing could be done first.
I felt this was a bit dodgy when I did it. But I don’t know how else to do this?
note this usage should never lead to cycles since BlockBandedMatrices.jl itself depends on BandedMatrices.jl
Put another way: if package B depends on A, then it’s natural that an extension for B depends on an extension on A
note this usage should never lead to cycles since BlockBandedMatrices.jl itself depends on BandedMatrices.jl
If you happen to get two of your triggers for your extensions (say BlockBandedMatrices and StaticArrays) into your full dependency graph of LazyArrays you get a cycle because loading LazyArrays will trigger one extension and loading LazyArrays in that will trigger the other (over and over).
You cannot control your full dependency graph so this can happen at any time.
I don't understand your example. What's the alternative solution?
It might be possible I can move functions/types into LazyArrays.jl in this case. But it seems like a major flaw in extensions combining with packages.
I think at present, extensions are only for adding methods to pre-existing functions from one package for pre-existing types from another. In this case, perhaps both the type LazyBandedLayout and function _mulbanded_copyto! need to me moved to LazyArrays, and only methods need to be added in the extension modules.
But it seems like a major flaw in extensions combining with packages
Well yeah, that's what we are trying to fix heh.
I still don’t understand the issue being solved. A directed tree doesn’t form cycles….
https://github.com/JuliaLang/julia/issues/52511#issuecomment-1983388150 is one example in the wild.
You can also just setup the test example I provided in this PR and see how precompilation fails for example.
I think I need a mathematical description to be convinced it’s not an implementational issue.
Packages with (or without) extensions are simply a tree. How does a cycle come up?
I how are extensions different to before when we could make packages to implement extra behaviour? Ie how is LazyArrays making extensions for BandedMatrices and BlockBandedMatrixes in any way different to making two packages called LazyArraysBandedMatrices and LazyArraysBlockBandedMatrices that depend on each other exactly like the extensions do??
Mainly because the edges point the unexpected direction from BandedMatrices and LazyArrays, since loading either of those can trigger loading the extension, so in the graph, they are the originators of the edge, rather than the terminators. That can cause a cycle to appear in the load graph that isn't as easily anticipated.
I've created https://github.com/JuliaArrays/LazyArrays.jl/pull/345 to avoid the inter-ext dependency
This kind of workaround isn't always possible though - This wouldn't work if the types I moved (e.g. AbstractLazyBandedLayout) had fields/supertypes from both BandedMatrices.jl (the trigger) and LazyArrays.jl
I think we need to make the change that Kristoffer described here: https://github.com/JuliaLang/julia/issues/48734#issuecomment-1554626135 and give this situation a definite load-/dependency-ordering, so that it's not a "cycle" any more - ~~With that change, I think the only "cycle" left is for extensions that share the exact same trigger/parent set (e.g. extBA and extAB) for which case this fix probably makes sense~~
OK, I think we probably want to add an exception to this to still allow loading of other extensions if their triggers are a strict subset of your own (e.g. {A,C} ⊊ {A,B,C})
That'd leave room for us to do something like https://github.com/JuliaLang/julia/pull/49891 to allow ext -> ext dependencies, like the LazyArrays.jl case.
Could a simple solution be that if an extensionA depends on extensionB it needs to list it in the Project.toml? I.e.
https://github.com/JuliaArrays/LazyArrays.jl/blob/d3e51078551d98ba951525931c77b2f259bfea29/Project.toml#L25
Would become
LazyArraysBlockBandedMatricesExt = ["LazyArraysBandedMatricesExt", "BlockBandedMatrices"]