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

Update github.com/pierrec/lz4 dependency to module version #192

Merged
merged 1 commit into from
Nov 14, 2019

Conversation

nmiyake
Copy link
Contributor

@nmiyake nmiyake commented Nov 14, 2019

Updates dependency to use github.com/pierrec/lz4/v3 at version
v3.0.1. This version is functionally equivalent to the previously
specified dependency.

Updates dependency to use github.com/pierrec/lz4/v3 at version
v3.0.1. This version is functionally equivalent to the previously
specified dependency.
@nmiyake
Copy link
Contributor Author

nmiyake commented Nov 14, 2019

I verified locally that there is no code change between v2.0.5 and v3.0.1, so this should not change any functionality.

Ran go mod tidy after updating imports and adding module dependency -- all further modifications to go.mod and go.sum are a result of that operation.

Copy link
Owner

@mholt mholt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thank you for the effort you've spent to improve this! Thanks for helping us stay up to date :)

@mholt mholt merged commit d939c22 into mholt:master Nov 14, 2019
@nmiyake nmiyake deleted the updateModDependency branch November 14, 2019 18:05
@finferflu
Copy link

@mholt, @nmiyake,

This seems to break my CI/CD pipelines (using Go 1.13.4), although I'm unable to replicate this locally using the same Go version:

+ go get -t ./...
package github.com/pierrec/lz4/v3: cannot find package "github.com/pierrec/lz4/v3" in any of:
	/usr/local/go/src/github.com/pierrec/lz4/v3 (from $GOROOT)
	/opt/atlassian/pipelines/agent/build/src/github.com/pierrec/lz4/v3 (from $GOPATH)

Any ideas?

Thanks!

@nmiyake
Copy link
Contributor Author

nmiyake commented Nov 15, 2019

@finferflu can you provide a link to the project/CI build, or is it internal? Happy to take a look.

@finferflu
Copy link

@nmiyake thanks for that, however, I’m afraid it’s a private project that I cannot share publicly. Is there anything else I can provide? I suppose I could provide you with snippets from my pipelines configuration, would that work?

Thanks!

@nmiyake
Copy link
Contributor Author

nmiyake commented Nov 15, 2019

Sure, no problem. One thing that looks a bit suspicious is that the error message seems to be trying to resolve the package using GOPATH mode, when it is actually a module. GOPATH resolution for github.com/pierrec/lz4/v3 won't work, since the lz4 repository is using modules for major versions but in a manner in which they aren't using a subdirectory for major version development.

If you're building a non-module project, then the quickest fix will likely be to pin your dependency for mholt/archiver to v3.2.0, as that does not use any of the new module import paths.

If you want to try to dig deeper/fix the issue with the newest code, knowing the answer to the following would be helpful for trouble-shooting:

  • Is the project that you're developing a module or not?
    • If it's a module, are you using vendor mode or not?
    • If it's not a module, what (if anything) are you using for your dependency management? Are you vendoring, or just fetching into GOPATH locally and depending on using GOPATH to build?
  • Output of go version in build environment
  • Output of go env in build environment
  • If possible, CI system being used (CircleCI, Travis, something else?) and what image/environment is being used to run

@finferflu
Copy link

@nmiyake Thanks very much for the detailed feedback.

I'm quite new to Go, so I'm not sure I'll be able to provide you with exhaustive or 100% correct answers.

I believe I'm building a non-module project, however, I'm not sure how to pin my dependency to a specific version. That would be the best short-term solution for me at this point as I just need to be able to build my project ASAP.

I'm (unfortunately) using Bitbucket Pipelines as the CI system, with the golang:1.13-alpine image (go version outputs go version go1.13.4 linux/amd64). The relevant bit of the Bitbucket Pipelines configuration looks like this:

...
          - source bitbucket-pipelines-go.sh
          - SRC=$BITBUCKET_CLONE_DIR/src/project
          - cd $SRC
          - go get -t ./... # this is where it fails
...

The bitbucket-pipelines-go.sh file looks like this:

export GOPATH=$BITBUCKET_CLONE_DIR
mkdir -p $GOPATH/src/project
mv $BITBUCKET_CLONE_DIR/cmd $BITBUCKET_CLONE_DIR/src/project
mv $BITBUCKET_CLONE_DIR/internal $BITBUCKET_CLONE_DIR/src/project
mv $BITBUCKET_CLONE_DIR/test $BITBUCKET_CLONE_DIR/src/project
mkdir -p $GOPATH/{bin,pkg}
export GOBIN=$GOPATH/bin
go get github.com/tebeka/go2xunit
mkdir $BITBUCKET_CLONE_DIR/test-reports

To that file, I've tried to add the following lines, in my attempt to pin the version down to the previous release:

GO111MODULE=on go get github.com/mholt/archiver@33320f6f730664f9896b05b777e53ae4c917f572

While that command didn't error out, I got the same error while running go get -t ./....

I'm unable to provide you with the output of go env at this time, however, hopefully, the contents of the setup script above might be enough. Please let me know if that's not the case and I'll get back to you with the actual output.

Again, thank you very much for taking the time to respond to my query.

@nmiyake
Copy link
Contributor Author

nmiyake commented Nov 16, 2019

Thanks for the extra information.

If you're not using Go modules (if your project doesn't declare a go.mod file), then you're building in GOPATH-mode. Based on your workflow, presumably you're building your project without a "vendor" directory (you're depending on the "go get" call to populate your local GOPATH and building based on that).

I've tried a few different approaches, but unfortunately I think the set of circumstances that currently exist will not allow go get to properly populate GOPATH based on the way the current imports/projects are structured -- this could be viewed as an issue with the "go get" command, so filed golang/go#35630 to track that. However, it's possible that this failure is expected and will not be fixed.

A slightly bigger issue here seems to be that your project doesn't appear to have a defined strategy for managing its dependencies -- even if you fix this issue, if you're depending on a "go get" to populate your GOPATH for dependencies, your build will be very fragile -- you won't be protected at all from possible breaks introduced by dependency updates in the future, and different developers may use different dependencies from each other (and from the build system).

In GOPATH mode, most Go projects tend to use a "vendor" directory to manage their dependencies, and often use a dependency management tool like dep to help manage dependency versions.

I would recommend the following:

(1) Investigate the possibility of building your project as a Go module
(2) Use a "vendor" directory to manage your dependencies, and use a tool like "dep" to manage the version of your vendored dependencies

However, there is a larger issue here of the "go get" in GOPATH mode now being broken for this repository -- I'll file a new issue for that and allow the maintainer to decide how they want to handle this.

@finferflu
Copy link

Thanks very much for the detailed response, @nmiyake, it's really appreciated.

I think I will go down the route of using a "vendor" directory, as it seems to be the most time-effective solution.

Thanks!

@finferflu
Copy link

@nmiyake I've actually started using modules and it's worked flawlessly. Thanks again for your feedback.

@awgh
Copy link

awgh commented Dec 2, 2019

This needs to be reverted on master until the lz4 change it depends on is merged into lz4 master branch.

@nmiyake
Copy link
Contributor Author

nmiyake commented Dec 2, 2019

This updates the module dependency -- the lz4 repository uses an unconventional (but valid) approach where they only add go.mod files on the release branches. The module dependency that is added is on a valid release version, and is the proper way to consume the lz4 module (this is the only way that the module will be properly compared for semantic version importing and handle the transitive dependencies without adding them as indirect imports).

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

Successfully merging this pull request may close these issues.

4 participants