-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
glue code (aka conditional dependencies) #43119
Comments
Duplicate of #6195? |
No. I'll grant you that it's related, but your definition of "duplicate" is very odd. That issue is closed and has been for four years. It also doesn't have a specific concrete plan of action. Should we really close this issue and reopen that one? Is that in any way more likely to lead to progress on this problem? |
My biggest question as I look into this is whether precompilation and PkgCompiler will work doing this with a little elbow grease? |
My guess is that they might just work, but @vtjnash and @KristofferC might have to weigh in. |
May I suggest a different title for this issue, like "Environment based conditional glue code"? At first I thought this was also about allowing to specify that a given package is only a dependency if e.g. the current julia version is a concrete or earlier version than some specified version, without having to split the releases & maintenance over two different releases. At least, something like that is what I understand under the term "conditional dependency". Sounds like a good idea in general though, to allow for special casing when some other package is available in the environment our package is loaded in and we have specialized code for that package. |
Looks like a nice feature. I have some extra questions though:
|
I feel this proposal should contrast how it works vs how Requires work (which already uses a post-load hook when a package is loaded to load arbitrary "glue" code) as well as the proposal in JuliaLang/Pkg.jl#1285. I must say, this looks very similar to Requires (I actually don't see what the difference is). It should probably also discuss how this fixes the problems identified under "What is the problem with Requires.jl" in JuliaLang/Pkg.jl#1285. In my opinion, this has the same weaknesses. Personally, I feel that a proposal that is based on when running something when packages are "loaded" is set up to fail. It should be declarative and something that can be checked during precompile time (for example, if a package is present in the current manifest) so that the glue code can be precompiled together with the package. (and yes, this requires modifications to Project.toml files to add a conditional dependency there so you have a name -> UUID mapping to load the conditional dependency to run the glue code). |
@bvdmitri, I think you're treating a social problem as if it were a technological problem. It's fine for glue definitions to go in either
Package authors have to not do this. Again, this is a social problem, not a technological one.
This is already possible and Julia does nothing to prevent it.
Ok, here's what you wrote about the issues with Requires over in JuliaLang/Pkg.jl#1285:
This doesn't run inside of
This is explicit and can be entirely determined from the project source.
In Requires the issue is that the external dependency isn't an explicit dependency. This has a similar issue in that neither
I don't really understand the performance problem with Requires, so it's hard to address this.
That's a good point that this proposal doesn't address. We could base it on what's in the manifest instead. In that case the glue hook would only be registered if I'm not sure that's the right call though. I think that people would be confused that when they load
Man, there's like three or four different morphing proposals over there. Not even sure what to cover. This is why this problem still isn't solved. Frankly, this isn't a pain point for me personally, it's something that @Wimmerer wanted to work on, @ViralBShah connected him with me, and this is the plan I conveyed to him, and then I promised to write it up on GitHub so he could work on it if he wanted to.
This seems like it could be precompiled straightforwardly. Precompilation would need to learn to precompile glue "packages" like it precompiles real packages, but that's doable. And it is declarative: you can tell, without running any code, wether you'll need glue code or not, assuming only that all the packages in the manifest are actually loaded. Of course, we already can't accurately make that assumption in any Julia project, but it's fair to just pre-load the entire manifest and then run the code and should generally work close to the same. Glue code doesn't affect that. |
I've added some additional Q&A at the bottom of the original post in an attempt to keep the proposal all in one place rather than spreading it out in a long discussion. |
Well, that's why I don't call these "conditional dependencies"—because I think it's a confusing and bad name. I call it glue code. Other people have called it conditional dependencies, but that's what they'll probably search for, so that's why I used the term here. |
You would need to extend the manifest format then to be able to see the "glue dependencies" of packages that are not direct dependencies though? This would require Pkg to emit those type of manifests.
With the "static" approach, they would always be able to do
This requires Pkg support.
I can do some comparisons:
That's identical to the linked proposal with the only difference being
This is identical to the proposal with the difference being the actual path:
This is the same as in the linked proposal:
So the proposal in JuliaLang/Pkg.jl#1285 looks functionally equivalent to the one here with the only difference being things like names and paths? To make the difference between this proposal and Requires.jl more clear here are some concrete points:
I think most of this is ok and would be an improvement (obviously since the proposal here is almost identical to JuliaLang/Pkg.jl#1285). I do think it can be slightly annoying to have to split up all the glue code between different files (like https://github.com/KristofferC/PGFPlotsX.jl/blob/master/src/requires.jl) but it allows the expression to be evaluated when the glue code "fires" to be statically known. What I think is needed is a concrete proposal is the precompilation of the glue code. If the glue code is included as part of the precompilation process (because only the information from the Manifest) is used, this works automatically. It basically turns the glue code into a normal file that is part of the package that just happens to be conditionally included. If not, these glue code files need to be fitted into the precompilation system:
|
Another thing that I think needs elaborating on is how the compat on conditional dependencies will work. In JuliaLang/Pkg.jl#1285 I wrote:
So in that proposal, compat on conditional dependencies never influence the resolver. The only thing they do is that decide on if the glue code should be loaded or not. So if you have a glue compat on |
We could print a warning in those cases, just so that the user is aware. |
I don't follow this. Why would the manifest need to have anything about glue dependencies in it?
Why is it hard to get right? If
I have a different take on this: I think that Suppose we do it the other way and we decide whether to load glue code or not based on compatibility information. What do we consider to the "one true source" of compatibility? Is it the registry or the project files of the packages we are loading? For everything else, we consider the registry to be definitive and only use project files as a source for registry data, but if we later edit the registry, then that is what we use for choosing versions. If we did that here, then a registry change could affect the behavior of a program: we resolved a manifest previously and picked compatible versions of Suppose we don't look in the registry and instead consider the compat info in package project files to be the source of compatibility truth for the purposes of deciding to load glue code or not. At least that is immutable since it's inside the package source, so it can't change over time. It still doesn't seem much better to me—it means that there are now two different notions of compatibility in play that both matter: the one in the registry, which is used to decide what versions to install, and the one in the project files that is used to decide whether to load glue code or not. Again, consider a situation where these have deviated. You could end up in a situation where two versions were marked as incompatible according to one or both of the project files, but that was wrong and the compatibility has since been fixed in the registry to indicate that the versions are actually compatible. Now you can resolve a single, apparently compatible manifest, where there is glue code for The main potential issue with unconditional glue code loading, regardless of compatibility, is that the glue code might not work as it should or fail to load at all. However, I don't see this as being any different from cases where
Fair point. Specifically, what is required is this: if we're resolving a manifest which happens to contain any of the packages in any
Yes, based on your breakdown, it's pretty similar. I think the key differences are these:
I think most of the other differences are superficial. However, I do think that the section name is significant even though it's superficial:
Yes, this is the meat of what needs to be decided.
We could name the module for
Yes, a
Yes, this is definitely part of what needs to be done. I haven't looked at it enough to know which is the better approach. |
For the same reason we have the
Okay, then compat for glue dependencies are needed in the registry.
The path I chose was just an example, the core is that the filename is based on a convention of the names of the conditional dependencies:
The exact path is pretty not really interesting at this point when it comes to the design because it doesn't influence anything. In the end it will just be a
If we want to talk about path names I think that this proposal would give very long module names in the stack traces and would look pretty ugly in the source code (>80 linewidth) (and you can't have commas and colon in module names). Files also tend to have the same name as the module (enforced for packages). |
I'm still not getting it—what specifically needs to go in the manifest? Can you give an example?
Yes. The big issue here is that we can't really put glue compat in normal
We could call them whatever we want because they're top-level and top-level module names don't have to be unique anyway. So I guess |
Since the design is that a certain file will be loaded deterministically based on when other packages are loaded, there is no need for the Requires style package callback of arbitrary code as is described in part 1. Instead, code loading looks at the manifest, sees what glue dependency a package each package have, and loads the corresponding file when both a package and its glue dependency happen to be loaded. I don't see how Part 1 in this proposal has any benefit in a system where the code that will be run is deterministic based on the "input" packages. Might as well put that logic in code loading then instead of having all packages push this as callbacks. Right now, Requires.jl does exactly this except you push arbitrary code as a callback instead of a specific file to be included, as in this proposal.
Yes, many choices in JuliaLang/Pkg.jl#1285 were made to reduce the number of separate places that need to be touched. That's why it doesn't involve the registry, or the resolver, or (with environment-based glue code inclusion during precompile time) not the precompilation system. Everything gets pushed to code loading. |
Is this obsolete now with weakdeps? |
This is related to the discussion in JuliaLang/Pkg.jl#1285 but the plan I propose doesn't touch Pkg at all, so I'm writing it up here. @Wimmerer contacted me about wanting to work on this feature and we had a back and forth about what needs to be done. Here's what I propose as a concrete solution to the glue code problem, aka conditional dependencies.
The problem we want to address occurs when there are packages, say
A
andB
, which don't depend on each other, but when both are loaded at the same time, there is are some additional definitions—usually of methods—that are needed to make them work well together. For example, supposeA
provides a functionA.f
andB
defines a typeB.T
and there's a methodA.f(::B.T)
that would be useful to have. This method cannot be defined inA
sinceB.T
isn't available there and it can't be defined inB
sinceA.f
isn't available there. We need a way to provide that definition at the point when bothA
andB
have been loaded.The solution comes in two parts.
Part 1: post-load hooks for sets of packages
The first part is to implement and expose a mechanism whereby you can register a callback to be called when a set of packages have been loaded. The packages should be identified by UUID and it should be an arbitrary set of packages, not just one or two packages. This logic goes right after a new package has been loaded and should check for each registered hook, whether the set of loaded package UUIDs is a superset of the set of required package UUIDs, and if it is, then call the hook callback and then delete it.
Part 2:
glue/*.jl
entry-pointsThe second part leverages the first mechanism and uses it to load some glue code that is included in packages that are loaded. The idea is that package
A
will have a top-level directory calledglue
and a fileglue/B.jl
which contains the code that implements the methods that are useful when bothA
andB
are loaded. In more detail:A
, it should look at allglue/*.jl
filesglue/B.jl
in the packageA
would be the file providing glue definitions forA
andB
A
's project file entitled[glue]
with the same format as the[deps]
and[extras]
sections, i.e. mapping names to UUIDsSo,
glue/B.jl
in the packageA
would be the glue forA
andB
andglue/B,C.jl
would be glue for the packagesB
andC
together, i.e. definitions that depend on all three ofA
,B
andC
.The next question is how to use the glue file to define a hook. We want the behavior of the glue files to be fairly constrained, so I propose that the hook generated look something like this:
This evaluates the glue code in an anonymous module that imports
A
andB
. I'm not sure how easy it would be to rig it up so that this is done in a context where onlyA
andB
can be loaded and if that's really necessary or desirable, but otherwise the code can load anything that can be loaded in Main, which probably isn't ideal or sensible for glue hooks.This hook could probably be expressed generically and be written as
glue_hook(glue_path, A, B)
or something like that, which might avoid code some code generation. Or in the other direction, we could insert the contents of the glue file into the above template and generate that function. But I suspect that's not what we want. Probably better to be late binding and do less code generation.The actual contents of a glue file would be pretty simple. For example, the glue file for defining the method
A.f(::B.T)
would simply contain that method definition:This would either be
glue/B.jl
in package A orglue/A.jl
package B.Questions
What happens if
glue/B.jl
in package A andglue/A.jl
in package B both exist? Load them both.What about the order? Eh, whatever order they happen in is probably fine, but we could maybe have a defined ordering based on UUIDs or something.
What if they have conflicting definitions and it breaks? That's a normal package incompatibility between those versions of
A
andB
, although it's a slightly weird one because neitherA
norB
depends on the other but their versions are incompatible. I'm not certain if we can express that in[compat]
—it's possible that we can't. At the very least,[compat]
probably needs to know about the[glue]
section, which may end up being a reason to put glue dependencies in the[extras]
section since I think[compat]
may already know about that.Why not make glue packages real registered external packages that live outside of both
A
andB
? Because that complicates things massively. In that design, Pkg needs to know about glue packages and needs to makes sure that wheneverA
andB
are both in a manifest, thenGlue_A_B
is also included in the manifest. Also, registering glue packages and versioning them separately seems like a lot of overhead.What if the glue behavior depends on what features the particular version of
B
that gets loaded has? The glue code can do whatever metaprogramming it needs to, reflecting onB
, it’s version, and features.Edit: couple of added questions
Why does the glue code go in
A/glue/
instead of somewhere inA/src/
—isn't it part of theA
package? No, not really. If you just loadA
then you load what's inA/src
and you don't load anything fromA/glue
at all. The glue stuff is really external lightweight packages that depend onA
, which is why it makes sense to put them outside of the main codebase ofA
.What's really so bad about making glue packages explicit separate packages? For one thing, we don't even have a way of expressing that to Julia's version resolver and it's unclear how we would even do this. The resolver currently understands two things: (1) that a version of a package depends on some other set of packages and (2) that some versions of packages are incompatible with each other. The conditional dependency pattern can't be expressed in terms of these two features—it's a new kind of thing entirely. You need to express that for some subset of pairs of versions of two packages, you need to load yet another package if you've chosen both those versions. What about for other pairs of version of those two packages? Are those incompatible or are they just pairs for which the glue isn't necessary or doesn't work? Both are valid concepts. So now you need to express several things:
This is a lot of complicated new crap to cram into the registry and teach the resolver about. How does this proposal avoid this problem? It locks the glue code to the version of one or both reverse dependencies and then it's just the usual problem of picking those such that they're compatible.
The text was updated successfully, but these errors were encountered: