make: fix verify-dep target
#18205
make: fix verify-dep target
#18205
Conversation
|
Skipping CI for Draft Pull Request. |
|
/test all |
|
This is an example of a failure (with the current mismatch in dependency versions from the main branch): https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/etcd-io_etcd/18205/pull-etcd-verify/1803142325526859776 @ahrtr, could you PTAL at this? Thanks :) |
Just curious @ivanvc, is it possible to show the file paths where the inconsistent versions are extracted from? :) |
Without adding a lot of complexity to the code, I can achieve something like: |
Looks good! Thanks for the quick work! I think with this information it’s faster for reviewers to understand what’s doing on, as the source and type (direct, indirect, etc.) 😀 is very clear! |
ivanvc
force-pushed
the
add-make-verify-dep-versions
branch
from
7b1c81c
to
4322740
Compare
|
/test pull-etcd-verify |
|
As I was working on this, I realized that there's a similar check already in place: Lines 533 to 539 in 0430960 However, the approach is slightly different. This check runs against I wonder what approach we should follow, as the current didn't catch the inconsistencies we have right now. Find attached the output log when removing the Update: I found a potential issue on how the current code lists duplicates. After fixing it, the list is shorter. However, AFAIK we don't have the ability to update those deep-level dependencies, as they don't show on the |
|
/test pull-etcd-verify |
|
I updated this PR. I have two possible implementations:
I think I prefer the latter option. @jmhbnz, it would be great to have your input too. |
|
Great work, thanks @ivanvc
Thanks for the finding.
Yes, let's go for simpler solution. No need to add a separate script file |
ivanvc
force-pushed
the
add-make-verify-dep-versions
branch
from
0bec4c5
to
d08c8d4
Compare
k8s-ci-robot
added
area/clientv3
area/etcdctl
area/testing
area/tooling
size/M
and removed
size/XS
labels
Makefile's target `verify-dep` current behavior is to use `go list` to check consistent dependency versions from direct dependencies. Ignoring indirect dependencies in a multi-module project could lead to version mismatches. If module A imports module B, module B's dependency will be an indirect dependency in module A. Which can potentially have a version mismatch. Therefore, use `go mod edit` with indirect dependencies, too. So it can work with all dependencies defined in go.mod. Fix displaying dependencies with mismatches, as the old code was searching with grep just for the prefix, which would show other dependencies that shared the same prefix. Signed-off-by: Ivan Valdes <ivan@vald.es>
Ensure that golang.org/x/sys is at the same version across the modules from the project. Signed-off-by: Ivan Valdes <ivan@vald.es>
…20240520151616-dc85e6b867a5 Set the same version for google.golang.org/genproto/googleapis/api across the submodules. Signed-off-by: Ivan Valdes <ivan@vald.es>
ivanvc
force-pushed
the
add-make-verify-dep-versions
branch
from
d08c8d4
to
a016567
Compare
ivanvc
changed the title
verify-dependency-versions targetverify-dep target
|
Updated the description and finalized commits + addressed dependency version mismatches. /cc @ahrtr |
|
@henrybear327, do you want to give it a try at backporting this to 3.5? |
Yes! Thank you @ivanvc! /assign @henrybear327 |
Makefile's target `verify-dep` current behavior is to use `go list` to check consistent dependency versions from direct dependencies. Ignoring indirect dependencies in a multi-module project could lead to version mismatches. If module A imports module B, module B's dependency will be an indirect dependency in module A. Which can potentially have a version mismatch. Therefore, use `go mod edit` with indirect dependencies, too. So it can work with all dependencies defined in go.mod. Fix displaying dependencies with mismatches, as the old code was searching with grep just for the prefix, which would show other dependencies that shared the same prefix. Reference: - etcd-io#18205 Signed-off-by: Ivan Valdes <ivan@vald.es> Signed-off-by: Chun-Hung Tseng <henrybear327@gmail.com>
Makefile's target `verify-dep` current behavior is to use `go list` to check consistent dependency versions from direct dependencies. Ignoring indirect dependencies in a multi-module project could lead to version mismatches. If module A imports module B, module B's dependency will be an indirect dependency in module A. Which can potentially have a version mismatch. Therefore, use `go mod edit` with indirect dependencies, too. So it can work with all dependencies defined in go.mod. Fix displaying dependencies with mismatches, as the old code was searching with grep just for the prefix, which would show other dependencies that shared the same prefix. Reference: - etcd-io#18205 Signed-off-by: Ivan Valdes <ivan@vald.es> Signed-off-by: Chun-Hung Tseng <henrybear327@gmail.com>
Makefile's target `verify-dep` current behavior is to use `go list` to check consistent dependency versions from direct dependencies. Ignoring indirect dependencies in a multi-module project could lead to version mismatches. If module A imports module B, module B's dependency will be an indirect dependency in module A. Which can potentially have a version mismatch. Therefore, use `go mod edit` with indirect dependencies, too. So it can work with all dependencies defined in go.mod. Fix displaying dependencies with mismatches, as the old code was searching with grep just for the prefix, which would show other dependencies that shared the same prefix. Reference: - etcd-io#18205 Signed-off-by: Ivan Valdes <ivan@vald.es> Signed-off-by: Chun-Hung Tseng <henrybear327@gmail.com>
make verify-deptarget to scan also indirect dependencies.Part of #18180.
Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.