Re: [GUILT v2 14/29] Use "git check-ref-format" to validate patch names.

2014-05-15 Thread Jeff Sipek 
On Tue, May 13, 2014 at 10:30:50PM +0200, Per Cederqvist wrote:
> The valid_patchname now lets "git check-ref-format" do its job instead
> of trying (and failing) to implement the same rules.  See
> git-check-ref-format(1) for a list of the rules.
> 
> Refer to the git-check-ref-format(1) man page in the error messages
> produced when valid_patchname indicates that the name is bad.
> 
> Added testcases that breaks most of the rules in that man-page.
> 
> Git version 1.8.5 no longer allows the single character "@" as a
> branch name.  Guilt always rejects that name, for increased
> compatibility.

Very nice.

Signed-off-by: Josef 'Jeff' Sipek 


> Signed-off-by: Per Cederqvist 
> ---
>  guilt|  21 ++-
>  guilt-fork   |   2 +-
>  guilt-import |   2 +-
>  guilt-new|   2 +-
>  regression/t-025.out | 426 
> +--
>  regression/t-025.sh  |  12 +-
>  regression/t-032.out |   4 +-
>  7 files changed, 446 insertions(+), 23 deletions(-)
> 
> diff --git a/guilt b/guilt
> index 3fc524e..23cc2da 100755
> --- a/guilt
> +++ b/guilt
> @@ -132,14 +132,19 @@ fi
>  # usage: valid_patchname 
>  valid_patchname()
>  {
> - case "$1" in
> - /*|./*|../*|*/./*|*/../*|*/.|*/..|*/|*\ *|*\*)
> - return 1;;
> - *:*)
> - return 1;;
> - *)
> - return 0;;
> - esac
> + if git check-ref-format --allow-onelevel "$1"; then
> + # Starting with Git version 1.8.5, a branch cannot be
> + # the single character "@".  Make sure guilt rejects
> + # that name even if we are currently using an older
> + # version of Git.  This ensures that the test suite
> + # runs fine using any version of Git.
> + if [ "$1" = "@" ]; then
> + return 1
> + fi
> + return 0
> + else
> + return 1
> + fi
>  }
>  
>  get_branch()
> diff --git a/guilt-fork b/guilt-fork
> index a85d391..6447e55 100755
> --- a/guilt-fork
> +++ b/guilt-fork
> @@ -37,7 +37,7 @@ else
>  fi
>  
>  if ! valid_patchname "$newpatch"; then
> - die "The specified patch name contains invalid characters (:)."
> + die "The specified patch name is invalid according to 
> git-check-ref-format(1)."
>  fi
>  
>  if [ -e "$GUILT_DIR/$branch/$newpatch" ]; then
> diff --git a/guilt-import b/guilt-import
> index 3e9b3bb..928e325 100755
> --- a/guilt-import
> +++ b/guilt-import
> @@ -40,7 +40,7 @@ if [ -e "$GUILT_DIR/$branch/$newname" ]; then
>  fi
>  
>  if ! valid_patchname "$newname"; then
> - die "The specified patch name contains invalid characters (:)."
> + die "The specified patch name is invalid according to 
> git-check-ref-format(1)."
>  fi
>  
>  # create any directories as needed
> diff --git a/guilt-new b/guilt-new
> index 9528438..9f7fa44 100755
> --- a/guilt-new
> +++ b/guilt-new
> @@ -64,7 +64,7 @@ fi
>  
>  if ! valid_patchname "$patch"; then
>   disp "Patchname is invalid." >&2
> - die "it cannot begin with '/', './' or '../', or contain /./, /../, or 
> whitespace"
> + die "It must follow the rules in git-check-ref-format(1)."
>  fi
>  
>  # create any directories as needed
> diff --git a/regression/t-025.out b/regression/t-025.out
> index 7811ab1..01bc406 100644
> --- a/regression/t-025.out
> +++ b/regression/t-025.out
> @@ -141,7 +141,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709  
> .git/patches/master/prepend
>  r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8  .git/refs/patches/master/prepend
>  % guilt new white space
>  Patchname is invalid.
> -it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
> +It must follow the rules in git-check-ref-format(1).
>  % list_files
>  d .git/patches
>  d .git/patches/master
> @@ -211,7 +211,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709  
> .git/patches/master/prepend
>  r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8  .git/refs/patches/master/prepend
>  % guilt new /abc
>  Patchname is invalid.
> -it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
> +It must follow the rules in git-check-ref-format(1).
>  % list_files
>  d .git/patches
>  d .git/patches/master
> @@ -235,7 +235,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709  
> .git/patches/master/prepend
>  r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8  .git/refs/patches/master/prepend
>  % guilt new ./blah
>  Patchname is invalid.
> -it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
> +It must follow the rules in git-check-ref-format(1).
>  % list_files
>  d .git/patches
>  d .git/patches/master
> @@ -259,7 +259,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709  
> .git/patches/master/prepend
>  r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8  .git/refs/patches/master/prepend
>  % guilt new ../blah
>  Patchname is invalid.
> -it cannot begin with '/', './' or '../', or contain /./,

[GUILT v2 14/29] Use "git check-ref-format" to validate patch names.

2014-05-13 Thread Per Cederqvist 
The valid_patchname now lets "git check-ref-format" do its job instead
of trying (and failing) to implement the same rules.  See
git-check-ref-format(1) for a list of the rules.

Refer to the git-check-ref-format(1) man page in the error messages
produced when valid_patchname indicates that the name is bad.

Added testcases that breaks most of the rules in that man-page.

Git version 1.8.5 no longer allows the single character "@" as a
branch name.  Guilt always rejects that name, for increased
compatibility.

Signed-off-by: Per Cederqvist 
---
 guilt|  21 ++-
 guilt-fork   |   2 +-
 guilt-import |   2 +-
 guilt-new|   2 +-
 regression/t-025.out | 426 +--
 regression/t-025.sh  |  12 +-
 regression/t-032.out |   4 +-
 7 files changed, 446 insertions(+), 23 deletions(-)

diff --git a/guilt b/guilt
index 3fc524e..23cc2da 100755
--- a/guilt
+++ b/guilt
@@ -132,14 +132,19 @@ fi
 # usage: valid_patchname 
 valid_patchname()
 {
-   case "$1" in
-   /*|./*|../*|*/./*|*/../*|*/.|*/..|*/|*\ *|*\*)
-   return 1;;
-   *:*)
-   return 1;;
-   *)
-   return 0;;
-   esac
+   if git check-ref-format --allow-onelevel "$1"; then
+   # Starting with Git version 1.8.5, a branch cannot be
+   # the single character "@".  Make sure guilt rejects
+   # that name even if we are currently using an older
+   # version of Git.  This ensures that the test suite
+   # runs fine using any version of Git.
+   if [ "$1" = "@" ]; then
+   return 1
+   fi
+   return 0
+   else
+   return 1
+   fi
 }
 
 get_branch()
diff --git a/guilt-fork b/guilt-fork
index a85d391..6447e55 100755
--- a/guilt-fork
+++ b/guilt-fork
@@ -37,7 +37,7 @@ else
 fi
 
 if ! valid_patchname "$newpatch"; then
-   die "The specified patch name contains invalid characters (:)."
+   die "The specified patch name is invalid according to 
git-check-ref-format(1)."
 fi
 
 if [ -e "$GUILT_DIR/$branch/$newpatch" ]; then
diff --git a/guilt-import b/guilt-import
index 3e9b3bb..928e325 100755
--- a/guilt-import
+++ b/guilt-import
@@ -40,7 +40,7 @@ if [ -e "$GUILT_DIR/$branch/$newname" ]; then
 fi
 
 if ! valid_patchname "$newname"; then
-   die "The specified patch name contains invalid characters (:)."
+   die "The specified patch name is invalid according to 
git-check-ref-format(1)."
 fi
 
 # create any directories as needed
diff --git a/guilt-new b/guilt-new
index 9528438..9f7fa44 100755
--- a/guilt-new
+++ b/guilt-new
@@ -64,7 +64,7 @@ fi
 
 if ! valid_patchname "$patch"; then
disp "Patchname is invalid." >&2
-   die "it cannot begin with '/', './' or '../', or contain /./, /../, or 
whitespace"
+   die "It must follow the rules in git-check-ref-format(1)."
 fi
 
 # create any directories as needed
diff --git a/regression/t-025.out b/regression/t-025.out
index 7811ab1..01bc406 100644
--- a/regression/t-025.out
+++ b/regression/t-025.out
@@ -141,7 +141,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709  
.git/patches/master/prepend
 r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8  .git/refs/patches/master/prepend
 % guilt new white space
 Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
 % list_files
 d .git/patches
 d .git/patches/master
@@ -211,7 +211,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709  
.git/patches/master/prepend
 r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8  .git/refs/patches/master/prepend
 % guilt new /abc
 Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
 % list_files
 d .git/patches
 d .git/patches/master
@@ -235,7 +235,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709  
.git/patches/master/prepend
 r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8  .git/refs/patches/master/prepend
 % guilt new ./blah
 Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
 % list_files
 d .git/patches
 d .git/patches/master
@@ -259,7 +259,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709  
.git/patches/master/prepend
 r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8  .git/refs/patches/master/prepend
 % guilt new ../blah
 Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
 % list_files
 d .git/patches
 d .git/patches/master
@@ -283,7 +283,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709  
.git/patches/master/prepend
 r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8  .git/refs/patches/master/prepend
 % guilt new abc/./blah
 Patchname