search

crc-org / crc

CRC is a tool to help you run containers. It manages a local OpenShift 4.x cluster, Microshift or a Podman VM optimized for testing and development purposes
https://crc.dev
Apache License 2.0
1.26k stars 242 forks source link

build: Simplify/improve verify-vendor.sh #4132

Closed cfergeau closed 6 months ago

cfergeau commented 6 months ago

verify-vendor.sh compares the content of the vendor directory after running make vendor with a copy which was made before running make vendor. This does not catch changes to go.mod/go.sum made by go mod tidy which we forgot to commit.

This commit uses git diff --exit-code instead of running diff against the vendor directory.

This will catch all uncommitted changes, which should be fine when make vendorcheck is run.

Using git diff instead of diff allows to remove quite some code from verify-vendor.sh

openshift-ci[bot] commented 6 months ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please ask for approval from cfergeau. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/crc-org/crc/blob/main/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
cfergeau commented 6 months ago

The "Build RPM" failure means this change is working as expected. However, I realize now that it would be good if make spec failed when gomod2rpmdeps fails.

praveenkumar commented 6 months ago

About https://github.com/crc-org/crc/pull/4132/commits/07add4ff6a24d2fd918251c1014268a24ded50e2 I was thinking instead to use go-1.21 tool chain can we revert ec6cbfb3d35f433a6285bda4b12badb45e97f915 one and use go mod tidy -go 1.20 as part of make vendor ?

praveenkumar commented 6 months ago

/hold

openshift-ci[bot] commented 6 months ago

@cfergeau: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-microshift-crc b525b854f193e2c20bc25d73350a0ac1a2c20ea2 link true /test e2e-microshift-crc
ci/prow/integration-crc b525b854f193e2c20bc25d73350a0ac1a2c20ea2 link true /test integration-crc
ci/prow/security b525b854f193e2c20bc25d73350a0ac1a2c20ea2 link false /test security
ci/prow/e2e-crc b525b854f193e2c20bc25d73350a0ac1a2c20ea2 link true /test e2e-crc
ci/prow/images b525b854f193e2c20bc25d73350a0ac1a2c20ea2 link true /test images

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
praveenkumar commented 6 months ago

should we close it because #4139 have all those commits also?

cfergeau commented 6 months ago

Since this PR won't work without updating to 1.21, sure.