Skip to content

Commit

Permalink
Makefile: alias format to cargo fmt, add format-check command
Browse files Browse the repository at this point in the history
This removes the previous `make format` command which, in essence,
runs some bash magic to achieve the exact same job that `cargo fmt`
does, if its run within workspace instead of a particular
crate. Besides this, many people's editors have a native rustfmt
integration anyways, which makes this script and its prompt for
unstaged files largely redundant.

Instead, we replace it with the more useful `make format-check` and
the associated `tools/check_format.sh`. This script does more than
`cargo fmt --check` does, for instance also ensuring that no Rust
source file contains tab characters (which rustfmt sometimes leaves
behind).

As part of `make prepush`, `check-format` is now more in line with the
other jobs, such as `cargo check` or Clippy: it'll complain, but not
touch any source code.

To retain compatibility, we introduce a `make format` and `make fmt`,
aliased to `cargo fmt` -- without prompting the user.

This also removes an optimization that uses Make's logic to check for
changed files in order to skip formatting. However, I doubt that this
will meaningfully impact users (`make format-check` completes in about
2sec and does not take a meaningful amount of time compared to the
other jobs executed as part of `prepush`).
  • Loading branch information
lschuermann committed Jul 5, 2024
1 parent 5a65d68 commit 12ae6a1
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 116 deletions.
27 changes: 15 additions & 12 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,12 @@ usage:
@echo " doc: Builds Tock documentation for all boards"
@echo " stack: Prints a basic stack frame analysis for all boards"
@echo " clean: Clean all builds"
@echo " format: Runs the rustfmt tool on all kernel sources"
@echo " format-check: Checks for formatting errors in kernel sources"
@echo " list: Lists available boards"
@echo
@echo "We also define the following aliases:"
@echo " format: cargo fmt"
@echo
@echo "The make system also drives all continuous integration and testing:"
@echo " $$(tput bold)prepush$$(tput sgr0): Fast checks to run before pushing changes upstream"
@echo " ci-all: Run all continuous integration tests (possibly slow!)"
Expand Down Expand Up @@ -175,14 +178,15 @@ clean:
@echo "$$(tput bold)Clean ci-artifacts" && rm -rf tools/ci-artifacts

.PHONY: fmt format
fmt format: tools/.format_fresh
$(call banner,Formatting complete)
fmt format:
$(call banner,Running \"cargo fmt\" -- for a complete format check run \"make format-check\")
cargo fmt

# Get a list of all rust source files (everything fmt operates on)
$(eval RUST_FILES_IN_TREE := $(shell (git ls-files | grep '\.rs$$') || find . -type f -name '*.rs'))
tools/.format_fresh: $(RUST_FILES_IN_TREE)
@./tools/run_cargo_fmt.sh $(TOCK_FORMAT_MODE)
@touch tools/.format_fresh
.PHONY: format-check
format-check:
$(call banner,Formatting checker)
@./tools/check_format.sh
$(call banner,Check for formatting complete)

.PHONY: list
list:
Expand Down Expand Up @@ -215,7 +219,7 @@ ci-nosetup:
# This is designed for developers, to be run often and before submitting code upstream.
.PHONY: prepush
prepush:\
format\
format-check\
ci-job-clippy\
ci-job-syntax\
licensecheck
Expand Down Expand Up @@ -350,9 +354,8 @@ ci-runner-netlify:\

### ci-runner-github-format jobs:
.PHONY: ci-job-format
ci-job-format: licensecheck
$(call banner,CI-Job: Format Check)
@NOWARNINGS=true TOCK_FORMAT_MODE=diff $(MAKE) format
ci-job-format: licensecheck format-check
$(call banner,CI-Job: Format Check DONE)

define ci_setup_markdown_toc
$(call banner,CI-Setup: Install markdown-toc)
Expand Down
53 changes: 53 additions & 0 deletions tools/check_format.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
#!/usr/bin/env bash

# Licensed under the Apache License, Version 2.0 or the MIT License.
# SPDX-License-Identifier: Apache-2.0 OR MIT
# Copyright Tock Contributors 2023.
#
# Author: Leon Schuermann <[email protected]>
# Author: Pat Pannuto <[email protected]>
# Author: Brad Campbell <[email protected]>
#
set -e

# Verify that we're running in the base directory
if [ ! -x tools/check_format.sh ]; then
echo ERROR: $0 must be run from the tock repository root.
echo ""
exit 1
fi

set +e
let FAIL=0
set -e

# Run `cargo fmt --check` in the workspace root, which will check all
# crates in the workspace.
echo "Running \`cargo fmt --check\`..."
CARGO_FMT_CHECK_EXIT_CODE=0
cargo fmt --check || CARGO_FMT_CHECK_EXIT_CODE=$?
if [[ $CARGO_FMT_CHECK_EXIT_CODE -ne 0 ]]; then
let FAIL=FAIL+1
else
echo "\`cargo fmt --check\` suceeded."
fi
printf "\n"

# Check for tab characters in Rust source files that haven't been
# removed by rustfmt
echo "Checking for Rust source files with tab characters..."
RUST_FILES_WITH_TABS="$(git grep --files-with-matches $'\t' -- '*.rs' || grep -lr --include '*.rs' $'\t' . || true)"
if [ "$RUST_FILES_WITH_TABS" != "" ]; then
echo "ERROR: The following files contain tab characters, please use spaces instead:"
echo "$RUST_FILES_WITH_TABS" | sed 's/^/ -> /'
let FAIL=FAIL+1
else
echo "No Rust source file containing tab characters found."
fi

if [[ $FAIL -ne 0 ]]; then
printf "\n"
echo "$(tput bold)Formatting errors.$(tput sgr0)"
echo "See above for details. You can try running \`cargo fmt\` to correct them."
fi
exit $FAIL
104 changes: 0 additions & 104 deletions tools/run_cargo_fmt.sh

This file was deleted.

0 comments on commit 12ae6a1

Please sign in to comment.