Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

[WIP] write partial manifest upon init failure #1280

Closed
wants to merge 1 commit into from

Conversation

JKobyP
Copy link

@JKobyP JKobyP commented Oct 17, 2017

What does this do / why do we need it?

It writes out part of the manifest and lock if dep init fails during solve (and indicates in the manifest that it's incomplete). It's helpful because it gives the confused programmer something tangible to work from.

What should your reviewer look out for in this PR?

Unintended side effects, proper idiomatic code. Some caveats are also listed in the list below.

Do you need help or clarification on anything?

What kind of tests ought to be written for this change?

Which issue(s) does this PR fix?

fixes #909

Comments on the state of the WIP (I wanted to get something up):

  • It builds and tests pass
  • My changes somehow squashed the typically helpful error logging on StdErr. I'll be looking into that issue as I push commits.
  • I've tested this by hand by running dep init on a project that imports a project with an invalid manifest (one that contains version conflicts). The partial written manifest was empty, which I think caused more confusion. An upcoming commit will check for that, and won't bother writing the partial if it'll be empty.
  • An upcoming commit will also add the change to the changelog

The proposed changes write a partial manifest and lock
in the case of a failed solve. The manifest contains a
note describing that the manifest is incomplete.
Solves golang#909

Signed-off-by: Koby Picker <[email protected]>
@JKobyP
Copy link
Author

JKobyP commented Oct 17, 2017

Yikes, looks like I broke a few integration tests. It didn't occur to me that it wouldn't be sufficient to run go test without arguments in the top level directory.

@sdboyer sdboyer added this to the v0.3.3 milestone Oct 24, 2017
@sdboyer
Copy link
Member

sdboyer commented Oct 25, 2017

whew - you really jumped right in, straight to solver modifications and everything 😄

that said, if we can avoid solver changes for a first pass on this, i think it's worth trying to do so. there are so many varieties of failures we can encounter there. if, instead, we can focus on just getting a Gopkg.toml put together based purely on the inputs we sent to the solver in the first place, i think that's an adequate starting point.

then, in a follow-up, we can discuss out what partial solutions from the solver might look like 🎉

@sdboyer sdboyer removed this from the v0.4.0 milestone Nov 29, 2017
@sdboyer
Copy link
Member

sdboyer commented Dec 23, 2017

bump

@sdboyer
Copy link
Member

sdboyer commented Jan 16, 2018

OK, closing this one out to make it clear that someone else can grab it. Please feel free to reopen if you have time to return to it, and we haven't fixed it already 😄

@sdboyer sdboyer closed this Jan 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failed dep init runs should still write out config files
3 participants