flox upgrade reports no packages upgraded with catalog even though they are

[zmitchell]

Steps to reproduce:

• Using the catalog server install hello and dump the response
• Modify the version number and rev_count from the response to make the package look "old"
  • Even if this isn't the correct way to make the package look old, you'll see that it still updates the lockfile later
• Uninstall the package, then install it again using this dumped and modified response
• Now you have an "old" installed package
• FLOX_FEATURES_USE_CATALOG=true flox upgrade
• See a message saying that no packages needed upgrading
• Check the lockfile and see that the package was actually upgraded

From discussion:
There's two issues

1. For our tests, we need to change the derivation rather than just the version, and then we can unskip things
2. We need to polish if we report upgrades when the page changes but not the derivation

[mkenigs]

As part of this ticket, we should unskip the tests added in #1488

[dcarley]

I'm not able to reproduce this using the original instructions:

% _FLOX_CATALOG_DUMP_RESPONSE_FILE=catalog.json flox install hello
✅ 'hello' installed to environment 'tmp'

% flox list        
hello: hello (2.12.1)

% flox remove hello
🗑️  'hello' uninstalled from environment 'tmp'

% _FLOX_USE_CATALOG_MOCK=<(jq '.[0][0].pages[0].packages[0] |= ( .version |= "1.2.3" | .rev_count |= 123)' catalog.json) flox install hello
✅ 'hello' installed to environment 'tmp'

% flox list
hello: hello (1.2.3)

% flox upgrade 
⬆️  Upgraded 'hello' in environment 'tmp'.

% flox list   
hello: hello (2.12.1)

Removing the version constraint on an existing package also works:

% flox install hello@2.12
✅ 'hello' installed to environment 'tmp'

% flox list              
hello: hello (2.12)

% sed -iE '/hello.version/d' .flox/env/manifest.toml

% flox upgrade
⬆️  Upgraded 'hello' in environment 'tmp'.

% flox list
hello: hello (2.12.1)

Although it says that it's upgraded every time, even when there are no changes to the lock:

% flox list
hello: hello (2.12.1)

% flox upgrade
⬆️  Upgraded 'hello' in environment 'tmp'.

% flox list
hello: hello (2.12.1)

[dcarley]

Ahh, I see a difference between what's happening above and the commented tests. The "old_hello" fixture also modifies the name. If name is left unmodified or the derivation is modified to match then the tests pass:

• flox/cli/tests/catalog_responses/install_old_hello.json

  Line 16 in 2712b63

  "name": "hello-2.0",

• flox/cli/tests/catalog_responses/install_old_hello.json

  Line 11 in 2712b63

  "derivation": "/nix/store/052y1njzgp8kkrxpk42zplvwn1b9pig3-hello-2.12.1.drv",

It will still require some work to:

1. Massage the responses for "catalog: upgrade errors on iid in group with other packages" to work.
2. Add other systems to the mock response files so that the bats matrix tests pass on other platforms.

[ysndr]

when using the catalog the diff is calculated after locking the manifest here:

flox/cli/flox-rust-sdk/src/models/environment/core_environment.rs

Lines 659 to 671 in 6474bf7

	// find all packages that after upgrading have a different derivation
	let package_diff = upgraded
	.packages
	.iter()
	.filter_map(move |pkg| {
	previous_packages
	.iter()
	.find(|prev| {
	prev.install_id == pkg.install_id && prev.derivation != pkg.derivation
	})
	.map(|prev| (prev.clone(), pkg.clone()))
	})
	.collect();

in particular the current predicate for a package to count as "upgraded" is a change of its derivation attribute.
I.e. a package is not upgraded when the derivation remains the same even though the revision of nixpkgs it got pulled form changes, as both revisions produce the exact same package.

Also it's hard to call the lockfile "upgraded" when wrt a single package both revisions are indeed equivalent.

IMO the missing message is "New lockfile written, no package upgraded" if we do write a lockfile.
OR, we wholly ignore a lockfile or, more granular, package upgrade if its derivations are equivalent.

Saying "hello was upgraded from version A to version A" seems like "the worst" of both options.

[mkenigs]

Split into #1568 and #1570