Split input components out in Gopkg.lock #584
Conversation
|
This now fixes #421, as well. |
| @@ -162,7 +162,7 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error { | |||
| writeV = dep.VendorAlways | |||
| } | |||
|
|
|||
| newLock := dep.LockFromInterface(solution) | |||
| newLock := dep.LockFromSolution(solution) | |||
There was a problem hiding this comment.
🎉 I know this is a small change, but the name is so much more clear that this gave me a happy.
| } | ||
|
|
||
| func (s *solver) Name() string { | ||
| return "gps-cdcl" |
There was a problem hiding this comment.
Conflict-Driven Clause Learning. It's the general class of SAT solver into which gps' solver falls.
| @@ -2,6 +2,7 @@ | |||
| "commands": [ | |||
| ["remove", "github.com/not/used"] | |||
| ], | |||
| "error-expected": "", | |||
There was a problem hiding this comment.
👍 I've been seeing this pop up when I run tests locally. Any idea why the build didn't break and force us to update the testdata when it was first introduced?
There was a problem hiding this comment.
I've noticed it elsewhere, as well. I assume the build doesn't break because we're not actually validating that file - it's just the testcase procedure. What actually strikes me as odd is that passing the -update flag writes these files out at all. That seems like the actual mistake bug.
There was a problem hiding this comment.
Oh right, we only compare golden files, not the testcases. I don't think it's a bug to update testcase files when -update is specified, e.g. updating the final vendor contents or expected-error message. It just looks odd in this instance because it's not fixing the test, and is just futzing with the schema/formatting.
| // The version of the ProjectAnalyzer used in generating this solution. | ||
| AnalyzerVersion() int | ||
| // The name of the Solver used in generating this solution. | ||
| SolverName() string |
There was a problem hiding this comment.
Looks like there's a plan for having multiple solvers in the future?
There was a problem hiding this comment.
I have no actual plan to do so, but it's possible that circumstances could demand it. We definitely needed the solver version field; as long as I was adding one, it seemed only to make sense to add the other.
Also...I wanted to make the cdcl thing explicit, because people would ask questions. Like you did. And then maybe get interested in working on the solver 😄
lock.go
Outdated
| } | ||
|
|
||
| type solveMeta struct { | ||
| Memo string `toml:"inputs-hash"` |
There was a problem hiding this comment.
How do you feel about renaming Memo to InputsHash? Small nit but would make it easier on contributors if the names lined up.
There was a problem hiding this comment.
Yeah I kinda flip-flopped on this. There are a bunch of different similar names that get used for this - both the method that does it, and the name of fields that store it. (This mirrors the common conflation of "hashing" with "hash digest"). I didn't change it only because I kinda didn't want to start unpacking that can of worms...though still, yes, it would be incrementally better to change it here now and then change it again later than have the incongruity.
testdata/lock/error0.toml
Outdated
| @@ -1,4 +1,5 @@ | |||
| memo = "2252a285ab27944a4d7adcba8dbd03980f59ba652f12db39fa93b927c345593e" | |||
| [solve-meta] | |||
| inputs-hash = "2252a285ab27944a4d7adcba8dbd03980f59ba652f12db39fa93b927c345593e" | |||
There was a problem hiding this comment.
Nit, but can we indent this consistently with the rest of the file?
| @@ -1,5 +1,3 @@ | |||
| Memo: 595716d270828e763c811ef79c9c41f85b1d1bfbdfe85280036405c03772206c -> 2252a285ab27944a4d7adcba8dbd03980f59ba652f12db39fa93b927c345593e | |||
There was a problem hiding this comment.
This is going to be cleaned up in #623, right?
There was a problem hiding this comment.
Ah crap, I think I just plain did this part wrong. Thanks for catching it.
There was a problem hiding this comment.
That is, if there was a memo diff before, there should be a memo diff now.
|
this is integrated in the |
Fixes #508
Fixes #421
The essential changes are as follows:
Locknow has aLock.Inputsthat contains the three bits of input info we want to track (for now): the old top-levelMemoproperty, plus the name and version of the analyzer.gps.Solutionadds two methods to return the name and version of theProjectAnalyzerthat was used to create the solution.LockFromInterface(in gps.Lock) *LockbecomesLockFromSolution(in gps.Solution) *Lock.