Fix build and use go modules for build #435

gnufied · 2020-05-22T16:50:51Z

Use go modules to pin dependency versions.

Fixes #434

humblec · 2020-05-23T03:27:30Z

.travis.yml

@@ -29,7 +29,7 @@ jobs:
    # Lang stage: Go
    - stage:               lang
      language:            go
-      go:                  1.10.4
+      go:                  1.13.8


may be we could use go version go1.14.2 linux/amd64

I was trying to use slightly older version on purpose, since go 1.14 was released just couple of months ago. I can change it though.

jdef · 2020-05-25T19:27:32Z

lib/go/go.sum

@@ -0,0 +1,51 @@
+cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=


does go mod tidy clean up this dep list at all? there's a ton here..

it does not. If anything it seems to be adding one more (duplicate) dependecy:

diff --git a/lib/go/go.sum b/lib/go/go.sum index 6f6d303..ca54368 100644 --- a/lib/go/go.sum +++ b/lib/go/go.sum @@ -12,6 +12,7 @@ github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5y github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.3 h1:gyjaxf+svBWX08ZjK86iN9geUJF0H6gp2IRKX6Nf6/I= github.com/golang/protobuf v1.3.3/go.mod h1:vzj43D7+SQXF/4pzW/hwtAqwc6iTitCiVSaWz5lYuqw= +github.com/google/go-cmp v0.2.0 h1:+dTQ8DZQJz0Mb/HjFlkptS1FeQ4cWSnN941F8aEG4SQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=

jdef · 2020-05-25T19:31:45Z

lib/go/go.mod

@@ -0,0 +1,8 @@
+module github.com/container-storage-interface/spec/lib/go/csi


maybe this go.mod file should be moved into lib/go/csi instead of keeping it adjacent to the Makefile?

hmm, it seems logical to have it here since top level csi package declaration exists in this directory.

it looks like the (empty) top level file was put there to satisfy go-dep, which didn't support multiple modules per repo unlike go mod. i'd rather be more precise with our module definitions.

would strongly prefer to move the go.mod file as suggested.

actually, this is tricker. I am okay with moving go.mod file to csi directory but it looks like even though our build is green, having go.mod file in a subdirectory prevents the csi module from being usable.

afaict - if we want to use go modules, we will have to place go.mod at the root of the repo.

jieyu

LGTM

saad-ali · 2020-05-28T00:38:26Z

I'll let @jdef review and approve this.

gnufied · 2020-05-28T20:08:30Z

@jdef @jieyu To fix the build and keep import paths same - I had to move go.mod file to root of the project. But I have verified by importing it in bunch of project and it works same as before. I do not see a way around this if we want to use go modules.

jieyu · 2020-05-28T21:04:34Z

@gnufied have you tried placing go.mod in both the root dir and in lib/go/csi (like many multiple module go project)?

gnufied · 2020-05-28T21:29:19Z

@jieyu I have and it looks like exporting/publishing a module from a subdirectory is treated different from how local multiple modules refer each other from same repo.

Take https://github.com/gnufied/sandbox2/tree/v1.8.0 for example. If you go get

~> go get github.com/gnufied/[email protected]
# you get an empty clone in pkg/mod/github.com/gnufied/[email protected] with just go.mod file

# get nested module
~> go get github.com/gnufied/sandbox2/lib/spec/[email protected]
go: finding github.com/gnufied/sandbox2/lib/spec v1.8.0
go: finding github.com v1.8.0
go: finding github.com/gnufied v1.8.0
go: finding github.com/gnufied/sandbox2/lib/spec/sandbox2 v1.8.0
go: finding github.com/gnufied/sandbox2/lib v1.8.0
go get github.com/gnufied/sandbox2/lib/spec/[email protected]: module github.com/gnufied/[email protected] found, but does not contain package github.com/gnufied/sandbox2/lib/spec/sandbox2

jdef · 2020-05-28T21:51:45Z

maybe we should not yet upgrade to go.mod and instead patch the makefile to pin the grpc-go version like it already does for protobufs

…

On Thu, May 28, 2020 at 5:29 PM Hemant Kumar ***@***.***> wrote: @jieyu <https://github.com/jieyu> I have and it looks like exporting/publishing a repsitory from a subdirectory is treated different from how local multiple modules refer each other from same repo. Take https://github.com/gnufied/sandbox2/tree/v1.8.0 for example. If you go get ~> go get ***@***.*** # you get an empty clone in ***@***.*** with just go.mod file # get nested module ~> go get ***@***.*** go: finding github.com/gnufied/sandbox2/lib/spec/sandbox v1.8.0 go: finding github.com v1.8.0 go: finding github.com/gnufied/sandbox2/lib/spec v1.8.0 go: finding github.com/gnufied/sandbox2/lib v1.8.0 go: finding github.com/gnufied v1.8.0 go: extracting github.com/gnufied/sandbox2 v1.8.0 go get ***@***.***: module ***@***.*** found, but does not contain package github.com/gnufied/sandbox2/lib/spec/sandbox — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#435 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAR5KLCPXDT6GW57OYPAUVDRT3JT3ANCNFSM4NIABGNQ> .

-- James DeFelice 585.241.9488 (voice) 650.649.6071 (fax)

gnufied · 2020-05-28T22:06:37Z

@jdef the trick is - we will have to probably pin grpc-go and https://github.com/googleapis/go-genproto - so it gets bit uglier as time flies.

This PR however works at the cost of having go.mod file at root level. For example:

# This runs k8s unit tests by replacing github.com/container-storage-interface
# with github.com/gnufied/1.6.0 which is built from this PR.
go: downloading github.com/gnufied/spec v1.6.0
go: extracting github.com/gnufied/spec v1.6.0
go: finding github.com/gnufied/spec v1.6.0
=== RUN   TestNodeExpandVolume
--- PASS: TestNodeExpandVolume (0.00s)

Alternately we can start thinking about moving language bindings in their own repo, which will be more cleaner (and may be take the bitter pill of pinning version via Makefile for now)

xing-yang · 2020-05-29T18:14:53Z

LGTM

saad-ali · 2020-05-29T21:11:04Z

Discussed this offline. Given the concerns that @jdef raised, and since we are so close to cutting a CSI 1.3, we agreed to go with the less risky (but more hacky) approach (patch the makefile to pin the grpc-go version), and move to go mod approach afterwards for the CSI 1.4 release. This is just to mitigate unforeseen issues with switching to go modules and rushing the decision before we can think it though.

Thank you @gnufied for your patience and for driving this!

saad-ali · 2021-05-07T04:00:57Z

maybe we should not yet upgrade to go.mod and instead patch the makefile to
pin the grpc-go version like it already does for protobufs

Doesn't look like there is a cleaner way to do this. golang/go#34055 asks for the ability to serve a module from a subdirectory but the proposed solution won't work for us because it depends on being able to return a <meta name="go-import" ...> tag which we can't do on github repos (see golang/go#34055 (comment)).

maybe we should not yet upgrade to go.mod and instead patch the makefile to
pin the grpc-go version like it already does for protobufs

Pinning dependencies in this way is a game of whack-a-mole. For example, the build is broken again right now.

Alternately we can start thinking about moving language bindings in their own repo, which will be more cleaner (and may be take the bitter pill of pinning version via Makefile for now)

That seems like a lot of unnecessary overhead.

This PR however works at the cost of having go.mod file at root level. For example:

I am in favor of this approach. go.mod at root is a little ugly but not the end of the world. Overall the benefit of go modules to fix dependencies vs existing solution outweighs the drawback, IMO.

Update travis to use go 1.13

gnufied · 2021-05-10T20:23:53Z

@saad-ali @jdef I have updated the branch and tested and verified that using via go get or go mod works.

saad-ali · 2021-05-12T07:11:21Z

Thanks @gnufied

/lgtm

Anyone have any objections to this change?

ddebroy

/lgtm

ddebroy · 2021-05-12T14:54:43Z

.travis.yml

@@ -29,10 +29,10 @@ jobs:
    # Lang stage: Go
    - stage:               lang
      language:            go
-      go:                  1.10.4
+      go:                  1.16.4


Huge bump here :-) but nice!

humblec · 2021-05-12T15:12:51Z

Thanks @gnufied

/lgtm

Anyone have any objections to this change?

lgtm from my side as well 👍

saad-ali · 2021-05-12T18:25:46Z

Thanks everyone, merging!

gnufied force-pushed the fix-grpc-build branch from a0fb73e to 556fc7d Compare May 22, 2020 18:06

gnufied changed the title ~~{WIP} Fix grpc build~~ {WIP} Fix build and use go modules for build May 22, 2020

gnufied changed the title ~~{WIP} Fix build and use go modules for build~~ Fix build and use go modules for build May 22, 2020

gnufied mentioned this pull request May 22, 2020

Floating grpc dependencies and build failures #434

Closed

humblec reviewed May 23, 2020

View reviewed changes

jdef reviewed May 25, 2020

View reviewed changes

jieyu approved these changes May 27, 2020

View reviewed changes

gnufied force-pushed the fix-grpc-build branch 3 times, most recently from b159550 to 1e5a9bd Compare May 28, 2020 20:04

gnufied added 2 commits May 10, 2021 12:32

Use go mods to fix build

53eee1f

Update travis to use go 1.13

Rebase go.mod changes

7257d75

gnufied force-pushed the fix-grpc-build branch from 1e5a9bd to 7257d75 Compare May 10, 2021 16:36

Bump go version in travis

644edb5

ddebroy reviewed May 12, 2021

View reviewed changes

gnufied mentioned this pull request May 12, 2021

Go 1.10.4 build fails with undef imports in http2 pkg #481

Closed

saad-ali merged commit 5914b37 into container-storage-interface:master May 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix build and use go modules for build #435

Fix build and use go modules for build #435

gnufied commented May 22, 2020 •

edited

Loading

humblec May 23, 2020

gnufied May 25, 2020

jdef May 25, 2020

gnufied May 25, 2020

jdef May 25, 2020

gnufied May 26, 2020

jdef May 27, 2020

gnufied May 28, 2020

gnufied May 28, 2020

jieyu left a comment

saad-ali commented May 28, 2020

gnufied commented May 28, 2020

jieyu commented May 28, 2020

gnufied commented May 28, 2020 •

edited

Loading

jdef commented May 28, 2020 via email

gnufied commented May 28, 2020 •

edited

Loading

xing-yang commented May 29, 2020

saad-ali commented May 29, 2020

saad-ali commented May 7, 2021

gnufied commented May 10, 2021

saad-ali commented May 12, 2021

ddebroy left a comment

ddebroy May 12, 2021 •

edited

Loading

humblec commented May 12, 2021

saad-ali commented May 12, 2021

		@@ -0,0 +1,51 @@
		cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=

		@@ -0,0 +1,8 @@
		module github.com/container-storage-interface/spec/lib/go/csi

Fix build and use go modules for build #435

Fix build and use go modules for build #435

Conversation

gnufied commented May 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jieyu left a comment

Choose a reason for hiding this comment

saad-ali commented May 28, 2020

gnufied commented May 28, 2020

jieyu commented May 28, 2020

gnufied commented May 28, 2020 • edited Loading

jdef commented May 28, 2020 via email

gnufied commented May 28, 2020 • edited Loading

xing-yang commented May 29, 2020

saad-ali commented May 29, 2020

saad-ali commented May 7, 2021

gnufied commented May 10, 2021

saad-ali commented May 12, 2021

ddebroy left a comment

Choose a reason for hiding this comment

ddebroy May 12, 2021 • edited Loading

Choose a reason for hiding this comment

humblec commented May 12, 2021

saad-ali commented May 12, 2021

gnufied commented May 22, 2020 •

edited

Loading

gnufied commented May 28, 2020 •

edited

Loading

gnufied commented May 28, 2020 •

edited

Loading

ddebroy May 12, 2021 •

edited

Loading