it's hard to know what to do when inputs-digest in Gopkg.lock conflicts

[glasser]

Basically every change to Gopkg.lock updates the solve-metadata.inputs-digest field. That means that whenever you're merging two changes that touch Gopkg.lock you're going to get a merge conflict.

It's not the end of the world, but it would be nice if there was more guidance for what to do when this happens. I assume the answer is basically just "resolve the conflict randomly, run dep ensure again" but I'm a little paranoid, like, what if choosing the wrong digest in my resolution means that dep ensure won't work (either crashing or incorrectly assuming that it doesn't have to do work due to a cache)? Maybe this is safe now but might get worse when some of the in-progress caching work for dep is done?

I think it would be great if the docs answered the question of what to do if you get an inputs-digest conflict. It would be even better if the file was self-documenting, with a comment above the inputs-digest line explaining how to resolve merge conflicts; I guess go-toml doesn't support adding comments, though I guess you could just add an otherwise ignored field whose value is effectively a comment.

Experienced with dep v0.3.1.

[sdboyer]

hi, welcome! interesting problem, this is the first time someone's raised this. to answer most effectively, i have to start with another question:

Basically every change to Gopkg.lock updates the solve-metadata.inputs-digest field.

so, it's normal for some changes to produce inputs-digest changes: if you import a new package, drop all imports to a package, or change constraints in Gopkg.toml. however, if your experience that "basically every" change to Gopkg.lock involves changes to the inputs-digest, then i suspect you may be filling Gopkg.toml with revision constraints, and manually updating them all the time. if so, that's really not the way dep is designed to work (though it is how e.g. Godep has worked) - you're basically treating Gopkg.toml like a lock, which is likely adding very little in terms of reliability.

so, before i continue on with the rest of the answer - is that how you/your folks are using dep right now? 😄 if not, what are the sorts of changes that you're performing so regularly that result in the hash changes?

[glasser]

I'm mostly referring to adding new packages, yeah. One person adds package X on a branch and in parallel another adds package Y. We're new to dep and haven't been doing much upgrading yet, just adding :)

(As a separate issue, I am putting some revision constraints in Gopkg.toml, but only as part of converting from govendor because I don't want to immediately upgrade lots of old libraries as part of migration. I don't intend to replace revision constraints with other revision constraints.)

[sdboyer]

OK, sounds like you're holding it right, then :)

so, there actually isn't a terribly easy answer here, unfortunately. the inputs-digest hash is computed based from the set of inputs to dep's solver. by the very nature of hashing, when those value change, the hash digest will change in an unpredictable way. the closest hack i can offer you is that, if you have the code locally, you could run dep hash-inputs | tr -d '\n' | shasum -a 256 on the code with the merge conflict, and it'll produce the hash that should be in Gopkg.lock.

however, that's really not a good solution. while it's quite unlikely, it's just not possible to guarantee that even though the rest of the Gopkg.lock merge went smoothly, that the output is actually sound with respect to dep's rules. the safe thing to do is to reconcile the inputs in Gopkg.lock as best you can, pick an inputs-digest from either side (it doesn't matter), then run dep ensure against the result. without any arguments, it will follow the most conservative path possible, and update Gopkg.lock accordingly.

note that i have been considering dropping the inputs-digest entirely, and this hiccup pushes me further towards it. there are uses for that hash value, but it's not actually necessary that we rely on it in the way that we do - it's very nearly as efficient to just check the values in the Gopkg.lock directly to make sure they align with what's in your imports and Gopkg.toml.

[glasser]

Yeah, I had just been setting it to a syntactically valid but wrong digest (other strings return an error) and running dep ensure and it fixed it. A comment or fake comment or FAQ entry saying "on conflict just delete the field and run dep ensure, it'll be ok" would be nice (if true).

I saw this today which is clever but maybe more the kind of advanced thing that folks working on v5 of their tool have time for: https://twitter.com/maybekatz/status/914006850550370304

[glasser]

Oh hey, yarn too: yarnpkg/yarn#3544

[sdboyer]

oooooooh. fancy! i'm currently ribbing the npm folks for creating work for me 😜

i think it's not impossible that we could do that, though they get it for almost free because they already have a tree fixup phase, from what i understand. we don't have such a phase, and we need to create something like one in order to do it. i'd happily guide a PR that wanted to work on that 😄

[glasser]

Heh, that is probably beyond my capacities right now.

On the other hand I'd be happy to try to do something simpler. And hey, our TOML lib just added the ability to write (static) comments a few days ago!

From a quick skim, it looks like it is always safe (if perhaps not the best performance) to delete the inputs-digest line and re-run dep ensure. All we ever do with an inputs digest is compare it to another one (and validate that it parses as hex, but empty is fine too). It might be good to slightly tweak the dep status error message in this case.

So what do you think about improving dep status a bit, upgrading go-toml, and adding a comment to Gopkg.lock saying that you should just delete the line and re-run ensure on conflict?

(Other things I noticed while investigating: errInputDigestMismatch should pluralize "input". The errors that were changed to warnings in #1225 are preceded by now-misleading comments claiming that we're still going to bail out.)

[sethrosenblum]

note that i have been considering dropping the inputs-digest entirely, and this hiccup pushes me further towards it. there are uses for that hash value, but it's not actually necessary that we rely on it in the way that we do

I definitely agree with removing the inputs-digest from the lockfile. The fact that it's un-mergeable between unrelated feature branches makes it impossible to work on dependency changes in parallel, especially when changes are submitted as PRs. That's why similar examples like Gemfile.lock and Berksfile.lock don't include this. Instead, they generate files that will conflict only if the actual dependencies conflict.

If this is needed, maybe it should be moved to a file or location that's not intended to be committed to version control.

[sdboyer]

yes, we're definitely going to be removing the inputs-digest entirely. doing so won't solve all merge problems - i'd love to see us get some adaptive behavior in akin to what npm has implemented - but doing so will get us a long way.

it will require writing several new verification routines as substitutes, though. those are likely to go hand-in-hand with the ideas outlined in this doc for a dep check subcommand.