Re: [PATCH] dim: Check that patches have git-format-patch default prefixes

2021-12-14 Thread Daniel Vetter
On Mon, Nov 29, 2021 at 11:51:58AM +0100, Javier Martinez Canillas wrote:
> By default the git-format-patch command generates patches with prefixes
> for the source and destination (-p1) and is also what git-am uses as a
> default. The command strips the first leading path component when patch
> is applied (unless a different -p argument is used).
> 
> But the patch generating behaviour can be changed with git-format-patch
> --no-prefix argument (or setting 'diff.noprefix = true' in .gitconfig).
> 
> Patches with no source and destination prefixes will confuse the git-am
> 3-way merge logic, since stripping the first path component will lead
> to wrong paths for newly added files.
> 
> To avoid this, check that patches to apply are using git-format-patch's
> defaults prefixes to make sure that git-am defaults are safe to use too.
> 
> Signed-off-by: Javier Martinez Canillas 
> ---
> 
>  dim | 19 +++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/dim b/dim
> index bbe9308ac695..1a07cc177fb5 100755
> --- a/dim
> +++ b/dim
> @@ -1113,6 +1113,18 @@ function check_merge_baseline
>   fi
>  }
>  
> +# ensure the patch has prefixes (-p1), since otherwise it can confuse the 
> git am
> +# 3-way merge logic. check the default source (a/) and destination (b/) 
> prefixes.
> +function check_diff_prefix
> +{
> + local rv
> + patch="$1"
> +
> + rv=$(grep -q -E "^diff --git a\/.+ b\/.+$" $patch)
> +
> + return $rv
> +}
> +
>  # ensure we're on branch $1, and apply patches. the rest of the arguments are
>  # passed to git am.
>  dim_alias_ab=apply-branch
> @@ -1139,6 +1151,13 @@ function dim_apply_branch
>   git mailsplit -b -o$dir $file > /dev/null
>  
>   for patch in "$dir"/*; do
> +
> + if ! check_diff_prefix "$patch"; then
> + echoerr "ERROR: The patch does not contain prefixes in 
> its diff."
> + echoerr "ERROR: This format can confuse git am when 
> applying it."
> + exit 1

I think there are still people around who love pain too much and generate
patches without git, which I think would trip your check. Can you make
this a warn_or_fail instead. Also I think ERROR: in your echoerr is a bit
overkill.

Otherwise makes sense.
-Daniel

> + fi
> +
>   if ! apply_patch $patch "$@"; then
>   rv=1
>   fi
> -- 
> 2.33.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


[PATCH] dim: Check that patches have git-format-patch default prefixes

2021-11-29 Thread Javier Martinez Canillas
By default the git-format-patch command generates patches with prefixes
for the source and destination (-p1) and is also what git-am uses as a
default. The command strips the first leading path component when patch
is applied (unless a different -p argument is used).

But the patch generating behaviour can be changed with git-format-patch
--no-prefix argument (or setting 'diff.noprefix = true' in .gitconfig).

Patches with no source and destination prefixes will confuse the git-am
3-way merge logic, since stripping the first path component will lead
to wrong paths for newly added files.

To avoid this, check that patches to apply are using git-format-patch's
defaults prefixes to make sure that git-am defaults are safe to use too.

Signed-off-by: Javier Martinez Canillas 
---

 dim | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/dim b/dim
index bbe9308ac695..1a07cc177fb5 100755
--- a/dim
+++ b/dim
@@ -1113,6 +1113,18 @@ function check_merge_baseline
fi
 }
 
+# ensure the patch has prefixes (-p1), since otherwise it can confuse the git 
am
+# 3-way merge logic. check the default source (a/) and destination (b/) 
prefixes.
+function check_diff_prefix
+{
+   local rv
+   patch="$1"
+
+   rv=$(grep -q -E "^diff --git a\/.+ b\/.+$" $patch)
+
+   return $rv
+}
+
 # ensure we're on branch $1, and apply patches. the rest of the arguments are
 # passed to git am.
 dim_alias_ab=apply-branch
@@ -1139,6 +1151,13 @@ function dim_apply_branch
git mailsplit -b -o$dir $file > /dev/null
 
for patch in "$dir"/*; do
+
+   if ! check_diff_prefix "$patch"; then
+   echoerr "ERROR: The patch does not contain prefixes in 
its diff."
+   echoerr "ERROR: This format can confuse git am when 
applying it."
+   exit 1
+   fi
+
if ! apply_patch $patch "$@"; then
rv=1
fi
-- 
2.33.1