From 45ba86e4297dc67c16eaeb72aa1573cbaf97ca4a Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Wed, 28 Dec 2016 17:29:24 -0800 Subject: [PATCH 1/8] Add check_style in presubmit test. --- .travis.yml | 1 + contrib/endpoints/src/api_manager/config.cc | 10 +++--- script/check-style | 36 +++++++++++++++++++++ src/envoy/prototype/api_manager_env.cc | 11 +++---- src/envoy/prototype/api_manager_env.h | 2 +- src/envoy/prototype/api_manager_filter.cc | 10 +++--- 6 files changed, 53 insertions(+), 17 deletions(-) create mode 100755 script/check-style diff --git a/.travis.yml b/.travis.yml index 8471d0d3c61..ab2705a989e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -34,6 +34,7 @@ before_install: - cat .bazelrc.travis .bazelrc.orig > .bazelrc script: + - script/check-style - bazel --output_base=${HOME}/bazel/outbase test //... notifications: diff --git a/contrib/endpoints/src/api_manager/config.cc b/contrib/endpoints/src/api_manager/config.cc index dcd3d15dcc5..b0a1b85d119 100644 --- a/contrib/endpoints/src/api_manager/config.cc +++ b/contrib/endpoints/src/api_manager/config.cc @@ -258,7 +258,7 @@ bool Config::LoadRpcMethods(ApiManagerEnvInterface *env, bool Config::LoadAuthentication(ApiManagerEnvInterface *env) { // Parsing auth config. const ::google::api::Authentication &auth = service_.authentication(); - map provider_id_provider_map; + map provider_id_provider_map; for (const auto &provider : auth.providers()) { if (provider.id().empty()) { env->LogError("Missing id field in AuthProvider."); @@ -296,15 +296,15 @@ bool Config::LoadAuthentication(ApiManagerEnvInterface *env) { env->LogError(error.c_str()); continue; } - auto provider = utils::FindPtrOrNull(provider_id_provider_map, - provider_id); + auto provider = + utils::FindPtrOrNull(provider_id_provider_map, provider_id); if (provider == nullptr) { std::string error = "Undefined provider_id: " + provider_id; env->LogError(error.c_str()); } else { const std::string &audiences = provider->audiences().empty() - ? requirement.audiences() - : provider->audiences(); + ? requirement.audiences() + : provider->audiences(); (*method)->addAudiencesForIssuer(provider->issuer(), audiences); } } diff --git a/script/check-style b/script/check-style new file mode 100755 index 00000000000..0eac9c928f9 --- /dev/null +++ b/script/check-style @@ -0,0 +1,36 @@ +#!/bin/bash +# +# Copyright 2016 Google Inc. All Rights Reserved. +# +# 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. +# +################################################################################ +# +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" + +CLANG_FORMAT="$(which clang-format)" || CLANG_FORMAT="${HOME}/clang/bin/clang-format" + +pushd ${ROOT} > /dev/null + +SOURCE_FILES=($(git ls-tree -r HEAD --name-only | grep -E '\.(h|c|cc|proto)$')) +"${CLANG_FORMAT}" -style=Google -i "${SOURCE_FILES[@]}" \ + || { echo "Could not run clang-format." ; exit 1 ; } + +CHANGED_FILES=($(git diff HEAD --name-only | grep -E '\.(h|c|cc|proto)$')) + +if [[ "${#CHANGED_FILES}" -ne 0 ]]; then + echo "Files not formatted: ${CHANGED_FILES[@]}" + exit 1 +fi + +popd diff --git a/src/envoy/prototype/api_manager_env.cc b/src/envoy/prototype/api_manager_env.cc index d694fab49a2..0ef8cb004d5 100644 --- a/src/envoy/prototype/api_manager_env.cc +++ b/src/envoy/prototype/api_manager_env.cc @@ -24,7 +24,6 @@ void Http::ApiManager::Env::Log(LogLevel level, const char *message) { } } - class PeriodicTimer : public google::api_manager::PeriodicTimer, public Logger::Loggable { private: @@ -73,9 +72,8 @@ class HTTPRequest : public Http::Message { header_map_.addStatic(Headers::get().Scheme, "http"); header_map_.addStatic(Headers::get().Host, "localhost"); header_map_.addStatic(Headers::get().ContentLength, - std::to_string(body_.length())); - header_map_.addStatic(LowerCaseString("x-api-manager-url"), - request->url()); + std::to_string(body_.length())); + header_map_.addStatic(LowerCaseString("x-api-manager-url"), request->url()); for (auto header : request->request_headers()) { LowerCaseString key(header.first); header_map_.addStatic(key, header.second); @@ -102,10 +100,11 @@ class RequestCallbacks : public AsyncClient::Callbacks { std::stoi(response->headers().Status()->value().c_str()), ""); std::map headers; response->headers().iterate( - [&](const HeaderEntry& header, void*) -> void { + [&](const HeaderEntry &header, void *) -> void { // TODO: fix it // headers.emplace(header.key().c_str(), header.value().c_str()); - }, nullptr); + }, + nullptr); request_->OnComplete(status, std::move(headers), response->bodyAsString()); delete this; } diff --git a/src/envoy/prototype/api_manager_env.h b/src/envoy/prototype/api_manager_env.h index ea045532ae2..6e5e63676aa 100644 --- a/src/envoy/prototype/api_manager_env.h +++ b/src/envoy/prototype/api_manager_env.h @@ -3,8 +3,8 @@ #include "precompiled/precompiled.h" #include "common/common/logger.h" -#include "envoy/upstream/cluster_manager.h" #include "contrib/endpoints/include/api_manager/env_interface.h" +#include "envoy/upstream/cluster_manager.h" #include "server/server.h" namespace Http { diff --git a/src/envoy/prototype/api_manager_filter.cc b/src/envoy/prototype/api_manager_filter.cc index d54258e30c4..d310508a592 100644 --- a/src/envoy/prototype/api_manager_filter.cc +++ b/src/envoy/prototype/api_manager_filter.cc @@ -1,15 +1,15 @@ #include "precompiled/precompiled.h" #include "api_manager_env.h" -#include "common/grpc/common.h" #include "common/common/logger.h" +#include "common/grpc/common.h" #include "common/http/filter/ratelimit.h" #include "common/http/headers.h" #include "common/http/utility.h" -#include "envoy/server/instance.h" #include "contrib/endpoints/include/api_manager/api_manager.h" -#include "server/config/network/http_connection_manager.h" #include "contrib/endpoints/src/grpc/transcoding/transcoder.h" +#include "envoy/server/instance.h" +#include "server/config/network/http_connection_manager.h" namespace Http { namespace ApiManager { @@ -157,7 +157,7 @@ class Instance : public Http::StreamFilter, FilterDataStatus decodeData(Buffer::Instance& data, bool end_stream) override { log().debug("Called ApiManager::Instance : {} ({}, {})", __func__, - data.length(), end_stream); + data.length(), end_stream); if (state_ == Calling) { return FilterDataStatus::StopIterationAndBuffer; } @@ -180,7 +180,7 @@ class Instance : public Http::StreamFilter, } void completeCheck(const google::api_manager::utils::Status& status) { log().debug("Called ApiManager::Instance : check complete {}", - status.ToJson()); + status.ToJson()); if (!status.ok() && state_ != Responded) { state_ = Responded; Utility::sendLocalReply(*decoder_callbacks_, Code(status.HttpCode()), From 33fff838a00ee84fba71c506a5665f468d35ffad Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Wed, 28 Dec 2016 17:50:15 -0800 Subject: [PATCH 2/8] Checks clang-format version. --- script/check-style | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/script/check-style b/script/check-style index 0eac9c928f9..2fe0b09f296 100755 --- a/script/check-style +++ b/script/check-style @@ -20,6 +20,14 @@ ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" CLANG_FORMAT="$(which clang-format)" || CLANG_FORMAT="${HOME}/clang/bin/clang-format" +CLANG_FORMAT_VERSION="$(${CLANG_FORMAT} -version | cut -d ' ' -f 3)" +CLANG_FORMAT_VERSION_REQUIRED="3.8.0" +if [[ "${CLANG_FORMAT_VERSION}" != "${CLANG_FORMAT_VERSION_REQUIRED}" ]]; then + echo "Skipping: clang-format ${CLANG_FORMAT_VERSION_REQUIRED} required." + echo "Current version: ${CLANG_FORMAT_VERSION}" + exit 0 +fi + pushd ${ROOT} > /dev/null SOURCE_FILES=($(git ls-tree -r HEAD --name-only | grep -E '\.(h|c|cc|proto)$')) From c254ce53e23771752fb1fc177869f9a32a606186 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Wed, 28 Dec 2016 20:11:40 -0800 Subject: [PATCH 3/8] Install required clang-fromat version. --- script/check-style | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/script/check-style b/script/check-style index 2fe0b09f296..195b2394808 100755 --- a/script/check-style +++ b/script/check-style @@ -20,12 +20,17 @@ ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" CLANG_FORMAT="$(which clang-format)" || CLANG_FORMAT="${HOME}/clang/bin/clang-format" -CLANG_FORMAT_VERSION="$(${CLANG_FORMAT} -version | cut -d ' ' -f 3)" -CLANG_FORMAT_VERSION_REQUIRED="3.8.0" -if [[ "${CLANG_FORMAT_VERSION}" != "${CLANG_FORMAT_VERSION_REQUIRED}" ]]; then - echo "Skipping: clang-format ${CLANG_FORMAT_VERSION_REQUIRED} required." - echo "Current version: ${CLANG_FORMAT_VERSION}" - exit 0 +CLANG_VERSION="$(${CLANG_FORMAT} -version | cut -d ' ' -f 3)" +CLANG_VERSION_REQUIRED="3.8.0" +if [[ "${CLANG_VERSION}" != "${CLANG_VERSION_REQUIRED}" ]]; then + echo "Current version: ${CLANG_VERSION}" + CLANG_DIRECTORY=$(mktemp -d /tmp/clang_XXXXX) + echo "Installing required clang-format ${CLANG_VERSION_REQUIRED} to ${CLANG_DIRECTORY}" + curl --silent --show-error --retry 10 \ + "http://releases.llvm.org/${CLANG_VERSION_REQUIRED}/clang+llvm-${CLANG_VERSION_REQUIRED}-x86_64-linux-gnu-debian8.tar.xz" \ + | tar Jx -C "${CLANG_DIRECTORY}" --strip=1 \ + && CLANG_FORMAT="${CLANG_DIRECTORY}/bin/clang-format" \ + || { echo "Could not install required clang-format. Skip formating." ; exit 0 ; } fi pushd ${ROOT} > /dev/null From c74e9aa09cbf16124201268223e8237c61271b54 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Wed, 28 Dec 2016 20:26:55 -0800 Subject: [PATCH 4/8] Download the clang-format for ubuntu. --- script/check-style | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/check-style b/script/check-style index 195b2394808..86f82ac1bd4 100755 --- a/script/check-style +++ b/script/check-style @@ -27,7 +27,7 @@ if [[ "${CLANG_VERSION}" != "${CLANG_VERSION_REQUIRED}" ]]; then CLANG_DIRECTORY=$(mktemp -d /tmp/clang_XXXXX) echo "Installing required clang-format ${CLANG_VERSION_REQUIRED} to ${CLANG_DIRECTORY}" curl --silent --show-error --retry 10 \ - "http://releases.llvm.org/${CLANG_VERSION_REQUIRED}/clang+llvm-${CLANG_VERSION_REQUIRED}-x86_64-linux-gnu-debian8.tar.xz" \ + "http://releases.llvm.org/${CLANG_VERSION_REQUIRED}/clang+llvm-${CLANG_VERSION_REQUIRED}-x86_64-linux-gnu-ubuntu-14.04.tar.xz" \ | tar Jx -C "${CLANG_DIRECTORY}" --strip=1 \ && CLANG_FORMAT="${CLANG_DIRECTORY}/bin/clang-format" \ || { echo "Could not install required clang-format. Skip formating." ; exit 0 ; } From 7e34fc6973887a3e2eaed4092b18c7b66c5662de Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Wed, 28 Dec 2016 21:04:53 -0800 Subject: [PATCH 5/8] Cache the clang-format folder. --- .travis.yml | 1 + script/check-style | 16 +++++++++------- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/.travis.yml b/.travis.yml index ab2705a989e..c2e753a867e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -21,6 +21,7 @@ cache: directories: - $HOME/bazel/install - $HOME/bazel/outbase + - $HOME/clang before_install: - mkdir -p ${HOME}/bazel/install diff --git a/script/check-style b/script/check-style index 86f82ac1bd4..e79fec7abdc 100755 --- a/script/check-style +++ b/script/check-style @@ -18,21 +18,22 @@ # ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -CLANG_FORMAT="$(which clang-format)" || CLANG_FORMAT="${HOME}/clang/bin/clang-format" - -CLANG_VERSION="$(${CLANG_FORMAT} -version | cut -d ' ' -f 3)" +# Install required clang version to a folder and cache it. CLANG_VERSION_REQUIRED="3.8.0" -if [[ "${CLANG_VERSION}" != "${CLANG_VERSION_REQUIRED}" ]]; then - echo "Current version: ${CLANG_VERSION}" - CLANG_DIRECTORY=$(mktemp -d /tmp/clang_XXXXX) +CLANG_DIRECTORY="${HOME}/clang" +CLANG_FORMAT="${CLANG_DIRECTORY}/bin/clang-format" + +if [[ ! -x "${CLANG_FORMAT}" ]]; then echo "Installing required clang-format ${CLANG_VERSION_REQUIRED} to ${CLANG_DIRECTORY}" + mkdir -p ${CLANG_DIRECTORY} curl --silent --show-error --retry 10 \ "http://releases.llvm.org/${CLANG_VERSION_REQUIRED}/clang+llvm-${CLANG_VERSION_REQUIRED}-x86_64-linux-gnu-ubuntu-14.04.tar.xz" \ | tar Jx -C "${CLANG_DIRECTORY}" --strip=1 \ - && CLANG_FORMAT="${CLANG_DIRECTORY}/bin/clang-format" \ || { echo "Could not install required clang-format. Skip formating." ; exit 0 ; } fi +echo "Checking file format ..." + pushd ${ROOT} > /dev/null SOURCE_FILES=($(git ls-tree -r HEAD --name-only | grep -E '\.(h|c|cc|proto)$')) @@ -45,5 +46,6 @@ if [[ "${#CHANGED_FILES}" -ne 0 ]]; then echo "Files not formatted: ${CHANGED_FILES[@]}" exit 1 fi +echo "All files are properly formatted." popd From 12d30c7f85af21a87316757f71cea53a4ca4359a Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Wed, 28 Dec 2016 21:06:43 -0800 Subject: [PATCH 6/8] Use ubuntu 16.4 --- script/check-style | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/check-style b/script/check-style index e79fec7abdc..a2a7dad5648 100755 --- a/script/check-style +++ b/script/check-style @@ -27,7 +27,7 @@ if [[ ! -x "${CLANG_FORMAT}" ]]; then echo "Installing required clang-format ${CLANG_VERSION_REQUIRED} to ${CLANG_DIRECTORY}" mkdir -p ${CLANG_DIRECTORY} curl --silent --show-error --retry 10 \ - "http://releases.llvm.org/${CLANG_VERSION_REQUIRED}/clang+llvm-${CLANG_VERSION_REQUIRED}-x86_64-linux-gnu-ubuntu-14.04.tar.xz" \ + "http://releases.llvm.org/${CLANG_VERSION_REQUIRED}/clang+llvm-${CLANG_VERSION_REQUIRED}-x86_64-linux-gnu-ubuntu-16.04.tar.xz" \ | tar Jx -C "${CLANG_DIRECTORY}" --strip=1 \ || { echo "Could not install required clang-format. Skip formating." ; exit 0 ; } fi From 9ab7f6f6671ff76e86ac07d95b85ad73025342bc Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Wed, 28 Dec 2016 21:18:58 -0800 Subject: [PATCH 7/8] Revert ubuntu version back to 14.04. --- script/check-style | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/check-style b/script/check-style index a2a7dad5648..e79fec7abdc 100755 --- a/script/check-style +++ b/script/check-style @@ -27,7 +27,7 @@ if [[ ! -x "${CLANG_FORMAT}" ]]; then echo "Installing required clang-format ${CLANG_VERSION_REQUIRED} to ${CLANG_DIRECTORY}" mkdir -p ${CLANG_DIRECTORY} curl --silent --show-error --retry 10 \ - "http://releases.llvm.org/${CLANG_VERSION_REQUIRED}/clang+llvm-${CLANG_VERSION_REQUIRED}-x86_64-linux-gnu-ubuntu-16.04.tar.xz" \ + "http://releases.llvm.org/${CLANG_VERSION_REQUIRED}/clang+llvm-${CLANG_VERSION_REQUIRED}-x86_64-linux-gnu-ubuntu-14.04.tar.xz" \ | tar Jx -C "${CLANG_DIRECTORY}" --strip=1 \ || { echo "Could not install required clang-format. Skip formating." ; exit 0 ; } fi From 04fcbd682f235907b0815fb74f5bb2853044b172 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Wed, 28 Dec 2016 21:28:09 -0800 Subject: [PATCH 8/8] Make sure clang-format can be run. --- script/check-style | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/script/check-style b/script/check-style index e79fec7abdc..443fabd087f 100755 --- a/script/check-style +++ b/script/check-style @@ -23,7 +23,8 @@ CLANG_VERSION_REQUIRED="3.8.0" CLANG_DIRECTORY="${HOME}/clang" CLANG_FORMAT="${CLANG_DIRECTORY}/bin/clang-format" -if [[ ! -x "${CLANG_FORMAT}" ]]; then +CLANG_VERSION="$(${CLANG_FORMAT} -version | cut -d ' ' -f 3)" +if [[ "${CLANG_VERSION}" != "${CLANG_VERSION_REQUIRED}" ]]; then echo "Installing required clang-format ${CLANG_VERSION_REQUIRED} to ${CLANG_DIRECTORY}" mkdir -p ${CLANG_DIRECTORY} curl --silent --show-error --retry 10 \