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

[WIP] Skip failed remove package vendor #1089

Closed
wants to merge 4 commits into from
Closed

[WIP] Skip failed remove package vendor #1089

wants to merge 4 commits into from

Conversation

cypherfox
Copy link

What does this do / why do we need it?

Will skip failed calls to removeAll() when removing the vendor directories in loaded packages.

If the removeAll fails, the whole dep ensure run will fail and there is no method to work around as dep will start over each time.

What should your reviewer look out for in this PR?

The current code simply ignores the failure to delete the vendor dir. Should it retry? how often?

Which issue(s) does this PR fix?

It is a extension of the Problems discussed in #1087

This is an update to #1085

@sdboyer
Copy link
Member

sdboyer commented Sep 5, 2017

If the removeAll fails, the whole dep ensure run will fail and there is no method to work around as dep will start over each time.

so, i appreciate that it can be frustrating when these relatively small errors cause an entire ensure run to fail, but that's also an important part of dep's design. if there are known errors that resulting in the state on disk not being exactly what the user requested, then we must abort.

the solution here is to fix the underlying cause of the os.RemoveAll() failures, not to undermine that guarantee.

@sdboyer sdboyer closed this Sep 5, 2017
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.

3 participants