-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
hack/update-codegen.sh is broken on macOS 10.14.6 (Mojave) #9792
Comments
For reference, here's the change (partial revert of 250b0f2) that lets this run to the second failure point: index b11cd33be..ce3b43156 100644
--- a/vendor/knative.dev/test-infra/scripts/library.sh
+++ b/vendor/knative.dev/test-infra/scripts/library.sh
@@ -22,7 +22,7 @@
readonly KNATIVE_TESTS_PROJECT=knative-tests
# Conveniently set GOPATH if unset
-if [[ ! -v GOPATH ]]; then
+if [[ -z "${GOPATH:-}" ]]; then
export GOPATH="$(go env GOPATH)"
if [[ -z "${GOPATH}" ]]; then
echo "WARNING: GOPATH not set and go binary unable to provide it"
@@ -30,9 +30,9 @@ if [[ ! -v GOPATH ]]; then
fi
# Useful environment variables
-[[ -v PROW_JOB_ID ]] && IS_PROW=1 || IS_PROW=0
+[[ -n "${PROW_JOB_ID:-}" ]] && IS_PROW=1 || IS_PROW=0
readonly IS_PROW
-[[ ! -v REPO_ROOT_DIR ]] && REPO_ROOT_DIR="$(git rev-parse --show-toplevel)"
+[[ -z "${REPO_ROOT_DIR:-}" ]] && REPO_ROOT_DIR="$(git rev-parse --show-toplevel)"
readonly REPO_ROOT_DIR
readonly REPO_NAME="$(basename ${REPO_ROOT_DIR})"
@@ -478,19 +478,24 @@ function start_latest_eventing_sugar_controller() {
function run_go_tool() {
local tool=$2
local install_failed=0
- if [[ -z "$(which ${tool})" ]]; then
- local action=get
- [[ $1 =~ ^[\./].* ]] && action=install
+ local action="get"
+ [[ $1 =~ ^[\./].* ]] && action="install"
+ # Install the tool in the following situations:
+ # - The tool does not exist.
+ # - The tool needs to be installed from a local path.
+ # - Version of the tool is specificied in the given tool path.
+ # TODO(chizhg): derive a better versioning story for the tools being used.
+ if [[ -z "$(which "${tool}")" || "${action}" == "install" || "${tool}" =~ "@" ]]; then
# Avoid running `go get` from root dir of the repository, as it can change go.sum and go.mod files.
# See discussions in https://github.com/golang/go/issues/27643.
if [[ ${action} == "get" && $(pwd) == "${REPO_ROOT_DIR}" ]]; then
local temp_dir="$(mktemp -d)"
# Swallow the output as we are returning the stdout in the end.
pushd "${temp_dir}" > /dev/null 2>&1
- GOFLAGS="" go ${action} "$1" || install_failed=1
+ GOFLAGS="" go ${action} $1 || install_failed=1
popd > /dev/null 2>&1
else
- GOFLAGS="" go ${action} "$1" || install_failed=1
+ GOFLAGS="" go ${action} $1 || install_failed=1
fi
fi
(( install_failed )) && return ${install_failed}
@@ -587,6 +592,17 @@ function run_kntest() {
fi
}
+# Run "kntest" tool
+# Parameters: $1..$n - parameters passed to the tool
+function run_kntest() {
+ if [[ "${REPO_NAME}" == "test-infra" ]]; then
+ go run "${REPO_ROOT_DIR}"/kntest/cmd/kntest "$@"
+ else
+ # Always run the latest version.
+ run_go_tool knative.dev/test-infra/kntest/cmd/kntest@master "$@"
+ fi
+}
+
# Run go-licenses to update licenses.
# Parameters: $1 - output file, relative to repo root dir.
# $2 - directory to inspect.
@@ -671,7 +687,7 @@ function get_canonical_path() {
# This is implemented as a function so it can be mocked in unit tests.
# It will fail if a file name ever contained a newline character (which is bad practice anyway)
function list_changed_files() {
- if [[ -v PULL_BASE_SHA ]] && [[ -v PULL_PULL_SHA ]]; then
+ if [[ ! -z "${PULL_BASE_SHA:-}" ]] && [[ ! -z "${PULL_PULL_SHA:-}" ]]; then
# Avoid warning when there are more than 1085 files renamed:
# https://stackoverflow.com/questions/7830728/warning-on-diff-renamelimit-variable-when-doing-git-push
git config diff.renames 0
@@ -724,29 +740,6 @@ function get_latest_knative_yaml_source() {
echo "https://storage.googleapis.com/knative-nightly/${repo_name}/latest/${yaml_name}.yaml"
}
-function shellcheck_new_files() {
- declare -a array_of_files
- local failed=0
- readarray -t -d '\n' array_of_files < <(list_changed_files)
- for filename in "${array_of_files[@]}"; do
- if echo "${filename}" | grep -q "^vendor/"; then
- continue
- fi
- if file "${filename}" | grep -q "shell script"; then
- # SC1090 is "Can't follow non-constant source"; we will scan files individually
- if shellcheck -e SC1090 "${filename}"; then
- echo "--- PASS: shellcheck on ${filename}"
- else
- echo "--- FAIL: shellcheck on ${filename}"
- failed=1
- fi
- fi
- done
- if [[ ${failed} -eq 1 ]]; then
- fail_script "shellcheck failures"
- fi
-}
-
# Initializations that depend on previous functions.
# These MUST come last. |
Using bash 4 should solve this for you. It is why we use #!/usr/bin/env bash Vs #!/bin/bash Several other options are bash 4 only that are in use. |
looks like we don't currently document this in DEVELOPMENT.md, Ill PR something |
Fwiw, the reason This would have returned something vaguely useful: $ git describe --dirty --always
9dc2ab410-dirty |
I think this has been resolved via @julz documentation update |
In what area(s)?
/area build
What version of Knative?
master?
Expected Behavior
running:
should work (I don't have a definition for this, as so far, I have not managed to see this)
Actual Behavior
Initially #9778 (comment):
$ hack/update-codegen.sh
hack/../vendor/knative.dev/test-infra/scripts/library.sh: line 25: conditional binary operator expected
Steps to Reproduce the Problem
Run
hack/update-codegen.sh
on macOS 10.14.6 (using the branch from #9778 / commit: af905a7)The text was updated successfully, but these errors were encountered: