[backports-release-1.11] Allow ext → ext dependency if triggers are a strict superset #56368
Conversation
|
Hello! Having this feature would be really great, I just wanted to add though that for our usecase Bijectors.jl would have one of its strong deps as an extension trigger, and (as I understand it) the following Pkg.jl issue needs to be resolved so that we can get it to work on 1.10 as well (this issue doesn't happen on 1.11, I haven't yet looked into why): (Re. our usecase: there's more context here, but TLDR we want |
|
Thanks for the heads up! Yeah, you're right - this does mean that we need triggers from That sounds like #54009, which wasn't backported to 1.10 (yet). I'm curious how @KristofferC would feel about a potential backport. |
|
I think it is fine to backport since it is arguably a bugfix. |
topolarity
force-pushed
the
ct/ext-to-ext-dep
branch
from
51870b5
to
75763a7
Compare
base/precompilation.jl
Outdated
| keep_loaded_modules = false | ||
| if haskey(exts, pkg) | ||
| # any extension in our direct dependencies is one we have a right to load | ||
| loadable_exts = filter((dep)->haskey(exts, dep), depsmap[pkg]) |
There was a problem hiding this comment.
The set of loadable_exts has to be computed from the loading code, no? There is no guarantee that it is the parallel precompile code that precompiles the extension?
There was a problem hiding this comment.
Hmm, I think you're right.
In fact, it looks like it should have been computing isext already to avoid cycles but I think we missed that
topolarity
force-pushed
the
ct/ext-to-ext-dep
branch
2 times, most recently
from
2bf7616
to
4139614
Compare
|
otherwise CI is looking green |
|
This adds no test, is the new behavior not observable or not worth testing? It seems to me that a setup like in #56204 (comment) which then checks that a |
|
Yeah, I've been waiting to add a test until #49891 lands - will follow up with that soon |
topolarity
force-pushed
the
ct/ext-to-ext-dep
branch
3 times, most recently
from
1646cb3
to
efbbab5
Compare
This allows for one extension to depend on another if its triggers are a strict superset of the other's. For example, in a Project.toml: ```toml [extensions] PlottingExt = "Plots" StatisticsPlottingExt = ["Plots", "Statistics"] ``` Here `StatisticsPlottingExt` is allowed to depend on `PlottingExt` This provides a way to declare `ext → ext` dependencies while still avoiding any extension cycles. The same trick can also be used to make an extension in one package depend on an extension provided in another. Also requires something like JuliaLang#49891, so that we guarantee these load in the right order.
topolarity
force-pushed
the
ct/ext-to-ext-dep
branch
2 times, most recently
from
243d783
to
d358d6b
Compare
topolarity
force-pushed
the
ct/ext-to-ext-dep
branch
from
d358d6b
to
85218bc
Compare
KristofferC
merged commit bf5675b
into
JuliaLang:backports-release-1.11
…ng#56368) This is an effort at a proper workaround (feature?) to resolve JuliaLang#56204. For example: ```toml [extensions] PlottingExt = "Plots" StatisticsPlottingExt = ["Plots", "Statistics"] ``` Here `StatisticsPlottingExt` is allowed to depend on `PlottingExt` This provides a way to declare `ext → ext` dependencies while still avoiding any extension cycles. The same trick can also be used to make an extension in one package depend on an extension provided in another. We'll also need to land JuliaLang#49891, so that we guarantee these load in the right order.
This is an effort at a proper workaround (feature?) to resolve #56204.
For example:
Here
StatisticsPlottingExtis allowed to depend onPlottingExtThis provides a way to declareext → extdependencies while still avoiding any extension cycles. The same trick can also be used to make an extension in one package depend on an extension provided in another.We'll also need to land #49891, so that we guarantee these load in the right order.