Fwd: Re: [PATCH] hooks--pre-commit.sample: check for chars, that are not allowed for a windows file name

[dex teritas]

Am 15.06.2016 um 19:54 schrieb Junio C Hamano:
>
> dexteritas writes:
>>
>> After the ASCII-check, test the windows compatibility of file names.
>> Can be disabled by: git config hooks.allownonwindowschars true ---
>
> Missing sign off.
Will be there next time.
>>
>> templates/hooks--pre-commit.sample | 22 ++ 1 file
>> changed, 22 insertions(+) diff --git
>> a/templates/hooks--pre-commit.sample
>> b/templates/hooks--pre-commit.sample index 68d62d5..120daf1 100755
>> --- a/templates/hooks--pre-commit.sample +++
>> b/templates/hooks--pre-commit.sample @@ -17,6 +17,7 @@ fi # If you
>> want to allow non-ASCII filenames set this variable to true.
>> allownonascii=$(git config --bool hooks.allownonascii)
>> +allownonwindowschars=$(git config --bool hooks.allownonwindowschars)
>> # Redirect output to stderr. exec 1>&2 @@ -43,6 +44,27 @@ If you know
>> what you are doing you can disable this check using: git config
>> hooks.allownonascii true EOF exit 1 +elif [ "$allownonwindowschars"
>> != "true" ] &&
>
> This line is doubly irritating because (1) the double negation is
> somewhat hard to grok, and (2) [] is not part of our CodingStyle.
> Because you inherited this from the existing "allow-non-ascii" one,
> however, I do not want to see you change this line, if you need to
> reroll.
ok
>>
>> + # If you work with linux and windows, there is a problem, if you
>> use + # chars like \ / : * ? " < > |
>
> There is no reason to single out Linux, is there? This new check is
> only about Windows and because people on other platforms, not limited
> to Linux, may not be aware of some characters that are not usable on
> Windows, you are trying to help them, no?
You're right. Linux is just an example.
>
>
>>
>> + # Check if there are used only windows compatible chars + test
>> $(git diff --cached --name-only --diff-filter=A -z $against | +
>> LC_ALL=C tr -d '[0-9A-Za-z\[\]\{\}_ -)+-.]\0' | wc -c) != 0
>
> Because this is in "elif", we know we are allowing non-ascii
> characters when we come here. So you need to do a quite similar check
> from scratch, which is sensible. I do not offhand know if this covers
> all the characters that Windows users cannot use, though.
Actually the first if checks:
"$allownonascii" != "true" ] &&
test $(git diff --cached --name-only --diff-filter=A -z $against |
LC_ALL=C tr -d '[ -~]\0' | wc -c) != 0

If we come to the elif, we either allow non-ascii characters or we don't
allow them and they don't occur.

Well, it covers the characters, but some reserved names and files with
trailing period or space are missing, as Thomas Braun wrote before.
>>
>> +then + cat <<\EOF +Error: Attempt to add a chars that are not
>> allowed for a windows file name. + +This can cause problems if you
>> want to work with people on other platforms. + +To be portable it is
>> advisable to rename the file. + +Check your filenames for: \ / : * ?
>> " < > | + +If you know what you are doing you can disable this check
>> using: + + git config hooks.allownonwindowschars true +EOF + exit 2
>
> Why 2?
Should be 1.
>>
>> fi # If there are whitespace errors, print the offending file names
>> and fail.
>
> When the user says [hooks] allownonascii = false allownonwindowschars
> = false what happens? The user's intention clearly is that the project
> wants to be usable on Windows and also wants to exclude characters
> from codepages that are not ASCII. I however suspect that the hook
> with your patch will allow people to create a "path>like?this.txt"
> happily.
That would throw an error (on the check for nonwindowschars), because
">" and "?" are not inlcuded in the expression '[0-9A-Za-z\[\]\{\}_
-)+-.]\0'.
This one also checks, if there are only ascii chars, because if
allownonwindowschars = false, non-ascii should be excluded no matter
what "allownonascii" was set to.

It may follow a reroll sometime.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hooks--pre-commit.sample: check for chars, that are not allowed for a windows file name

[dex teritas]

Am 15.06.2016 um 13:15 schrieb Thomas Braun:
> Am 15.06.2016 um 10:02 schrieb dexteritas:
>> After the ASCII-check, test the windows compatibility of file names.
>> Can be disabled by:
>> git config hooks.allownonwindowschars true
>> ---
>>  templates/hooks--pre-commit.sample | 22 ++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/templates/hooks--pre-commit.sample 
>> b/templates/hooks--pre-commit.sample
>> index 68d62d5..120daf1 100755
>> --- a/templates/hooks--pre-commit.sample
>> +++ b/templates/hooks--pre-commit.sample
>> @@ -17,6 +17,7 @@ fi
>>  
>>  # If you want to allow non-ASCII filenames set this variable to true.
>>  allownonascii=$(git config --bool hooks.allownonascii)
>> +allownonwindowschars=$(git config --bool hooks.allownonwindowschars)
>>  
>>  # Redirect output to stderr.
>>  exec 1>&2
>> @@ -43,6 +44,27 @@ If you know what you are doing you can disable this check 
>> using:
>>git config hooks.allownonascii true
>>  EOF
>>  exit 1
>> +elif [ "$allownonwindowschars" != "true" ] &&
>> +# If you work with linux and windows, there is a problem, if you use
>> +# chars like \ / : * ? " < > |
>> +# Check if there are used only windows compatible chars
>> +test $(git diff --cached --name-only --diff-filter=A -z $against |
>> +  LC_ALL=C tr -d '[0-9A-Za-z\[\]\{\}_ -)+-.]\0' | wc -c) != 0
>> +then
>> +cat <<\EOF
>> +Error: Attempt to add a chars that are not allowed for a windows file name.
>> +
>> +This can cause problems if you want to work with people on other platforms.
>> +
>> +To be portable it is advisable to rename the file.
>> +
>> +Check your filenames for: \ / : * ? " < > |
>> +
>> +If you know what you are doing you can disable this check using:
>> +
>> +  git config hooks.allownonwindowschars true
>> +EOF
>> +exit 2
>>  fi
>>  
>>  # If there are whitespace errors, print the offending file names and fail.
> There are some cases of illegal file names missing. E.g. reserved names,
> trailing period and space. My trial with a precommit hook for avoiding
> illegal filenames on windows can be found at [1]. Feel free to loot my
> version for a reroll.
>
> [1]:
> https://github.com/t-b/git-pre-commit-hook-windows-filenames/blob/master/pre-commit
>
You're right. Thanks for the example.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hooks--pre-commit.sample: check for chars, that are not allowed for a windows file name

[Junio C Hamano]

dexteritas  writes:

> After the ASCII-check, test the windows compatibility of file names.
> Can be disabled by:
> git config hooks.allownonwindowschars true
> ---

Missing sign off.

>  templates/hooks--pre-commit.sample | 22 ++
>  1 file changed, 22 insertions(+)
>
> diff --git a/templates/hooks--pre-commit.sample 
> b/templates/hooks--pre-commit.sample
> index 68d62d5..120daf1 100755
> --- a/templates/hooks--pre-commit.sample
> +++ b/templates/hooks--pre-commit.sample
> @@ -17,6 +17,7 @@ fi
>  
>  # If you want to allow non-ASCII filenames set this variable to true.
>  allownonascii=$(git config --bool hooks.allownonascii)
> +allownonwindowschars=$(git config --bool hooks.allownonwindowschars)
>  
>  # Redirect output to stderr.
>  exec 1>&2
> @@ -43,6 +44,27 @@ If you know what you are doing you can disable this check 
> using:
>git config hooks.allownonascii true
>  EOF
>   exit 1
> +elif [ "$allownonwindowschars" != "true" ] &&

This line is doubly irritating because (1) the double negation is
somewhat hard to grok, and (2) [] is not part of our CodingStyle.

Because you inherited this from the existing "allow-non-ascii" one,
however, I do not want to see you change this line, if you need to
reroll.

> + # If you work with linux and windows, there is a problem, if you use
> + # chars like \ / : * ? " < > |

There is no reason to single out Linux, is there?

This new check is only about Windows and because people on other
platforms, not limited to Linux, may not be aware of some characters
that are not usable on Windows, you are trying to help them, no?

> + # Check if there are used only windows compatible chars
> + test $(git diff --cached --name-only --diff-filter=A -z $against |
> +   LC_ALL=C tr -d '[0-9A-Za-z\[\]\{\}_ -)+-.]\0' | wc -c) != 0

Because this is in "elif", we know we are allowing non-ascii
characters when we come here.  So you need to do a quite similar
check from scratch, which is sensible.  I do not offhand know if
this covers all the characters that Windows users cannot use,
though.

> +then
> + cat <<\EOF
> +Error: Attempt to add a chars that are not allowed for a windows file name.
> +
> +This can cause problems if you want to work with people on other platforms.
> +
> +To be portable it is advisable to rename the file.
> +
> +Check your filenames for: \ / : * ? " < > |
> +
> +If you know what you are doing you can disable this check using:
> +
> +  git config hooks.allownonwindowschars true
> +EOF
> + exit 2

Why 2?

>  fi
>  
>  # If there are whitespace errors, print the offending file names and fail.

When the user says

[hooks]
allownonascii = false
allownonwindowschars = false

what happens?

The user's intention clearly is that the project wants to be usable
on Windows and also wants to exclude characters from codepages that
are not ASCII.  I however suspect that the hook with your patch will
allow people to create a "path>like?this.txt" happily.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hooks--pre-commit.sample: check for chars, that are not allowed for a windows file name

[Thomas Braun]

Am 15.06.2016 um 10:02 schrieb dexteritas:
> After the ASCII-check, test the windows compatibility of file names.
> Can be disabled by:
> git config hooks.allownonwindowschars true
> ---
>  templates/hooks--pre-commit.sample | 22 ++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/templates/hooks--pre-commit.sample 
> b/templates/hooks--pre-commit.sample
> index 68d62d5..120daf1 100755
> --- a/templates/hooks--pre-commit.sample
> +++ b/templates/hooks--pre-commit.sample
> @@ -17,6 +17,7 @@ fi
>  
>  # If you want to allow non-ASCII filenames set this variable to true.
>  allownonascii=$(git config --bool hooks.allownonascii)
> +allownonwindowschars=$(git config --bool hooks.allownonwindowschars)
>  
>  # Redirect output to stderr.
>  exec 1>&2
> @@ -43,6 +44,27 @@ If you know what you are doing you can disable this check 
> using:
>git config hooks.allownonascii true
>  EOF
>   exit 1
> +elif [ "$allownonwindowschars" != "true" ] &&
> + # If you work with linux and windows, there is a problem, if you use
> + # chars like \ / : * ? " < > |
> + # Check if there are used only windows compatible chars
> + test $(git diff --cached --name-only --diff-filter=A -z $against |
> +   LC_ALL=C tr -d '[0-9A-Za-z\[\]\{\}_ -)+-.]\0' | wc -c) != 0
> +then
> + cat <<\EOF
> +Error: Attempt to add a chars that are not allowed for a windows file name.
> +
> +This can cause problems if you want to work with people on other platforms.
> +
> +To be portable it is advisable to rename the file.
> +
> +Check your filenames for: \ / : * ? " < > |
> +
> +If you know what you are doing you can disable this check using:
> +
> +  git config hooks.allownonwindowschars true
> +EOF
> + exit 2
>  fi
>  
>  # If there are whitespace errors, print the offending file names and fail.

There are some cases of illegal file names missing. E.g. reserved names,
trailing period and space. My trial with a precommit hook for avoiding
illegal filenames on windows can be found at [1]. Feel free to loot my
version for a reroll.

[1]:
https://github.com/t-b/git-pre-commit-hook-windows-filenames/blob/master/pre-commit

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] hooks--pre-commit.sample: check for chars, that are not allowed for a windows file name

[dexteritas]

After the ASCII-check, test the windows compatibility of file names.
Can be disabled by:
git config hooks.allownonwindowschars true
---
 templates/hooks--pre-commit.sample | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/templates/hooks--pre-commit.sample 
b/templates/hooks--pre-commit.sample
index 68d62d5..120daf1 100755
--- a/templates/hooks--pre-commit.sample
+++ b/templates/hooks--pre-commit.sample
@@ -17,6 +17,7 @@ fi
 
 # If you want to allow non-ASCII filenames set this variable to true.
 allownonascii=$(git config --bool hooks.allownonascii)
+allownonwindowschars=$(git config --bool hooks.allownonwindowschars)
 
 # Redirect output to stderr.
 exec 1>&2
@@ -43,6 +44,27 @@ If you know what you are doing you can disable this check 
using:
   git config hooks.allownonascii true
 EOF
exit 1
+elif [ "$allownonwindowschars" != "true" ] &&
+   # If you work with linux and windows, there is a problem, if you use
+   # chars like \ / : * ? " < > |
+   # Check if there are used only windows compatible chars
+   test $(git diff --cached --name-only --diff-filter=A -z $against |
+ LC_ALL=C tr -d '[0-9A-Za-z\[\]\{\}_ -)+-.]\0' | wc -c) != 0
+then
+   cat <<\EOF
+Error: Attempt to add a chars that are not allowed for a windows file name.
+
+This can cause problems if you want to work with people on other platforms.
+
+To be portable it is advisable to rename the file.
+
+Check your filenames for: \ / : * ? " < > |
+
+If you know what you are doing you can disable this check using:
+
+  git config hooks.allownonwindowschars true
+EOF
+   exit 2
 fi
 
 # If there are whitespace errors, print the offending file names and fail.

--
https://github.com/git/git/pull/252
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html