Skip to content
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

Support "friend dependencies"? #3051

Closed
jakemac53 opened this issue Jul 22, 2021 · 14 comments
Closed

Support "friend dependencies"? #3051

jakemac53 opened this issue Jul 22, 2021 · 14 comments
Labels
type-enhancement A request for a change that isn't a bug

Comments

@jakemac53
Copy link
Contributor

jakemac53 commented Jul 22, 2021

This is a proposal for a new dependency type, a "friend dependency" (not attached to the name).

These dependencies would not be real dependencies. However, if this dependency was a part of the version solve, then it would take the constraints into account.

The motivation would be issues such as dart-lang/sdk#46687. There are cases where your package may only be compatible with a certain version range of another package - but you don't actually depend on that package, and don't want to introduce that dependency just to enforce the version constraint.

In particular if you want to move something from one package to another, this can be useful and allow you to do it without any breaking changes in either package.

@jakemac53 jakemac53 added the type-enhancement A request for a change that isn't a bug label Jul 22, 2021
@sigurdm
Copy link
Contributor

sigurdm commented Aug 6, 2021

Hmm interesting concept!
It sounds like a rather rare use-case for the added complexity, and a bit hard to get right for the user.
What is the advantage over adding it as a real dependency?

@jakemac53
Copy link
Contributor Author

What is the advantage over adding it as a real dependency?

You don't want to add a real dependency where you don't actually use it, because that adds bloat for anybody using your package but not the other package.

For instance in the linked example (dart-lang/sdk#46687), adding analyzer as a dependency to meta version 1.7.0 would not be a viable option. Meta has zero dependencies today and adding analyzer would bring in a whole host of new dependencies (and corresponding dependency constraints).

It sounds like a rather rare use-case for the added complexity, and a bit hard to get right for the user.

I agree it isn't going to be incredibly common, but it could be very useful and beneficial for users when it is used. This comes both in terms of:

  • safer dependency constraints
  • fewer breaking package versions
  • no extra bloat where not needed

@sigurdm
Copy link
Contributor

sigurdm commented Aug 9, 2021

We have plans for a mechanism allowing one to mark package-versions 'broken' in the pub.dev interface.

We would then teach the pub solver to avoid broken versions altogether (unless explicitly demanded).

Would that work for solving issues like: dart-lang/sdk#46687 ? Then the versions of analyzer that conflicts with meta could be marked broken, and patch-releases could be published that would always allow solving within the same ^-ranges.

That seems to me to be a more general solution - and carrying this information outside the pubspec, means that things can be solved as they are discovered.

One possible downside compared to the "friend dependencies" is that it only works when you control the other package...

cc @jonasfj might have opinions?

@jonasfj
Copy link
Member

jonasfj commented Aug 9, 2021

Years ago npm had something called peerDependencies, useful for plugin systems.. but it was eventually deprecated not sure why..

A crazy idea in this space might be to allow package authors to publish incompatibilities for a package version long after it has been published. It's probably way to complicated to actually do. But a fun way to think about the problem.

IMO the downside to adding complexity might outweigh the benefits. Especially if we introduce simple measures like retraction / broken.

@jakemac53
Copy link
Contributor Author

jakemac53 commented Aug 9, 2021

We have plans for a mechanism allowing one to mark package-versions 'broken' in the pub.dev interface.

Allowing you to mark broken versions isn't directly related - if you didn't realize the issue in the first place you would be unlikely to add the "friend dependency" either. It is a good way to fix this problem when you accidentally cause it, but it doesn't allow you to actually release a change like what was attempted in dart-lang/sdk#46687 safely.

In most situations like this today you would either just decide not to go ahead with the change at all, or do a breaking change to safely release it. But this feature would allow you to move things from one package to another without a breaking change in either package, which would be nice (especially for "core" packages).

Consider also the case where you have a package that defines some abstract classes, but where all implementations of that class are known, and you want to add a method to that class. This would simplify the versioning of packages in that case, allowing the "core" package to have fewer breaking changes, and instead just have friend dependencies on the versions of all the packages that implement that class, requiring the version that is compatible. This is the case for package:build today, and we manage it by inventing our own versioning policy, which is that implementors of these classes must keep a tight constraint (to the feature version) on the build package. But that really isn't an ideal situation.

@jakemac53
Copy link
Contributor Author

IMO the downside to adding complexity might outweigh the benefits.

I think the complexity should be pretty minimal, although things are never as easy as they seem :). But essentially you just collect a list of all the non-friend dependencies. Then you throw away all friend dependencies not in that set and only keep the ones that do exist in that set, and treat them as normal dependencies. So there would be probably an extra pass somewhere early on, and the rest of the code should be unchanged.

@jonasfj
Copy link
Member

jonasfj commented Jan 3, 2022

@jakemac53, am I right, that what you really want to do here is specify incompatibilities with non-dependencies?

In most situations like this today you would either just decide not to go ahead with the change at all, or do a breaking change to safely release it. But this feature would allow you to move things from one package to another without a breaking change in either package, which would be nice (especially for "core" packages).

I tried pitching making namespaced imports the default for the new import-syntax, effectively encourage everybody to do:
import 'foo.dart' as foo';
I don't really think I want that (I like it for code readability, as Dart code is impossible to read without an editor that supports "go-to definition"). But I wanted to point out the underlying name conflict issue with two libraries defines the same name. Especially, if the name is moved one package to another. Which is your motivation here, right?

I think it was @lrhn who made the reasonable argument that the compatibility promise in semver only covers the API offered by the package. So it follows that if two packages introduce the same top-level symbol neither of packages needs to bump major version, as no breaking change has happened. The fact that code using both packages might have hard to debug problems is an acceptable limitation of semantic versioning.

I think it's fair to argue that semantic versioning has limitations. Which also implies that sometimes upgrade experiences will be painful.


If we wanted to mitigate this problem, we could:

  • (A) introduce friend_dependencies, which is version constraints on optional-dependencies -- meaning a friend-dependency constraint is ignored unless the friend dependency is installed.
  • (B) introduce dynamic publication of incompatibilities on pub.dev -- effectively allow package authors to publish incompatibility claims after package publication.
  • (C) Use code analysis to ensure that only top-level symbols available from the version in the lower-bound constraint is visible, requiring the that user bumps the lower-bound constraint to access newer symbols.

I think (A) has limitations because you are likely to only discover incompatibilities after publishing. (B) makes the constraint graph dynamic, and could possibly lend itself to abuse -- but it is nonetheless interesting. (C) probably requires some non-trivial engineering, and most likely has limitations too -- but it might solve problem.

@lrhn
Copy link
Member

lrhn commented Jan 4, 2022

Sounds like what you really want is to be able to add upper bounds on dependencies on your package to specific versions of other packages.

That is, package meta wants to do something in version x.y.0 that package analyzer v.w.? is not ready for.
So, package meta wants to add an upper bound on analyzer's dependency on meta, to < x.y.0, for versions of analyzer < v.w+1.0.

That makes sense because the two packages are tightly coupled. It's probably rare otherwise.

It's a kind of inverse dependency (aka., not a dependency). More like an extra conditional constraint of the form analyzer < v.w+1.0 => meta < x.y.0. It's trivially satisfied by having no analyzer package in the project.
We can (probably) add such an extra constraint at any point, but doing it in one of the related packages makes the most sense. Doing it in analyzer is trivial if it actually depends on meta, it's just a normal dependency. Doing it in meta is the new thing, but when it disallows the current version of meta, it just reduces to analyzer >= v.w+1.0, as a requirement, but not a dependency.

So, it makes sense to me. Even when I try to generalize it, I end up at the same thing.

I can see why putting it into meta version x.y.0's pubspec.yaml is tempting. That's the one thing that you have access to when you make the change to meta. It's just the right time and place.

So, it's just a way to add a constraint without adding a dependency, and where the constraint is trivially satisfied if the constrained package is not part of the program.

@jonasfj
Copy link
Member

jonasfj commented Jan 4, 2022

So, it's just a way to add a constraint without adding a dependency, and where the constraint is trivially satisfied if the constrained package is not part of the program.

Yes, in effect it's adding an "incompatibility", because it's a statement that says package meta version a.b.c is incompatible with analyzer version < x.y.z.


My side note was that: afaik this only happens when introducing conflicting top-level symbols in two packages.
And the "correct" answer is foranalyzer to:

  • (1) import meta using import "package:meta/meta.dart" as meta;,
  • (2) have a tighter upper-bound constraint on package:meta (like meta: >= 1.0.0 <1.8.0, so as to disallow future minor versions).

(1) could be done in the language by encouraging import .. as ..
(2) is actually encouraged when exporting symbols from a dependency)

If the primary motivation is conflicting top-level symbols, then maybe the root of the problem is language features that doesn't encourage forward compatibility (1).

@jakemac53
Copy link
Contributor Author

I think it's fair to argue that semantic versioning has limitations. Which also implies that sometimes upgrade experiences will be painful.

Fwiw I think in general semantic versioning does cover this use case, the goal is not to change how semantic versioning works. Instead what I am proposing is I think best described by @lrhn :

So, it's just a way to add a constraint without adding a dependency, and where the constraint is trivially satisfied if the constrained package is not part of the program.

@jakemac53
Copy link
Contributor Author

I do think that option B could be interesting as well, it would help solve some additional cases that come up. But I do worry about the UX implications for some users:

  • (B) introduce dynamic publication of incompatibilities on pub.dev -- effectively allow package authors to publish incompatibility claims after package publication.

Possibly it would be enough to just log a warning to users who run a pub get if they have an incompatible solve due to a dynamically added constraint, but still give them the exact solve they had before.

@jonasfj
Copy link
Member

jonasfj commented Jan 4, 2022

Option (B) definitely has UX implications. On the other hand, it would be a really cool and innovative :D

It would effectively make dependency constraints dynamically tweakable after package publication.
I guess it's a more general case of package version retraction.

@jakemac53
Copy link
Contributor Author

Ya, it would be a better alternative to retraction for many use cases I think.

It would also enable some packages to avoid breaking changes, where the breakage is not actually expected to affect general users, and only a certain other tightly coupled package. You could add a new upper bound dynamically to that one tightly coupled package instead.

@sigurdm
Copy link
Contributor

sigurdm commented Aug 28, 2024

I don't think we have any plans for this in the foreseeable future.

Closing as won't-fix for now.

Please file a new issue if this is still pressing.

@sigurdm sigurdm closed this as not planned Won't fix, can't repro, duplicate, stale Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants