-
Notifications
You must be signed in to change notification settings - Fork 85
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
Use dep for repo-infra deps #54
Changes from 1 commit
bc5179d
20ddc6f
53ed540
4db8f53
65f7e87
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
#!/usr/bin/env bash | ||
# Copyright 2017 The Kubernetes Authors. | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
set -o errexit | ||
set -o nounset | ||
set -o pipefail | ||
|
||
PKG=$1 | ||
COMMIT=$2 | ||
export GOPATH=$3 | ||
export GOBIN="$GOPATH/bin" | ||
|
||
go get -d -u "${PKG}" | ||
cd "${GOPATH}/src/${PKG}" | ||
git checkout -q "${COMMIT}" | ||
go install "${PKG}" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
#!/usr/bin/env bash | ||
# Copyright 2016 The Kubernetes Authors. | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
set -o errexit | ||
set -o nounset | ||
set -o pipefail | ||
|
||
REPOINFRA_ROOT=$(git rev-parse --show-toplevel) | ||
# https://github.com/kubernetes/test-infra/issues/5699#issuecomment-348350792 | ||
cd ${REPOINFRA_ROOT} | ||
TMP_GOPATH=$(mktemp -d) | ||
|
||
# no unit tests in vendor | ||
# previously we used godeps which did this, but `dep` does not handle this | ||
# properly yet. some of these tests don't build well. see: | ||
# ref: https://github.com/kubernetes/test-infra/pull/5411 | ||
find ${REPOINFRA_ROOT}/vendor/ -name "*_test.go" -delete | ||
|
||
# manually remove BUILD file for github.com/bazelbuild/buildtools/BUILD.bazel if it | ||
# exists; there is a specific test_suite rule that breaks importing | ||
rm -f ${REPOINFRA_ROOT}/vendor/github.com/bazelbuild/buildtools/BUILD.bazel | ||
|
||
GOBIN="${TMP_GOPATH}/bin" go get github.com/kubernetes/repo-infra/kazel | ||
|
||
"${REPOINFRA_ROOT}/verify/go_install_from_commit.sh" \ | ||
github.com/bazelbuild/bazel-gazelle/cmd/gazelle \ | ||
0.8 \ | ||
"${TMP_GOPATH}" | ||
|
||
touch "${REPOINFRA_ROOT}/vendor/BUILD" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI this isn't really needed for repo-infra (since we don't use the srcs rules here), but it makes sense if this script is used elsewhere, so I'm fine leaving it. |
||
|
||
"${TMP_GOPATH}/bin/gazelle" fix \ | ||
-build_file_name=BUILD,BUILD.bazel \ | ||
-external=vendored \ | ||
-proto=legacy \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can remove this line here. it's been removed from test-infra as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will remove |
||
-mode=fix \ | ||
-repo_root="${REPOINFRA_ROOT}" | ||
|
||
"${TMP_GOPATH}/bin/kazel" -root="${REPOINFRA_ROOT}" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
#!/usr/bin/env bash | ||
# Copyright 2016 The Kubernetes Authors. | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
set -o errexit | ||
set -o nounset | ||
set -o pipefail | ||
|
||
REPOINFRA_ROOT=$(git rev-parse --show-toplevel) | ||
TMP_GOPATH=$(mktemp -d) | ||
|
||
GOBIN="${TMP_GOPATH}/bin" go get github.com/kubernetes/repo-infra/kazel | ||
|
||
"${REPOINFRA_ROOT}/verify/go_install_from_commit.sh" \ | ||
github.com/bazelbuild/bazel-gazelle/cmd/gazelle \ | ||
0.8 \ | ||
"${TMP_GOPATH}" | ||
|
||
touch "${REPOINFRA_ROOT}/vendor/BUILD" | ||
|
||
gazelle_diff=$("${TMP_GOPATH}/bin/gazelle" fix \ | ||
-build_file_name=BUILD,BUILD.bazel \ | ||
-external=vendored \ | ||
-proto=legacy \ | ||
-mode=diff \ | ||
-repo_root="${REPOINFRA_ROOT}") | ||
|
||
kazel_diff=$("${TMP_GOPATH}/bin/kazel" \ | ||
-dry-run \ | ||
-print-diff \ | ||
-root="${REPOINFRA_ROOT}") | ||
|
||
# check if there are vendor/*_test.go | ||
# previously we used godeps which did this, but `dep` does not handle this | ||
# properly yet. some of these tests don't build well. see: | ||
# ref: https://github.com/kubernetes/test-infra/pull/5411 | ||
vendor_tests=$(find ${REPOINFRA_ROOT}/vendor/ -name "*_test.go" | wc -l) | ||
|
||
if [[ -n "${gazelle_diff}" || -n "${kazel_diff}" || "${vendor_tests}" -ne "0" ]]; then | ||
echo "${gazelle_diff}" | ||
echo "${kazel_diff}" | ||
echo "number of vendor/*_test.go: ${vendor_tests} (want: 0)" | ||
echo | ||
echo "Run ./verify/update-bazel.sh" | ||
exit 1 | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ugh, mixing go vendoring + bazel is the worst. :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it's worth just.. find/deleting all BUILD/BUILD.bazel files in
vendor/
. @BenTheElder had suggested this before in test-infra, but I wasn't sure if there was possibly other rules in those files that would be useful in the context of go vendoring (when we strip tests and other files).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This SGTM except that it would stink to need to regenerate all of these every update run, lots of update runs shouldn't touch
vendor/
at all 🤔There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BenTheElder We could perhaps use
kube::util::has_changes_against_upstream_branch
to check if there has been changes invendor/
against upstream, and only trigger if there has. This is used for verifying core godeps only if there has been changes (or forced in CI)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deleting all existing
BUILD
files can be dangerous, since there might begenrule
s or other customizations we need.I filed bazelbuild/bazel-gazelle#63 upstream to figure out a better long-term fix for this issue.