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

evaluate glide for vendor #5658

Closed
gyuho opened this issue Jun 13, 2016 · 12 comments
Closed

evaluate glide for vendor #5658

gyuho opened this issue Jun 13, 2016 · 12 comments
Assignees
Milestone

Comments

@gyuho
Copy link
Contributor

gyuho commented Jun 13, 2016

Many CoreOS projects are already using glide for its vendoring: torus, dex, clair, etcd-play, dbtester, etc..

And other projects as well:

From my personal experience:

Pros.

  • works well with go tip (godep doesn't work with go tip, failing to parse go version string)
  • feels easier to update (edit glide.lock and glide install)

Cons.

  • still early in development
  • test files are included in vendor (seems like this will be fixed soon)
    • in the meantime, repo size with glide could be bloated

Almost every contributor to etcd had trouble with godep when introducing a new dependency, if I remember correctly. It might be worth trying out.

/cc @xiang90 @heyitsanthony

What do you think?

@gyuho gyuho added this to the unplanned milestone Jun 13, 2016
@chancez
Copy link
Contributor

chancez commented Jun 14, 2016

To remove test files, and non-code files form vendored deps: https://github.com/sgotti/glide-vc

@gyuho
Copy link
Contributor Author

gyuho commented Jun 14, 2016

Cool. Will check that out. Thanks!

@gyuho
Copy link
Contributor Author

gyuho commented Jun 15, 2016

Got it to work, but glide doesn't work well with symlinks. On Linux, writing symlinks back to root directory works ln -s cmd/vendor/ . && ./build, but not sure if we want to do that.

For future reference:

##################################################
# converting Godeps to glide
cd $GOPATH/src/github.com/coreos/etcd/
mv cmd/Godeps .
rm -rf cmd/vendor
mv cmd/ ./.cmd

glide init

glide update --strip-vendor --strip-vcs --update-vendored
glide vc --only-code --no-tests

mv ./.cmd ./cmd
mv glide.lock ./cmd
mv glide.yaml ./cmd
mv vendor/ ./cmd

##################################################
# update gRPC dependency
vim $GOPATH/src/github.com/coreos/etcd/cmd/glide.yaml
# change version to:
# e78224b060cf3215247b7be455f80ea22e469b66

cd $GOPATH/src/github.com/coreos/etcd/
./scripts/updatedep.sh
# OR
./scripts/updatedep.sh github.com/dustin/go-humanize

##################################################

#!/usr/bin/env bash

# A script for updating godep dependencies for the vendored directory /cmd/
# without pulling in etcd itself as a dependency.

if ! [[ "$0" =~ "scripts/updatedep.sh" ]]; then
    echo "must be run from repository root"
    exit 255
fi

GLIDE_ROOT="${GOPATH}/src/github.com/Masterminds/glide"
GLIDE_SHA="23f6a9d8e774f597faabae8b58b8c6020b3f79ef"
go get -u github.com/Masterminds/glide

pushd "${GLIDE_ROOT}"
    git reset --hard "${GLIDE_SHA}"
    echo "installing glide"
    go install
popd
echo "installed glide"

rm -rf vendor
mv cmd/glide.lock glide.lock
mv cmd/glide.yaml glide.yaml
mv cmd/vendor vendor

if [ -n "$1" ]; then
    echo "glide update on $(echo $1)"
    glide update --strip-vendor --strip-vcs --update-vendored $1
else
    echo "glide update on *"
    glide update --strip-vendor --strip-vcs --update-vendored
fi;

echo "removing test files"
glide vc --only-code --no-tests

mv glide.lock ./cmd
mv glide.yaml ./cmd
mv vendor ./cmd

echo "Done!"

##################################################

@chancez
Copy link
Contributor

chancez commented Jun 15, 2016

Yeah, symlinks will screw glide up, a lot. You'll need to avoid that if possible.

@mattfarina
Copy link

Folks... if there is a problem with symlinks it's a bug. We should fix that on Glide.

Note, I write glide so I'm happy to help get any symlink bugs fixed.

@gyuho
Copy link
Contributor Author

gyuho commented Jul 21, 2016

@mattfarina Thanks! I will give another try sometime next month. And keep you updated.

@gyuho gyuho self-assigned this Jul 21, 2016
@gyuho gyuho modified the milestones: v3.2.0, unplanned Aug 11, 2016
@gyuho
Copy link
Contributor Author

gyuho commented Aug 12, 2016

@mattfarina I reported the issue that etcd is facing here Masterminds/glide#546

Thanks!

@gyuho
Copy link
Contributor Author

gyuho commented Aug 15, 2016

Closing via #6163.

If anybody is interested, this is the commands that I used:

# converting Godeps to glide
cd $GOPATH/src/github.com/coreos/etcd
rm -rf ./vendor
rm -rf ./cmd/vendor

# temporarily move directory
mv ./cmd/Godeps .
mv ./cmd ..

# initialize with glide
glide create

# remove legacy Godep directory
rm -rf ./Godeps

# into vendor directory
glide --verbose update --delete --strip-vendor --strip-vcs --update-vendored --skip-test
glide vc --only-code --no-tests

# move back cmd, vendor directories
mv ../cmd ./cmd
mv ./vendor ./cmd/

# glide doesn't play well with symlink
# need to recreate symlink that was created by godep
rm -f $GOPATH/src/github.com/coreos/etcd/cmd/vendor/github.com/coreos/etcd
ln -s ../../../../ cmd/vendor/github.com/coreos/etcd
readlink $GOPATH/src/github.com/coreos/etcd/cmd/vendor/github.com/coreos/etcd


# to update dependencies
vim glide.yaml
# edit git SHA
./scripts/updatedep.sh


# to add dependency to vendor
./scripts/updatedep.sh github.com/klauspost/reedsolomon#9b772b54b3bf0be1eec083c9669766a56332559a

@gyuho gyuho closed this as completed Aug 15, 2016
@joshk0
Copy link

joshk0 commented Aug 29, 2016

This all looks great but I have one question. Why does the vendor directory not live at the top directory of etcd.git?

The build script is such that it can only grab the vendor directory when you choose to build cmd/ (e.g. building etcd and etcdctl yourself.) I use the client library in client/ extensively, and it can't benefit from the vendored dependencies in cmd/. Instead, i have to make them available manually in my workspace.

@chancez
Copy link
Contributor

chancez commented Aug 29, 2016

@joshk0 that's the exact reason. the etcd team doesn't want the client library to pull use the vendor directories within etcd since it's a library. That behavior is intended.

@joshk0
Copy link

joshk0 commented Aug 29, 2016

But isn't there a specific version of each dependency that the library modules are designed to work with? Or should I assume that the latest released version of each one should work? What's the convention here?

@heyitsanthony
Copy link
Contributor

@joshk0 cf. #4913 if you're interested in the discussion around putting vendor in the repo root. Things break.

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

No branches or pull requests

5 participants