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

[pub] Reconsider the default versioning-strategy #4979

Open
jonasfj opened this issue Apr 8, 2022 · 6 comments
Open

[pub] Reconsider the default versioning-strategy #4979

jonasfj opened this issue Apr 8, 2022 · 6 comments
Labels
L: dart:pub Dart packages via pub T: bug 🐞 Something isn't working versioning

Comments

@jonasfj
Copy link
Contributor

jonasfj commented Apr 8, 2022

Reconsider the default versioning-strategy.

We have the following versioning-strategies:

  • widen, extend only the upper bound to include the new version.
  • increase, always update the constraint lower bound to match the new version.
  • increase-if-necessary, leave the constraint if the original constraint allows the new version, otherwise, bump the constraint.
Constraint Current Version New Version Strategy New Constraint
^1.0.0 1.0.0 1.2.0 'widen' ^1.0.0
'increase' ^1.2.0
'increase-if-necessary' ^1.0.0
^1.0.0 1.0.0 2.0.0 'widen' >=1.0.0 <3.0.0
'increase' ^2.0.0
'increase-if-necessary' ^2.0.0
(side note: maybe we should include tables like this in the documentation, feel free to steal)

Considerations

For packages using the widen strategy is attractive because increasing the lower-bound constraint can cause resolution conflicts (as we can only have one version of each package).

For applications using the increase strategy is attractive because it makes accidental downgrades unlikely.

However, using widen is likely to cause problems unless humans are carefully considering the scenario. Especially, since most developers aren't using dart pub downgrade to test with their lower-bound constraint (assuming it's even possible to resolve the lower-bound -- as other dependencies might have higher lower-bound constraints on the same package).

Similarly, using increase does slightly increase the risk that the package author runs into resolution conflict in the future. But it is perhaps a bit safer, though debugging an issue due to accidental downgrade isn't too hard.


On top of all this, is the fact that we don't have good ways to detect if a pubspec.yaml is an application or a package.
The current logic assumes the project is a package if pubspec.yaml contains version. This is unfortunate, because Flutter default application template includes version key, used to indicate application version. This can be remedied by checking if publish_to: none is specified, which indicates that it's an application (and also present in Flutter default template).

But even if we can probably detect that something is a package, unless it is a core package used by a lot of users, it's probably best to do increase-if-necessary. Using widen can easily lead to bugs, so maybe it's best to only used that core packages that are used by many other packages. I could also be convinced that we should merely patch the logic for detecting whether something is a package or application.

@devoncarew
Copy link

Dropping in to add a +1 for defaulting to the increase-if-necessary strategy.

Additionally, I think it'll be tough to accurately detect when we're looking at an app or a library. Using version has issues - mentioned in the comment above. Looking for the presence of a checked in pubspec.lock file might be a more reliable indication that we have an app. Even that will likely mis-identify both apps and libraries - it's a convention to check in a pubspec.lock file for apps, but not very consistently followed. I think having the behavior of the pub dependabot's versioning strategy be predictable - having one default for both apps and libraries - would be useful, and make easier to reason about what dependabot will do. For use cases where they need a different strategy (increase, for apps which want to restrict possibly bringing in an older package dep? widen, for highly used packages, that we willing to sign up for testing both the high and low end of their dep ranges?), they can opt into the more specific strategy.

@brrygrdn brrygrdn added the L: dart:pub Dart packages via pub label Apr 14, 2022
@sigurdm
Copy link
Contributor

sigurdm commented Apr 19, 2022

cc @jurre what do you think about defaulting to increase-if-necessary for pub? It seems to be the preference in the dart world. Or do you prefer internal consistency for dependabot?

@jurre
Copy link
Member

jurre commented Apr 20, 2022

I think if we document it clearly I have no real concerns, we should default to what makes most sense for users, having everyone needing to configure a version strategy goes against the philosophy of trying to do the right thing by default

@jurre
Copy link
Member

jurre commented Apr 20, 2022

Including @mctofu in the conversation, as I'm going to be out on parental leave for a good while starting next week

@jeffwidman
Copy link
Member

Circling back on this:

@jurre: I think if we document it clearly I have no real concerns, we should default to what makes most sense for users, having everyone needing to configure a version strategy goes against the philosophy of trying to do the right thing by default

👍 makes sense to me.

@jonasfj / @sigurdm you're the experts here, what would you prefer?

Please take a look at @devoncarew 's PR and see if it matches what you're wanting so he knows whether to spend time fixing tests or not:

@sigurdm
Copy link
Contributor

sigurdm commented Aug 8, 2023

@jonasfj / @sigurdm you're the experts here, what would you prefer?

I think we are in agreement with devon, and I already fixed the tests I believe :)

atsansone pushed a commit to flutter/website that referenced this issue Oct 20, 2023
This likely should be the default, but it's [not
yet](dependabot/dependabot-core#4979). `widen`
doesn't make sense for example use cases, we just want to use the
latest.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: dart:pub Dart packages via pub T: bug 🐞 Something isn't working versioning
Projects
None yet
Development

No branches or pull requests

7 participants