Subject: Incorrect propagation chain reporting on profile collision

From: Ulf Herrman <[email-masked]>
Date: Fri, 05 May 2023 15:32:51 -0500

A <manifest-entry> has a `parent' field that contains a promise that
returns either #f or another <manifest-entry>.  This is somewhat
incomplete information - a given entry can be a propagated-input to
multiple entries, so there can actually be multiple parents.  Given that
this is only used for tracing a propagation chain to display to the
user, though, that shouldn't be a problem.  What is a problem, though,
is that this field is determined when the manifest is read in and
remains static thereafter.  To demonstrate this, consider the following
example:

User installs package A1, which propagates B1.  User also installs
package C, which also propagates B1.  Later the package definition for A
is updated with a newer version, A2, and this propagates a newer version
of B, B2.  User runs 'guix pull', and subsequently runs 'guix package
-u'.  Note that C is unchanged and still propagates B1.  Guix sees that
the proposed new manifest contains A2, C, B1, and B2, and properly
detects that B1 and B2 represent a collision.  It traces B2 back to A2
correctly, and then goes to hunt down what propagated B1.  What happens
next depends somewhat on the arbitrary ordering of entries in the
manifest, but it is entirely possible that the parent recorded for B1 is
not C, but A1, even though the proposed new manifest doesn't even
contain A1.

So you end up with bizarre error messages like this:

-----------------------------------------
$ guix package -u certbot
The following package will be upgraded:
   certbot 1.28.0 → 2.3.0

guix package: error: profile contains conflicting entries for python-cryptography
guix package: error:   first entry: [email-masked] /gnu/store/bcixgv8vg9bz6gyvijavqiha77h17icv-python-cryptography-40.0.1
guix package: error:    ... propagated from [email-masked]
guix package: error:    ... propagated from [email-masked]
guix package: error:    ... propagated from [email-masked]
guix package: error:   second entry: [email-masked] /gnu/store/2kz701xhwvcswdp4dj3fd7v86v2ra70x-python-cryptography-3.4.8
guix package: error:    ... propagated from [email-masked]
guix package: error:    ... propagated from [email-masked]
guix package: error:    ... propagated from [email-masked]
hint: You cannot have two different versions or variants of `certbot' in the same
profile.
-----------------------------------------

This happens because the collision check is performed on the new
manifest, and the parent fields are leftover from the old manifest.

Ultimately, the parent field is just an optimization - it can always be
re-derived from a manifest, and indeed, when a <manifest-entry> is
shared across multiple manifests, *must* be re-derived with respect to a
given manifest in order for it to make sense.  I propose that we remove
the 'parent' field entirely and modify &profile-collision-error so that
it also reports the manifest the collision occurred in.  (guix ui) can
then use this to provide more complete (and also accurate) error
messages.  Note that at present check-for-collisions actually can report
manifest entries that aren't "part of" *any* manifest, since they're
freshly created by lower-manifest-entry.

Tangentially related: it's worth noting that only showing two
propagation chains tends to lead to frustrating "whack-a-mole"-style
package transactions, since a decision about which package to keep,
remove, or upgrade has to be made before the next collision source can
be found.  We should probably add a profile mode to 'guix graph' that
shows the propagation graph for a proposed new profile manifest, or at
least let 'guix package' show all collisions when invoked with higher
verbosity.