All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] libxfs-apply: support filterdiff >= 0.4.2 only
@ 2021-07-13 23:53 Dave Chinner
  2022-01-11  1:30 ` Eric Sandeen
  0 siblings, 1 reply; 3+ messages in thread
From: Dave Chinner @ 2021-07-13 23:53 UTC (permalink / raw)
  To: linux-xfs

From: Dave Chinner <dchinner@redhat.com>

We currently require filterdiff v0.3.4 as a minimum for handling git
based patches. This was the first version to handle git diff
metadata well enough to do patch reformatting. It was, however, very
buggy and required several workarounds to get it to do what we
needed.

However, these bugs have been fixed and on a machine with v0.4.2,
the workarounds result in libxfs-apply breaking and creating corrupt
patches. Rather than try to carry around workarounds for a broken
filterdiff version and one that just works, just increase the
minimum required version to 0.4.2 and remove all the workarounds for
the bugs in 0.3.4.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 tools/libxfs-apply | 42 +++++++++++++++++-------------------------
 1 file changed, 17 insertions(+), 25 deletions(-)

diff --git a/tools/libxfs-apply b/tools/libxfs-apply
index 9271db380198..097a695f942b 100755
--- a/tools/libxfs-apply
+++ b/tools/libxfs-apply
@@ -30,21 +30,22 @@ fail()
 	exit
 }
 
-# filterdiff 0.3.4 is the first version that handles git diff metadata (almost)
-# correctly. It just doesn't work properly in prior versions, so those versions
-# can't be used to extract the commit message prior to the diff. Hence just
-# abort and tell the user to upgrade if an old version is detected. We need to
+# filterdiff didn't start handling git diff metadata correctly until some time
+# after 0.3.4. The handling in 0.3.4 was buggy and broken, requiring working
+# around that bugs to use it. Now that 0.4.2 has fixed all those bugs, the
+# work-arounds for 0.3.4 do not work. Hence set 0.4.2 as the minimum required
+# version and tell the user to upgrade if an old version is detected. We need to
 # check against x.y.z version numbers here.
 _version=`filterdiff --version | cut -d " " -f 5`
 _major=`echo $_version | cut -d "." -f 1`
 _minor=`echo $_version | cut -d "." -f 2`
 _patch=`echo $_version | cut -d "." -f 3`
 if [ $_major -eq 0 ]; then
-	if [ $_minor -lt 3 ]; then
-		fail "filterdiff $_version found. 0.3.4 or greater is required."
+	if [ $_minor -lt 4 ]; then
+		fail "filterdiff $_version found. 0.4.2 or greater is required."
 	fi
-	if [ $_minor -eq 3 -a $_patch -le 3 ]; then
-		fail "filterdiff $_version found. 0.3.4 or greater is required."
+	if [ $_minor -eq 4 -a $_patch -lt 2 ]; then
+		fail "filterdiff $_version found. 0.4.2 or greater is required."
 	fi
 fi
 
@@ -158,8 +159,7 @@ filter_kernel_patch()
 			--addoldprefix=a/fs/xfs/ \
 			--addnewprefix=b/fs/xfs/ \
 			$_patch | \
-		sed -e 's, [ab]\/fs\/xfs\/\(\/dev\/null\), \1,' \
-		    -e '/^diff --git/d'
+		sed -e 's, [ab]\/fs\/xfs\/\(\/dev\/null\), \1,'
 
 
 	rm -f $_libxfs_files
@@ -187,8 +187,7 @@ filter_xfsprogs_patch()
 			--addoldprefix=a/ \
 			--addnewprefix=b/ \
 			$_patch | \
-		sed -e 's, [ab]\/\(\/dev\/null\), \1,' \
-		    -e '/^diff --git/d'
+		sed -e 's, [ab]\/\(\/dev\/null\), \1,'
 
 	rm -f $_libxfs_files
 }
@@ -209,30 +208,23 @@ fixup_header_format()
 	local _diff=`mktemp`
 	local _new_hdr=$_hdr.new
 
-	# there's a bug in filterdiff that leaves a line at the end of the
-	# header in the filtered git show output like:
-	#
-	# difflibxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
-	#
-	# split the header on that (convenient!)
-	sed -e /^difflib/q $_patch > $_hdr
+	# Split the header on the first ^diff --git line (convenient!)
+	sed -e /^diff/q $_patch > $_hdr
 	cat $_patch | awk '
-		BEGIN { difflib_seen = 0; index_seen = 0 }
-		/^difflib/ { difflib_seen++; next }
+		BEGIN { diff_seen = 0; index_seen = 0 }
+		/^diff/ { diff_seen++; next }
 		/^index/ { if (++index_seen == 1) { next } }
-		// { if (difflib_seen) { print $0 } }' > $_diff
+		// { if (diff_seen) { print $0 } }' > $_diff
 
 	# the header now has the format:
 	# commit 0d5a75e9e23ee39cd0d8a167393dcedb4f0f47b2
 	# Author: Eric Sandeen <sandeen@sandeen.net>
 	# Date:   Wed Jun 1 17:38:15 2016 +1000
-	# 
+	#
 	#     xfs: make several functions static
 	#....
 	#     Signed-off-by: Dave Chinner <david@fromorbit.com>
 	#
-	#difflibxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
-	#
 	# We want to format it like a normal patch with a line to say what repo
 	# and commit it was sourced from:
 	#
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] libxfs-apply: support filterdiff >= 0.4.2 only
  2021-07-13 23:53 [PATCH] libxfs-apply: support filterdiff >= 0.4.2 only Dave Chinner
@ 2022-01-11  1:30 ` Eric Sandeen
  2022-01-11 17:39   ` Darrick J. Wong
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Sandeen @ 2022-01-11  1:30 UTC (permalink / raw)
  To: Dave Chinner, linux-xfs

On 7/13/21 6:53 PM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> We currently require filterdiff v0.3.4 as a minimum for handling git
> based patches. This was the first version to handle git diff
> metadata well enough to do patch reformatting. It was, however, very
> buggy and required several workarounds to get it to do what we
> needed.
> 
> However, these bugs have been fixed and on a machine with v0.4.2,
> the workarounds result in libxfs-apply breaking and creating corrupt
> patches. Rather than try to carry around workarounds for a broken
> filterdiff version and one that just works, just increase the
> minimum required version to 0.4.2 and remove all the workarounds for
> the bugs in 0.3.4.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

I had missed this, sorry. I'm using it now, seems to work fine.

Darrick is still on the older version, so we probably need to come
to agreement on requiring something newer.  Thanks for working through
this!

-Eric

> ---
>   tools/libxfs-apply | 42 +++++++++++++++++-------------------------
>   1 file changed, 17 insertions(+), 25 deletions(-)
> 
> diff --git a/tools/libxfs-apply b/tools/libxfs-apply
> index 9271db380198..097a695f942b 100755
> --- a/tools/libxfs-apply
> +++ b/tools/libxfs-apply
> @@ -30,21 +30,22 @@ fail()
>   	exit
>   }
>   
> -# filterdiff 0.3.4 is the first version that handles git diff metadata (almost)
> -# correctly. It just doesn't work properly in prior versions, so those versions
> -# can't be used to extract the commit message prior to the diff. Hence just
> -# abort and tell the user to upgrade if an old version is detected. We need to
> +# filterdiff didn't start handling git diff metadata correctly until some time
> +# after 0.3.4. The handling in 0.3.4 was buggy and broken, requiring working
> +# around that bugs to use it. Now that 0.4.2 has fixed all those bugs, the
> +# work-arounds for 0.3.4 do not work. Hence set 0.4.2 as the minimum required
> +# version and tell the user to upgrade if an old version is detected. We need to
>   # check against x.y.z version numbers here.
>   _version=`filterdiff --version | cut -d " " -f 5`
>   _major=`echo $_version | cut -d "." -f 1`
>   _minor=`echo $_version | cut -d "." -f 2`
>   _patch=`echo $_version | cut -d "." -f 3`
>   if [ $_major -eq 0 ]; then
> -	if [ $_minor -lt 3 ]; then
> -		fail "filterdiff $_version found. 0.3.4 or greater is required."
> +	if [ $_minor -lt 4 ]; then
> +		fail "filterdiff $_version found. 0.4.2 or greater is required."
>   	fi
> -	if [ $_minor -eq 3 -a $_patch -le 3 ]; then
> -		fail "filterdiff $_version found. 0.3.4 or greater is required."
> +	if [ $_minor -eq 4 -a $_patch -lt 2 ]; then
> +		fail "filterdiff $_version found. 0.4.2 or greater is required."
>   	fi
>   fi
>   
> @@ -158,8 +159,7 @@ filter_kernel_patch()
>   			--addoldprefix=a/fs/xfs/ \
>   			--addnewprefix=b/fs/xfs/ \
>   			$_patch | \
> -		sed -e 's, [ab]\/fs\/xfs\/\(\/dev\/null\), \1,' \
> -		    -e '/^diff --git/d'
> +		sed -e 's, [ab]\/fs\/xfs\/\(\/dev\/null\), \1,'
>   
>   
>   	rm -f $_libxfs_files
> @@ -187,8 +187,7 @@ filter_xfsprogs_patch()
>   			--addoldprefix=a/ \
>   			--addnewprefix=b/ \
>   			$_patch | \
> -		sed -e 's, [ab]\/\(\/dev\/null\), \1,' \
> -		    -e '/^diff --git/d'
> +		sed -e 's, [ab]\/\(\/dev\/null\), \1,'
>   
>   	rm -f $_libxfs_files
>   }
> @@ -209,30 +208,23 @@ fixup_header_format()
>   	local _diff=`mktemp`
>   	local _new_hdr=$_hdr.new
>   
> -	# there's a bug in filterdiff that leaves a line at the end of the
> -	# header in the filtered git show output like:
> -	#
> -	# difflibxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> -	#
> -	# split the header on that (convenient!)
> -	sed -e /^difflib/q $_patch > $_hdr
> +	# Split the header on the first ^diff --git line (convenient!)
> +	sed -e /^diff/q $_patch > $_hdr
>   	cat $_patch | awk '
> -		BEGIN { difflib_seen = 0; index_seen = 0 }
> -		/^difflib/ { difflib_seen++; next }
> +		BEGIN { diff_seen = 0; index_seen = 0 }
> +		/^diff/ { diff_seen++; next }
>   		/^index/ { if (++index_seen == 1) { next } }
> -		// { if (difflib_seen) { print $0 } }' > $_diff
> +		// { if (diff_seen) { print $0 } }' > $_diff
>   
>   	# the header now has the format:
>   	# commit 0d5a75e9e23ee39cd0d8a167393dcedb4f0f47b2
>   	# Author: Eric Sandeen <sandeen@sandeen.net>
>   	# Date:   Wed Jun 1 17:38:15 2016 +1000
> -	#
> +	#
>   	#     xfs: make several functions static
>   	#....
>   	#     Signed-off-by: Dave Chinner <david@fromorbit.com>
>   	#
> -	#difflibxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> -	#
>   	# We want to format it like a normal patch with a line to say what repo
>   	# and commit it was sourced from:
>   	#

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] libxfs-apply: support filterdiff >= 0.4.2 only
  2022-01-11  1:30 ` Eric Sandeen
@ 2022-01-11 17:39   ` Darrick J. Wong
  0 siblings, 0 replies; 3+ messages in thread
From: Darrick J. Wong @ 2022-01-11 17:39 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: Dave Chinner, linux-xfs

On Mon, Jan 10, 2022 at 07:30:26PM -0600, Eric Sandeen wrote:
> On 7/13/21 6:53 PM, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > We currently require filterdiff v0.3.4 as a minimum for handling git
> > based patches. This was the first version to handle git diff
> > metadata well enough to do patch reformatting. It was, however, very
> > buggy and required several workarounds to get it to do what we
> > needed.
> > 
> > However, these bugs have been fixed and on a machine with v0.4.2,
> > the workarounds result in libxfs-apply breaking and creating corrupt
> > patches. Rather than try to carry around workarounds for a broken
> > filterdiff version and one that just works, just increase the
> > minimum required version to 0.4.2 and remove all the workarounds for
> > the bugs in 0.3.4.
> > 
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> 
> I had missed this, sorry. I'm using it now, seems to work fine.
> 
> Darrick is still on the older version, so we probably need to come
> to agreement on requiring something newer.  Thanks for working through
> this!

/me built 0.4.2 for his local systems and drops his objections.  The
plan is to update to Ubuntu 2022 in May, but we'll see how much of
Ubuntu gets borg'd^Wsnapify'd in that timeframe.

--D

> -Eric
> 
> > ---
> >   tools/libxfs-apply | 42 +++++++++++++++++-------------------------
> >   1 file changed, 17 insertions(+), 25 deletions(-)
> > 
> > diff --git a/tools/libxfs-apply b/tools/libxfs-apply
> > index 9271db380198..097a695f942b 100755
> > --- a/tools/libxfs-apply
> > +++ b/tools/libxfs-apply
> > @@ -30,21 +30,22 @@ fail()
> >   	exit
> >   }
> > -# filterdiff 0.3.4 is the first version that handles git diff metadata (almost)
> > -# correctly. It just doesn't work properly in prior versions, so those versions
> > -# can't be used to extract the commit message prior to the diff. Hence just
> > -# abort and tell the user to upgrade if an old version is detected. We need to
> > +# filterdiff didn't start handling git diff metadata correctly until some time
> > +# after 0.3.4. The handling in 0.3.4 was buggy and broken, requiring working
> > +# around that bugs to use it. Now that 0.4.2 has fixed all those bugs, the
> > +# work-arounds for 0.3.4 do not work. Hence set 0.4.2 as the minimum required
> > +# version and tell the user to upgrade if an old version is detected. We need to
> >   # check against x.y.z version numbers here.
> >   _version=`filterdiff --version | cut -d " " -f 5`
> >   _major=`echo $_version | cut -d "." -f 1`
> >   _minor=`echo $_version | cut -d "." -f 2`
> >   _patch=`echo $_version | cut -d "." -f 3`
> >   if [ $_major -eq 0 ]; then
> > -	if [ $_minor -lt 3 ]; then
> > -		fail "filterdiff $_version found. 0.3.4 or greater is required."
> > +	if [ $_minor -lt 4 ]; then
> > +		fail "filterdiff $_version found. 0.4.2 or greater is required."
> >   	fi
> > -	if [ $_minor -eq 3 -a $_patch -le 3 ]; then
> > -		fail "filterdiff $_version found. 0.3.4 or greater is required."
> > +	if [ $_minor -eq 4 -a $_patch -lt 2 ]; then
> > +		fail "filterdiff $_version found. 0.4.2 or greater is required."
> >   	fi
> >   fi
> > @@ -158,8 +159,7 @@ filter_kernel_patch()
> >   			--addoldprefix=a/fs/xfs/ \
> >   			--addnewprefix=b/fs/xfs/ \
> >   			$_patch | \
> > -		sed -e 's, [ab]\/fs\/xfs\/\(\/dev\/null\), \1,' \
> > -		    -e '/^diff --git/d'
> > +		sed -e 's, [ab]\/fs\/xfs\/\(\/dev\/null\), \1,'
> >   	rm -f $_libxfs_files
> > @@ -187,8 +187,7 @@ filter_xfsprogs_patch()
> >   			--addoldprefix=a/ \
> >   			--addnewprefix=b/ \
> >   			$_patch | \
> > -		sed -e 's, [ab]\/\(\/dev\/null\), \1,' \
> > -		    -e '/^diff --git/d'
> > +		sed -e 's, [ab]\/\(\/dev\/null\), \1,'
> >   	rm -f $_libxfs_files
> >   }
> > @@ -209,30 +208,23 @@ fixup_header_format()
> >   	local _diff=`mktemp`
> >   	local _new_hdr=$_hdr.new
> > -	# there's a bug in filterdiff that leaves a line at the end of the
> > -	# header in the filtered git show output like:
> > -	#
> > -	# difflibxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> > -	#
> > -	# split the header on that (convenient!)
> > -	sed -e /^difflib/q $_patch > $_hdr
> > +	# Split the header on the first ^diff --git line (convenient!)
> > +	sed -e /^diff/q $_patch > $_hdr
> >   	cat $_patch | awk '
> > -		BEGIN { difflib_seen = 0; index_seen = 0 }
> > -		/^difflib/ { difflib_seen++; next }
> > +		BEGIN { diff_seen = 0; index_seen = 0 }
> > +		/^diff/ { diff_seen++; next }
> >   		/^index/ { if (++index_seen == 1) { next } }
> > -		// { if (difflib_seen) { print $0 } }' > $_diff
> > +		// { if (diff_seen) { print $0 } }' > $_diff
> >   	# the header now has the format:
> >   	# commit 0d5a75e9e23ee39cd0d8a167393dcedb4f0f47b2
> >   	# Author: Eric Sandeen <sandeen@sandeen.net>
> >   	# Date:   Wed Jun 1 17:38:15 2016 +1000
> > -	#
> > +	#
> >   	#     xfs: make several functions static
> >   	#....
> >   	#     Signed-off-by: Dave Chinner <david@fromorbit.com>
> >   	#
> > -	#difflibxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> > -	#
> >   	# We want to format it like a normal patch with a line to say what repo
> >   	# and commit it was sourced from:
> >   	#

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-01-11 17:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-13 23:53 [PATCH] libxfs-apply: support filterdiff >= 0.4.2 only Dave Chinner
2022-01-11  1:30 ` Eric Sandeen
2022-01-11 17:39   ` Darrick J. Wong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.