diff mbox series

[1/5] libxfs-apply: don't add duplicate headers

Message ID 160633668210.634603.16132006317248436755.stgit@magnolia (mailing list archive)
State Accepted, archived
Headers show
Series xfsprogs: fixes for 5.10-rc1 | expand

Commit Message

Darrick J. Wong Nov. 25, 2020, 8:38 p.m. UTC 
From: Darrick J. Wong <darrick.wong@oracle.com>

When we're backporting patches from libxfs, don't add a S-o-b header if
there's already one at the end of the headers of the patch being ported.

That way, we avoid things like:
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tools/libxfs-apply |   14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Comments

Eric Sandeen Nov. 25, 2020, 9:12 p.m. UTC | #1 
On 11/25/20 2:38 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> When we're backporting patches from libxfs, don't add a S-o-b header if
> there's already one at the end of the headers of the patch being ported.
> 
> That way, we avoid things like:
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

But it will still not add my additional SOB if I merge something across to
userspace that starts out with:

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

And I always felt like the committer should be adding their SOB to the end of
the chain when they move code from one place to another, especially if any
tweaks got made along the way.

IOWs I see the rationale for removing duplicate /sequential/ SOBs, but not
for removing duplicate SOBs in general.

Am I thinking about that wrong?

> ---
>  tools/libxfs-apply |   14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/tools/libxfs-apply b/tools/libxfs-apply
> index 3258272d6189..9271db380198 100755
> --- a/tools/libxfs-apply
> +++ b/tools/libxfs-apply
> @@ -193,6 +193,14 @@ filter_xfsprogs_patch()
>  	rm -f $_libxfs_files
>  }
>  
> +add_header()
> +{
> +	local hdr="$1"
> +	local hdrfile="$2"
> +
> +	tail -n 1 "$hdrfile" | grep -q "^${hdr}$" || echo "$hdr" >> "$hdrfile"
> +}
> +
>  fixup_header_format()
>  {
>  	local _source=$1
> @@ -280,13 +288,13 @@ fixup_header_format()
>  	sed -i '${/^[[:space:]]*$/d;}' $_hdr.new
>  
>  	# Add Signed-off-by: header if specified
> -	if [ ! -z ${SIGNED_OFF_BY+x} ]; then 
> -		echo "Signed-off-by: $SIGNED_OFF_BY" >> $_hdr.new
> +	if [ ! -z ${SIGNED_OFF_BY+x} ]; then
> +		add_header "Signed-off-by: $SIGNED_OFF_BY" $_hdr.new
>  	else	# get it from git config if present
>  		SOB_NAME=`git config --get user.name`
>  		SOB_EMAIL=`git config --get user.email`
>  		if [ ! -z ${SOB_NAME+x} ]; then
> -			echo "Signed-off-by: $SOB_NAME <$SOB_EMAIL>" >> $_hdr.new
> +			add_header "Signed-off-by: $SOB_NAME <$SOB_EMAIL>" $_hdr.new
>  		fi
>  	fi
>  
>
Darrick J. Wong Nov. 30, 2020, 5:13 p.m. UTC | #2 
On Wed, Nov 25, 2020 at 03:12:39PM -0600, Eric Sandeen wrote:
> On 11/25/20 2:38 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > When we're backporting patches from libxfs, don't add a S-o-b header if
> > there's already one at the end of the headers of the patch being ported.
> > 
> > That way, we avoid things like:
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> But it will still not add my additional SOB if I merge something across to
> userspace that starts out with:
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> And I always felt like the committer should be adding their SOB to the end of
> the chain when they move code from one place to another, especially if any
> tweaks got made along the way.
> 
> IOWs I see the rationale for removing duplicate /sequential/ SOBs, but not
> for removing duplicate SOBs in general.
> 
> Am I thinking about that wrong?

add_header is different from last time -- whereas before it would search
the entire $hdrfile for $hdr and add $hdr if it didn't find a match, now
it only looks at the last line of $hdrfile for a match.

For your example above, it would not add another "SOB: Darrick" because
there's already one at the bottom of the headers; but it would add
another "SOB: Eric" because this commit acquired other tags after your
first signoff.

--D

> > ---
> >  tools/libxfs-apply |   14 +++++++++++---
> >  1 file changed, 11 insertions(+), 3 deletions(-)
> > 
> > 
> > diff --git a/tools/libxfs-apply b/tools/libxfs-apply
> > index 3258272d6189..9271db380198 100755
> > --- a/tools/libxfs-apply
> > +++ b/tools/libxfs-apply
> > @@ -193,6 +193,14 @@ filter_xfsprogs_patch()
> >  	rm -f $_libxfs_files
> >  }
> >  
> > +add_header()
> > +{
> > +	local hdr="$1"
> > +	local hdrfile="$2"
> > +
> > +	tail -n 1 "$hdrfile" | grep -q "^${hdr}$" || echo "$hdr" >> "$hdrfile"
> > +}
> > +
> >  fixup_header_format()
> >  {
> >  	local _source=$1
> > @@ -280,13 +288,13 @@ fixup_header_format()
> >  	sed -i '${/^[[:space:]]*$/d;}' $_hdr.new
> >  
> >  	# Add Signed-off-by: header if specified
> > -	if [ ! -z ${SIGNED_OFF_BY+x} ]; then 
> > -		echo "Signed-off-by: $SIGNED_OFF_BY" >> $_hdr.new
> > +	if [ ! -z ${SIGNED_OFF_BY+x} ]; then
> > +		add_header "Signed-off-by: $SIGNED_OFF_BY" $_hdr.new
> >  	else	# get it from git config if present
> >  		SOB_NAME=`git config --get user.name`
> >  		SOB_EMAIL=`git config --get user.email`
> >  		if [ ! -z ${SOB_NAME+x} ]; then
> > -			echo "Signed-off-by: $SOB_NAME <$SOB_EMAIL>" >> $_hdr.new
> > +			add_header "Signed-off-by: $SOB_NAME <$SOB_EMAIL>" $_hdr.new
> >  		fi
> >  	fi
> >  
> >
Eric Sandeen Nov. 30, 2020, 5:16 p.m. UTC | #3 
On 11/30/20 11:13 AM, Darrick J. Wong wrote:
> On Wed, Nov 25, 2020 at 03:12:39PM -0600, Eric Sandeen wrote:
>> On 11/25/20 2:38 PM, Darrick J. Wong wrote:
>>> From: Darrick J. Wong <darrick.wong@oracle.com>
>>>
>>> When we're backporting patches from libxfs, don't add a S-o-b header if
>>> there's already one at the end of the headers of the patch being ported.
>>>
>>> That way, we avoid things like:
>>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>>
>> But it will still not add my additional SOB if I merge something across to
>> userspace that starts out with:
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> Reviewed-by: Dave Chinner <dchinner@redhat.com>
>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>>
>> And I always felt like the committer should be adding their SOB to the end of
>> the chain when they move code from one place to another, especially if any
>> tweaks got made along the way.
>>
>> IOWs I see the rationale for removing duplicate /sequential/ SOBs, but not
>> for removing duplicate SOBs in general.
>>
>> Am I thinking about that wrong?
> 
> add_header is different from last time -- whereas before it would search
> the entire $hdrfile for $hdr and add $hdr if it didn't find a match, now
> it only looks at the last line of $hdrfile for a match.

I'm sorry, I visually checked and thought it was same as last time.  Will
look again, more closely.

Thanks,
-Eric

> For your example above, it would not add another "SOB: Darrick" because
> there's already one at the bottom of the headers; but it would add
> another "SOB: Eric" because this commit acquired other tags after your
> first signoff.
> 
> --D
> 
>>> ---
>>>  tools/libxfs-apply |   14 +++++++++++---
>>>  1 file changed, 11 insertions(+), 3 deletions(-)
>>>
>>>
>>> diff --git a/tools/libxfs-apply b/tools/libxfs-apply
>>> index 3258272d6189..9271db380198 100755
>>> --- a/tools/libxfs-apply
>>> +++ b/tools/libxfs-apply
>>> @@ -193,6 +193,14 @@ filter_xfsprogs_patch()
>>>  	rm -f $_libxfs_files
>>>  }
>>>  
>>> +add_header()
>>> +{
>>> +	local hdr="$1"
>>> +	local hdrfile="$2"
>>> +
>>> +	tail -n 1 "$hdrfile" | grep -q "^${hdr}$" || echo "$hdr" >> "$hdrfile"
>>> +}
>>> +
>>>  fixup_header_format()
>>>  {
>>>  	local _source=$1
>>> @@ -280,13 +288,13 @@ fixup_header_format()
>>>  	sed -i '${/^[[:space:]]*$/d;}' $_hdr.new
>>>  
>>>  	# Add Signed-off-by: header if specified
>>> -	if [ ! -z ${SIGNED_OFF_BY+x} ]; then 
>>> -		echo "Signed-off-by: $SIGNED_OFF_BY" >> $_hdr.new
>>> +	if [ ! -z ${SIGNED_OFF_BY+x} ]; then
>>> +		add_header "Signed-off-by: $SIGNED_OFF_BY" $_hdr.new
>>>  	else	# get it from git config if present
>>>  		SOB_NAME=`git config --get user.name`
>>>  		SOB_EMAIL=`git config --get user.email`
>>>  		if [ ! -z ${SOB_NAME+x} ]; then
>>> -			echo "Signed-off-by: $SOB_NAME <$SOB_EMAIL>" >> $_hdr.new
>>> +			add_header "Signed-off-by: $SOB_NAME <$SOB_EMAIL>" $_hdr.new
>>>  		fi
>>>  	fi
>>>  
>>>
>
Eric Sandeen Dec. 4, 2020, 5:16 p.m. UTC | #4 
On 11/25/20 2:38 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> When we're backporting patches from libxfs, don't add a S-o-b header if
> there's already one at the end of the headers of the patch being ported.
> 
> That way, we avoid things like:
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

I like how that SOB is part of the changelog and is also part of
the changelog ;)

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  tools/libxfs-apply |   14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/tools/libxfs-apply b/tools/libxfs-apply
> index 3258272d6189..9271db380198 100755
> --- a/tools/libxfs-apply
> +++ b/tools/libxfs-apply
> @@ -193,6 +193,14 @@ filter_xfsprogs_patch()
>  	rm -f $_libxfs_files
>  }
>  
> +add_header()
> +{
> +	local hdr="$1"
> +	local hdrfile="$2"
> +
> +	tail -n 1 "$hdrfile" | grep -q "^${hdr}$" || echo "$hdr" >> "$hdrfile"
> +}
> +
>  fixup_header_format()
>  {
>  	local _source=$1
> @@ -280,13 +288,13 @@ fixup_header_format()
>  	sed -i '${/^[[:space:]]*$/d;}' $_hdr.new
>  
>  	# Add Signed-off-by: header if specified
> -	if [ ! -z ${SIGNED_OFF_BY+x} ]; then 
> -		echo "Signed-off-by: $SIGNED_OFF_BY" >> $_hdr.new
> +	if [ ! -z ${SIGNED_OFF_BY+x} ]; then
> +		add_header "Signed-off-by: $SIGNED_OFF_BY" $_hdr.new
>  	else	# get it from git config if present
>  		SOB_NAME=`git config --get user.name`
>  		SOB_EMAIL=`git config --get user.email`
>  		if [ ! -z ${SOB_NAME+x} ]; then
> -			echo "Signed-off-by: $SOB_NAME <$SOB_EMAIL>" >> $_hdr.new
> +			add_header "Signed-off-by: $SOB_NAME <$SOB_EMAIL>" $_hdr.new
>  		fi
>  	fi
>  
>
diff mbox series

Patch

diff --git a/tools/libxfs-apply b/tools/libxfs-apply
index 3258272d6189..9271db380198 100755
--- a/tools/libxfs-apply
+++ b/tools/libxfs-apply
@@ -193,6 +193,14 @@  filter_xfsprogs_patch()
 	rm -f $_libxfs_files
 }
 
+add_header()
+{
+	local hdr="$1"
+	local hdrfile="$2"
+
+	tail -n 1 "$hdrfile" | grep -q "^${hdr}$" || echo "$hdr" >> "$hdrfile"
+}
+
 fixup_header_format()
 {
 	local _source=$1
@@ -280,13 +288,13 @@  fixup_header_format()
 	sed -i '${/^[[:space:]]*$/d;}' $_hdr.new
 
 	# Add Signed-off-by: header if specified
-	if [ ! -z ${SIGNED_OFF_BY+x} ]; then 
-		echo "Signed-off-by: $SIGNED_OFF_BY" >> $_hdr.new
+	if [ ! -z ${SIGNED_OFF_BY+x} ]; then
+		add_header "Signed-off-by: $SIGNED_OFF_BY" $_hdr.new
 	else	# get it from git config if present
 		SOB_NAME=`git config --get user.name`
 		SOB_EMAIL=`git config --get user.email`
 		if [ ! -z ${SOB_NAME+x} ]; then
-			echo "Signed-off-by: $SOB_NAME <$SOB_EMAIL>" >> $_hdr.new
+			add_header "Signed-off-by: $SOB_NAME <$SOB_EMAIL>" $_hdr.new
 		fi
 	fi