Skip to content

Commit

Permalink
chore: pre-commit hook to warn of constant.nr change (#11507)
Browse files Browse the repository at this point in the history
Edit: A git hook might not be the best approach to this, so maybe
there's a better CI solution that can be built?

I just lost 6 hours of my life hunting a bug, because a few days ago I'd
changed a single number in constants.nr. I'd forgotten to update the
downstream constants files, or perhaps I'd wrongly assumed that
bootstrap.sh or CI or something would update the associated constants
`.ts` and `.sol` etc. files.

Anyway, I have channelled my furious anger into trying to prevent
someone else wasting countless hours over something so easily avoidable
and automatable.

Out of interest, why isn't this automatically caught already?

## About the PR:

- It identifies whether the `constants.nr` file has been changed _and_
staged. (No point doing anything if it's not been staged).
- It just warns the dev about this, to make them aware that they might
need to run some scripts.
- ~It runs the remake-constants script.~
- *It does not re-stage the resulting, re-generated constant files*,
because I noticed (by chance I had a `build:dev` watch window open) that
there are other typescript files that need to be generated, which aren't
covered by the `remake-constants` script. Ideally, if someone knows
where the scripts are to regenerate all of those lingering constants
files as well, we would run those scripts too within this script. But of
course, the auto-generated files themselves don't necessarily tell you
how to generate them (sigh). E.g.
/mnt/user-data/mike/aztec-packages/yarn-project/noir-protocol-circuits-types/src/types/index.ts

---------

Co-authored-by: ludamad <[email protected]>
  • Loading branch information
iAmMichaelConnor and ludamad authored Jan 26, 2025
1 parent d1e33a3 commit ad604ea
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 0 deletions.
1 change: 1 addition & 0 deletions bootstrap.sh
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ hooks_dir=$(git rev-parse --git-path hooks)
echo "(cd barretenberg/cpp && ./format.sh staged)" >$hooks_dir/pre-commit
echo "./yarn-project/precommit.sh" >>$hooks_dir/pre-commit
echo "./noir-projects/precommit.sh" >>$hooks_dir/pre-commit
echo "./yarn-project/circuits.js/precommit.sh" >>$hooks_dir/pre-commit
chmod +x $hooks_dir/pre-commit

github_group "pull submodules"
Expand Down
32 changes: 32 additions & 0 deletions yarn-project/circuits.js/precommit.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#!/bin/bash

# Precommit hook simply to warn the user that they've staged a change to constants.nr.
# Longer-term, there might be a more-robust solution, but the goal of this script is
# simply to warn devs, so that they don't forget to regenerate the constants.
# Lots of hours lost to people forgetting to regenerate all the downstream constants.
# This script DOES NOT regenerate the constants, because there's too much to build.
# In the case where they've already generated and staged all the constant files of
# this commit, they'll have to cope with this small bit of noise.

#!/bin/bash
set -euo pipefail # Fail on errors, unset variables, and pipeline failures

cd "$(dirname "$0")" # Change to the script's directory

export FORCE_COLOR=true

FILE_TO_WATCH="noir-projects/noir-protocol-circuits/crates/types/src/constants.nr"

# Check if constants.nr is staged for commit
if git diff --cached --name-only | grep -Fxq "$FILE_TO_WATCH"; then
echo "It looks like you changed $FILE_TO_WATCH."
echo ""
echo -e "\033[33mPlease remember to regenerate the other constants files. If you've already regenerated the constants, please ignore this message.\033[0m"
echo ""
echo "Depending on the constants you've changed, these might include: constants.gen.ts, ConstantsGen.sol, constants_gen.pil, aztec_constants.hpp."
echo ""
echo -e "You can regenerate these by running: '\033[33myarn remake-constants\033[0m' from the 'yarn-project/circuits.js' dir. If you have changed tree sizes, also run ./yarn-project/update-snapshots.sh."
echo ""
echo "We don't automatically regenerate them for you in this git hook, because you'll likely need to also re-build components of the repo. End."
echo ""
fi

0 comments on commit ad604ea

Please sign in to comment.