Skip to content

Make the git hooks fast: no analysis on commit, only what a push affects #38

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.

Sign up for GitHub

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

Merged
burhankhanzada merged 1 commit into from
Oct 4, 2026
Merged

Make the git hooks fast: no analysis on commit, only what a push affects #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
70 changes: 21 additions & 49 deletions .githooks/pre-commit
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,21 @@ if [ -n "$staged_scratch_files" ]; then
done
fi

# 1. Update templates and sort pubspec if it was staged or templates changed
echo "📦 Syncing templates to pubspec.yaml..."
dart run tools/update_templates.dart
staged_templates=$(git diff --cached --name-only | grep '^assets/templates/')

# 1. Sync the template folders into pubspec.yaml's assets, when they changed.
#
# Only then: it is a list of folders, and nothing else can change it. It used
# to run on every commit, and `dart run` builds the dependency graph's native
# hooks first whenever they are stale — after any branch switch or dependency
# change — which made it ~45 s of a commit that touched no template.
if [ -n "$staged_templates" ]; then
echo "📦 Syncing templates to pubspec.yaml..."
dart run tools/update_templates.dart
fi

# 2. Sort pubspec if it was staged
if [ -n "$staged_pubspec" ] || git diff --cached --name-only | grep -q 'assets/templates/'; then
if [ -n "$staged_pubspec" ] || [ -n "$staged_templates" ]; then
echo "📦 Sorting pubspec.yaml dependencies..."
dart run sort_pubspec_dependencies
git add pubspec.yaml
Expand Down Expand Up @@ -55,52 +64,15 @@ if [ -n "$staged_dart_files" ]; then
done
fi

# 4. Analyze, when the commit could actually affect analysis
#
# This was commented out, which meant a broken commit was accepted and the
# breakage surfaced at the next `git push` — several commits later, blamed on
# whoever happened to run into it. Both of the last two push failures were
# that.
#
# `dart analyze`, not `flutter analyze`. The note that used to sit here had it
# backwards: it said `flutter analyze` was used *because* the Flutter tool sets
# up the analyzer plugins this repo's analysis_options registers. It does not.
# `flutter analyze` never loads them, which is why 34 riverpod_lint findings
# accumulated while every hook and CI run reported a clean tree — they were
# only ever visible in the IDE. `dart analyze` does load them.
# No analysis here. `dart analyze --fatal-infos` takes ~40-55 s on this
# project even for a single file — most of it loading the analyzer plugins —
# which is a commit you learn to skip with --no-verify. The IDE shows the same
# findings as you type, and pre-push analyzes the whole tree before anything
# reaches the remote, so a broken commit still cannot leave your machine.
#
# `--fatal-infos` because the two disagree on what counts as failure:
# `flutter analyze` exits non-zero on an info, plain `dart analyze` exits 0.
# Without the flag this swap would have *loosened* the gate — and most of what
# riverpod_lint reports (`avoid_public_notifier_properties`, and every lint in
# `all_lints` that is info-level) would have stopped blocking anything.
#
# Costs about 11s more than `flutter analyze` did, which is the plugin loading.
#
# Skipped for commits that cannot change the result (docs, assets, workflows)
# so that writing a README stays instant. pre-push analyzes unconditionally,
# so nothing reaches the remote unchecked either way.
staged_analysis_config=$(git diff --cached --name-only --diff-filter=ACM | grep -E '(^|/)analysis_options\.yaml$')

if [ -n "$staged_dart_files" ] || [ -n "$staged_pubspec" ] || [ -n "$staged_analysis_config" ]; then
echo "🔍 Analyzing..."
if ! dart analyze --fatal-infos; then
# Generated *.g.dart files are gitignored, so a fresh clone or a branch
# switch can leave the tree needing a regenerate. Say so plainly rather
# than leaving a wall of undefined-symbol errors that all have the same
# one-line cause.
echo ""
echo "❌ Analysis failed. If these are missing generated symbols, run:"
echo " dart run build_runner build --delete-conflicting-outputs"
echo "Commit blocked; use --no-verify to override."
exit 1
fi
fi

# Tests deliberately stay in pre-push. They take ~30s against ~430 tests,
# which is tolerable once per push but not once per commit — and a commit is
# not the thing that reaches anyone else. Run them yourself before
# committing; these hooks are the backstop, not the gate.
# `dart analyze`, never `flutter analyze`, wherever it runs: `flutter analyze`
# does not load the analyzer plugins analysis_options.yaml registers, which is
# how 34 riverpod_lint findings once accumulated behind a clean hook.

echo "✅ All pre-commit checks passed!"
exit 0
160 changes: 102 additions & 58 deletions .githooks/pre-push
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,27 @@
#!/bin/sh

# Runs the gates CI runs, before the push rather than after it.
# Runs the gates CI runs, before the push rather than after it — but only the
# ones the pushed commits can affect.
#
# This is deliberately NOT in pre-commit. Analysis and the suite take about a
# minute together, which is fine once per push and not fine on every commit —
# a hook slow enough to be worth skipping gets skipped, and then it protects
# nothing. Committing stays instant; pushing is where the cost belongs,
# because that is the point after which a red result costs someone else time.
# This is deliberately NOT in pre-commit: analysis alone is ~40-55 s here, and a
# hook slow enough to be worth skipping gets skipped, and then it protects
# nothing. Pushing is where the cost belongs, because that is the point after
# which a red result costs someone else time.
#
# Mirrors the analyze-and-test job in .github/workflows/ci.yml. Formatting and
# import sorting are pre-commit's job and are not repeated here.
# It mirrors CI's analyze, test and package jobs, which it used to run in full
# on every push — ~2.5 min warm on an M-series Mac, docs included. Now:
#
# - a push that touches nothing they check (docs, workflows) runs none;
# - the packages are checked only when something under packages/ changed:
# they cannot import the app, so app code cannot break them. An app-only
# push went from ~155 s to ~95 s.
#
# One thing at a time, deliberately. Measured on an 8-core Mac: analysis
# alongside the tests took 135 s against 96 s in turn, and the eleven packages
# all at once 107 s against 86 s in turn (three at a time saved 9 s). Each
# already spreads over every core, so running them together only fights.
#
# Formatting and import sorting are pre-commit's job and are not repeated here.
#
# `dart analyze --fatal-infos`, not `flutter analyze`: the latter does not load
# the analyzer plugins analysis_options.yaml registers, so it reports no
Expand All @@ -29,19 +41,52 @@
# below runs in the working tree it was invoked from.
unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE

# Deleting a remote branch pushes no code, so there is nothing to check.
# stdin carries "<local ref> <local sha> <remote ref> <remote sha>" per ref;
# an all-zero local sha is a delete.
zero=0000000000000000000000000000000000000000

# What the push changes, across every ref in it. stdin carries
# "<local ref> <local sha> <remote ref> <remote sha>" per ref. A deleted ref
# (all-zero local sha) pushes no code. A new branch is compared with where it
# left main. If no base can be found, everything is checked: a wasted minute
# is cheaper than an unchecked push.
pushing_code=0
while read -r _ local_sha _ _; do
check_everything=0
changed=""
while read -r _ local_sha _ remote_sha; do
case "$local_sha" in
0000000000000000000000000000000000000000) ;;
'') ;;
*) pushing_code=1 ;;
"$zero" | '') continue ;;
esac
pushing_code=1
if [ "$remote_sha" = "$zero" ]; then
base=$(git merge-base "$local_sha" origin/main 2>/dev/null || true)
else
base=$remote_sha
fi
if [ -n "$base" ] && git cat-file -e "$base^{commit}" 2>/dev/null; then
changed="$changed
$(git diff --name-only "$base" "$local_sha")"
else
check_everything=1
fi
done
[ "$pushing_code" -eq 1 ] || exit 0

touches() {
[ "$check_everything" -eq 1 ] || printf '%s\n' "$changed" | grep -qE "$1"
}

# What can change the app's analysis or tests: its code, its tests, the
# packages and assets it builds from, its dependencies and its lint setup.
app_paths='^(lib/|test/|packages/|assets/|tools/|pubspec\.(yaml|lock)$|analysis_options\.yaml$|build\.yaml$|dart_test\.yaml$)'
check_app=0
check_packages=0
touches "$app_paths" && check_app=1
touches '^packages/' && check_packages=1

if [ "$check_app" -eq 0 ] && [ "$check_packages" -eq 0 ]; then
echo "✅ Nothing in this push changes what the checks cover — skipping them."
exit 0
fi

# A fresh clone or worktree has no dependencies or generated code yet, which
# would fail every check below for reasons that have nothing to do with the
# push. Set up whatever is missing first; on a ready checkout this is quick.
Expand All @@ -50,56 +95,55 @@ if ! tools/bootstrap.sh; then
exit 1
fi

echo "🔍 Analyzing..."
if ! dart analyze --fatal-infos; then
# Generated *.g.dart files are gitignored, so a fresh clone or a branch
# switch can leave the tree needing a regenerate. Say so plainly rather
# than leaving a wall of undefined-symbol errors that all have the same
# one-line cause.
echo ""
echo "❌ Analysis failed. If these are missing generated symbols, run:"
echo " dart run build_runner build --delete-conflicting-outputs"
exit 1
fi
if [ "$check_app" -eq 1 ]; then
echo "🔍 Analyzing..."
if ! dart analyze --fatal-infos; then
# Generated *.g.dart files are gitignored, so a fresh clone or a branch
# switch can leave the tree needing a regenerate. Say so plainly rather
# than leaving a wall of undefined-symbol errors that all have the same
# one-line cause.
echo ""
echo "❌ Analysis failed. If these are missing generated symbols, run:"
echo " dart run build_runner build --delete-conflicting-outputs"
exit 1
fi

# The 'arduino' tag marks tests that shell out to a local arduino-cli
# toolchain. CI excludes them and so does this — run them yourself if you
# touched the compile path.
echo "🧪 Running tests..."
if ! flutter test --exclude-tags arduino; then
echo ""
echo "❌ Tests failed. Push blocked; use --no-verify to override."
exit 1
# The 'arduino' tag marks tests that shell out to a local arduino-cli
# toolchain. CI excludes them and so does this — run them yourself if you
# touched the compile path.
echo "🧪 Running tests..."
if ! flutter test --exclude-tags arduino; then
echo ""
echo "❌ Tests failed. Push blocked; use --no-verify to override."
exit 1
fi
fi

# Packages are separate pub roots: the root analyze excludes them and the root
# test run does not descend into them, so without this they reach CI unchecked.
for pkg in packages/*/; do
[ -f "$pkg/pubspec.yaml" ] || continue
echo "📦 Checking $pkg"
# No package registers analyzer plugins — only the root does — so here the
# difference from `flutter analyze` is purely `--fatal-infos`. Kept
# identical to the root command anyway, so there is one analyze incantation
# in this repo rather than two that drift.
if ! (cd "$pkg" && dart analyze --fatal-infos); then
echo ""
echo "❌ $pkg failed analysis. Push blocked; use --no-verify to override."
exit 1
fi
# Not every package is a library with a suite — composite_plugin is a
# generated analyzer plugin. `flutter test` exits 1 on "Test directory not
# found", which is not a test failure, so check before running rather than
# treating a testless package as broken. Said out loud so a package that
# *should* have tests and lost them is visible rather than silently passing.
if [ -d "$pkg/test" ]; then
if ! (cd "$pkg" && flutter test); then
if [ "$check_packages" -eq 1 ]; then
for pkg in packages/*/; do
[ -f "$pkg/pubspec.yaml" ] || continue
echo "📦 Checking $pkg"
if ! (cd "$pkg" && dart analyze --fatal-infos); then
echo ""
echo "❌ $pkg failed its tests. Push blocked; use --no-verify to override."
echo "❌ $pkg failed analysis. Push blocked; use --no-verify to override."
exit 1
fi
else
echo " (no test/ directory — analysis only)"
fi
done
# Not every package has a suite — composite_plugin is a generated
# analyzer plugin. `flutter test` exits 1 on "Test directory not
# found", which is not a test failure, so check before running. Said
# out loud so a package that *should* have tests and lost them shows.
if [ -d "$pkg/test" ]; then
if ! (cd "$pkg" && flutter test); then
echo ""
echo "❌ $pkg failed its tests. Push blocked; use --no-verify to override."
exit 1
fi
else
echo " (no test/ directory — analysis only)"
fi
done
fi

echo "✅ Ready to push."
11 changes: 7 additions & 4 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,10 @@ you touch the compile path.

CI runs these on every pull request; running them first saves a round trip.
`./setup_hooks.sh`, once after cloning, has git do it for you: the pre-commit
hook formats, sorts imports and analyzes what you staged, and the pre-push hook
runs the analysis and the tests below.
hook formats and sorts imports in what you staged (analysis is left to your IDE,
since it takes the better part of a minute here), and the pre-push hook runs the
analysis and the tests below — those the push can affect, so a docs change runs
none of them.

```bash
dart analyze --fatal-infos # must be clean, infos included — as CI runs it
Expand All @@ -72,8 +74,9 @@ for p in packages/*/; do [ -d "$p/test" ] && (cd "$p" && flutter test); done
Packages under `packages/` carry their own `pubspec.yaml` and
`analysis_options.yaml` and are checked on their own terms — the root
`flutter analyze` excludes them and the root `flutter test` does not descend
into them. CI loops over every package, and so does the pre-push hook: all of
them are analyzed, and the ones with a `test/` directory are tested.
into them. CI checks every package, and the pre-push hook does too whenever a
push changes something under `packages/`: all of them are analyzed, and the ones
with a `test/` directory are tested.
(`composite_plugin` is a generated analyzer plugin and has none.)

`flutter analyze` exits non-zero on **warnings and infos**, not just errors, so
Expand Down
11 changes: 7 additions & 4 deletions setup_hooks.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,13 @@
# ./setup_hooks.sh
#
# What you get:
# pre-commit — formats staged Dart, sorts imports, syncs templates into
# pubspec.yaml, sorts dependencies. Fast; runs on every commit.
# pre-push — flutter analyze + flutter test, the same gates CI runs.
# About a minute, which is why it is not on commit.
# pre-commit — formats staged Dart and sorts its imports; syncs templates
# into pubspec.yaml and sorts dependencies when those changed.
# A few seconds; runs on every commit. No analysis — that is
# the IDE's and pre-push's.
# pre-push — `dart analyze` and the tests, the same gates CI runs, for
# what the push changes: none for docs, the packages only when
# packages/ changed.
#
# Both live in .githooks/, which is committed — unlike .git/hooks/, which is
# why `git config core.hooksPath` exists at all.
Expand Down
Loading