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

Vendor all dependencies? #1523

Closed
achille-roussel opened this Issue Feb 13, 2018 · 7 comments

Comments

Projects
None yet
3 participants
@achille-roussel

achille-roussel commented Feb 13, 2018

Hello,

I'm having a bit of trouble getting reproducible builds with CoreDNS, which makes tracking down issues much harder.

I have a build that I made 5 days ago and worked just fine, but the build I'm making today have broken the config we used.

Most of the pain comes from the custom build process that CoreDNS uses with its makefile, which does go get + git checkout to lock down dependencies, where this could be better served by leveraging the vendor folder.

There are already a bunch of dependencies checked in into the vendor folder, my question is why not vendor all dependencies? That way building coredns would basically just be

go generate
go build

We can't just use one of the tagged releases because we add external plugins so we need to rebuild CoreDNS, and the go get steps fail if the coredns repo is checked out in a non-master branch or a branch that does not exist on the remote repo (due to interdependencies between coredns and plugins).

I'd love to hear about how people have addressed those kinds of problems as well, maybe I'm not approaching this the right way.

@chrisohaver

This comment has been minimized.

Member

chrisohaver commented Feb 13, 2018

This go get "version guarding" was added about a month ago (#1370). There isn't really an explanation as to why vendoring is not used. Vendoring has its own set of pain points. @yongtang @miekg

The change caused similar breakage as you describe in the continuous integration tests when an external plugin (forward) was included by default in the coredns Makefile. The temporary fix (#1435) was to ignore the failure of the step in the Makefile. This had the effect of not re-getting coredns/coredns, which you already have at correct version if you are building from a branch. IMO, it seems like Go should know this and not be confused, but maybe there is ambiguity there and I'm only seeing one case. Since then, forward has been moved into coredns/coredns, so its no longer an external plugin.

@miekg

This comment has been minimized.

Member

miekg commented Feb 14, 2018

We try to vendor everything, but because of external plugins we can't sometimes. This is situation is pretty terrible. Go 1.9 and type aliases might make this better, see #792 for instance.

@miekg miekg added the duplicate label Feb 14, 2018

@achille-roussel

This comment has been minimized.

achille-roussel commented Feb 14, 2018

You make a good point about the difficulty that external plugins represent.

However, they can be vendored too for custom builds, if all dependencies live in ./vendor there should be no issues to build coredns.

What do you think?

@miekg

This comment has been minimized.

Member

miekg commented Feb 15, 2018

@miekg

This comment has been minimized.

Member

miekg commented Feb 16, 2018

If there are things we could to make it easier for CoreDNS devs we should most definitively do that, while keeping in mind that the experience for plugin developers should be seamless

@miekg

This comment has been minimized.

Member

miekg commented Feb 23, 2018

I hope vgo will improve this situation. Can't wait to rm -rf vendor; git commit -a'ahhh gone'

@achille-roussel

This comment has been minimized.

achille-roussel commented Feb 23, 2018

Yep, there’s hope for a brighter future!

Thanks for your feedback you all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment