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

Investigate using multiple Go modules to solve the scripts vendor issue #2078

Closed
chizhg opened this issue May 15, 2020 · 2 comments · Fixed by #2141
Closed

Investigate using multiple Go modules to solve the scripts vendor issue #2078

chizhg opened this issue May 15, 2020 · 2 comments · Fixed by #2141
Assignees

Comments

@chizhg
Copy link
Member

chizhg commented May 15, 2020

At this point (May 15th, 2020), we have switched all Knative projects to use Go modules, but it brings in a regression that makes it impossible to only update vendored scripts from test-infra, when vendored pkg is not at HEAD. More discussions can be found from #2069.

Using multiple Go modules in the test-infra repo can potentially solve this issue, but we need to evaluate carefully to make sure it does not introduce other side effects.

The doc for multiple modules repository is in https://github.com/golang/go/wiki/Modules#faqs--multi-module-repositories.

/assign
FYI @coryrc @chaodaiG @albertomilan

@chizhg
Copy link
Member Author

chizhg commented Jun 2, 2020

I did some experiment with Go multiple modules, and did not see any issues.

The only changes needed in each repo will be changing the floating deps from knative.dev/test-infra to knative.dev/test-infra/scripts in for example https://github.com/knative/serving/blob/master/hack/update-deps.sh#L35. And even if it's not changed, it can still work in the old way.

@chizhg
Copy link
Member Author

chizhg commented Jun 29, 2020

It turned out this approach has some problems, so I reverted the PR in #2185.

The reason is by using this approach, repos which imports the scripts package will get an ambiguous import error as the module can be either knative.dev/test-infra/scripts or knative.dev/test-infra. One solution given in https://github.com/golang/go/wiki/Modules#is-it-possible-to-add-a-module-to-a-multi-module-repository suggests adding knative.dev/test-infra as a required module in knative.dev/test-infra/scripts, but this will defeat our objective to get rid of the circular dependencies in the first place.

We will consider other solutions to solve this issue.

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 a pull request may close this issue.

1 participant