Add ability to remove nested vendor and Godeps/_workspace directories

[mattfarina]

@technosophos @sgotti @thockin can you please take a look at this.

[sgotti]

Thanks for doing this.

Godeps/_workspace cannot be removed if the project used godep save -r since all project import paths were rewritten to point to it and compilation will fail.
Many projects actually uses it since it was the best solution to make it self contained before GOVENDOREXPERIMENT.
I don't see a direct way to detect if this option was used in the Godeps.json file without scanning packages imports in the source.

Instead of scanning for its usage and given that this option is deprecated and not allowed with go 1.6 (godep uses standard vendor dirs) probably the fastest solution will be to not remove them.

[mattfarina]

@sgotti good catch. I was looking at Kubernetes because it's a more complicated example than most. Some of the packages it imports have Godeps storing the dependencies in the vendor/ folder and others in the _workspace/src folder with rewrites. Somewhere in the process Godeps resolves all of this. Rewrites are a pain. We may need to undo them.

[thockin]

I don't think Kubernetes has any deps that rewrite, or at least those don't get used by us. That said, yeah, rewriting is a good loophole here that probably needs to be either un-rewritten or left as-is (probably with a warning). Anyone who is doing rewriting needs to stop, or tools like glide will be come hopeless.

[thockin]

This patch looks like what we want. You need to be careful, though, that you don't let the tail wag the dog. As much as I appreciate your enthusiasm, many of these issues are subtle, and the kubernetes approach is just one of potentially many valid approaches. As maintainer, you get the fun job of considering all you users and trying to strike balance.

That said, aside from the rewrites mess, I would totally use this.

[sdboyer]

@thockin glad it's something you'd want! 👏

fwiw, i don't think this is a rash thing to do in general - there are well-defined ways that vendor works, and a strong and clear conceptual basis for stripping the dirs.

[mattfarina]

@thockin We have desire for something as elegant and simple (not sure it's really simple) as is available in Rust, Python, JavaScript, PHP, and other languages. The way the Go ecosystem for dependencies has grown up we have to be a little pragmatic until we have a better overall situation (hopefully someday). I'm trying to figure out where to be pragmatic.

Feedback helps shape that. I'm not going to let the tail wag the dog but I will take everything I can to be pragmatic. I'm an engineer not a scholar. I'm looking for solutions not theoretically proper.

It might surprise you but there is a fair rewriting that needs to be undone to remove the Godeps/_workspace directory. From a quick pass of a quick import you can see the files that need to have their rewrites removed. Godeps does this for you as well.

@sgotti This is opt-in for a reason. It has potential to break things. Before releasing we'd need to add some messaging around that.

[technosophos]

Man... this is gonna be dangerous. I agree with adding some messaging. I wish there was a way to make this really clear to users (up front) that this is an experimental feature, and that it is destructive.

That said, I feel like the community has expressed a strong desire to at least have the option of doing this. I still think we should go ahead with it.

[thockin]

If we started with only nuking recursive /vendor dirs, there's less danger,
maybe? Do any tools do rewriting AND store in /vendor?

That said, I think "Godep rewriting is not supported" is not such a bad
position to take, IMO. Then you can do better IFF it is actually a pain
point for people.

On Thu, Mar 17, 2016 at 3:12 PM, Matt Butcher [email protected]
wrote:

Man... this is gonna be dangerous. I agree with adding some messaging. I
wish there was a way to make this really clear to users (up front) that
this is an experimental feature, and that it is destructive.

That said, I feel like the community has expressed a strong desire to at
least have the option of doing this. I still think we should go ahead with
it.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#339 (comment)

[thockin]

codewise, do you want to hardcode anything godep in a non-godep dir? If it were me, I would do something like:

main():

plugins := []ToolPlugin{NewGoToolPlugin()}
plugins = append(plugins, godep.ProbePlugins()...)
plugins = append(plugins, gom.ProbePlugins()...)
// etc

...where ToolPlugin is an interface. One of the methods on ToolPlugin might be RemoveVendoredJunk(). So here you would just loop over all plugins and have them remove their own vendored junk.

Something like that...(I am a big fan of opaque plugins)

[mattfarina]

I've been ponder this and a few things are relevant:

1. I think we need to support Godeps/_workspace and undoing rewrites for awhile. Godep users are moving to vendor directories but we need to support the old way for awhile. So, this functionality is deprecated from the moment it goes in because it will be removed at some point.
2. I moved the stripping functionality to a custom sub-package within godep. Go uses vendor, Godep is moving to that and has its legacy method, GB uses vendor/src, and most other tools now use vendor. Everything but the old Godep is handled by the vendor stripping. I considered a plugin here but I don't thing another method is going to come along.
3. Docs are important and I added some docs to help including a note about the old Godep method.

With that I think it's ready to go.