Copilot commented on code in PR #13267:
URL: https://github.com/apache/trafficserver/pull/13267#discussion_r3410908916
##########
tools/clang-format.sh:
##########
@@ -23,7 +23,9 @@ PKGDATE="20200514"
function main() {
set -e # exit on error
- ROOT=${ROOT:-$(cd $(dirname $0) && git rev-parse
--show-toplevel)/.git/fmt/${PKGDATE}}
+
+ GIT_DIR=$(git rev-parse --absolute-git-dir)
+ ROOT=${ROOT:-${GIT_DIR}/fmt/${PKGDATE}}
Review Comment:
`git rev-parse --absolute-git-dir` is run in the current working directory.
If this script is invoked from an out-of-tree build directory (common for
CMake) or any directory outside the worktree, this will fail even though the
script path points into the repo. Run `git rev-parse` from the script’s
directory (or use `git -C`) to make it independent of the caller’s CWD.
##########
tools/clang-format.sh:
##########
@@ -118,5 +120,6 @@ EOF
if [[ "$(basename -- "$0")" == 'clang-format.sh' ]]; then
main "$@"
else
- ROOT=${ROOT:-$(git rev-parse --show-toplevel)/.git/fmt/${PKGDATE}}
+ GIT_DIR=$(git rev-parse --absolute-git-dir)
+ ROOT=${ROOT:-${GIT_DIR}/fmt/${PKGDATE}}
Review Comment:
Same issue as above: when this file is sourced from a directory outside the
worktree, `git rev-parse --absolute-git-dir` will fail. Resolve the git dir
relative to the script location so sourcing works from any CWD.
##########
tools/yapf.sh:
##########
@@ -47,7 +47,8 @@ _END_
fi
REPO_ROOT=$(cd $(dirname $0) && git rev-parse --show-toplevel)
- YAPF_VENV=${YAPF_VENV:-${REPO_ROOT}/.git/fmt/yapf_${YAPF_VERSION}_venv}
+ GIT_DIR=$(git rev-parse --absolute-git-dir)
+ YAPF_VENV=${YAPF_VENV:-${GIT_DIR}/fmt/yapf_${YAPF_VERSION}_venv}
Review Comment:
`git rev-parse --absolute-git-dir` is run in the current working directory.
If this script is invoked from an out-of-tree build directory (common for
CMake) or any directory outside the worktree, this will fail even though
`REPO_ROOT` is computed relative to the script path. Run `git rev-parse` from
the script’s directory (or use `git -C`) for consistency and robustness.
##########
tools/yapf.sh:
##########
@@ -105,5 +106,6 @@ _END_
if [[ "$(basename -- "$0")" == 'yapf.sh' ]]; then
main "$@"
else
- YAPF_VENV=${YAPF_VENV:-$(git rev-parse
--show-toplevel)/.git/fmt/yapf_${YAPF_VERSION}_venv}
+ GIT_DIR=$(git rev-parse --absolute-git-dir)
+ YAPF_VENV=${YAPF_VENV:-${GIT_DIR}/fmt/yapf_${YAPF_VERSION}_venv}
Review Comment:
Same issue as above: when this file is sourced from a directory outside the
worktree, `git rev-parse --absolute-git-dir` will fail. Resolve the git dir
relative to the script location so sourcing works from any CWD.
##########
tools/autopep8.sh:
##########
@@ -115,5 +116,6 @@ function main() {
if [[ "$(basename -- "$0")" == 'autopep8.sh' ]]; then
main "$@"
else
- AUTOPEP8_VENV=${AUTOPEP8_VENV:-$(git rev-parse
--show-toplevel)/.git/fmt/autopep8_${AUTOPEP8_VERSION}_venv}
+ GIT_DIR=$(git rev-parse --absolute-git-dir)
+
AUTOPEP8_VENV=${AUTOPEP8_VENV:-${GIT_DIR}/fmt/autopep8_${AUTOPEP8_VERSION}_venv}
Review Comment:
Same issue as above: when this file is sourced from a directory outside the
worktree, `git rev-parse --absolute-git-dir` will fail. Resolve the git dir
relative to the script location so sourcing works from any CWD.
##########
tools/autopep8.sh:
##########
@@ -40,7 +40,8 @@ function main() {
pip install -q virtualenv
fi
- AUTOPEP8_VENV=${AUTOPEP8_VENV:-$(cd $(dirname $0) && git rev-parse
--show-toplevel)/.git/fmt/autopep8_${AUTOPEP8_VERSION}_venv}
+ GIT_DIR=$(git rev-parse --absolute-git-dir)
+
AUTOPEP8_VENV=${AUTOPEP8_VENV:-${GIT_DIR}/fmt/autopep8_${AUTOPEP8_VERSION}_venv}
Review Comment:
`git rev-parse --absolute-git-dir` is run in the current working directory.
If `tools/autopep8.sh` is executed from an out-of-tree build directory (or any
directory outside the repo), this will fail. Run `git rev-parse` from the
script’s directory (or use `git -C`) to make it independent of the caller’s CWD.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]