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.

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

Fix formatter issue #20

Merged
merged 24 commits into from
Apr 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
dc2d2a1
Print clang-format version
i05nagai Apr 13, 2019
42b8280
Add options to skip checking with clang-formatter
i05nagai Apr 13, 2019
725f009
Install buildifier
i05nagai Apr 13, 2019
e41b0db
Use clang-format 5.0 explicitly
i05nagai Apr 13, 2019
3f28ba5
Disable checking with clang-format on OSX
i05nagai Apr 13, 2019
3e4c65c
Add script to run formatter inside docker container
i05nagai Apr 13, 2019
6bf3881
Update docs for formatter
i05nagai Apr 13, 2019
5a13f2f
Fix the names of functions
i05nagai Apr 13, 2019
432065f
Use sudo if option is specified
i05nagai Apr 13, 2019
e47b0ae
Remove unused go-dep
i05nagai Apr 13, 2019
655308e
Delete duplicated package
i05nagai Apr 13, 2019
0ffefd9
Add sudo to install buildifier
i05nagai Apr 13, 2019
97088e8
Update specify path to golang explicitly
i05nagai Apr 13, 2019
dba46b3
Add command to show user information
i05nagai Apr 13, 2019
8d969c2
Specify GOPATH for root
i05nagai Apr 13, 2019
ff61191
Change not to install golang
i05nagai Apr 19, 2019
00ec457
Support to install clang-format-5.0 for ubuntu 14.04
i05nagai Apr 19, 2019
b8cd6c2
Fix for clang-format-5.0
i05nagai Apr 19, 2019
273e740
Add option to download package from unauthenticate repository
i05nagai Apr 19, 2019
a11c361
Update install newer version of bazel on osx
i05nagai Apr 19, 2019
a338818
Remove unnecessary statement
i05nagai Apr 19, 2019
7cf8a0e
Fix numerical error in case of gcc compiler
i05nagai Apr 19, 2019
c6e6a6d
Fix typos
i05nagai Apr 19, 2019
8f28c1a
Format code
i05nagai Apr 19, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ matrix:
install:
- ci/travis/install.sh
script:
- ci/build/check_sanity.sh
# behavior of clang-formatter depends on the version of clang. In OSX, it's hard to install specified version. We only checks files on Linux
- if [ "$TRAVIS_OS_NAME" == "linux" ]; then ci/build/check_sanity.sh; fi
- if [ "$TRAVIS_OS_NAME" == "osx" ]; then ci/build/check_sanity.sh --skip-clang-format; fi
- ci/build/build.sh
after_success:
- ci/travis/after_success.sh
Expand Down
6 changes: 5 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,10 @@ diff <my_cc_file> /tmp/my_cc_file.cc
To format all of code in the repository, you can run the script on the root of the repository by executing the command:

```
./tools/formatter.sh
# Build docker images for formatter. Behaviour of formatter depends on the version of formatter.
./tools/docker/gcc/docker_build_ubuntu1604.sh
# Run formatter with docker images
./tools/docker_run_formatter.sh
```

### bazel
Expand Down Expand Up @@ -146,6 +149,7 @@ There is no visualization of the result of the test coverage for now.
* [x] lint
* [x] C++ format check
* [ClangFormat — Clang 8 documentation](https://clang.llvm.org/docs/ClangFormat.html)
* Using clnag-fromat version 5.0
* [x] bazel format check
* [buildtools/buildifier at master · bazelbuild/buildtools](https://github.com/bazelbuild/buildtools/tree/master/buildifier)
* build with cmake
Expand Down
7 changes: 4 additions & 3 deletions ci/_lib/install_bazel.sh
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ install_bazel_osx()
brew update
fi

brew cask install homebrew/cask-versions/java8
brew install \
bazel
# tap and install
brew tap bazelbuild/tap
brew tap-pin bazelbuild/tap
brew install bazel
}
58 changes: 58 additions & 0 deletions ci/_lib/install_buildifier.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
#!/bin/bash

set -e
if [[ ! -z ${RECIPE_DEBUG+x} ]]; then
set -x
fi

#
# install_buildifier.sh
#
# variables:
# * _do_update=1
# * _use_sudo=1
#
install_buildifier_linux()
{

if [[ ${_use_sudo} -eq 1 ]]; then
local SUDO="sudo"
fi

if [[ ${_do_update} -eq 1 ]]; then
${SUDO} apt-get update
fi

#
# install golang
#
# export GOPATH="/opt/local/golang"
# export PATH="$PATH:/usr/lib/go-1.10/bin:$GOPATH/bin"
# echo 'export GOPATH="'$GOPATH'"' | ${SUDO} tee -a ${HOME}/.bashrc
# echo 'export PATH="'$PATH'"' | ${SUDO} tee -a ${HOME}/.bashrc

# ${SUDO} apt-get install -y \
# software-properties-common \
# curl
# ${SUDO} mkdir -p \
# $GOPATH/bin
# ${SUDO} add-apt-repository ppa:gophers/archive < /dev/null
# ${SUDO} apt-get update
# ${SUDO} GOPATH="$GOPATH" PATH="$PATH" apt-get install -y \
# git \
# golang-1.10-go
# ${SUDO} GOPATH="$GOPATH" PATH="$PATH" /usr/lib/go-1.10/bin/go get github.com/bazelbuild/buildtools/buildifier
}

#
# variables:
# * _do_update=1
#
install_buildifier_osx()
{
if [[ ${_do_update} -eq 1 ]]; then
brew update
fi

brew install buildifier
}
12 changes: 10 additions & 2 deletions ci/_lib/install_clang_format.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,18 @@ install_clang_format_linux()

UBUNTU_VERSION=`get_ubuntu_version`
if [[ "$UBUNTU_VERSION" == "14.04" ]]; then
${SUDO} apt-get install -y clang-format-3.9
# to install newer version
${SUDO} apt-get install -y software-properties-common
${SUDO} apt-add-repository "deb http://apt.llvm.org/trusty/ llvm-toolchain-trusty-5.0 main"
${SUDO} apt-get update

${SUDO} apt-get install -y --allow-unauthenticated clang-format-5.0
${SUDO} ln -f -s /usr/bin/clang-format-5.0 /usr/bin/clang-format
else
${SUDO} apt-get install -y clang-format
${SUDO} apt-get install -y clang-format-5.0
${SUDO} ln -f -s /usr/bin/clang-format-5.0 /usr/bin/clang-format
fi
clang-format --version
}

#
Expand Down
4 changes: 4 additions & 0 deletions ci/build/build.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
#!/bin/bash

cat /etc/group
cat /etc/passwd
id

set -e
if [ ! -z "${RECIPE_DEBUG}" ]; then
set -x
Expand Down
60 changes: 41 additions & 19 deletions ci/build/check_sanity.sh
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
#!/bin/bash

do_buildifier(){
local BUILD_FILES=$(find . -type f -name BUILD -or -name 'BUILD.*' -or -name BUILD.bazel)
local NUM_BUILD_FILES=$(echo ${BUILD_FILES} | wc -w)
local build_files=$(find . -type f -name BUILD -or -name 'BUILD.*' -or -name BUILD.bazel)
local num_build_files=$(echo ${build_files} | wc -w)

echo "Running do_buildifier on ${NUM_BUILD_FILES} files"
echo "Running do_buildifier on ${num_build_files} files"
echo ""

local BUILDIFIER_START_TIME=$(date +'%s')
local BUILDIFIER_OUTPUT_FILE="$(mktemp)_buildifier_output.log"
local buildifier_start_time=$(date +'%s')
local buildifier_output_file="$(mktemp)_buildifier_output.log"

rm -rf ${BUILDIFIER_OUTPUT_FILE}
rm -rf ${buildifier_output_file}

buildifier -showlog -v -mode=check \
${BUILD_FILES} 2>&1 | tee ${BUILDIFIER_OUTPUT_FILE}
${build_files} 2>&1 | tee ${buildifier_output_file}
local BUILDIFIER_END_TIME=$(date +'%s')

echo ""
echo "buildifier took $((BUILDIFIER_END_TIME - BUILDIFIER_START_TIME)) s"
echo "buildifier took $((BUILDIFIER_END_TIME - buildifier_start_time)) s"
echo ""

if [[ -s ${BUILDIFIER_OUTPUT_FILE} ]]; then
if [[ -s ${buildifier_output_file} ]]; then
echo "FAIL: buildifier found errors and/or warnings in above BUILD files."
echo "buildifier suggested the following changes:"
buildifier -showlog -v -mode=diff ${BUILD_FILES}
buildifier -showlog -v -mode=diff ${build_files}
echo "Please fix manually or run buildifier <file> to auto-fix."
return 1
else
Expand Down Expand Up @@ -74,16 +74,18 @@ do_clang_format_check() {
return 1
fi

local clang_src_files

if [[ "$1" == "--incremental" ]]; then
CLANG_SRC_FILES=$(get_clang_files_to_check --incremental)
clang_src_files=$(get_clang_files_to_check --incremental)

if [[ -z "${CLANG_SRC_FILES}" ]]; then
if [[ -z "${clang_src_files}" ]]; then
echo "do_clang_format_check will NOT run due to --incremental flag and "\
"due to the absence of .h or .cc code changes in the last commit."
return 0
fi
elif [[ -z "$1" ]]; then
CLANG_SRC_FILES=$(get_clang_files_to_check)
clang_src_files=$(get_clang_files_to_check)
else
echo "Invalid syntax for invoking do_clang_format_check"
echo "Usage: do_clang_format_check [--incremental]"
Expand All @@ -92,13 +94,12 @@ do_clang_format_check() {

CLANG_FORMAT=${CLANG_FORMAT:-clang-format}

success=1
for filename in $CLANG_SRC_FILES; do
local diff=$($CLANG_FORMAT --style=google $filename | diff $filename -)
local success=1
for filename in $clang_src_files; do
local diff=$($CLANG_FORMAT $filename | diff $filename -)
if [[ "$diff" != "" ]]; then
success=0
echo File $filename is not properly formatted with "clang-format "\
"--style=google"
echo File $filename is not properly formatted with "clang-format"
fi
done

Expand All @@ -109,5 +110,26 @@ do_clang_format_check() {
echo Clang format check success.
}

while [ $# -gt 0 ];
do
case ${1} in
--skip-clang-format|-c)
shift || true # in case of set -e
skip_clang_format=1
;;

*)
echo "[ERROR] Invalid option '${1}'"
exit 1
;;
esac
shift || true # in case of set -e
done

do_buildifier
do_clang_format_check

echo ""

if [[ -z ${skip_clang_format} ]]; then
do_clang_format_check
fi
11 changes: 11 additions & 0 deletions ci/linux/install_buildifier.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/bin/bash

set -e
if [[ ! -z ${RECIPE_DEBUG+x} ]]; then
set -x
fi

PATH_TO_CI=$(cd $(dirname ${0});cd ..;pwd)
source ${PATH_TO_CI}/_lib/install_buildifier.sh

_use_sudo=${_use_sudo} _do_update=${_do_update} install_buildifier_linux
1 change: 1 addition & 0 deletions ci/travis/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ fi
# sourcing is required to export some variables
#
source ${PATH_TO_ROOT}/ci/travis/install_bazel.sh
source ${PATH_TO_ROOT}/ci/travis/install_buildifier.sh
source ${PATH_TO_ROOT}/ci/travis/install_clang_format.sh
source ${PATH_TO_ROOT}/ci/travis/install_gcc.sh
# ${PATH_TO_ROOT}/ci/travis/install_valgrind.sh
Expand Down
22 changes: 22 additions & 0 deletions ci/travis/install_buildifier.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#!/bin/bash

set -e
if [[ ! -z ${RECIPE_DEBUG+x} ]]; then
set -x
fi

PATH_TO_CI=$(cd $(dirname ${0});cd ..;pwd)
source ${PATH_TO_CI}/_lib/install_buildifier.sh

if [[ "$TRAVIS_OS_NAME" == "" ]]; then
echo "\$TRAVIS_OS_NAME is not set."
exit 1
fi

if [[ "$TRAVIS_OS_NAME" == "linux" ]]; then
_use_sudo=1 install_buildifier_linux
fi

if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then
install_buildifier_osx
fi
5 changes: 4 additions & 1 deletion recipe/linear_algebra/vector_test.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#include "recipe/linear_algebra/vector.h"
#include <gtest/gtest.h>
#include <limits>
#include <sstream>
#include "recipe/linear_algebra/test_util/gtest_assertion.h"
#include "recipe/linear_algebra/test_util/test_data.h"

namespace recipe {
Expand All @@ -13,7 +15,8 @@ TEST(VectorTest, CopyConstructor) {
Vector expect(3);
Vector actual(expect);

EXPECT_EQ(expect, actual);
EXPECT_VECTOR_ELEMENT_NEAR(expect, actual,
std::numeric_limits<double>::epsilon());
}

TEST(VectorTest, ConstructorInitializer) {
Expand Down
4 changes: 2 additions & 2 deletions tools/docker/gcc/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
Build docker images with the command

```
./docker_build.sh
./docker_build_ubuntu1604.sh
```

Run docker container and attach to it

```
./docker_run.sh
./docker_run_ubuntu1604.sh
```
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,16 @@

PATH_TO_REPOSITORY=$(cd $(dirname ${0});cd ../../..;pwd)

# if no arguments are provided
args=$@
if [[ -z "${args}" ]]; then
args=/bin/bash
fi

docker run \
--rm \
-it \
--volume ${PATH_TO_REPOSITORY}:/tmp/repository \
--workdir /tmp/repository \
recipe/gcc:4.9 \
/bin/bash
$args
8 changes: 7 additions & 1 deletion tools/docker/gcc/docker_run_ubuntu1604.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,16 @@

PATH_TO_REPOSITORY=$(cd $(dirname ${0});cd ../../..;pwd)

# if no arguments are provided
args=$@
if [[ -z "${args}" ]]; then
args=/bin/bash
fi

docker run \
--rm \
-it \
--volume ${PATH_TO_REPOSITORY}:/tmp/repository \
--workdir /tmp/repository \
recipe/gcc:4.9 \
/bin/bash
$args
1 change: 1 addition & 0 deletions tools/docker/gcc/ubuntu1404/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ COPY ci /tmp/ci
RUN \
apt-get update \
&& /tmp/ci/linux/install_bazel.sh \
&& /tmp/ci/linux/install_buildifier.sh \
&& /tmp/ci/linux/install_clang_format.sh \
&& /tmp/ci/linux/install_gcc.sh \
&& rm -rf /var/lib/apt/lists/* \
Expand Down
1 change: 1 addition & 0 deletions tools/docker/gcc/ubuntu1604/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ COPY ci /tmp/ci
RUN \
apt-get update \
&& /tmp/ci/linux/install_bazel.sh \
&& /tmp/ci/linux/install_buildifier.sh \
&& /tmp/ci/linux/install_clang_format.sh \
&& /tmp/ci/linux/install_gcc.sh \
&& rm -rf /var/lib/apt/lists/* \
Expand Down
6 changes: 6 additions & 0 deletions tools/docker_run_formatter.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#!/bin/bash

PATH_TO_REPOSITORY=$(cd $(dirname ${0});cd ..;pwd)
cd $PATH_TO_REPOSITORY

./tools/docker/gcc/docker_run_ubuntu1604.sh ./tools/formatter.sh