Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep the nested vendor directories #1

Closed
sgotti opened this issue Mar 4, 2016 · 9 comments
Closed

Keep the nested vendor directories #1

sgotti opened this issue Mar 4, 2016 · 9 comments
Milestone

Comments

@sgotti
Copy link
Owner

sgotti commented Mar 4, 2016

Probably the nested vendor directories should be kept.

If so, how to clean them from unneeded packages/files?

@sgotti sgotti changed the title Keep the sub vendor directories Keep the nested vendor directories Mar 4, 2016
@sgotti
Copy link
Owner Author

sgotti commented Mar 4, 2016

From what I understand Glide should always flattens the vendor tree so this shouln't happen. But I have a repo where nested vendor folders are kept. I have to create a smaller reproducer and take a look.

@mattfarina
Copy link

This is a good idea.

@sgotti
Copy link
Owner Author

sgotti commented Mar 5, 2016

@mattfarina looks like Glide is flattening the vendor tree but not removing the vendor directories commmitted in the dependencies. Since keeping or removing them causes different behaviors I opened Masterminds/glide#303 to share my thoughts.

If I got it right, with the current Glide behavior, gvs should keep, inside every nested vendor directory only the packages provided by the glide.lock file (since it contains all the flattened dependencies).

@mattfarina
Copy link

While Glide flattens the vendor tree, there are cases that the top level project developer needs to negotiate, to decide if you use the nested or top level dependency. Sometimes you need to use the nested one.

npm, the package manager for node.js, in version 3 changed their strategy from one of always nesting the whole tree to only nesting differences from the top level. It may be worth poking at their docs on this to understand what's going on. Of course, the way you can pass instances around in JavaScript is entirely different than Go so they don't have some of the issues we encounter.

That's my long winded way of suggesting that you should keep the nested vendor directories if they are present.

@sgotti
Copy link
Owner Author

sgotti commented Mar 11, 2016

@mattfarina thanks for your explanation. I'd like to discuss on it more deeply, perhaps on glide issue Masterminds/glide#303 or on a new one.

Last day I merged PR #5 which keeps all the nested vendor directories (removing unneeded packages). My original idea on glide-vc is to help users cleaning up their vendor directories so, if they want to commit it on VCS, they are going to commit only needed files. But I don't want to change compilation and runtime behavior (and removing nested vendor dirs will change it). Do you agree on this?

@mattfarina
Copy link

Removing nested vendor directories will change compile behavior which is why it's good to leave them.

It may be worth opting-in to remove nested vendor directories which will change compile time behavior.

@sgotti
Copy link
Owner Author

sgotti commented Mar 11, 2016

Removing nested vendor directories will change compile behavior which is why it's good to leave them

Good. That's the current behavior (I tried to explain it in the README)

It may be worth opting-in to remove nested vendor directories which will change compile time behavior.

I started to implement it. I have to think what is the best way to let an user to keep just some vendor directories.

In future the best option will be to determine which nested vendor is not needed because it's the same of a parent one (maybe possible if all vendored deps contains information like glide.yaml but actually looks very difficult)

@mattfarina
Copy link

@sgotti difficult problems are run. I hope you have fun with this one. 😉

@sgotti
Copy link
Owner Author

sgotti commented Mar 31, 2016

@mattfarina Now that glide-vc doesn't remove nested vendor directories and that can be done by glide 0.10 I'm going to close this. Thanks for your support!

@sgotti sgotti closed this as completed Mar 31, 2016
@sgotti sgotti modified the milestone: v0.1.0 Sep 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants