Pub: `pub upgrade <package>` does not respect version constraints

[walnutdust]

Environment

• pub version or flutter pub version: Pub 2.9.1

Problem

pub upgrade <package> does not seem to be respecting version constraints, for example in the following test:

void main() {
  test('upgrade test', () async {
    await servePackages((builder) {
      builder.serve('foo', '1.0.0');
    });

    await d.appDir({'foo': '^1.0.0'}).create();

    await pubGet();

    await d.appPackagesFile({'foo': '1.0.0'}).validate();

    globalPackageServer.add((builder) {
      builder.serve('foo', '1.5.0');
      builder.serve('foo', '2.0.0');
    });

    await pubUpgrade(args: ['foo']);
  });
}

Expected behavior

Expected the test to pass without issues, with foo getting bumped to 1.5.0.

Actual behavior

The version solver tries to force version 2.0.0 of foo.

--trace output

[e] FINE: Pub 0.1.2+3
    MSG : Resolving dependencies...
[e] SLVR: fact: myapp is 0.0.0
[e] SLVR: derived: myapp
[e] SLVR: fact: myapp depends on foo ^1.0.0
[e] SLVR:   selecting myapp
[e] SLVR:   derived: foo ^1.0.0
[e] IO  : Get versions from http://localhost:44351/api/packages/foo.
[e] IO  : HTTP GET http://localhost:44351/api/packages/foo
[e]     | Accept: application/vnd.pub.v2+json
[e]     | X-Pub-OS: linux
[e]     | X-Pub-Command: upgrade
[e]     | X-Pub-Session-ID: A8B4AB61-AED0-4464-8A5B-AFE54C9F820F
[e]     | X-Pub-Environment: test-environment
[e]     | X-Pub-Reason: direct
[e]     | user-agent: Dart pub 0.1.2+3
[e] IO  : HTTP response 200 OK for GET http://localhost:44351/api/packages/foo
[e]     | took 0:00:00.041970
[e]     | transfer-encoding: chunked
[e]     | date: Fri, 21 Aug 2020 13:48:30 GMT
[e]     | x-frame-options: SAMEORIGIN
[e]     | content-type: text/plain; charset=utf-8
[e]     | x-xss-protection: 1; mode=block
[e]     | x-content-type-options: nosniff
[e]     | server: dart:io with Shelf
[e] SLVR:   fact: the latest version of foo (2.0.0) is required
[e] SLVR:   conflict: the latest version of foo (2.0.0) is required
[e] SLVR:   ! foo <2.0.0-∞ or >2.0.0 is satisfied by foo ^1.0.0
[e] SLVR:   ! which is caused by "myapp depends on foo ^1.0.0"
[e] SLVR:   ! thus: version solving failed
[e] SLVR: Version solving took 0:00:00.100353 seconds.
[e]     | Tried 1 solutions.
[e] FINE: Resolving dependencies finished (0.116s).
[e] ERR : Because myapp depends on foo <2.0.0-∞ or >2.0.0 but the latest version (2.0.0) is required, version solving failed.
[e] FINE: Exception type: SolveFailure
[e] FINE: package:pub/src/solver/version_solver.dart 312:5   VersionSolver._resolveConflict
[e]     | package:pub/src/solver/version_solver.dart 133:27  VersionSolver._propagate
[e]     | package:pub/src/solver/version_solver.dart 97:11   VersionSolver.solve.<fn>
[e]     | ===== asynchronous gap ===========================
[e]     | dart:async                                         Future.catchError
[e]     | package:pub/src/utils.dart 113:52                  captureErrors.wrappedCallback
[e]     | package:stack_trace                                Chain.capture
[e]     | package:pub/src/utils.dart 126:11                  captureErrors
[e]     | package:pub/src/command_runner.dart 182:13         PubCommandRunner.runCommand

Notes

Upon inspection, it is likely that the issue is caused by useLatest in acquireDependencies, which forces the newest version regardless of constraint on pubspec.yaml in packageLister.bestVersion.

[walnutdust]

@jonasfj

[jonasfj]

I think I wish pub upgrade foo meant:

Upgrade foo to the latest version that:
(i) is allowed by pubspec.yaml,
(ii) is mutually compatible with other dependency constraints in pubspec.yaml
While during a reasonable effort to keep other dependencies locked to their current version.

---

I think a reasonably heuristic for this might be something along the lines of:
(1) Backup the pubspec.lock for later
(2) Run pub upgrade
(3) Take the version of foo that we were upgraded to let's call this version x.y.z
(4) Revert pubspec.lock
(5) Backup pubspec.yaml for later
(6) Update pubspec.yaml to say: foo: ^x.y.z
(7) Run pub get
(8) Restore pubspec.yaml

In step (3) we discover a version of foo that satisfies (i) and (ii), it may not be the latest version of foo, but it might be a reasonable heuristic.

In step (7) we run pub get with the original pubspec.lock file, and with a requirement that foo is upgraded to ^x.y.z (this is a heuristic that attempts to preserve the lock).
We know from step (3) that x.y.z is compatible with the original constraint on foo, so restoring pubspec.yaml is valid.

I say _heuristic_ here, because the solver doesn't explore the entire solution-space. Instead it has some heuristics for exploring certain paths first, and those will affect whether the version picked in step (3), as well what locked versions are preserved.

---

Whether this is a better heuristic for pub upgrade foo is a little unclear, we could also modify the solver to change the order in which the solution-space is explored.

This might require a bit more thinking...

/cc @sigurdm

[munificent]

That heuristic feels a little dubious to me, and I always hesitate to take pub's solver and wrap it in some larger iterative algorithm. The solver's job is to do that iteration.

If we take a step back, what is a user asking for when they run pub upgrade foo while also having some dependency constraint on foo and having it already present in their lockfile? The user has told pub:

1. There is some range of versions of foo I'm OK with, because I put it in my pubspec constraint.
2. I don't want it to be the current version of foo from the lockfile.
3. I do want it to be something higher than that.

I think we can model that intention directly as a constraint:

1. Take the version constraint on foo from the root package's pubspec.
2. Create a new constraint >x where x is the current version of foo in the lockfile. For example, if the lockfile is 1.2.3, we create >1.2.3.
3. Intersect those two constraints together. For example, if the pubspec has foo: >=1.0.0 <2.0.0 and the lockfile has 1.2.3, we get >1.2.3 <2.0.0.
4. Run the solver using that new constraint for foo instead of the constraint in the pubspec.

Since that constraint explicitly excludes the current version, it will implicitly unlock it. Then it should start hunting for the highest version that also fits their pubspec constraint, unlocking other packages as it goes as needs to. If it's possible to upgrade foo at all, this should find a solution.

I'm not positive, but I think this could do what we want.

[jonasfj]

That's an interesting idea. However, since all locked packages only have one version the solver will pick those first.

This means that while we do force an upgrade of foo. We are still going to explore some of branches that retain other existing locks first. Hence, if locked versions allow an upgrade to foo version 1.2.4, but not 1.3.0, we may not get 1.3.0. This now depends on the arbitrary order in which branches are explored.

We could the run this repeatedly until no further upgrades of foo is possible, hehe,

This could very well be a corner case, but I fear the branch order will play tricks on us here.

We could of course tweak the branching priority to explore branches that upgrade foo as much as possible first. This is possible and plausibly sensible.
However, it means that pub upgrade foo might give you a newer version of foo than pub upgrade will.
That however, is probably a rare corner case.

[munificent]

This means that while we do force an upgrade of foo. We are still going to explore some of branches that retain other existing locks first. Hence, if locked versions allow an upgrade to foo version 1.2.4, but not 1.3.0, we may not get 1.3.0. This now depends on the arbitrary order in which branches are explored.

Ah, right. This is a hard problem. It gets a lot closer to looking like a soft constraint solver: find the highest version of foo that drags the other dependencies upwards as little as possible. We probably don't want to go down the road of soft constraint solving. :)

Maybe it is enough to just unlock foo and if you don't get a version that's quite as high as you wanted... well you can always change your constraint on foo in your pubspec to tell pub what you want. If you just run pub upgrade foo, all you've really said is "Something higher please, I don't care what."

[jonasfj]

Maybe it is enough to just unlock foo and if you don't get a version that's quite as high as you wanted...

Yeah, that seems simple, but unless we get the latest version of foo allowed by pubspec.yaml, we would probably need to print a message saying that further upgrades may be possible by running pub upgrade and not pub upgrade foo which only unlocks foo.

It's probably less useful, but I doubt this is a frequently used feature anyways.

We probably don't want to go down the road of soft constraint solving. :)

I agree, if we start using some best-first-search wrt. optimization criteria, we'll likely find that: (i) the optimization criteria has to be a heuristic because there is no absolute rank of solutions, (ii) the impact of this will be very small, as most packages don't conflict and newer versions are frequently compatible with each other.

---

Context: we started looking at this because @walnutdust was working on pub upgrade --major-version (which upgrades version constraints in pubspec.yaml), here it suddenly becomes very attractive to use pub upgrade --major-version foo for cases where you only want the major version bumped for foo.

In the case of --major-versions I think it's rather desirable to unlock the other dependencies, and but only allow major version bumps for foo in pubspec.yaml. Since bumping major version of foo is what is desired, but locking all the other dependencies would be an unfortunate way of holding back a major version bump for foo. On the flip side this would be less consistent.

[munificent]

Since bumping major version of foo is what is desired, but locking all the other dependencies would be an unfortunate way of holding back a major version bump for foo. On the flip side this would be less consistent.

I don't have a lot of context so I'm probably missing something but it seems like the easiest answer here is to upgrade the constraint on foo in the pubspec to require the next major version and then let pub get sort it out. The solver will then unlock whatever else it needs to unlock in order to find a solution that lets foo use that next major version.

[jonasfj]

@sigurdm and I took some time to discuss the various options, and have concluded the following.

• pub upgrade foo, should:
  
  
  
  • Remove foo from pubspec.lock and runs pub get
  
  
  • pros:
    
    
    
    
    
    • This is simple to explain and understand.
    
    
    
    • This is idempotent/stable, running pub upgrade foo an extra time has no effect (unless new versions haven been published).
    
    
    
  
  
  • cons:
    
    
    
    
    
    • Another dependency bar might hold back a newer compatible version of foo.
    
    
    
  
  
  • Rationale:
    
    
    
    
    
    • It's rare that bar would have an upper-bound constraint on a minor version of foo.
    
    
    
    • If you are willing to upgrade other packages, then just run pub upgrade. In other words, pub upgrade foo is for the special where you only want to upgrade foo.
    
    
    
  
  
• pub upgrade --major-version foo, should:
  
  
  
  • Remove upper-bound on foo from pubspec.yaml (retain lower-bound)
  
  
  • Run pub upgrade
  
  
  • Add an appropriate ^-bound on foo in pubspec.yaml (based on the version selected during resolution).
    
    
    
    
    
    • pros:
    
    
    
    • It's simple to explain, that:
      
      
      
      
      
      
      
      • you are running pub upgrade without the upper-bound constraints on foo.
      
      
      
      
      • all other dependencies will be upgraded the same as running pub upgrade would do.
      
      
      
      
    
    
    
    • This is idempotent/stable, running pub upgrade --major-versions foo an extra time has no effect (unless new versions haven been published).
    
    
    
    • cons:
    
    
    
    • Unrelated dependencies will have minor versions upgraded unnecessarily.
      
      
      (If users wanted this they could just run pub upgrade an extra turn.)
    
    
    
    • rationale:
    
    
    
    • If users really want to retain minor versions of unrelated packages, they can increase the constraint on foo in pubspec.yaml and run pub get instead.
    
    
    
  
  

We also note that, if we wanted to change these semantics in the future, this would probably not be a breaking change. Because users are very unlikely to rely upon this behavior. Hence, we are not prevented from improving our heuristics in the future (even changing semantics wrt. when to retain or partially upgrade minor versions).