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

Don't vendor dependencies #634

Closed
dlsniper opened this issue Apr 14, 2016 · 13 comments
Closed

Don't vendor dependencies #634

dlsniper opened this issue Apr 14, 2016 · 13 comments
Assignees
Labels
feature-request A feature should be added or improved.

Comments

@dlsniper
Copy link

Hi,

While it's a good practice to vendor dependencies, in case of libraries, this can result in headaches.
In this case, it would be ok to have a file with the revision / version of the libraries but let the users pull the dependencies manually / via a tool of their preference.
I know there's no agreed format just yet for Go but maybe this would also help the discussion around it and push things forward.
Thank you.

@jurij
Copy link

jurij commented Apr 14, 2016

Related Use Case:

jkmb:server juRiii$ godep save ./...
godep: Package (github.com/jmespath/go-jmespath) not found
jkmb:server juRiii$ grep -R --include="*.go" "github.com/jmespath/go-jmespath" /Users/juRiii/dev/go/src/
/Users/juRiii/dev/go/src//github.com/aws/aws-sdk-go/aws/awsutil/path_value.go:    "github.com/jmespath/go-jmespath"
/Users/juRiii/dev/go/src//github.com/aws/aws-sdk-go/vendor/github.com/jmespath/go-jmespath/fuzz/jmespath.go:import "github.com/jmespath/go-jmespath"
jkmb:Godeps juRiii$ go version
go version go1.6 darwin/amd64

@StabbyCutyou
Copy link

Not sure how widely supported it is, but the author of govendor has been maintaining a spec for dependency files in hopes to get it adopted by more of the tools.

Something to consider https://github.com/kardianos/vendor-spec

@jasdel
Copy link
Contributor

jasdel commented Apr 14, 2016

Thanks for contacting us. I'd like to learn more about the issues you are seeing with the SDK vendoring its dependancies?

If there were a generic pattern the SDK could follow supported by the base Go tooling it would be great to investigate how the SDK could use this. Regrettably there is no champion pattern accepted by the community or Go tooling. This is why the SDK uses vendoring to ensure the upstream dependancies needed by the SDK will not cause users to break if the upstream dependancies makes a breaking change. Which has happened.

@dlsniper
Copy link
Author

Hi, thanks for the awesome fast reply.

In the case of @juriii the problem is that even if this project vendors the code, the tool he's using is not aware of the the fact that the library already has the dependencies vendored and it tries to copy them from GOPATH (which will fail since they are not there, would be interesting to know how he go getted the package, by doing go get aws-sdk-go or go get aws-sdk-go/...).

Maybe a simple fix could be to tell the users to do the later, with appending /... to the package when go getting it (needs to be tested) (cc @juriii, see this).

On the large side of the issues, this is still pretty much a yet to be solved problem in Go but the general consensus is to not vendor the dependencies in case the project is a library and not an application. Fortunately enough, you are doing the nice thing and not also rewriting the import path (which a bigger problem). Of course, you could use a tool which has a file in which it saves the revision for each dependency and have only that added to the repository (but that means a big transition and forcing users to use that specific tool + the tool should be aware of the vendor/aws/vendor revisions)

This should also be asked / discussed on the mailing list / with the Go team as we can either have a community driven solution, like @StabbyCutyou mentioned vendor-spec, or we can have the Go team creating a standard for this.

@jurij
Copy link

jurij commented Apr 15, 2016

@dlsniper I have used go get -u github.com/aws/aws-sdk-go because of go1.6. I have not thought about that vendoring feature of any dependency I am using.

@jasdel Since most users are using this as a Library and not contributing, at least an Information in the README.md would be helpful.

@jasdel jasdel self-assigned this Apr 15, 2016
@jasdel
Copy link
Contributor

jasdel commented Apr 15, 2016

Thanks for your feedback, and suggestion of tools. We're taking a look at the options, but currently we don't think there is a good solution thats widely accepted by the Go community. With multiple competing solutions like Glide, Godep, and multiple vendor spec formats, we think its too soon to choose a specific format and be stuck with it. The /vendor folder is supported by all Go 1.5 with 1.5 vendoring experiment and +1.6 out of the box.

The downside of go get -u github.com/aws/aws-sdk-go/... is that users would be downloading several more dependancies than those needed to build applications against the SDK. Specifically, the testing libraries used by the SDK. Along with the golang.org/x/tools dependancies used by the deprecated awsmigration tool.

Instead of requiring users of the SDK to use a non-standard tool, we'd prefer to provide the most generic support possible, while also providing dependency versioning locking. With the vendoring folder we can guarantee that the SDK will work out of the box when downloaded without worrying about vendoring conflicts.

In addition, the SDK's README.md can use some updates to include more detailed information how retrieve the SDK, and what the different methods of retrieval will entail.

@jasdel jasdel added the feature-request A feature should be added or improved. label Apr 15, 2016
@dlsniper
Copy link
Author

@jasdel I completely agree with your points.
I would suggest raising the issue with the Go team on the mailing list https://groups.google.com/d/forum/golang-dev to see what's their opinion on this as come Go 1.7 the current solution will be here to stay (for better or worse) and feedback like this could help them out to understand the issue and either have them or the community come to an agreement on what needs to be done moving forward. I've raised a issue as well, unrelated to this, and there are several other threads with different problems as well, most recently (and somewhat similar) https://groups.google.com/forum/#!topic/golang-dev/4FfTBfN2YaI

Thank you!

@e-dard
Copy link

e-dard commented Apr 17, 2016

@jasdel

The downside of go get -u github.com/aws/aws-sdk-go/... is that users would be downloading several more dependancies than those needed to build applications against the SDK. Specifically, the testing libraries used by the SDK.

That shouldn't be the case. As far as I'm aware go get has never fetched test dependencies; you have to explicitly provide the -t flag to go get to fetch those.

@jasdel
Copy link
Contributor

jasdel commented Apr 25, 2016

@e-dard I verified that retrieving the SDK with go get -u github.com/aws/aws-sdk-go/... will still download some of the testing libraries because they are imported from utility packages within the SDK's repository. Specifically the awstesting package.

With that said the simplest way to retrieve the SDK with only decencies needed for runtime is: go get -u github.com/aws/aws-sdk-go/service/...

@jasdel
Copy link
Contributor

jasdel commented Apr 25, 2016

Thanks for the feedback, I've updated the README.md to be clearer on what to expect when retrieving the SDK.

jasdel added a commit that referenced this issue Mar 13, 2017
Updates the SDK's go-ini dependency from a pre 1.0 version to the latest 1.25.4

This fixes the old version of go-ini, but does not address the general desire to remove dependencies from the SDK. Lets continue discussion on #634 to determine how the SDK can improve its dependency references.

Fix #1125
@jasdel
Copy link
Contributor

jasdel commented Sep 23, 2017

I created a PR #1544 which adds the dep metadata files to the SDK. This will allow applications to correctly manage the SDK's dependencies in the context of the application.

I do not think we can safely remove the vendor folder from the SDK without breaking users that do not vendor, and not using dep. In a future version of the SDK a vendor folder would not be needed.

jasdel added a commit that referenced this issue Sep 25, 2017
Adds the dep Go dependency management metadata files to the SDK. The Gopkg files allow your tools to detect the version of dependencies the SDK uses.

Non-runtime dependencies such as test, example, and code generation packages are excluded from the Gopkg files as they are not needed to use the SDK.

Fix #1451
Fix #634
@awstools awstools mentioned this issue Sep 26, 2017
@roytanmoy
Copy link

roytanmoy commented Dec 13, 2018

Is this issue resolved? I am still getting the similar error with Go version 1.11.2.
../aws/aws-sdk-go/aws/awsutil/path_value.go:9:2: cannot find package "github.com/jmespath/go-jmespath"

@jasdel
Copy link
Contributor

jasdel commented Dec 13, 2018

@roytanmoy Are you using the latest version of the SDK? v1.16.2 updated the SDK's dependency on go-jmespath to be the latest version.

skotambkar pushed a commit to skotambkar/aws-sdk-go that referenced this issue May 20, 2021
Adds the ContentLengthMiddleware to the operation's stack, and updates generated clients and protocol tests to use the content-length check.

Depends on:
* aws/smithy-go#108
* smithy-lang/smithy#491
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

6 participants