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

Failed dep init runs should still write out config files #909

Closed
carolynvs opened this issue Jul 26, 2017 · 8 comments
Closed

Failed dep init runs should still write out config files #909

carolynvs opened this issue Jul 26, 2017 · 8 comments

Comments

@carolynvs
Copy link
Collaborator

When a dep init run fails, no configuration files are written. This means that even a small problem results in the user having to figure out the problem and start all over, or get a bug fixed in dep, or give up and live in a yurt. It would be helpful if dep wrote out the config files anyway, explained that they are incomplete but provide a jumping off point for tweaking.

We could write out the files to a non-standard path, e.g. Gopkg.incomplete.toml so that the user must take action in order to use the config. Then explain in the output what the user should do to fix and use the config files.

@JKobyP
Copy link

JKobyP commented Sep 2, 2017

Would this be a change in cmd/dep/init.go? Something near handleAllTheFailuresOfTheWorld()?

@carolynvs
Copy link
Collaborator Author

@JKobyP Yup! That is the right place to be looking in. 👍

@carolynvs
Copy link
Collaborator Author

carolynvs commented Sep 4, 2017

Ideally dep would recognize a few categories of "init failure" and handle them appropriately (below). There are a lot of scenarios listed here, they don't all fall under this issue but hopefully they help put this issue in context.

  1. An importer returned an error during import. This is a situation that we are actively fixing so that it instead prints a warning and keeps going (Warn when an importer cannot preserve a constraint #907).
  2. The gopath scanner returns an error. We don't have an issue for this but it should instead print a warning and keep going.
  3. Solve returns an error:
    • Can we tell from the error why solve failed and how catastrophic the failure is?
    • Do we have a full manifest and lock to write out? Or did it die part way through and we only have something that is half complete. I hope we can compare the project's dependencies against what's in the files and highlight what requires attention. This is what I would like to tackle in this issue.
    • Here are some types of errors off the top of my head that would be great to detect and provide an improved user message:
      • The solver couldn't find a version for a project that met all the constraints.
      • The solver couldn't find a version that has all the required sub-packages. Does this indicate a different problem, or different user intervention? Or can it be treated the same as above?
      • One of the projects is inaccessible, most likely due to being private or requiring separate credentials. We have several open issues/epics for this. I'd just like to be able to detect them, and link them to a FAQ.
  4. The imported revisions don't match the final solved revisions. This means that we had to pick a different revision than what they were expecting. Validate that imported locked revisions did not change after solving #908

@JKobyP
Copy link

JKobyP commented Sep 4, 2017

Would this be appropriate for a first contribution? If so, I'd like to try it. Otherwise, feel free to tell me to buzz off!
Thanks for providing the context above, it helps me understand where this issue fits.

Right now, I'm confused about finding a concrete case for the first two of the error types you mentioned above. I'm just starting to form a mental model for dep. It seems to me that

  • The solver couldn't find a version for a project that met all the constraints.

would fail in the import step, rather than the solve step (since the initial solve won't have to consider an existing Gopkg.toml) -- am I wrong? Is there a failure case I can look to as an example?

@carolynvs
Copy link
Collaborator Author

Sorry it wasn't clear from my brain dump, but for this issue, let's just focus on writing out the files and clearly indicate that they are incomplete. The rest is just context which may (or may) not impact how we go about things, as I expect to build on this over time implementing the other concerns.

Do we have a full manifest and lock to write out? Or did it die part way through and we only have something that is half complete. I hope we can compare the project's dependencies against what's in the files and highlight what requires attention. This is what I would like to tackle in this issue.

I think this part is suitable for a first contribution, though it may require a bit of back and forth on the PR. If you are up for that, please have at it and I'd welcome a WIP PR so that we can answer questions as they come up or spot any gaps in my plan. 😀

@JKobyP
Copy link

JKobyP commented Sep 20, 2017

Sorry I'm running 0mph here! Finding time to volunteer can be hard. I'll keep thinking about this in the meantime, but anybody can (and should) take this issue if they want!

@JKobyP
Copy link

JKobyP commented Oct 12, 2017

Looking at this again - it appears that the solve implementation at solver.go:472 currently returns nil during error cases.

Then the content of this PR would be to:

  1. Make sure the data in sel.projects is properly returned by the solver even when there's a failure in solve() (Essentially, reusing solver.go:560-577).
  2. Potentially making a modification in Solve() around 447? I still don't really grok what's going on there, I need to look at it more.
  3. Making sure to write the Manifest and Lock partials while making clear they're flawed (back in init.go)
    Does this sound about right?

JKobyP added a commit to JKobyP/dep that referenced this issue Oct 17, 2017
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 <jkp46@case.edu>
@carolynvs
Copy link
Collaborator Author

@JKobyP I don't have time for a full review, but I took a peek and I think you are on the right track!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants