From e8be87c1c7d680506466d693734d4af2bd640cfb Mon Sep 17 00:00:00 2001 From: iAmMichaelConnor Date: Sat, 25 Jan 2025 21:50:50 +0000 Subject: [PATCH 1/3] pre-commit --- bootstrap.sh | 1 + yarn-project/circuits.js/precommit.sh | 35 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100755 yarn-project/circuits.js/precommit.sh diff --git a/bootstrap.sh b/bootstrap.sh index 6c20b48f5de..2a848a05f25 100755 --- a/bootstrap.sh +++ b/bootstrap.sh @@ -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" diff --git a/yarn-project/circuits.js/precommit.sh b/yarn-project/circuits.js/precommit.sh new file mode 100755 index 00000000000..30d635ea5fe --- /dev/null +++ b/yarn-project/circuits.js/precommit.sh @@ -0,0 +1,35 @@ +#!/bin/bash + +# Precommit hook to generate the constants if constants.nr is changed. This will save developers hours of time hunting a bug, when in fact all they needed to do was run this script (trust me, I have trodden this dark path before...). + +#!/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" # Relative path + +# 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 "Regenerating the other constants files, so you don't lose a day of your life wondering why things aren't working..." + echo "" + + COMMAND="yarn remake-constants" + + echo "Running `$COMMAND`..." + echo "" + $COMMAND # Run the command + + # Stage all the constants files, if they've been changed by the script: + # We move to the top-level of the repo first, so that we don't have to specify full relative paths, in case someone does some refiling that breaks those paths. + # cd ../../ + # git add -u -- *constants.gen.ts *ConstantsGen.sol *constants_gen.pil *aztec_constants.hpp + + echo "Constants files re-generated." + echo "" + echo "We haven't actually re-staged those re-generated files for you, because there might actually be additional files that you'll need to manually update, e.g. yarn-project/noir-protocol-circuits-types/src/types/index.ts. Sorry about that. But at least it's caught your attention as something that needs to be fixed!" +fi \ No newline at end of file From be8502fac2fdad1c6887399ea6df885d6a3a07b1 Mon Sep 17 00:00:00 2001 From: iAmMichaelConnor Date: Sun, 26 Jan 2025 16:45:47 +0000 Subject: [PATCH 2/3] simplify to just a warning message that doesn't execute any scripts --- yarn-project/circuits.js/precommit.sh | 29 ++++++++++++--------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/yarn-project/circuits.js/precommit.sh b/yarn-project/circuits.js/precommit.sh index 30d635ea5fe..3d76871539f 100755 --- a/yarn-project/circuits.js/precommit.sh +++ b/yarn-project/circuits.js/precommit.sh @@ -1,6 +1,12 @@ #!/bin/bash -# Precommit hook to generate the constants if constants.nr is changed. This will save developers hours of time hunting a bug, when in fact all they needed to do was run this script (trust me, I have trodden this dark path before...). +# 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 @@ -9,27 +15,18 @@ 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" # Relative path +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 "Regenerating the other constants files, so you don't lose a day of your life wondering why things aren't working..." + 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 "" - - COMMAND="yarn remake-constants" - - echo "Running `$COMMAND`..." + echo "Depending on the constants you've changed, these might include: constants.gen.ts, ConstantsGen.sol, constants_gen.pil, aztec_constants.hpp." echo "" - $COMMAND # Run the command - - # Stage all the constants files, if they've been changed by the script: - # We move to the top-level of the repo first, so that we don't have to specify full relative paths, in case someone does some refiling that breaks those paths. - # cd ../../ - # git add -u -- *constants.gen.ts *ConstantsGen.sol *constants_gen.pil *aztec_constants.hpp - - echo "Constants files re-generated." + echo -e "You can regenerate these by running: '\033[33myarn remake-constants\033[0m' from the 'yarn-project/circuits.js' dir." + 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 "" - echo "We haven't actually re-staged those re-generated files for you, because there might actually be additional files that you'll need to manually update, e.g. yarn-project/noir-protocol-circuits-types/src/types/index.ts. Sorry about that. But at least it's caught your attention as something that needs to be fixed!" fi \ No newline at end of file From cd67dfdc918da62f675915926f16ea322285562b Mon Sep 17 00:00:00 2001 From: ludamad Date: Sun, 26 Jan 2025 21:26:35 +0000 Subject: [PATCH 3/3] Update precommit.sh --- yarn-project/circuits.js/precommit.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/yarn-project/circuits.js/precommit.sh b/yarn-project/circuits.js/precommit.sh index 3d76871539f..78a56c9ea4d 100755 --- a/yarn-project/circuits.js/precommit.sh +++ b/yarn-project/circuits.js/precommit.sh @@ -25,8 +25,8 @@ if git diff --cached --name-only | grep -Fxq "$FILE_TO_WATCH"; then 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." + 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 \ No newline at end of file +fi