-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
compute/metadata: dependency on compute doesn’t avoid ambiguous imports in all cases #7209
Comments
I believe this is to be expected and nothing can be done to avoid that case. The only way to avoid the ambiguous import is to upgrade cloud.google.com/go to the version referenced in the mod of compute/metadata. If you upgrade to the latest compute/metadata this weak ref was re-added for the time being to help facilitate an easier migration. |
This is all related in a way to kubernetes/kubernetes#113366 |
Right, I was wondering if this could be handled in cloud.google.com/go/compute somehow but that already has an appropriate dependency on cloud.google.com/go. AFAICT reliably fixing this would require adding a dependency from /go/compute/metadata to /go, defeating the purpose of the metadata split. |
that sort of defeats the purpose of the isolated module, right? |
I agree with this, but did not expect the metadata module to add a backref that forces all consumers of that module to also depend on cloud.google.com/go (and transitively, on all other submodules) |
https://github.com/googleapis/google-cloud-go/blob/main/compute/metadata/tidyfix.go#L23 is supposed to allow safe module upgrades, but it isn’t sufficient in all cases; specifically, when an existing dependency on cloud.google.com/go is present with an old enough version.
Starting from k/k master (commit kubernetes/kubernetes@0b2e541 right now):
k/k has an indirect dependency on cloud.google.com/go v0.97.0. Bumping that to v0.104.0 allows the upgrade to complete:
(Yes, I know that’s not how k/k dependency bumps are supposed to be handled, this is just for illustrative purposes.)
The text was updated successfully, but these errors were encountered: