Merge pull request #67675 from cblecker/fix-golint

Automatic merge from submit-queue (batch tested with PRs 67378, 67675, 67654). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Pass files to golint one at a time

**What this PR does / why we need it**:
When we pass multiple files into golint at once, and golint detects a fatal error in one of them, it will actually exit and not process any more of the files passed. For large packages, this increases the chance that golint will exit.

This change will, instead of golinting once per package, will use xargs to run each file through golint individually.

**Special notes for your reviewer**:
Out of interest, I timed the difference between using find and a regex to exclude the generated files, but `ls | grep` was about 15-20 seconds faster throughout the entire duration of the run (about 2 minutes).

**Release note**:
```release-note
NONE
```
This commit is contained in:
Kubernetes Submit Queue 2018-08-22 02:45:11 -07:00 committed by GitHub
commit 0c75bf6e4b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 98 additions and 54 deletions

View File

@ -23,6 +23,8 @@ pkg/api/v1/pod
pkg/api/v1/resource
pkg/apis/abac
pkg/apis/abac/latest
pkg/apis/abac/v0
pkg/apis/abac/v1beta1
pkg/apis/admission
pkg/apis/admission/v1beta1
pkg/apis/admissionregistration
@ -30,6 +32,9 @@ pkg/apis/admissionregistration/v1alpha1
pkg/apis/admissionregistration/v1beta1
pkg/apis/admissionregistration/validation
pkg/apis/apps
pkg/apis/apps/v1
pkg/apis/apps/v1beta1
pkg/apis/apps/v1beta2
pkg/apis/apps/validation
pkg/apis/authentication
pkg/apis/authentication/v1
@ -39,8 +44,13 @@ pkg/apis/authorization/v1
pkg/apis/authorization/v1beta1
pkg/apis/authorization/validation
pkg/apis/autoscaling
pkg/apis/autoscaling/v1
pkg/apis/autoscaling/v2beta1
pkg/apis/autoscaling/validation
pkg/apis/batch
pkg/apis/batch/v1
pkg/apis/batch/v1beta1
pkg/apis/batch/v2alpha1
pkg/apis/batch/validation
pkg/apis/certificates
pkg/apis/certificates/v1beta1
@ -52,6 +62,7 @@ pkg/apis/coordination/v1beta1
pkg/apis/core
pkg/apis/core/helper
pkg/apis/core/helper/qos
pkg/apis/core/v1
pkg/apis/core/v1/helper
pkg/apis/core/v1/helper/qos
pkg/apis/core/v1/validation
@ -59,14 +70,18 @@ pkg/apis/core/validation
pkg/apis/events
pkg/apis/events/v1beta1
pkg/apis/extensions
pkg/apis/extensions/v1beta1
pkg/apis/extensions/validation
pkg/apis/imagepolicy
pkg/apis/imagepolicy/v1alpha1
pkg/apis/networking
pkg/apis/networking/v1
pkg/apis/policy
pkg/apis/policy/v1beta1
pkg/apis/policy/validation
pkg/apis/rbac
pkg/apis/rbac/v1
pkg/apis/rbac/v1alpha1
pkg/apis/rbac/v1beta1
pkg/apis/rbac/validation
pkg/apis/scheduling
@ -76,8 +91,10 @@ pkg/apis/settings
pkg/apis/settings/v1alpha1
pkg/apis/storage
pkg/apis/storage/util
pkg/apis/storage/v1
pkg/apis/storage/v1/util
pkg/apis/storage/v1alpha1
pkg/apis/storage/v1beta1
pkg/apis/storage/v1beta1/util
pkg/auth/authorizer/abac
pkg/capabilities
@ -92,6 +109,7 @@ pkg/cloudprovider/providers/gce/cloud
pkg/cloudprovider/providers/ovirt
pkg/cloudprovider/providers/photon
pkg/cloudprovider/providers/vsphere
pkg/cloudprovider/providers/vsphere/vclib
pkg/controller
pkg/controller/bootstrap
pkg/controller/certificates
@ -140,6 +158,7 @@ pkg/kubeapiserver/authorizer/modes
pkg/kubeapiserver/options
pkg/kubeapiserver/server
pkg/kubectl
pkg/kubectl/apps
pkg/kubectl/cmd
pkg/kubectl/cmd/auth
pkg/kubectl/cmd/config
@ -151,6 +170,7 @@ pkg/kubectl/cmd/templates
pkg/kubectl/cmd/testing
pkg/kubectl/cmd/util
pkg/kubectl/cmd/util/editor
pkg/kubectl/cmd/util/openapi
pkg/kubectl/cmd/util/sanity
pkg/kubectl/cmd/wait
pkg/kubectl/metricsutil
@ -176,6 +196,7 @@ pkg/kubelet/cm/devicemanager/checkpoint
pkg/kubelet/cm/util
pkg/kubelet/config
pkg/kubelet/configmap
pkg/kubelet/container
pkg/kubelet/container/testing
pkg/kubelet/custommetrics
pkg/kubelet/dockershim
@ -341,6 +362,7 @@ pkg/security/podsecuritypolicy/selinux
pkg/security/podsecuritypolicy/user
pkg/security/podsecuritypolicy/util
pkg/securitycontext
pkg/serviceaccount
pkg/ssh
pkg/util/bandwidth
pkg/util/config
@ -377,6 +399,7 @@ pkg/util/threading
pkg/util/tolerations
pkg/util/workqueue/prometheus
pkg/version/verflag
pkg/volume
pkg/volume/aws_ebs
pkg/volume/azure_dd
pkg/volume/azure_file
@ -425,6 +448,7 @@ plugin/pkg/admission/security/podsecuritypolicy
plugin/pkg/admission/serviceaccount
plugin/pkg/auth/authorizer/node
plugin/pkg/auth/authorizer/rbac
plugin/pkg/auth/authorizer/rbac/bootstrappolicy
staging/src/k8s.io/api/admission/v1beta1
staging/src/k8s.io/api/admissionregistration/v1alpha1
staging/src/k8s.io/api/admissionregistration/v1beta1
@ -466,23 +490,29 @@ staging/src/k8s.io/apiextensions-apiserver/pkg/cmd/server
staging/src/k8s.io/apiextensions-apiserver/pkg/controller/finalizer
staging/src/k8s.io/apiextensions-apiserver/pkg/controller/status
staging/src/k8s.io/apiextensions-apiserver/pkg/features
staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource
staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition
staging/src/k8s.io/apimachinery/pkg/api/apitesting/fuzzer
staging/src/k8s.io/apimachinery/pkg/api/apitesting/roundtrip
staging/src/k8s.io/apimachinery/pkg/api/meta
staging/src/k8s.io/apimachinery/pkg/api/resource
staging/src/k8s.io/apimachinery/pkg/api/validation
staging/src/k8s.io/apimachinery/pkg/api/validation/path
staging/src/k8s.io/apimachinery/pkg/apis/config/v1alpha1
staging/src/k8s.io/apimachinery/pkg/apis/meta/fuzzer
staging/src/k8s.io/apimachinery/pkg/apis/meta/internalversion
staging/src/k8s.io/apimachinery/pkg/apis/meta/v1
staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured
staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation
staging/src/k8s.io/apimachinery/pkg/apis/meta/v1beta1
staging/src/k8s.io/apimachinery/pkg/apis/testapigroup
staging/src/k8s.io/apimachinery/pkg/apis/testapigroup/v1
staging/src/k8s.io/apimachinery/pkg/conversion
staging/src/k8s.io/apimachinery/pkg/labels
staging/src/k8s.io/apimachinery/pkg/runtime
staging/src/k8s.io/apimachinery/pkg/runtime/schema
staging/src/k8s.io/apimachinery/pkg/runtime/serializer
staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json
staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf
staging/src/k8s.io/apimachinery/pkg/runtime/serializer/recognizer
staging/src/k8s.io/apimachinery/pkg/runtime/serializer/streaming
@ -512,8 +542,10 @@ staging/src/k8s.io/apimachinery/pkg/util/uuid
staging/src/k8s.io/apimachinery/pkg/util/validation
staging/src/k8s.io/apimachinery/pkg/util/wait
staging/src/k8s.io/apimachinery/pkg/util/yaml
staging/src/k8s.io/apimachinery/pkg/watch
staging/src/k8s.io/apiserver/pkg/admission
staging/src/k8s.io/apiserver/pkg/admission/configuration
staging/src/k8s.io/apiserver/pkg/admission/initializer
staging/src/k8s.io/apiserver/pkg/admission/plugin/initialization
staging/src/k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle
staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/config
@ -598,7 +630,9 @@ staging/src/k8s.io/cli-runtime/pkg/genericclioptions
staging/src/k8s.io/cli-runtime/pkg/genericclioptions/printers
staging/src/k8s.io/cli-runtime/pkg/genericclioptions/resource
staging/src/k8s.io/client-go/deprecated-dynamic
staging/src/k8s.io/client-go/discovery
staging/src/k8s.io/client-go/discovery/cached
staging/src/k8s.io/client-go/discovery/fake
staging/src/k8s.io/client-go/dynamic
staging/src/k8s.io/client-go/dynamic/fake
staging/src/k8s.io/client-go/examples/workqueue
@ -619,6 +653,7 @@ staging/src/k8s.io/client-go/kubernetes/typed/policy/v1beta1/fake
staging/src/k8s.io/client-go/plugin/pkg/client/auth/oidc
staging/src/k8s.io/client-go/rest
staging/src/k8s.io/client-go/rest/fake
staging/src/k8s.io/client-go/rest/watch
staging/src/k8s.io/client-go/scale
staging/src/k8s.io/client-go/scale/fake
staging/src/k8s.io/client-go/scale/scheme
@ -630,6 +665,7 @@ staging/src/k8s.io/client-go/scale/scheme/extensionsint
staging/src/k8s.io/client-go/scale/scheme/extensionsv1beta1
staging/src/k8s.io/client-go/scale/scheme/extensionsv1beta1
staging/src/k8s.io/client-go/testing
staging/src/k8s.io/client-go/tools/auth
staging/src/k8s.io/client-go/tools/cache
staging/src/k8s.io/client-go/tools/cache/testing
staging/src/k8s.io/client-go/tools/clientcmd
@ -649,6 +685,7 @@ staging/src/k8s.io/client-go/util/integer
staging/src/k8s.io/client-go/util/jsonpath
staging/src/k8s.io/client-go/util/retry
staging/src/k8s.io/client-go/util/testing
staging/src/k8s.io/client-go/util/workqueue
staging/src/k8s.io/code-generator/cmd/client-gen/args
staging/src/k8s.io/code-generator/cmd/client-gen/generators/fake
staging/src/k8s.io/code-generator/cmd/client-gen/generators/scheme
@ -677,6 +714,8 @@ staging/src/k8s.io/metrics/pkg/client/custom_metrics
staging/src/k8s.io/metrics/pkg/client/custom_metrics/fake
staging/src/k8s.io/metrics/pkg/client/external_metrics
staging/src/k8s.io/metrics/pkg/client/external_metrics/fake
staging/src/k8s.io/sample-apiserver/pkg/admission/plugin/banflunder
staging/src/k8s.io/sample-apiserver/pkg/admission/wardleinitializer
staging/src/k8s.io/sample-apiserver/pkg/apis/wardle
staging/src/k8s.io/sample-apiserver/pkg/apis/wardle/v1alpha1
staging/src/k8s.io/sample-apiserver/pkg/apiserver

View File

@ -46,14 +46,14 @@ array_contains () {
# Check that the file is in alphabetical order
failure_file="${KUBE_ROOT}/hack/.golint_failures"
if ! diff -u "${failure_file}" <(LC_ALL=C sort "${failure_file}"); then
{
echo
echo "hack/.golint_failures is not in alphabetical order. Please sort it:"
echo
echo " LC_ALL=C sort -o hack/.golint_failures hack/.golint_failures"
echo
} >&2
false
{
echo
echo "hack/.golint_failures is not in alphabetical order. Please sort it:"
echo
echo " LC_ALL=C sort -o hack/.golint_failures hack/.golint_failures"
echo
} >&2
false
fi
export IFS=$'\n'
@ -61,73 +61,78 @@ export IFS=$'\n'
# as the prefix, however if we run it outside it returns the full path of the file
# with a leading underscore. We'll need to support both scenarios for all_packages.
all_packages=(
$(go list -e ./... | egrep -v "/(third_party|vendor|staging/src/k8s.io/client-go/pkg|generated|clientset_generated)" | sed -e 's|^k8s.io/kubernetes/||' -e "s|^_${KUBE_ROOT}/\?||")
$(go list -e ./... | egrep -v "/(third_party|vendor|staging/src/k8s.io/client-go/pkg|generated|clientset_generated)" | sed -e 's|^k8s.io/kubernetes/||' -e "s|^_${KUBE_ROOT}/\?||")
)
failing_packages=(
$(cat $failure_file)
$(cat $failure_file)
)
unset IFS
errors=()
not_failing=()
for p in "${all_packages[@]}"; do
# Run golint on package/*.go file explicitly to validate all go files
# and not just the ones for the current platform.
# Packages with a corresponding foo_test package will make golint fail
# with a useless error. Just ignore that, see golang/lint#68.
failedLint=$(golint "$p"/*.go 2>/dev/null)
array_contains "$p" "${failing_packages[@]}" && in_failing=$? || in_failing=$?
if [[ -n "${failedLint}" ]] && [[ "${in_failing}" -ne "0" ]]; then
errors+=( "${failedLint}" )
fi
if [[ -z "${failedLint}" ]] && [[ "${in_failing}" -eq "0" ]]; then
not_failing+=( $p )
fi
# Run golint on package/*.go file explicitly to validate all go files
# and not just the ones for the current platform. This also will ensure that
# _test.go files are linted.
# Generated files are ignored, and each file is passed through golint
# individually, as if one file in the package contains a fatal error (such as
# a foo package with a corresponding foo_test package), golint seems to choke
# completely.
# Ref: https://github.com/kubernetes/kubernetes/pull/67675
# Ref: https://github.com/golang/lint/issues/68
failedLint=$(ls "$p"/*.go | egrep -v "(zz_generated.*.go|generated.pb.go|generated.proto|types_swagger_doc_generated.go)" | xargs -L1 golint 2>/dev/null)
array_contains "$p" "${failing_packages[@]}" && in_failing=$? || in_failing=$?
if [[ -n "${failedLint}" ]] && [[ "${in_failing}" -ne "0" ]]; then
errors+=( "${failedLint}" )
fi
if [[ -z "${failedLint}" ]] && [[ "${in_failing}" -eq "0" ]]; then
not_failing+=( $p )
fi
done
# Check that all failing_packages actually still exist
gone=()
for p in "${failing_packages[@]}"; do
array_contains "$p" "${all_packages[@]}" || gone+=( "$p" )
array_contains "$p" "${all_packages[@]}" || gone+=( "$p" )
done
# Check to be sure all the packages that should pass lint are.
if [ ${#errors[@]} -eq 0 ]; then
echo 'Congratulations! All Go source files have been linted.'
echo 'Congratulations! All Go source files have been linted.'
else
{
echo "Errors from golint:"
for err in "${errors[@]}"; do
echo "$err"
done
echo
echo 'Please review the above warnings. You can test via "golint" and commit the result.'
echo 'If the above warnings do not make sense, you can exempt this package from golint'
echo 'checking by adding it to hack/.golint_failures (if your reviewer is okay with it).'
echo
} >&2
false
{
echo "Errors from golint:"
for err in "${errors[@]}"; do
echo "$err"
done
echo
echo 'Please review the above warnings. You can test via "golint" and commit the result.'
echo 'If the above warnings do not make sense, you can exempt this package from golint'
echo 'checking by adding it to hack/.golint_failures (if your reviewer is okay with it).'
echo
} >&2
false
fi
if [[ ${#not_failing[@]} -gt 0 ]]; then
{
echo "Some packages in hack/.golint_failures are passing golint. Please remove them."
echo
for p in "${not_failing[@]}"; do
echo " $p"
done
echo
} >&2
false
{
echo "Some packages in hack/.golint_failures are passing golint. Please remove them."
echo
for p in "${not_failing[@]}"; do
echo " $p"
done
echo
} >&2
false
fi
if [[ ${#gone[@]} -gt 0 ]]; then
{
echo "Some packages in hack/.golint_failures do not exist anymore. Please remove them."
echo
for p in "${gone[@]}"; do
echo " $p"
done
echo
} >&2
false
{
echo "Some packages in hack/.golint_failures do not exist anymore. Please remove them."
echo
for p in "${gone[@]}"; do
echo " $p"
done
echo
} >&2
false
fi