From 36c7d2ee9a21a582d51b439e96c840b5c219a495 Mon Sep 17 00:00:00 2001 From: Tin Lai Date: Sun, 26 Oct 2025 12:10:19 +1100 Subject: [PATCH] default to --no-verify with .subrepo option to config it --- ReadMe.pod | 24 ++++++++++++- lib/git-subrepo | 58 ++++++++++++++++++++++-------- test/init-no-verify.t | 84 +++++++++++++++++++++++++++++++++++++++++++ test/no-verify.t | 77 +++++++++++++++++++++++++++++++++++++++ test/verify-config.t | 72 +++++++++++++++++++++++++++++++++++++ 5 files changed, 300 insertions(+), 15 deletions(-) create mode 100644 test/init-no-verify.t create mode 100644 test/no-verify.t create mode 100644 test/verify-config.t diff --git a/ReadMe.pod b/ReadMe.pod index 6f224bb0..2a15a1c9 100644 --- a/ReadMe.pod +++ b/ReadMe.pod @@ -329,12 +329,21 @@ Read or update configuration values in the subdir/.gitrepo file. Because most of the values stored in the .gitrepo file are generated you will need to use C<--force> if you want to change anything else then the -C option. +C and C options. Example to update the C option for a subrepo: git subrepo config foo method rebase +Example to enable git hooks for a subrepo: + + git subrepo config foo verify true + +The C option controls whether git hooks (pre-commit and commit-msg) +are run during subrepo operations. By default, hooks are bypassed to prevent +parent repository hooks from interfering with subrepo operations. Set +C in the .gitrepo file to always run hooks for a specific subrepo. + =item C<< git subrepo help [|--all] >> Same as C. Will launch the manpage. For the shorter usage, @@ -447,6 +456,19 @@ Squash all commits on a push into one new commit. If C<--branch> or C<--remote> are used, and the command updates the C<.gitrepo> file, include these values to the update. +=item C<--verify> (C<-V>) + +Run pre-commit and commit-msg hooks during subrepo operations. + +By default, git-subrepo bypasses git hooks (using C<--no-verify> internally) +to prevent hooks from the parent repository interfering with subrepo operations. +This is particularly useful when cloning subrepos that may contain code that +doesn't pass the parent repository's linting or validation rules. + +Use the C<--verify> flag to enable hooks for a specific operation, or set +C in the C<.gitrepo> file to always run hooks for a particular +subrepo. + =back =head1 Output Options diff --git a/lib/git-subrepo b/lib/git-subrepo index 954380b8..f35a1e6d 100755 --- a/lib/git-subrepo +++ b/lib/git-subrepo @@ -77,6 +77,7 @@ file= Specify a commit message file r,remote= Specify the upstream remote to push/pull/fetch s,squash Squash commits on push u,update Add the --branch and/or --remote overrides to .gitrepo +V,verify Run pre-commit and commit-msg hooks (default: skip hooks) q,quiet Show minimal output v,verbose Show verbose output @@ -100,6 +101,7 @@ main() { local fetch_wanted=false # Fetch requested before a command local squash_wanted=false # Squash commits on push local update_wanted=false # Update .gitrepo with --branch and/or --remote + local verify_wanted=false # Run pre-commit and commit-msg hooks (default: skip) local quiet_wanted=false # Output should be quiet local verbose_wanted=false # Output should be verbose @@ -366,7 +368,7 @@ command:config() { # shellcheck disable=2154 o "Update '$subdir' configuration with $config_option=${config_value-}" - if [[ ! $config_option =~ ^(branch|cmdver|commit|method|remote|version)$ ]]; then + if [[ ! $config_option =~ ^(branch|cmdver|commit|method|remote|verify|version)$ ]]; then error "Option $config_option not recognized" fi @@ -377,8 +379,8 @@ command:config() { fi if ! $force_wanted; then - # Only allow changing method without force - if [[ $config_option != method ]]; then + # Only allow changing method and verify without force + if [[ $config_option != method && $config_option != verify ]]; then error "This option is autogenerated, use '--force' to override." fi fi @@ -389,6 +391,12 @@ command:config() { fi fi + if [[ $config_option == verify ]]; then + if [[ ! $config_value =~ ^(true|false)$ ]]; then + error "Not a valid verify value. Valid options are 'true' or 'false'." + fi + fi + RUN git config --file="$gitrepo" "subrepo.$config_option" "$config_value" say "Subrepo '$subdir' option '$config_option' set to '$config_value'." } @@ -531,7 +539,10 @@ subrepo:init() { o "Commit new subrepo to the '$original_head_branch' branch." subrepo_commit_ref=$original_head_commit - RUN git commit -m "$(get-commit-message)" + local no_verify_flag=' --no-verify' + $verify_wanted && no_verify_flag='' + # shellcheck disable=2086 + RUN git commit$no_verify_flag -m "$(get-commit-message)" o "Create ref '$refs_subrepo_commit'." git:make-ref "$refs_subrepo_commit" "$subrepo_commit_ref" @@ -710,10 +721,14 @@ subrepo:push() { commit_message=$(get-commit-message) fi + local no_verify_flag=' --no-verify' + $verify_wanted && no_verify_flag='' if [[ $commit_msg_file ]]; then - RUN git command --file "$commit_msg_file" + # shellcheck disable=2086 + RUN git commit$no_verify_flag --file "$commit_msg_file" else - RUN git commit -m "$commit_message" + # shellcheck disable=2086 + RUN git commit$no_verify_flag -m "$commit_message" fi } @@ -919,12 +934,17 @@ subrepo:commit() { local edit_flag= $edit_wanted && edit_flag=--edit + local no_verify_flag=' --no-verify' + $verify_wanted && no_verify_flag='' + o "Commit to the '$original_head_branch' branch." if [[ $original_head_commit != none ]]; then if [[ $commit_msg_file ]]; then - RUN git commit $edit_flag --file "$commit_msg_file" + # shellcheck disable=2086 + RUN git commit$no_verify_flag $edit_flag --file "$commit_msg_file" else - RUN git commit $edit_flag -m "$commit_message" + # shellcheck disable=2086 + RUN git commit$no_verify_flag $edit_flag -m "$commit_message" fi else # We had cloned into an empty repo, side effect of prior git reset --mixed @@ -1116,6 +1136,7 @@ get-command-options() { -s) squash_wanted=true ;; -u) update_wanted=true commit_msg_args+=("--update") ;; + -V) verify_wanted=true ;; -q) quiet_wanted=true ;; -v) verbose_wanted=true ;; -d) debug_wanted=true ;; @@ -1152,7 +1173,7 @@ get-command-options() { fi commit_msg_args+=("${command_arguments[@]}") - for option in all ALL edit fetch force squash; do + for option in all ALL edit fetch force squash verify; do var=${option}_wanted if ${!var}; then check_option $option @@ -1179,13 +1200,13 @@ get-command-options() { options_help='all' options_branch='all fetch force' options_clean='ALL all force' -options_clone='branch edit force message method' +options_clone='branch edit force message method verify' options_config='force' -options_commit='edit fetch force message' +options_commit='edit fetch force message verify' options_fetch='all branch force remote' -options_init='branch remote method' -options_pull='all branch edit force message remote update' -options_push='all branch force message remote squash update' +options_init='branch remote method verify' +options_pull='all branch edit force message remote update verify' +options_push='all branch force message remote squash update verify' options_status='ALL all fetch' check_option() { local var=options_${command//-/_} @@ -1443,6 +1464,15 @@ read-gitrepo-file() { join_method=merge fi + # Read verify config if not already set by command line + if ! $verify_wanted; then + FAIL=false \ + SAY=false OUT=true RUN git config --file="$gitrepo" subrepo.verify + if [[ $output == true ]]; then + verify_wanted=true + fi + fi + if [[ -z $subrepo_parent ]]; then FAIL=false \ SAY=false OUT=true RUN git config --file="$gitrepo" subrepo.former diff --git a/test/init-no-verify.t b/test/init-no-verify.t new file mode 100644 index 00000000..a66c2fcb --- /dev/null +++ b/test/init-no-verify.t @@ -0,0 +1,84 @@ +#!/usr/bin/env bash + +set -e + +source test/setup + +use Test::More + +clone-foo-and-bar + +# Create a directory to init as subrepo +{ + ( + cd "$OWNER/foo" + mkdir -p testdir + echo "test content" > testdir/testfile + git add testdir/testfile + git commit -m "Add testdir" + ) &> /dev/null || die +} + +# Create a pre-commit hook that will fail +{ + hook_file="$OWNER/foo/.git/hooks/pre-commit" + mkdir -p "$(dirname "$hook_file")" + cat > "$hook_file" <<'EOF' +#!/bin/bash +# This hook will always fail +echo "Pre-commit hook triggered and failing" +exit 1 +EOF + chmod +x "$hook_file" +} + +# Test that init without --verify succeeds (hooks bypassed by default) +{ + init_status=0 + init_output=$( + cd "$OWNER/foo" + git subrepo init testdir 2>&1 + ) || init_status=$? + + is "$init_status" "0" \ + 'subrepo init succeeds by default (hooks bypassed)' + + unlike "$init_output" "Pre-commit hook triggered" \ + 'pre-commit hook was bypassed by default' + + like "$init_output" "Subrepo created from 'testdir'" \ + 'init output is correct' +} + +# Test that the .gitrepo file was created +{ + test-exists "$OWNER/foo/testdir/.gitrepo" +} + +# Clean up for next test +{ + ( + cd "$OWNER/foo" + git reset --hard HEAD~1 + rm -f testdir/.gitrepo + ) &> /dev/null || true +} + +# Test that init with --verify runs the hook and fails +{ + init_status=0 + init_output=$( + cd "$OWNER/foo" + git subrepo init --verify testdir 2>&1 + ) || init_status=$? + + isnt "$init_status" "0" \ + 'subrepo init with --verify fails when pre-commit hook fails' + + like "$init_output" "Pre-commit hook triggered and failing" \ + 'pre-commit hook was triggered with --verify' +} + +done_testing + +teardown diff --git a/test/no-verify.t b/test/no-verify.t new file mode 100644 index 00000000..36872e3a --- /dev/null +++ b/test/no-verify.t @@ -0,0 +1,77 @@ +#!/usr/bin/env bash + +set -e + +source test/setup + +use Test::More + +clone-foo-and-bar + +# Create a pre-commit hook that will fail +{ + hook_file="$OWNER/foo/.git/hooks/pre-commit" + mkdir -p "$(dirname "$hook_file")" + cat > "$hook_file" <<'EOF' +#!/bin/bash +# This hook will always fail +echo "Pre-commit hook triggered and failing" +exit 1 +EOF + chmod +x "$hook_file" +} + +# Test that clone without --verify succeeds (hooks bypassed by default) +{ + clone_status=0 + clone_output=$( + cd "$OWNER/foo" + git subrepo clone "$UPSTREAM/bar" 2>&1 + ) || clone_status=$? + + is "$clone_status" "0" \ + 'subrepo clone succeeds by default (hooks bypassed)' + + unlike "$clone_output" "Pre-commit hook triggered" \ + 'pre-commit hook was bypassed by default' + + is "$clone_output" \ + "Subrepo '$UPSTREAM/bar' (master) cloned into 'bar'." \ + 'clone output is correct' +} + +# Test that the subrepo was actually cloned +{ + test-exists \ + "$OWNER/foo/bar/" \ + "$OWNER/foo/bar/Bar" \ + "$OWNER/foo/bar/.gitrepo" +} + +# Clean up for next test +{ + ( + cd "$OWNER/foo" + git reset --hard HEAD~1 + rm -rf bar + ) &> /dev/null || true +} + +# Test that clone with --verify runs the hook and fails +{ + clone_status=0 + clone_output=$( + cd "$OWNER/foo" + git subrepo clone --verify "$UPSTREAM/bar" 2>&1 + ) || clone_status=$? + + isnt "$clone_status" "0" \ + 'subrepo clone with --verify fails when pre-commit hook fails' + + like "$clone_output" "Pre-commit hook triggered and failing" \ + 'pre-commit hook was triggered with --verify' +} + +done_testing + +teardown diff --git a/test/verify-config.t b/test/verify-config.t new file mode 100644 index 00000000..83cc677f --- /dev/null +++ b/test/verify-config.t @@ -0,0 +1,72 @@ +#!/usr/bin/env bash + +set -e + +source test/setup + +use Test::More + +clone-foo-and-bar + +# Create a pre-commit hook that will fail +{ + hook_file="$OWNER/foo/.git/hooks/pre-commit" + mkdir -p "$(dirname "$hook_file")" + cat > "$hook_file" <<'EOF' +#!/bin/bash +# This hook will always fail +echo "Pre-commit hook triggered and failing" +exit 1 +EOF + chmod +x "$hook_file" +} + +# Clone without verify - should succeed (default) +{ + clone_status=0 + clone_output=$( + cd "$OWNER/foo" + git subrepo clone "$UPSTREAM/bar" 2>&1 + ) || clone_status=$? + + is "$clone_status" "0" \ + 'subrepo clone succeeds by default (hooks bypassed)' + + test-exists "$OWNER/foo/bar/.gitrepo" +} + +# Test that verify config can be set in .gitrepo +{ + ( + cd "$OWNER/foo" + git config --file="bar/.gitrepo" subrepo.verify true + ) &> /dev/null || true + + verify_value=$( + cd "$OWNER/foo" + git config --file="bar/.gitrepo" subrepo.verify + ) + + is "$verify_value" "true" \ + 'verify=true can be set in .gitrepo config' +} + +# Test that verify config set to false works +{ + ( + cd "$OWNER/foo" + git config --file="bar/.gitrepo" subrepo.verify false + ) &> /dev/null || true + + verify_value=$( + cd "$OWNER/foo" + git config --file="bar/.gitrepo" subrepo.verify + ) + + is "$verify_value" "false" \ + 'verify=false can be set in .gitrepo config' +} + +done_testing + +teardown