Skip to content

Commit

Permalink
Makefile: organize for faster and more flexible dev workflows (tikv#8777
Browse files Browse the repository at this point in the history
)

Signed-off-by: Greg Weber <greg@pingcap.com>
  • Loading branch information
Greg Weber authored Oct 15, 2020
1 parent e6d5b6d commit 4dc97df
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 84 deletions.
6 changes: 3 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ rustup component add clippy

### Building and testing

TiKV includes a `Makefile` with common workflows, you can also use `cargo`, as you would in many other Rust projects.
TiKV includes a `Makefile` that has common workflows and sets up a standard build environment. You can also use `cargo`, as you would in many other Rust projects. It can help to run a command in the same environment as the Makefile: this can avoid re-compilations due to environment changes. This is done by prefixing a command with `scripts/env`, for example: `./scripts/env cargo build`

At this point, you can build TiKV:
You can build TiKV:

```bash
make build
Expand Down Expand Up @@ -78,7 +78,7 @@ You can run the test suite alone, or just run a specific test:
# Run the full suite
make test
# Run a specific test
cargo test --all $TESTNAME -- --nocapture
./scripts/test $TESTNAME -- --nocapture
```

TiKV follows the Rust community coding style. We use Rustfmt and [Clippy](https://github.com/Manishearth/rust-clippy) to automatically format and lint our code. Using these tools is checked in our CI. These are as part of `make dev`, you can also run them alone:
Expand Down
102 changes: 30 additions & 72 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,18 @@ export DOCKER_IMAGE_TAG ?= "latest"
# https://internals.rust-lang.org/t/evaluating-pipelined-rustc-compilation/10199/68
export CARGO_BUILD_PIPELINING=true

default: release
# Almost all the rules in this Makefile are PHONY
# Declaring a rule as PHONY could improve correctness
# But probably instead just improves performance by a little bit
.PHONY: audit clippy format pre-format pre-clippy pre-audit unset-override
.PHONY: all build clean dev check-udeps doc error-code fuzz run test
.PHONY: docker docker-tag docker-tag-with-git-hash docker-tag-with-git-tag
.PHONY: ctl dist_artifacts dist_tarballs x-build-dist
.PHONY: build_dist_release dist_release dist_unportable_release
.PHONY: fail_release prof_release release unportable_release


.PHONY: all
default: release

clean:
cargo clean
Expand Down Expand Up @@ -197,12 +206,10 @@ dist_unportable_release:
ROCKSDB_SYS_PORTABLE=0 make dist_release

# Create distributable artifacts. Binaries and Docker image tarballs.
.PHONY: dist_artifacts
dist_artifacts: dist_tarballs

# Build gzipped tarballs of the binaries and docker images.
# Used to build a `dist/` folder containing the release artifacts.
.PHONY: dist_tarballs
dist_tarballs: docker
docker rm -f tikv-binary-extraction-dummy || true
docker create --name tikv-binary-extraction-dummy pingcap/tikv
Expand All @@ -214,52 +221,33 @@ dist_tarballs: docker
docker rm tikv-binary-extraction-dummy

# Create tags of the docker images
.PHONY: docker-tag
docker-tag: docker-tag-with-git-hash docker-tag-with-git-tag

# Tag docker images with the git hash
.PHONY: docker-tag-with-git-hash
docker-tag-with-git-hash:
docker tag pingcap/tikv pingcap/tikv:${TIKV_BUILD_GIT_HASH}

# Tag docker images with the git tag
.PHONY: docker-tag-with-git-tag
docker-tag-with-git-tag:
docker tag pingcap/tikv pingcap/tikv:${TIKV_BUILD_GIT_TAG}


## Execution environment
## -------
## Run a command in the environment setup by the Makefile
##
## COMMAND="echo" make run
##
run:
@env MAKEFILE_RUN=1 $(COMMAND)

## Testing
## -------

# Run tests under a variety of conditions. This should pass before
# submitting pull requests. Note though that the CI system tests TiKV
# through its own scripts and does not use this rule.
.PHONY: run-test
run-test:
# When SIP is enabled, DYLD_LIBRARY_PATH will not work in subshell, so we have to set it
# again here. LOCAL_DIR is defined in .travis.yml.
# The special Linux case below is testing the mem-profiling
# features in tikv_alloc, which are marked #[ignore] since
# they require special compile-time and run-time setup
# Fortunately rebuilding with the mem-profiling feature will only
# rebuild starting at jemalloc-sys.
export DYLD_LIBRARY_PATH="${DYLD_LIBRARY_PATH}:${LOCAL_DIR}/lib" && \
export LOG_LEVEL=DEBUG && \
export RUST_BACKTRACE=1 && \
cargo -Zpackage-features test --workspace \
--exclude fuzzer-honggfuzz --exclude fuzzer-afl --exclude fuzzer-libfuzzer \
--features "${ENABLE_FEATURES}" ${EXTRA_CARGO_ARGS} -- --nocapture && \
if [[ "`uname`" == "Linux" ]]; then \
cargo -Zpackage-features test --workspace \
--exclude fuzzer-honggfuzz --exclude fuzzer-afl --exclude fuzzer-libfuzzer \
--features "${ENABLE_FEATURES}" ${EXTRA_CARGO_ARGS} -p tikv -p tikv_alloc --lib -- --nocapture --ignored; \
fi

.PHONY: test
test: run-test
@if [[ "`uname`" = "Linux" ]]; then \
env EXTRA_CARGO_ARGS="--message-format=json-render-diagnostics -q --no-run" make run-test |\
python scripts/check-bins.py --features "${ENABLE_FEATURES}" --check-tests; \
fi
# submitting pull requests.
test:
./scripts/test-all

## Static analysis
## ---------------
Expand All @@ -283,39 +271,8 @@ doc:
pre-clippy: unset-override
@rustup component add clippy

ALLOWED_CLIPPY_LINTS=-A clippy::module_inception -A clippy::needless_pass_by_value -A clippy::cognitive_complexity \
-A clippy::unreadable_literal -A clippy::should_implement_trait -A clippy::verbose_bit_mask \
-A clippy::implicit_hasher -A clippy::large_enum_variant -A clippy::new_without_default \
-A clippy::neg_cmp_op_on_partial_ord -A clippy::too_many_arguments \
-A clippy::excessive_precision -A clippy::collapsible_if -A clippy::blacklisted_name \
-A clippy::needless_range_loop -A clippy::redundant_closure \
-A clippy::match_wild_err_arm -A clippy::blacklisted_name -A clippy::redundant_closure_call \
-A clippy::useless_conversion -A clippy::new_ret_no_self -A clippy::unnecessary_sort_by

# PROST feature works differently in test cdc and backup package, they need to be checked under their folders.
ifneq (,$(findstring prost-codec,"$(ENABLE_FEATURES)"))
clippy: pre-clippy
@cargo clippy --workspace --all-targets --no-default-features \
--exclude cdc --exclude backup --exclude tests --exclude cmd \
--exclude fuzz-targets --exclude fuzzer-honggfuzz --exclude fuzzer-afl --exclude fuzzer-libfuzzer \
--features "${ENABLE_FEATURES}" -- $(ALLOWED_CLIPPY_LINTS)
@for pkg in "components/cdc" "components/backup" "cmd" "tests"; do \
cd $$pkg && \
cargo clippy --all-targets --no-default-features \
--features "${ENABLE_FEATURES}" -- $(ALLOWED_CLIPPY_LINTS) && \
cd - >/dev/null;\
done
@for pkg in "fuzz"; do \
cd $$pkg && \
cargo clippy --all-targets -- $(ALLOWED_CLIPPY_LINTS) && \
cd - >/dev/null; \
done
else
clippy: pre-clippy
@cargo clippy --workspace \
--exclude fuzzer-honggfuzz --exclude fuzzer-afl --exclude fuzzer-libfuzzer \
--features "${ENABLE_FEATURES}" -- $(ALLOWED_CLIPPY_LINTS)
endif
@./scripts/clippy-all

pre-audit:
$(eval LATEST_AUDIT_VERSION := $(strip $(shell cargo search cargo-audit | head -n 1 | awk '{ gsub(/"/, "", $$3); print $$3 }')))
Expand All @@ -328,13 +285,11 @@ pre-audit:
audit: pre-audit
cargo audit

.PHONY: check-udeps
check-udeps:
which cargo-udeps &>/dev/null || cargo install cargo-udeps && cargo udeps

FUZZER ?= Honggfuzz

.PHONY: fuzz
fuzz:
@cargo run --package fuzz --no-default-features --features "${ENABLE_FEATURES}" -- run ${FUZZER} ${FUZZ_TARGET} \
|| echo "" && echo "Set the target for fuzzing using FUZZ_TARGET and the fuzzer using FUZZER (default is Honggfuzz)"
Expand All @@ -349,11 +304,14 @@ ctl:
@mkdir -p ${BIN_PATH}
@cp -f ${CARGO_TARGET_DIR}/release/tikv-ctl ${BIN_PATH}/

error-code:
# Actually use make to track dependencies! This saves half a second.
error_code_files := $(shell find components/error_code/ -type f )
etc/error_code.toml: $(error_code_files)
cargo run --manifest-path components/error_code/Cargo.toml --features protobuf-codec

error-code: etc/error_code.toml

# A special target for building TiKV docker image.
.PHONY: docker
docker:
bash ./scripts/gen-dockerfile.sh | docker build \
-t ${DOCKER_IMAGE_NAME}:${DOCKER_IMAGE_TAG} \
Expand Down
24 changes: 15 additions & 9 deletions ci-build/test.sh
Original file line number Diff line number Diff line change
@@ -1,36 +1,42 @@
#!/usr/bin/env bash
# Copyright 2016 TiKV Project Authors

set -o pipefail
set -eo pipefail

panic() {
echo -e "$@" >&1
echo -e "$@" >&2
exit 1
}

if [[ "$SKIP_FORMAT_CHECK" != "true" ]]; then
# Move to project root
cd "$(dirname "$0")/.."

if [[ -z "$SKIP_FORMAT_CHECK" ]]; then
make format
git diff-index --quiet HEAD -- || (git diff; panic "\e[35mplease make format before creating a pr!!!\e[0m")
git diff-index --quiet HEAD -- || (git --no-pager diff; panic "\e[35mplease 'make format' before creating a pr!!!\e[0m")
fi

trap 'kill $(jobs -p) &> /dev/null || true' EXIT

export LOG_FILE=tests.log
if [[ "$TRAVIS" = "true" ]]; then
export RUST_TEST_THREADS=2
fi
export RUSTFLAGS=-Dwarnings

make clippy || panic "\e[35mplease fix the errors!!!\e[0m"
make clippy || panic "\e[35mplease fix the 'make clippy' errors!!!\e[0m"

if [[ "$SKIP_TESTS" != "true" ]]; then
set +e
export LOG_FILE=tests.log
if [[ -z "$SKIP_TESTS" ]]; then
make test 2>&1 | tee tests.out
else
EXTRA_CARGO_ARGS="--no-run" make test
exit $?
fi
status=$?
git diff-index --quiet HEAD -- || echo "\e[35mplease run tests before creating a pr!!!\e[0m"
if [[ -z "$SKIP_CHECK_DIRTY_TESTS" ]]; then
git diff-index --quiet HEAD -- || echo "\e[35mplease run 'make test' before creating a pr!!!\e[0m"
fi
for case in `cat tests.out | python -c "import sys
import re
p = re.compile(\"thread '([^']+)' panicked at\")
Expand All @@ -55,4 +61,4 @@ if [[ "$TRAVIS" != "true" ]]; then
rm tests.out || true
fi

exit $status
exit $status
43 changes: 43 additions & 0 deletions scripts/clippy-all
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
#!/usr/bin/env bash
# This script runs clippy with the most common configurations.
# Arguments given will be passed through to "cargo clippy"
# This runs in the Makefile environment via "make run"

set -euo pipefail

# Run from the Makefile environment
MAKEFILE_RUN=${MAKEFILE_RUN:-""}
if [[ -z $MAKEFILE_RUN ]] ; then
COMMAND="$0 $*" exec make run
fi

ALLOWED_CLIPPY_LINTS=(-A clippy::module_inception -A clippy::needless_pass_by_value -A clippy::cognitive_complexity \
-A clippy::unreadable_literal -A clippy::should_implement_trait -A clippy::verbose_bit_mask \
-A clippy::implicit_hasher -A clippy::large_enum_variant -A clippy::new_without_default \
-A clippy::neg_cmp_op_on_partial_ord -A clippy::too_many_arguments \
-A clippy::excessive_precision -A clippy::collapsible_if -A clippy::blacklisted_name \
-A clippy::needless_range_loop -A clippy::redundant_closure \
-A clippy::match_wild_err_arm -A clippy::blacklisted_name -A clippy::redundant_closure_call \
-A clippy::useless_conversion -A clippy::new_ret_no_self -A clippy::unnecessary_sort_by)

if [[ "${TIKV_ENABLE_FEATURES}" == *prost-codec* ]] ; then
cargo clippy --workspace --all-targets --no-default-features \
--exclude cdc --exclude backup --exclude tests --exclude cmd \
--exclude fuzz-targets --exclude fuzzer-honggfuzz --exclude fuzzer-afl --exclude fuzzer-libfuzzer \
--features "${TIKV_ENABLE_FEATURES}" -- "${ALLOWED_CLIPPY_LINTS[@]}"
for pkg in "components/cdc" "components/backup" "cmd" "tests"; do
cd $pkg
cargo clippy --all-targets --no-default-features \
--features "${TIKV_ENABLE_FEATURES}" -- "${ALLOWED_CLIPPY_LINTS[@]}"
cd - >/dev/null
done
for pkg in "fuzz"; do
cd $pkg
cargo clippy --all-targets -- "${ALLOWED_CLIPPY_LINTS[@]}"
cd - >/dev/null
done
else
cargo clippy --workspace \
--exclude fuzzer-honggfuzz --exclude fuzzer-afl --exclude fuzzer-libfuzzer \
--features "${TIKV_ENABLE_FEATURES}" -- "${ALLOWED_CLIPPY_LINTS[@]}"
fi
5 changes: 5 additions & 0 deletions scripts/env
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/usr/bin/env bash
# This script runs the arguments given in the environment setup by the Makefile

set -euo pipefail
COMMAND="$*" exec make run
32 changes: 32 additions & 0 deletions scripts/test
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#!/usr/bin/env bash
# This script runs cargo test with the most common testing configurations.
# Arguments given will be passed through to "cargo test"
# This runs in the Makefile environment via "make run"

set -euo pipefail

# Run from the Makefile environment
MAKEFILE_RUN=${MAKEFILE_RUN:-""}
if [[ -z $MAKEFILE_RUN ]] ; then
COMMAND="$0 $*" exec make run
fi
SHELL_DEBUG=${SHELL_DEBUG:-""}
if [[ -n "$SHELL_DEBUG" ]] ; then
set -x
fi

DYLD_LIBRARY_PATH=${DYLD_LIBRARY_PATH:-""}
LOCAL_DIR=${LOCAL_DIR:-""}
TIKV_ENABLE_FEATURES=${TIKV_ENABLE_FEATURES:-""}
# EXTRA_CARGO_ARGS is unecessary now: this can just be given as arguments to ./scripts/test-all or ./scripts/test
EXTRA_CARGO_ARGS=${EXTRA_CARGO_ARGS:-""}

# When SIP is enabled, DYLD_LIBRARY_PATH will not work in subshell, so we have to set it
# again here. LOCAL_DIR is defined in .travis.yml.
export DYLD_LIBRARY_PATH="${DYLD_LIBRARY_PATH}:${LOCAL_DIR}/lib"
export LOG_LEVEL=DEBUG
export RUST_BACKTRACE=1

cargo -Zpackage-features test --workspace \
--exclude fuzzer-honggfuzz --exclude fuzzer-afl --exclude fuzzer-libfuzzer \
--features "${TIKV_ENABLE_FEATURES}" ${EXTRA_CARGO_ARGS} "$@"
29 changes: 29 additions & 0 deletions scripts/test-all
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#!/usr/bin/env bash
# This script runs all the tests under a variety of conditions.
# This should pass before submitting pull requests.
# Arguments given will be passed through to "cargo test"
# This runs in the Makefile environment via "make run"

set -euo pipefail

# Run from the Makefile environment
MAKEFILE_RUN=${MAKEFILE_RUN:-""}
if [[ -z $MAKEFILE_RUN ]] ; then
COMMAND="$0 $*" exec make run
fi

./scripts/test "$@" -- --nocapture
# The special Linux case below is testing the mem-profiling
# features in tikv_alloc, which are marked #[ignore] since
# they require special compile-time and run-time setup
# Fortunately rebuilding with the mem-profiling feature will only
# rebuild starting at jemalloc-sys.
if [[ "$(uname)" == "Linux" ]]; then
export MALLOC_CONF=prof:true,prof_active:false
./scripts/test -p tikv -p tikv_alloc --lib "$@" -- --nocapture --ignored
fi

if [[ "$(uname)" = "Linux" ]]; then
EXTRA_CARGO_ARGS="" ./scripts/test --message-format=json-render-diagnostics -q --no-run -- --nocapture |
python scripts/check-bins.py --features "${TIKV_ENABLE_FEATURES}" --check-tests
fi

0 comments on commit 4dc97df

Please sign in to comment.