Discontinue use of inputs-digest in Gopkg.lock

[sdboyer]

Gopkg.lock contains a property called inputs-digest, which is a hash of the relevant inputs to the solver that were used to generate that Gopkg.lock. The digest value can be generated by running dep hash-inputs | tr -d “\n” | shasum -a256. For reference, this is dep hash-inputs output on dep itself:

-CONSTRAINTS-
github.com/Masterminds/semver
https://github.com/carolynvs/semver.git
b-parse-constraints-with-dash-in-pre
github.com/Masterminds/vcs
svc-^1.11.0
github.com/boltdb/bolt
svc-^1.0.0
github.com/go-yaml/yaml
b-v2
github.com/golang/protobuf
b-master
github.com/jmank88/nuts
svc-^0.2.0
github.com/pelletier/go-toml
b-master
github.com/pkg/errors
svc-^0.8.0
-IMPORTS/REQS-
github.com/Masterminds/semver
github.com/Masterminds/vcs
github.com/armon/go-radix
github.com/boltdb/bolt
github.com/go-yaml/yaml
github.com/golang/protobuf/proto
github.com/jmank88/nuts
github.com/nightlyone/lockfile
github.com/pelletier/go-toml
github.com/pkg/errors
github.com/sdboyer/constext
golang.org/x/sync/errgroup
-IGNORES-
-OVERRIDES-
-ANALYZER-
dep
1

Basically, a list of all the imports, constraints, overrides, requireds, and ignoreds.

There are a few reasons why relying on an explicitly-recorded hash digest for this is suboptimal:

1. It's just not saving us a meaningful amount of work. Instead of relying on the inputs-digest to match, we could directly compare the hash inputs against the values recorded Gopkg.toml to see if everything lines up.
2. Moving to a direct check would also allow us to give meaningful feedback when something's out of sync, rather than just complaining of a hash mismatch.
3. Relying on hash comparison makes the check unnecessarily brittle. Moving to a direct check would allow, for example, constraint changes to be made in Gopkg.toml without triggering a re-solve, so long as what's in Gopkg.lock is still acceptable with respect to those new constraints. (e.g., if we constraint on ^1.0.0 but have v1.1.0 locked in, then moving the constraint to ^1.1.0 would be fine with a direct-check system, but would trigger a pointless re-solve when relying on hash comparisons).
4. As it's hard to know what to do when inputs-digest in Gopkg.lock conflicts #1224 outlines, it creates the possibility for spurious merge conflicts that cannot be easily resolved.

To do this, we'll need to write some new general gps functions for checking if a lock is acceptable with respect to an input set, and probably tweak a bunch of our comparison logic. idk the full extent of it right now - i have to dig a bit. But, we need to do this.

To be clear, there's still potential value in these inputs hash digests, mostly around the possibility of pushing some computation to an edge cache, and/or enabling some prefetching or pipelining of data from upstream sources. But using it for this particular local-only check is just bad, and we need to stop.

[darkowlzz]

Here's a list of the checks that we would need, as discussed in the last maintainer's meeting:

• Check if the locked project versions match with manifest project constraints.
• Check if any alt source exists in manifest, exists in the locked projects too. Might not be true the other way. An existing locked project with alternate source and a manifest without alternate source should not remove source from the locked project.
• Check if all external imports (including required) obtained from solver are in locked project’s packages lists.
• Check if any of the ignored packages exist in any of the locked project’s package list.
• Check if any of the package imports can not be deduced from the locked projects and their package list.
• Once we have vendor verification, vendor verification hash should be checked.

Any corrections, additions or questions about the checks are welcome.

[ecbaldwin]

The inputs-digest field ensures that the the Gopkg.lock file has had the
correct value for it -- based on the source files -- written to it at some
point. From this I might extrapolate that all of the contents of the lock file
are correct given those inputs. From that -- if I'm really brave -- I might
extrapolate that the vendor directory also contains all the right contents.
These are leaps of faith that I'm not comfortable making.

It causes merge conflicts because it is the single hottest point of merge
contention in all of my projects that use vendoring. Besides that, I'm having
trouble figuring out how it is helping me.

Let's say I clone a project and I run dep hash-inputs | tr -d "\n" | shasum -a256 on it and it doesn't match? Say I run dep ensure and all it changes is
this hash. This probably just means that someone didn't commit something
completely. So what? Why is it important to me that this hash is correct enough
to create a new commit to fix it and post it for review?

Even then, it is still a big leap of faith to say that if these values match
then the entire contents of the vendor directory are good. The only way that I
know how to be sure that is true is to run dep ensure on a clean working tree
and check that nothing gets modified.

I'd suggest doing one of the following.

1. Just get rid of this field. To be backward compatible, add a Gopkg.toml
   setting to stop writing it. Seriously, I don't think anyone will miss it. I
   don't even think we need to do any of the "list of checks" outlined in the
   previous comment. Those can be done later.

2. Replace it with merge friendly toml encoding of dep hash-inputs (i.e. not a
   single value squashed into one line that will cause a merge conflict on every
   change). This would serve the same purpose but me merge friendly.

[sdboyer]

@ecbaldwin i think you'll find all your concerns, and more, addressed by #1912 😄