public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] libxf-apply: Ignore Merge commits
@ 2023-11-20 15:10 cem
  2023-11-20 15:10 ` [PATCH 2/2] libxfs-apply: Add option to only import patches into guilt stack cem
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: cem @ 2023-11-20 15:10 UTC (permalink / raw)
  To: linux-xfs; +Cc: djwong, david, sandeen

From: Carlos Maiolino <cem@kernel.org>

Merge commits in the kernel tree, only polutes the patch list to be
imported into libxfs, explicitly ignore them.

Signed-off-by: Carlos Maiolino <cem@kernel.org>
---

I'm considering here my own usecase, I never used merge commits, and sometimes
they break the synchronization, so they make no good for me during libxfs-sync.

 tools/libxfs-apply | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/libxfs-apply b/tools/libxfs-apply
index 097a695f9..aa2530f4d 100755
--- a/tools/libxfs-apply
+++ b/tools/libxfs-apply
@@ -445,8 +445,8 @@ fi
 
 # grab and echo the list of commits for confirmation
 echo "Commits to apply:"
-commit_list=`git rev-list $hashr | tac`
-git log --oneline $hashr |tac
+commit_list=`git rev-list --no-merges $hashr | tac`
+git log --oneline --no-merges $hashr |tac
 read -r -p "Proceed [y|N]? " response
 if [ -z "$response" -o "$response" != "y" ]; then
 	fail "Aborted!"
-- 
2.41.0


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

* [PATCH 2/2] libxfs-apply: Add option to only import patches into guilt stack
  2023-11-20 15:10 [PATCH 1/2] libxf-apply: Ignore Merge commits cem
@ 2023-11-20 15:10 ` cem
  2023-11-20 16:49   ` Darrick J. Wong
  2023-11-20 16:51 ` [PATCH 1/2] libxf-apply: Ignore Merge commits Darrick J. Wong
  2023-11-20 19:50 ` [RFC PATCH 3/2] libxfs-apply: allow stgit users to force-apply a patch Darrick J. Wong
  2 siblings, 1 reply; 7+ messages in thread
From: cem @ 2023-11-20 15:10 UTC (permalink / raw)
  To: linux-xfs; +Cc: djwong, david, sandeen

From: Carlos Maiolino <cem@kernel.org>

The script automatically detects the existence of a guilt stack, but
only conflict resolution mechanisms so far are either skip the patch or
fail the process.

It's easier to fix conflicts if all the patches are stacked in guilt, so
add an option to stack all the patches into guilt, without applying them,
for post-processing and conflict resolution in case automatic import fails.

stgit doesn't seem to have a way to import patches into its stack, so,
there is no similar patch aiming stgit.

The order of commits added to $commit_list also needs to be reversed
when only importing patches to the guilt stack.

Signed-off-by: Carlos Maiolino <cem@kernel.org>
---
 tools/libxfs-apply | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/tools/libxfs-apply b/tools/libxfs-apply
index aa2530f4d..2b65684ec 100755
--- a/tools/libxfs-apply
+++ b/tools/libxfs-apply
@@ -9,7 +9,7 @@ usage()
 	echo $*
 	echo
 	echo "Usage:"
-	echo "	libxfs-apply [--verbose] --sob <name/email> --source <repodir> --commit <commit_id>"
+	echo "	libxfs-apply [--import-only] [--verbose] --sob <name/email> --source <repodir> --commit <commit_id>"
 	echo "	libxfs-apply --patch <patchfile>"
 	echo
 	echo "libxfs-apply should be run in the destination git repository."
@@ -67,6 +67,7 @@ REPO=
 PATCH=
 COMMIT_ID=
 VERBOSE=
+IMPORT_ONLY=
 GUILT=0
 STGIT=0
 
@@ -76,6 +77,7 @@ while [ $# -gt 0 ]; do
 	--patch)	PATCH=$2; shift ;;
 	--commit)	COMMIT_ID=$2 ; shift ;;
 	--sob)		SIGNED_OFF_BY=$2 ; shift ;;
+	--import-only)	IMPORT_ONLY=true ;;
 	--verbose)	VERBOSE=true ;;
 	*)		usage ;;
 	esac
@@ -99,6 +101,10 @@ if [ $? -eq 0 ]; then
 	GUILT=1
 fi
 
+if [ -n $IMPORT_ONLY -a $GUILT -ne 1 ]; then
+	usage "--import_only can only be used with a guilt stack"
+fi
+
 # Are we using stgit? This works even if no patch is applied.
 stg top &> /dev/null
 if [ $? -eq 0 ]; then
@@ -359,6 +365,11 @@ apply_patch()
 		fi
 
 		guilt import -P $_patch_name $_new_patch.2
+
+		if [ -n "$IMPORT_ONLY" ]; then
+			return;
+		fi
+
 		guilt push
 		if [ $? -eq 0 ]; then
 			guilt refresh
@@ -443,10 +454,17 @@ else
 	hashr="$hashr -- libxfs"
 fi
 
+# When using --import-only, the commit list should be in reverse order.
+if [ "$GUILT" -eq 1 -a -n "$IMPORT_ONLY" ]; then
+	commit_list=`git rev-list --no-merges $hashr`
+else
+	commit_list=`git rev-list --no-merges $hashr | tac`
+fi
+
 # grab and echo the list of commits for confirmation
 echo "Commits to apply:"
-commit_list=`git rev-list --no-merges $hashr | tac`
 git log --oneline --no-merges $hashr |tac
+
 read -r -p "Proceed [y|N]? " response
 if [ -z "$response" -o "$response" != "y" ]; then
 	fail "Aborted!"
-- 
2.41.0


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

* Re: [PATCH 2/2] libxfs-apply: Add option to only import patches into guilt stack
  2023-11-20 15:10 ` [PATCH 2/2] libxfs-apply: Add option to only import patches into guilt stack cem
@ 2023-11-20 16:49   ` Darrick J. Wong
  2023-11-20 18:21     ` Carlos Maiolino
  0 siblings, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2023-11-20 16:49 UTC (permalink / raw)
  To: cem; +Cc: linux-xfs, david, sandeen

On Mon, Nov 20, 2023 at 04:10:47PM +0100, cem@kernel.org wrote:
> From: Carlos Maiolino <cem@kernel.org>
> 
> The script automatically detects the existence of a guilt stack, but
> only conflict resolution mechanisms so far are either skip the patch or
> fail the process.
> 
> It's easier to fix conflicts if all the patches are stacked in guilt, so
> add an option to stack all the patches into guilt, without applying them,
> for post-processing and conflict resolution in case automatic import fails.
> 
> stgit doesn't seem to have a way to import patches into its stack, so,
> there is no similar patch aiming stgit.

Well... there is a rather gross way to do it, since stg import (even in
--reject mode) is far picker and less aggressive about finding solutions
than regular patch(1):

# Import patch to get the metadata even though it might fail due to
# pickiness.  If the patch applies cleanly, we're done.  Otherwise, set
# up autocleanup for more complicated logic.
stg import --reject "${patch}" && exit 0

tmpfile=/tmp/$$.stgit
trap 'rm -f "${tmpfile}"' TERM INT EXIT QUIT

# Erase whatever stgit managed to apply, then use patch(1)'s more
# flexible heuristics and capture the output.
stg diff | patch -p1 -R
patch -p1 "$@" < "${patch}" > "${tmpfile}" 2>&1
cat "${tmpfile}"

# Attach any new files created by the patch
grep 'create mode' "${patch}" | sed -e 's/^.* mode [0-7]* //g' | while read -r f; do
        git add "$f"
done

# Remove any old files deleted by the patch
grep 'delete mode' "${patch}" | sed -e 's/^.* mode [0-7]* //g' | while read -r f; do
        git rm "$f"
done

# Force us to deal with the rejects.  Use this instead of "<<<" because
# the latter picks up empty output as a single line and does variable
# expansion...  stupid bash.
readarray -t rej_files < <(grep 'saving rejects to' "${tmpfile}" | \
                                sed -e 's/^.*saving rejects to file //g')
rm -f "${tmpfile}"
if [ "${#rej_files[@]}" -gt 0 ]; then
        $VISUAL "${rej_files[@]}"
fi

Once you're done fixing the rejects, 'stg refresh' commits the patch and
you can move on to the next one.

I hadn't thought about merging the above into libxfs-apply directly
though.  I should do that.

Your change for guilt looks ok to me, though I don't have any expertise
with guilt to know if it's good or not.  However, if it makes your life
easier then I say go for it. :)

> The order of commits added to $commit_list also needs to be reversed
> when only importing patches to the guilt stack.
> 
> Signed-off-by: Carlos Maiolino <cem@kernel.org>
> ---
>  tools/libxfs-apply | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxfs-apply b/tools/libxfs-apply
> index aa2530f4d..2b65684ec 100755
> --- a/tools/libxfs-apply
> +++ b/tools/libxfs-apply
> @@ -9,7 +9,7 @@ usage()
>  	echo $*
>  	echo
>  	echo "Usage:"
> -	echo "	libxfs-apply [--verbose] --sob <name/email> --source <repodir> --commit <commit_id>"
> +	echo "	libxfs-apply [--import-only] [--verbose] --sob <name/email> --source <repodir> --commit <commit_id>"
>  	echo "	libxfs-apply --patch <patchfile>"
>  	echo
>  	echo "libxfs-apply should be run in the destination git repository."
> @@ -67,6 +67,7 @@ REPO=
>  PATCH=
>  COMMIT_ID=
>  VERBOSE=
> +IMPORT_ONLY=
>  GUILT=0
>  STGIT=0
>  
> @@ -76,6 +77,7 @@ while [ $# -gt 0 ]; do
>  	--patch)	PATCH=$2; shift ;;
>  	--commit)	COMMIT_ID=$2 ; shift ;;
>  	--sob)		SIGNED_OFF_BY=$2 ; shift ;;
> +	--import-only)	IMPORT_ONLY=true ;;
>  	--verbose)	VERBOSE=true ;;
>  	*)		usage ;;
>  	esac
> @@ -99,6 +101,10 @@ if [ $? -eq 0 ]; then
>  	GUILT=1
>  fi
>  
> +if [ -n $IMPORT_ONLY -a $GUILT -ne 1 ]; then
> +	usage "--import_only can only be used with a guilt stack"
> +fi
> +
>  # Are we using stgit? This works even if no patch is applied.
>  stg top &> /dev/null
>  if [ $? -eq 0 ]; then
> @@ -359,6 +365,11 @@ apply_patch()
>  		fi
>  
>  		guilt import -P $_patch_name $_new_patch.2
> +
> +		if [ -n "$IMPORT_ONLY" ]; then
> +			return;
> +		fi
> +
>  		guilt push
>  		if [ $? -eq 0 ]; then
>  			guilt refresh
> @@ -443,10 +454,17 @@ else
>  	hashr="$hashr -- libxfs"
>  fi
>  
> +# When using --import-only, the commit list should be in reverse order.
> +if [ "$GUILT" -eq 1 -a -n "$IMPORT_ONLY" ]; then
> +	commit_list=`git rev-list --no-merges $hashr`
> +else
> +	commit_list=`git rev-list --no-merges $hashr | tac`

You probably ought to turn this into a proper bash array:

readarray -t commit_list < <(git rev-list --no-merges $hashr | tac)

so that we don't ever run the risk of overflowing line length:

for commit in "${commit_list[@]}"; do
	...
done

Though that's probably only a theoretical concern since ARG_MAX is like
2 million or something.

--D

> +fi
> +
>  # grab and echo the list of commits for confirmation
>  echo "Commits to apply:"
> -commit_list=`git rev-list --no-merges $hashr | tac`
>  git log --oneline --no-merges $hashr |tac
> +
>  read -r -p "Proceed [y|N]? " response
>  if [ -z "$response" -o "$response" != "y" ]; then
>  	fail "Aborted!"
> -- 
> 2.41.0
> 

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

* Re: [PATCH 1/2] libxf-apply: Ignore Merge commits
  2023-11-20 15:10 [PATCH 1/2] libxf-apply: Ignore Merge commits cem
  2023-11-20 15:10 ` [PATCH 2/2] libxfs-apply: Add option to only import patches into guilt stack cem
@ 2023-11-20 16:51 ` Darrick J. Wong
  2023-11-20 18:23   ` Carlos Maiolino
  2023-11-20 19:50 ` [RFC PATCH 3/2] libxfs-apply: allow stgit users to force-apply a patch Darrick J. Wong
  2 siblings, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2023-11-20 16:51 UTC (permalink / raw)
  To: cem; +Cc: linux-xfs, david, sandeen

On Mon, Nov 20, 2023 at 04:10:46PM +0100, cem@kernel.org wrote:
> From: Carlos Maiolino <cem@kernel.org>
> 
> Merge commits in the kernel tree, only polutes the patch list to be
> imported into libxfs, explicitly ignore them.
> 
> Signed-off-by: Carlos Maiolino <cem@kernel.org>
> ---
> 
> I'm considering here my own usecase, I never used merge commits, and sometimes
> they break the synchronization, so they make no good for me during libxfs-sync.

The downside of ignoring merge commits is that Linus edited
xfs_rtbitmap.c in the 6.7 merge commit to get rid of the weird code that
casts a struct timespec64 to a u64 rtpick sequence counter and has
screwed things up for years.

--D

>  tools/libxfs-apply | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxfs-apply b/tools/libxfs-apply
> index 097a695f9..aa2530f4d 100755
> --- a/tools/libxfs-apply
> +++ b/tools/libxfs-apply
> @@ -445,8 +445,8 @@ fi
>  
>  # grab and echo the list of commits for confirmation
>  echo "Commits to apply:"
> -commit_list=`git rev-list $hashr | tac`
> -git log --oneline $hashr |tac
> +commit_list=`git rev-list --no-merges $hashr | tac`
> +git log --oneline --no-merges $hashr |tac
>  read -r -p "Proceed [y|N]? " response
>  if [ -z "$response" -o "$response" != "y" ]; then
>  	fail "Aborted!"
> -- 
> 2.41.0
> 

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

* Re: [PATCH 2/2] libxfs-apply: Add option to only import patches into guilt stack
  2023-11-20 16:49   ` Darrick J. Wong
@ 2023-11-20 18:21     ` Carlos Maiolino
  0 siblings, 0 replies; 7+ messages in thread
From: Carlos Maiolino @ 2023-11-20 18:21 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, david, sandeen

On Mon, Nov 20, 2023 at 08:49:54AM -0800, Darrick J. Wong wrote:
> On Mon, Nov 20, 2023 at 04:10:47PM +0100, cem@kernel.org wrote:
> > From: Carlos Maiolino <cem@kernel.org>
> >
> > The script automatically detects the existence of a guilt stack, but
> > only conflict resolution mechanisms so far are either skip the patch or
> > fail the process.
> >
> > It's easier to fix conflicts if all the patches are stacked in guilt, so
> > add an option to stack all the patches into guilt, without applying them,
> > for post-processing and conflict resolution in case automatic import fails.
> >
> > stgit doesn't seem to have a way to import patches into its stack, so,
> > there is no similar patch aiming stgit.
> 
> Well... there is a rather gross way to do it, since stg import (even in
> --reject mode) is far picker and less aggressive about finding solutions
> than regular patch(1):
> 
> # Import patch to get the metadata even though it might fail due to
> # pickiness.  If the patch applies cleanly, we're done.  Otherwise, set
> # up autocleanup for more complicated logic.
> stg import --reject "${patch}" && exit 0
> 
> tmpfile=/tmp/$$.stgit
> trap 'rm -f "${tmpfile}"' TERM INT EXIT QUIT
> 
> # Erase whatever stgit managed to apply, then use patch(1)'s more
> # flexible heuristics and capture the output.
> stg diff | patch -p1 -R
> patch -p1 "$@" < "${patch}" > "${tmpfile}" 2>&1
> cat "${tmpfile}"
> 
> # Attach any new files created by the patch
> grep 'create mode' "${patch}" | sed -e 's/^.* mode [0-7]* //g' | while read -r f; do
>         git add "$f"
> done
> 
> # Remove any old files deleted by the patch
> grep 'delete mode' "${patch}" | sed -e 's/^.* mode [0-7]* //g' | while read -r f; do
>         git rm "$f"
> done
> 
> # Force us to deal with the rejects.  Use this instead of "<<<" because
> # the latter picks up empty output as a single line and does variable
> # expansion...  stupid bash.
> readarray -t rej_files < <(grep 'saving rejects to' "${tmpfile}" | \
>                                 sed -e 's/^.*saving rejects to file //g')
> rm -f "${tmpfile}"
> if [ "${#rej_files[@]}" -gt 0 ]; then
>         $VISUAL "${rej_files[@]}"
> fi
> 
> Once you're done fixing the rejects, 'stg refresh' commits the patch and
> you can move on to the next one.
> 
> I hadn't thought about merging the above into libxfs-apply directly
> though.  I should do that.
> 
> Your change for guilt looks ok to me, though I don't have any expertise
> with guilt to know if it's good or not.  However, if it makes your life
> easier then I say go for it. :)

I remember seeing something similar to that one day we were talking about it,
and you showed me a script you wrote, many moons ago, probably the same source
where you got it from.

I actually explicitly mentioned stgit, because I know you use it, so I initially
intended to implement the feature on stgit too, as I thought you might see use
for it, but I gave up when I realized stgit can't import it to the stack only,
at least not without hacking up the script as you mentioned above.

Btw, is this a RwB? :)

I'd wait for Dave's review too, as I think he uses guilt too.

Carlos
> 
> > The order of commits added to $commit_list also needs to be reversed
> > when only importing patches to the guilt stack.
> >
> > Signed-off-by: Carlos Maiolino <cem@kernel.org>
> > ---
> >  tools/libxfs-apply | 22 ++++++++++++++++++++--
> >  1 file changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/libxfs-apply b/tools/libxfs-apply
> > index aa2530f4d..2b65684ec 100755
> > --- a/tools/libxfs-apply
> > +++ b/tools/libxfs-apply
> > @@ -9,7 +9,7 @@ usage()
> >  	echo $*
> >  	echo
> >  	echo "Usage:"
> > -	echo "	libxfs-apply [--verbose] --sob <name/email> --source <repodir> --commit <commit_id>"
> > +	echo "	libxfs-apply [--import-only] [--verbose] --sob <name/email> --source <repodir> --commit <commit_id>"
> >  	echo "	libxfs-apply --patch <patchfile>"
> >  	echo
> >  	echo "libxfs-apply should be run in the destination git repository."
> > @@ -67,6 +67,7 @@ REPO=
> >  PATCH=
> >  COMMIT_ID=
> >  VERBOSE=
> > +IMPORT_ONLY=
> >  GUILT=0
> >  STGIT=0
> >
> > @@ -76,6 +77,7 @@ while [ $# -gt 0 ]; do
> >  	--patch)	PATCH=$2; shift ;;
> >  	--commit)	COMMIT_ID=$2 ; shift ;;
> >  	--sob)		SIGNED_OFF_BY=$2 ; shift ;;
> > +	--import-only)	IMPORT_ONLY=true ;;
> >  	--verbose)	VERBOSE=true ;;
> >  	*)		usage ;;
> >  	esac
> > @@ -99,6 +101,10 @@ if [ $? -eq 0 ]; then
> >  	GUILT=1
> >  fi
> >
> > +if [ -n $IMPORT_ONLY -a $GUILT -ne 1 ]; then
> > +	usage "--import_only can only be used with a guilt stack"
> > +fi
> > +
> >  # Are we using stgit? This works even if no patch is applied.
> >  stg top &> /dev/null
> >  if [ $? -eq 0 ]; then
> > @@ -359,6 +365,11 @@ apply_patch()
> >  		fi
> >
> >  		guilt import -P $_patch_name $_new_patch.2
> > +
> > +		if [ -n "$IMPORT_ONLY" ]; then
> > +			return;
> > +		fi
> > +
> >  		guilt push
> >  		if [ $? -eq 0 ]; then
> >  			guilt refresh
> > @@ -443,10 +454,17 @@ else
> >  	hashr="$hashr -- libxfs"
> >  fi
> >
> > +# When using --import-only, the commit list should be in reverse order.
> > +if [ "$GUILT" -eq 1 -a -n "$IMPORT_ONLY" ]; then
> > +	commit_list=`git rev-list --no-merges $hashr`
> > +else
> > +	commit_list=`git rev-list --no-merges $hashr | tac`
> 
> You probably ought to turn this into a proper bash array:
> 
> readarray -t commit_list < <(git rev-list --no-merges $hashr | tac)
> 
> so that we don't ever run the risk of overflowing line length:
> 
> for commit in "${commit_list[@]}"; do
> 	...
> done
> 
> Though that's probably only a theoretical concern since ARG_MAX is like
> 2 million or something.
> 
> --D
> 
> > +fi
> > +
> >  # grab and echo the list of commits for confirmation
> >  echo "Commits to apply:"
> > -commit_list=`git rev-list --no-merges $hashr | tac`
> >  git log --oneline --no-merges $hashr |tac
> > +
> >  read -r -p "Proceed [y|N]? " response
> >  if [ -z "$response" -o "$response" != "y" ]; then
> >  	fail "Aborted!"
> > --
> > 2.41.0
> >

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

* Re: [PATCH 1/2] libxf-apply: Ignore Merge commits
  2023-11-20 16:51 ` [PATCH 1/2] libxf-apply: Ignore Merge commits Darrick J. Wong
@ 2023-11-20 18:23   ` Carlos Maiolino
  0 siblings, 0 replies; 7+ messages in thread
From: Carlos Maiolino @ 2023-11-20 18:23 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, david, sandeen

On Mon, Nov 20, 2023 at 08:51:51AM -0800, Darrick J. Wong wrote:
> On Mon, Nov 20, 2023 at 04:10:46PM +0100, cem@kernel.org wrote:
> > From: Carlos Maiolino <cem@kernel.org>
> >
> > Merge commits in the kernel tree, only polutes the patch list to be
> > imported into libxfs, explicitly ignore them.
> >
> > Signed-off-by: Carlos Maiolino <cem@kernel.org>
> > ---
> >
> > I'm considering here my own usecase, I never used merge commits, and sometimes
> > they break the synchronization, so they make no good for me during libxfs-sync.
> 
> The downside of ignoring merge commits is that Linus edited
> xfs_rtbitmap.c in the 6.7 merge commit to get rid of the weird code that
> casts a struct timespec64 to a u64 rtpick sequence counter and has
> screwed things up for years.

Well... My curse catches me again... Every time I say "I am not going to use
it", something happens to prove me wrong :)

Thanks for letting me know, I didn't get to 6.7 libxfs-sync yet, so, please,
just ignore this patch.

Carlos
> 
> --D
> 
> >  tools/libxfs-apply | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/libxfs-apply b/tools/libxfs-apply
> > index 097a695f9..aa2530f4d 100755
> > --- a/tools/libxfs-apply
> > +++ b/tools/libxfs-apply
> > @@ -445,8 +445,8 @@ fi
> >
> >  # grab and echo the list of commits for confirmation
> >  echo "Commits to apply:"
> > -commit_list=`git rev-list $hashr | tac`
> > -git log --oneline $hashr |tac
> > +commit_list=`git rev-list --no-merges $hashr | tac`
> > +git log --oneline --no-merges $hashr |tac
> >  read -r -p "Proceed [y|N]? " response
> >  if [ -z "$response" -o "$response" != "y" ]; then
> >  	fail "Aborted!"
> > --
> > 2.41.0
> >

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

* [RFC PATCH 3/2] libxfs-apply: allow stgit users to force-apply a patch
  2023-11-20 15:10 [PATCH 1/2] libxf-apply: Ignore Merge commits cem
  2023-11-20 15:10 ` [PATCH 2/2] libxfs-apply: Add option to only import patches into guilt stack cem
  2023-11-20 16:51 ` [PATCH 1/2] libxf-apply: Ignore Merge commits Darrick J. Wong
@ 2023-11-20 19:50 ` Darrick J. Wong
  2 siblings, 0 replies; 7+ messages in thread
From: Darrick J. Wong @ 2023-11-20 19:50 UTC (permalink / raw)
  To: cem; +Cc: linux-xfs, david, sandeen

From: Darrick J. Wong <djwong@kernel.org>

Currently, libxfs-apply handles merge conflicts in the auto-backported
patches in a somewhat unfriendly way -- either it applies completely
cleanly, or the user has to ^Z, find the raw diff file in /tmp, apply it
by hand, resume the process, and then tell it to skip the patch.

This is annoying, and I've long worked around that by using my handy
stg-force-import script that imports the patch with --reject, undoes the
partially-complete diff, uses patch(1) to import as much of the diff as
possible, and then starts an editor so the caller can clean up the rest.

When patches are fuzzy, patch(1) is /much/ less strict about applying
changes than stg-import.  Since Carlos sent in his own workaround for
guilt, I figured I might as well port stg-force-import into libxfs-apply
and contribute that.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tools/libxfs-apply |   62 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 61 insertions(+), 1 deletion(-)

diff --git a/tools/libxfs-apply b/tools/libxfs-apply
index 097a695f942..3ff46a8cd2b 100755
--- a/tools/libxfs-apply
+++ b/tools/libxfs-apply
@@ -297,6 +297,64 @@ fixup_header_format()
 
 }
 
+editor() {
+	if [ -n "${EDITOR}" ]; then
+		${EDITOR} "$@"
+	elif [ -n "${VISUAL}" ]; then
+		${VISUAL} "$@"
+	elif command -v editor &>/dev/null; then
+		editor "$@"
+	elif command -v nano &>/dev/null; then
+		nano "$@"
+	else
+		echo "No editor available, aborting messily."
+		exit 1
+	fi
+}
+
+stg_force_import()
+{
+	local _patch="$1"
+	local _patch_name="$2"
+
+	# Import patch to get the metadata even though the diff application
+	# might fail due to stg import being very picky.  If the patch applies
+	# cleanly, we're done.
+	stg import --reject -n "${_patch_name}" "${_patch}" && return 0
+
+	local tmpfile="${_patch}.stgit"
+	rm -f "${tmpfile}"
+
+	# Erase whatever stgit managed to apply, then use patch(1)'s more
+	# flexible heuristics.  Capture the output for later use.
+	stg diff | patch -p1 -R
+	patch -p1 < "${patch}" &> "${tmpfile}"
+	cat "${tmpfile}"
+
+	# Attach any new files created by the patch
+	grep 'create mode' "${patch}" | sed -e 's/^.* mode [0-7]* //g' | while read -r f; do
+		git add "$f"
+	done
+
+	# Remove any existing files deleted by the patch
+	grep 'delete mode' "${patch}" | sed -e 's/^.* mode [0-7]* //g' | while read -r f; do
+		git rm "$f"
+	done
+
+	# Open an editor so the user can clean up the rejects.  Use readarray
+	# instead of "<<<" because the latter picks up empty output as a single
+	# line and does variable expansion...  stupid bash.
+	readarray -t rej_files < <(grep 'saving rejects to' "${tmpfile}" | \
+				   sed -e 's/^.*saving rejects to file //g')
+	rm -f "${tmpfile}"
+	if [ "${#rej_files[@]}" -gt 0 ]; then
+		echo "Opening editor to deal with rejects.  Changes commit when you close the editor."
+		editor "${rej_files[@]}"
+	fi
+
+	stg refresh
+}
+
 apply_patch()
 {
 	local _patch=$1
@@ -385,11 +443,13 @@ apply_patch()
 		stg import -n $_patch_name $_new_patch.2
 		if [ $? -ne 0 ]; then
 			echo "stgit push failed!"
-			read -r -p "Skip or Fail [s|F]? " response
+			read -r -p "Skip, force Apply, or Fail [s|a|F]? " response
 			if [ -z "$response" -o "$response" != "s" ]; then
 				echo "Force push patch, fix and refresh."
 				echo "Restart from commit $_current_commit"
 				fail "Manual cleanup required!"
+			elif [ "$response" = "a" ]; then
+				stg_force_import "$_patch_name" "$_new_patch.2"
 			else
 				echo "Skipping. Manual series file cleanup needed!"
 			fi

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

end of thread, other threads:[~2023-11-20 19:50 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-20 15:10 [PATCH 1/2] libxf-apply: Ignore Merge commits cem
2023-11-20 15:10 ` [PATCH 2/2] libxfs-apply: Add option to only import patches into guilt stack cem
2023-11-20 16:49   ` Darrick J. Wong
2023-11-20 18:21     ` Carlos Maiolino
2023-11-20 16:51 ` [PATCH 1/2] libxf-apply: Ignore Merge commits Darrick J. Wong
2023-11-20 18:23   ` Carlos Maiolino
2023-11-20 19:50 ` [RFC PATCH 3/2] libxfs-apply: allow stgit users to force-apply a patch Darrick J. Wong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox