{"thread":{"id":"58163","subject":"[PATCH] hooks--pre-commit: detect non-ASCII when renaming","startedAt":"2022-07-14T22:17:59Z","lastAt":"2023-12-03T13:15:12Z","messageCount":5,"participants":["Julian Prein via GitGitGadget","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"459099","messageId":"pull.1291.git.git.1657837073372.gitgitgadget@gmail.com","threadId":"58163","inReplyTo":null,"subject":"[PATCH] hooks--pre-commit: detect non-ASCII when renaming","fromName":"Julian Prein via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-14T22:17:53Z","receivedAt":"2022-07-14T22:17:59Z","isPatch":true,"sender":{"key":"julian@druckdev.xyz","avatar":"https://avatars.githubusercontent.com/u/63563978?v=4"},"body":"From: Julian Prein \n\nCurrently the diff-filter that is used to check for non-ASCII characters\nin filenames only checks new additions.\n\nExtend the diff-filter in the pre-commit sample to include `CR` as well.\nThis way non-ASCII character in filenames are detected on a rename/copy\nas well.\n\nSigned-off-by: Julian Prein \n---\n hooks--pre-commit: detect non-ASCII when renaming\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1291%2Fdruckdev%2Fpre-commit-renames-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1291/druckdev/pre-commit-renames-v1\nPull-Request: https://github.com/git/git/pull/1291\n\n templates/hooks--pre-commit.sample | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample\nindex e144712c85c..95c327832da 100755\n--- a/templates/hooks--pre-commit.sample\n+++ b/templates/hooks--pre-commit.sample\n@@ -28,7 +28,7 @@ if [ \"$allownonascii\" != \"true\" ] &&\n \t# Note that the use of brackets around a tr range is ok here, (it's\n \t# even required, for portability to Solaris 10's /usr/bin/tr), since\n \t# the square bracket bytes happen to fall in the designated range.\n-\ttest $(git diff --cached --name-only --diff-filter=A -z $against |\n+\ttest $(git diff --cached --name-only --diff-filter=ACR -z $against |\n \t LC_ALL=C tr -d '[ -~]\\0' | wc -c) != 0\n then\n \tcat <<\\EOF\n\nbase-commit: bbea4dcf42b28eb7ce64a6306cdde875ae5d09ca\n-- \ngitgitgadget\n"},{"id":"459102","messageId":"xmqqedyn2t6e.fsf@gitster.g","threadId":"58163","inReplyTo":"pull.1291.git.git.1657837073372.gitgitgadget@gmail.com","subject":"Re: [PATCH] hooks--pre-commit: detect non-ASCII when renaming","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-14T23:03:37Z","receivedAt":"2022-07-14T23:03:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Julian Prein via GitGitGadget\" writes:\n\n> From: Julian Prein \n>\n> Currently the diff-filter that is used to check for non-ASCII characters\n> in filenames only checks new additions.\n>\n> Extend the diff-filter in the pre-commit sample to include `CR` as well.\n> This way non-ASCII character in filenames are detected on a rename/copy\n> as well.\n\nIt would probably be a better implementation to disable the rename\ndetection, instead of adding Copies and Renames.\n\n> Signed-off-by: Julian Prein \n> ---\n> hooks--pre-commit: detect non-ASCII when renaming\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1291%2Fdruckdev%2Fpre-commit-renames-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1291/druckdev/pre-commit-renames-v1\n> Pull-Request: https://github.com/git/git/pull/1291\n>\n> templates/hooks--pre-commit.sample | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample\n> index e144712c85c..95c327832da 100755\n> --- a/templates/hooks--pre-commit.sample\n> +++ b/templates/hooks--pre-commit.sample\n> @@ -28,7 +28,7 @@ if [ \"$allownonascii\" != \"true\" ] &&\n> \t# Note that the use of brackets around a tr range is ok here, (it's\n> \t# even required, for portability to Solaris 10's /usr/bin/tr), since\n> \t# the square bracket bytes happen to fall in the designated range.\n> -\ttest $(git diff --cached --name-only --diff-filter=A -z $against |\n> +\ttest $(git diff --cached --name-only --diff-filter=ACR -z $against |\n> \t LC_ALL=C tr -d '[ -~]\\0' | wc -c) != 0\n> then\n> \tcat <<\\EOF\n>\n> base-commit: bbea4dcf42b28eb7ce64a6306cdde875ae5d09ca\n"},{"id":"459103","messageId":"xmqqa69b2syj.fsf@gitster.g","threadId":"58163","inReplyTo":"xmqqedyn2t6e.fsf@gitster.g","subject":"Re: [PATCH] hooks--pre-commit: detect non-ASCII when renaming","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-14T23:08:20Z","receivedAt":"2022-07-14T23:08:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> \"Julian Prein via GitGitGadget\" writes:\n>\n>> From: Julian Prein \n>>\n>> Currently the diff-filter that is used to check for non-ASCII characters\n>> in filenames only checks new additions.\n>>\n>> Extend the diff-filter in the pre-commit sample to include `CR` as well.\n>> This way non-ASCII character in filenames are detected on a rename/copy\n>> as well.\n>\n> It would probably be a better implementation to disable the rename\n> detection, instead of adding Copies and Renames.\n\nThat is ...\n\n>> -\ttest $(git diff --cached --name-only --diff-filter=A -z $against |\n\n... instead of using the \"git diff\" Porcelain, we can probably use\nthe \"git diff-index\" plumbing command here, which will not be\naffected by end-user configuration or by the fact that recent \"git\ndiff\" turns on the rename detection by default.\n"},{"id":"485271","messageId":"pull.1291.v2.git.git.1701360836307.gitgitgadget@gmail.com","threadId":"58163","inReplyTo":"pull.1291.git.git.1657837073372.gitgitgadget@gmail.com","subject":"[PATCH v2] hooks--pre-commit: detect non-ASCII when renaming","fromName":"Julian Prein via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-11-30T16:13:56Z","receivedAt":"2023-11-30T16:13:59Z","isPatch":true,"sender":{"key":"julian@druckdev.xyz","avatar":"https://avatars.githubusercontent.com/u/63563978?v=4"},"body":"From: Julian Prein \n\nWhen diff.renames is turned on, the diff-filter will not return renamed\nfiles (or copied ones with diff.renames=copy) and potential non-ASCII\ncharacters would not be caught by this hook.\n\nUse the plumbing command diff-index instead of the porcelain one to not\nbe affected by diff.rename.\n\nSigned-off-by: Julian Prein \n---\n hooks--pre-commit: detect non-ASCII when renaming\n \n A bit later than I expected, but here is v2.\n \n Changes since v1:\n \n * Switched to using diff-index and back to just the A filter as\n suggested by Junio C Hamano\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1291%2Fdruckdev%2Fpre-commit-renames-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1291/druckdev/pre-commit-renames-v2\nPull-Request: https://github.com/git/git/pull/1291\n\nRange-diff vs v1:\n\n 1: 101f327a040 ! 1: 1f6ca0dd3eb hooks--pre-commit: detect non-ASCII when renaming\n @@ Metadata\n ## Commit message ##\n hooks--pre-commit: detect non-ASCII when renaming\n \n - Currently the diff-filter that is used to check for non-ASCII characters\n - in filenames only checks new additions.\n + When diff.renames is turned on, the diff-filter will not return renamed\n + files (or copied ones with diff.renames=copy) and potential non-ASCII\n + characters would not be caught by this hook.\n \n - Extend the diff-filter in the pre-commit sample to include `CR` as well.\n - This way non-ASCII character in filenames are detected on a rename/copy\n - as well.\n + Use the plumbing command diff-index instead of the porcelain one to not\n + be affected by diff.rename.\n \n Signed-off-by: Julian Prein \n \n @@ templates/hooks--pre-commit.sample: if [ \"$allownonascii\" != \"true\" ] &&\n \t# even required, for portability to Solaris 10's /usr/bin/tr), since\n \t# the square bracket bytes happen to fall in the designated range.\n -\ttest $(git diff --cached --name-only --diff-filter=A -z $against |\n -+\ttest $(git diff --cached --name-only --diff-filter=ACR -z $against |\n ++\ttest $(git diff-index --cached --name-only --diff-filter=A -z $against |\n \t LC_ALL=C tr -d '[ -~]\\0' | wc -c) != 0\n then\n \tcat <<\\EOF\n\n\n templates/hooks--pre-commit.sample | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample\nindex e144712c85c..29ed5ee486a 100755\n--- a/templates/hooks--pre-commit.sample\n+++ b/templates/hooks--pre-commit.sample\n@@ -28,7 +28,7 @@ if [ \"$allownonascii\" != \"true\" ] &&\n \t# Note that the use of brackets around a tr range is ok here, (it's\n \t# even required, for portability to Solaris 10's /usr/bin/tr), since\n \t# the square bracket bytes happen to fall in the designated range.\n-\ttest $(git diff --cached --name-only --diff-filter=A -z $against |\n+\ttest $(git diff-index --cached --name-only --diff-filter=A -z $against |\n \t LC_ALL=C tr -d '[ -~]\\0' | wc -c) != 0\n then\n \tcat <<\\EOF\n\nbase-commit: 61a22ddaf0626111193a17ac12f366bd6d167dff\n-- \ngitgitgadget\n"},{"id":"485334","messageId":"xmqqplzn4eci.fsf@gitster.g","threadId":"58163","inReplyTo":"pull.1291.v2.git.git.1701360836307.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] hooks--pre-commit: detect non-ASCII when renaming","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-12-03T13:15:09Z","receivedAt":"2023-12-03T13:15:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Julian Prein via GitGitGadget\" writes:\n\n> From: Julian Prein \n>\n> When diff.renames is turned on, the diff-filter will not return renamed\n> files (or copied ones with diff.renames=copy) and potential non-ASCII\n> characters would not be caught by this hook.\n>\n> Use the plumbing command diff-index instead of the porcelain one to not\n> be affected by diff.rename.\n\nMakes sense.\n\nAn obvious alternative would be to pass \"--no-renames\" and keep\nusing the Porcelain \"git diff\", but this is how the plumbing\n\"diff-index\" and friends are meant to be used. Looking good.\n\nWill queue. Thanks.\n"}]}