Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

wip: Move to Go modules #1911

Closed
wants to merge 5 commits into from
Closed

Conversation

amshinde
Copy link
Member

@amshinde amshinde commented Jul 26, 2019

Move from go dep to go modules.

Fixes #1331

@amshinde amshinde requested a review from a team as a code owner July 26, 2019 08:14
Copy link
Contributor

@jodh-intel jodh-intel left a comment

Choose a reason for hiding this comment

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

lgtm

export GO111MODULE=on \
go mod tidy && \
go mod vendor && \
go mod verify
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice. Could you add a check like this to https://github.com/kata-containers/tests/blob/master/.ci/static-checks.sh for repos that are using go modules? Yes, I know the check is already here, but having such a central and PR-gating check will hopefully keep all the repos protected.

Copy link
Member Author

Choose a reason for hiding this comment

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

@jodh-intel I have finally addressed this after my unexpected interim break.
I have opened a PR in the tests repo to account for go modules.
See: kata-containers/tests#1880

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @amshinde.

@ganeshmaharaj
Copy link
Contributor

@amshinde Is the plan to get runtime moved while we move rest of the repos? In that case, tests repository needs to be bostered to run go mod verify against dep check for just these repos.

@jodh-intel
Copy link
Contributor

@amshinde, @ganeshmaharaj - please can we document the plan on kata-containers/kata-containers#42 to allow others to understand (and comment on) it.

amshinde added 5 commits July 31, 2019 15:22
Add the go.mod file required for converting runtime to go module.
This file is generated with `go mod init` using the existing
Gopkg.lock and running a `go build` followed by `go test`.

Fixes kata-containers#1331

Signed-off-by: Archana Shinde <[email protected]>
Removing all dep related metadata files and the vendoring from the
dep tool.

Signed-off-by: Archana Shinde <[email protected]>
Remove any unneeded dependencies with `go mod tidy`

Signed-off-by: Archana Shinde <[email protected]>
Provide a make target for developers for generating vendor directory.

Signed-off-by: Archana Shinde <[email protected]>
Provide a make target for developers for generating vendor directory.

Signed-off-by: Archana Shinde <[email protected]>
@amshinde
Copy link
Member Author

/test

@amshinde
Copy link
Member Author

amshinde commented Aug 1, 2019

@jodh-intel Will update the issue.

@caoruidong
Copy link
Member

ping @amshinde

@umarcor
Copy link

umarcor commented Sep 5, 2019

Hi @amshinde, @jodh-intel! Coming from #1420, I'm sorry to be the one bringing bad news here...

Context:

  • Currently, this repository retrieves the tests repo with 'go get' and expects it to be available in GOPATH/src/github.com/kata-containers/tests. However, in any environment where go modules are enabled, go get will NOT download sources of packages in that path. This is why this PR will fail when go modules are enabled in Travis: https://travis-ci.com/umarcor/kata-runtime/jobs/231319730. See Migrate to go modules (deprecate 'vendor') #1420 (comment).
  • With go modules, 'vendor' is expected not to be required and there is no idiomatic way to properly 'mix' sources available in the modules caches and those in 'vendor'. This is related to modules: Verification for go modules tests#1880 (comment).
    • Any build with go modules should be reproducible, as long as remotes are not gone. This is the only use case of 'vendor' in the new environment: a backup, just in case some of the remotes fail. As a result, 'vendor' should not be included in the VCS. In any case, vendor packages could be distributed as tarballs for users with very specific requirements to have them.
    • By the same token, build commands need to be specifically forced to use the vendor folder.

Therefore, I think that this PR should ensure that go modules are not used, except for running go mod vendor.

@WeiZhang555
Copy link
Member

I think go mod vendor can be an option. How about we only use go mod to manage our "vendor" dir instead of remove whole "vendor" from the repo?

@umarcor
Copy link

umarcor commented Sep 5, 2019

I think go mod vendor can be an option. How about we only use go mod to manage our "vendor" dir instead of remove whole "vendor" from the repo?

That's a temporal solution that will only work as long as go modules are optional. Nevertheless, as commented, this approach would require:

  • this PR should ensure that go modules are not used, except for running go mod vendor.
  • build commands need to be specifically forced to use the vendor folder.

@jcvenegas
Copy link
Member

@amshinde ping this needs rebase

@raravena80
Copy link
Member

@amshinde any updates on this PR? Thx!

1 similar comment
@HZ89
Copy link

HZ89 commented Nov 27, 2019

@amshinde any updates on this PR? Thx!

@amshinde
Copy link
Member Author

I'll revisit this PR this week.

@raravena80
Copy link
Member

@amshinde any updates? Thx.

Your weekly Kata herder.

@raravena80
Copy link
Member

@amshinde ping 😄

@moricho moricho mentioned this pull request Feb 26, 2020
@bergwolf
Copy link
Member

Currently, this repository retrieves the tests repo with 'go get' and expects it to be available in GOPATH/src/github.com/kata-containers/tests. However, in any environment where go modules are enabled, go get will NOT download sources of packages in that path. This is why this PR will fail when go modules are enabled in Travis: https://travis-ci.com/umarcor/kata-runtime/jobs/231319730. See #1420 (comment).

We should not use the new go get to download code or binary inside a go module directory, -- it would fetch the code and add it to your go.mod dependency. git clone is your friend ;)

And kindly ping @amshinde from your Kata Herder this week ;)

@umarcor
Copy link

umarcor commented Mar 11, 2020

We should not use the new go get to download code or binary inside a go module directory, -- it would fetch the code and add it to your go.mod dependency. git clone is your friend ;)

@bergwolf, thanks for acknowledging this point. I've been trying to make it clear for almost a year now.. #1420, #1413. I believe that's the critical issue that is preventing any practical movement towards go modules.

@raravena80
Copy link
Member

@amshinde @bergwolf @umarcor ping.

@amshinde amshinde closed this Sep 29, 2020
@umarcor
Copy link

umarcor commented Sep 29, 2020

See #1420 (comment).

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

Successfully merging this pull request may close these issues.

Migration from 'go dep' to 'go mod'
10 participants