Skip to content

Conversation

@timholy
Copy link
Member

@timholy timholy commented Dec 16, 2022

I'm writing my first package extension, many thanks to @KristofferC and anyone else who help make weakdeps happen. This PR has two independent components:

  • some tweaks I made to wording in the existing documentation
  • a new subsection that describes ways to handle another mode of extension

Obviously you can pick and choose what you want from this. Also, writing documentation is often a moment for reflection about design, and if you don't like how the 2nd commit reads then perhaps we could talk about alternatives. (I think it's mostly fine, but register_error_hint makes things more complicated, and perhaps we could handle this automatically in some way.)

@KristofferC
Copy link
Member

KristofferC commented Dec 16, 2022

Thanks for all the polish!

I think the example here is very related to a discussion I and @fredrikekre has had about a somewhat dangerous situation where you accidentally end up relying on the dependencies of your dependencies (which is an implementation detail)

The issue with overloading list_footware(wardrobe) is that you are doing "extension piracy" (a completely new type of piracy that I just made up) where an extension overloads a method in the parent package where none of the argument types are owned by the extension dependencies.

The problem here is that you are able to call list_footware(wardrobe) even if you don't directly depend on Socks and the extension is loaded just because someone else in the dependency graph happened to depend on Socks. If they remove that dependency, code that called list_footware(wardrobe) will stop working (since the extension will no longer load).

To solve this, we need to force callers of list_footware to provide some kind of "proof" that they are in fact depend on Socks. The only reliable way to do that is to provide the Socks module to the function (which proves you depend on it). What we had in mind was to use JuliaLang/julia#47749 to instead of defining list_footware as done here, define something like:

list_wares{Socks} = ...

Callers of list_wares do now have to pass in the Socks module to do the call which proves that they have access to it and there is now no chance that they accidentally rely on someone else loading Sock and defining the method for you that way.

Any thoughts on this?

@timholy
Copy link
Member Author

timholy commented Dec 16, 2022

That's very interesting. I'm not certain I follow everything as deeply as you've thought about it, but this concern rings true. I've at least once had that experience of regretting depending on an implicit dependent.

My only concern about the proposed solution is, what if your wardrobe has multiple kinds of footware? Do you get into a situation where you have to supply all of the potential modules? I should say this concern might be theoretical: even the real-world example that prompted me to write the Wardrobe/Socks example is something I later reconsidered. (Originally I made the function in the extension a new function, but later I decided it was better as a new specialization of methodinstances.)

@timholy
Copy link
Member Author

timholy commented Dec 31, 2022

I'd be happy to split the "polish" into a separate PR and then we can think about the bigger change. Want me to do that?

@timholy
Copy link
Member Author

timholy commented Jan 20, 2023

I stripped the second commit and may submit that as a separate PR later. Seems better to discuss the wording tweaks separately from the bigger design issues.

@KristofferC KristofferC merged commit 4f94d8f into master Jan 24, 2023
@KristofferC KristofferC deleted the teh/weakdeps_docs branch January 24, 2023 15:01
IanButterworth pushed a commit to IanButterworth/Pkg.jl that referenced this pull request Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants