Skip to content
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.

Sign up for GitHub

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

Jump to bottom

hack/verify-staging-imports.sh: print actual dependencies #40659

Merged
merged 1 commit into from Jan 31, 2017
Merged

hack/verify-staging-imports.sh: print actual dependencies #40659

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 32 additions & 6 deletions hack/verify-staging-imports.sh
View file
Open in desktop
Expand Up @@ -23,13 +23,39 @@ source "${KUBE_ROOT}/hack/lib/init.sh"

kube::golang::setup_env

function print_forbidden_imports () {
set -o errexit # this was unset by ||
local PACKAGE="$1"
shift
local RE=""
local SEP=""
for CLAUSE in "$@"; do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why doesn't this pick up the first argument?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shift above removes it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of the shift on line 28

RE+="${SEP}${CLAUSE}"
SEP='\|'
done
local FORBIDDEN=$(
go list -f $'{{with $package := .ImportPath}}{{range $.Imports}}{{$package}} imports {{.}}\n{{end}}{{end}}' ./vendor/k8s.io/${PACKAGE}/... |
sed 's|^k8s.io/kubernetes/vendor/||;s| k8s.io/kubernetes/vendor/| |' |
grep -v " k8s.io/${PACKAGE}" |
grep -e "imports \(${RE}\)"
)
if [ -n "${FORBIDDEN}" ]; then
echo "${PACKAGE} has a forbidden dependency:"
echo
echo "${FORBIDDEN}" | sed 's/^/ /'
echo
return 1
fi
return 0
}

for dep in $(ls -1 ${KUBE_ROOT}/staging/src/k8s.io/); do
if go list -f {{.Deps}} ./vendor/k8s.io/${dep}/... | tr " " '\n' | grep k8s.io/kubernetes | grep -v 'k8s.io/kubernetes/vendor' | LC_ALL=C sort -u | grep -e "."; then
echo "${dep} has a cyclical dependency"
exit 1
fi
done
RC=0
print_forbidden_imports apimachinery k8s.io/ || RC=1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this ended up a lot better.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If at any point you expect set -o errexit to alter the flow of commands inside of print_forbidden_imports -- like, a failure somewhere in there you would expect to exit early ... in this construct, it would not. function_call || something will turn off errexit inside the function and there is no way to turn it back on.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point. thanks. Those details of shell code...

print_forbidden_imports apiserver k8s.io/kubernetes || RC=1
print_forbidden_imports client-go k8s.io/kubernetes k8s.io/apiserver || RC=1
if [ ${RC} != 0 ]; then
exit ${RC}
fi

if grep -rq '// import "k8s.io/kubernetes/' 'staging/'; then
echo 'file has "// import "k8s.io/kubernetes/"'
Expand Down