Add warnings if the lockfile doesn't match the Registry hashes #1360
Conversation
src/Spago/Command/Fetch.purs
Outdated
| Just expectedIntegrity | expectedIntegrity /= versionMetadata.hash -> | ||
| logWarn $ "Package " <> packageVersion | ||
| <> " has a different hash in the lockfile (" <> Sha256.print expectedIntegrity | ||
| <> ") than in the registry metadata (" <> Sha256.print versionMetadata.hash <> ")" |
There was a problem hiding this comment.
Potentially we could provide a link to an issue on Spago or a Discourse thread or something where we can post why this might happen, so it can be followed by users? For instance, an issue that notes the registry regen as the primary reason, and if so you can regenerate your lockfile; if that isn't why it's happening, then they should open an issue so we can understand the situation. This shouldn't really happen.
There was a problem hiding this comment.
I am thinking we release this together with #1362 and together with the new registry, so that lockfiles will be regenerated anyways and no such warning will pop up then.
Then we can make the warning a bit scarier since as you say it shouldn't really happen 🙂
There was a problem hiding this comment.
I have updated the error message pointing to the issue tracker, but I'm not really happy with it
| -- Verify that the lockfile integrity matches the registry metadata | ||
| case Map.lookup (Tuple name v) lockfileIntegrities of | ||
| Just expectedIntegrity | expectedIntegrity /= versionMetadata.hash -> | ||
| logWarn $ Array.intercalate "\n" |
There was a problem hiding this comment.
An alternative would be printing an error and exiting the process, and expecting that people can come to the issue tracker if they hit a weird error in Spago such as this
There was a problem hiding this comment.
I am worried about the opposite problem: flooding the issue tracker if someone accidentally stumbles on this 😄
In the same vein as purescript/registry-dev#723, we add hash verification for the lockfile hashes: the check between tarball hash and what we store in the metadata is error-worthy, but we currently say nothing if the lockfile doesn't have good hashes. This makes sense as those hashes were initially put there for tools such as Nix.
In any case, as we are approaching the Registry-regen, it might make sense to start warning about these mismatch and make them actionable. @thomashoneyman the current warning is not very actionable, I would like to tell users to regen the lockfile, do you have any ideas on how to not make it overly scary?