Skip to content

Adjust pre-commit/pre-push hooks to be compatible with git-worktree #17706

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

Closed
wants to merge 1 commit into from
Closed

Adjust pre-commit/pre-push hooks to be compatible with git-worktree #17706

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
12 changes: 6 additions & 6 deletions hooks/install-hooks.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ if [ $# != 1 ]
fi

DRUID_ROOT=$1

cp_if_not_exist ${DRUID_ROOT}/hooks/run-all-in-dir.py ${DRUID_ROOT}/.git/hooks/run-all-in-dir.py
cp_if_not_exist ${DRUID_ROOT}/hooks/pre-commit ${DRUID_ROOT}/.git/hooks/pre-commit
cp_if_not_exist ${DRUID_ROOT}/hooks/pre-push ${DRUID_ROOT}/.git/hooks/pre-push
cp_if_not_exist ${DRUID_ROOT}/hooks/pre-commits ${DRUID_ROOT}/.git/hooks/pre-commits
cp_if_not_exist ${DRUID_ROOT}/hooks/pre-pushes ${DRUID_ROOT}/.git/hooks/pre-pushes
GIT_COMMON_DIR=$(eval "git -C $DRUID_ROOT rev-parse --path-format=absolute --git-common-dir")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the usage of eval is necessary here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's probably not necessary, but while i was working on this i realized that this would change some files (run-all-in-dir.py, pre-commit, pre-push), and our script does cp_if_not_exist and throws if file is different from the ones in .git dir, so maybe we'd need to add a force flag to installing hook script to overwrite those. that might cause minor disruption to ppl's workflow.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cript does cp_if_not_exist and throws if file is different from the ones in .git dir, so maybe we'd need to add a force flag to installing hook script to overwrite those.

that would be what a plain cp does - but I doubt the initial intention was that.
I don't think these scripts change often; but people may modify it locally...

I think cp_if_not_exist should have nothing to do with the usage of eval

there is nothing in this which would make eval necessary; please remove it.

cp_if_not_exist ${DRUID_ROOT}/hooks/run-all-in-dir.py ${GIT_COMMON_DIR}/hooks/run-all-in-dir.py
cp_if_not_exist ${DRUID_ROOT}/hooks/pre-commit ${GIT_COMMON_DIR}/hooks/pre-commit
cp_if_not_exist ${DRUID_ROOT}/hooks/pre-push ${GIT_COMMON_DIR}/hooks/pre-push
cp_if_not_exist ${DRUID_ROOT}/hooks/pre-commits ${GIT_COMMON_DIR}/hooks/pre-commits
cp_if_not_exist ${DRUID_ROOT}/hooks/pre-pushes ${GIT_COMMON_DIR}/hooks/pre-pushes
3 changes: 2 additions & 1 deletion hooks/pre-commit
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,5 @@
# See the License for the specific language governing permissions and
# limitations under the License.

.git/hooks/run-all-in-dir.py .git/hooks/pre-commits
GIT_COMMON_DIR=$(eval "git rev-parse --path-format=absolute --git-common-dir")
$GIT_COMMON_DIR/hooks/run-all-in-dir.py $GIT_COMMON_DIR/hooks/pre-commits
3 changes: 2 additions & 1 deletion hooks/pre-push
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,5 @@
# under the License.
#

.git/hooks/run-all-in-dir.py .git/hooks/pre-pushes $1 $2
GIT_COMMON_DIR=$(eval "git rev-parse --path-format=absolute --git-common-dir")
$GIT_COMMON_DIR/hooks/run-all-in-dir.py $GIT_COMMON_DIR/hooks/pre-pushes $1 $2
Loading