* [PATCH] patman: Add a 'keep_change_id' setting
@ 2023-10-13 1:19 Maxim Cournoyer
2023-10-13 3:06 ` [PATCH v2] " Maxim Cournoyer
0 siblings, 1 reply; 5+ messages in thread
From: Maxim Cournoyer @ 2023-10-13 1:19 UTC (permalink / raw)
To: u-boot; +Cc: Maxim Cournoyer, Simon Glass
A Change-Id can be useful for traceability purposes, and some projects
may wish to have them preserved. This change makes it configurable
via a new 'keep_change_id' setting.
Signed-off-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
---
tools/patman/control.py | 9 +++++++--
tools/patman/patchstream.py | 17 ++++++++++++-----
tools/patman/patman.rst | 11 ++++++-----
tools/patman/test_checkpatch.py | 16 ++++++++++++++++
4 files changed, 41 insertions(+), 12 deletions(-)
diff --git a/tools/patman/control.py b/tools/patman/control.py
index 916ddf8fcf..29383551ed 100644
--- a/tools/patman/control.py
+++ b/tools/patman/control.py
@@ -16,11 +16,14 @@ from patman import gitutil
from patman import patchstream
from u_boot_pylib import terminal
+
def setup():
"""Do required setup before doing anything"""
gitutil.setup()
-def prepare_patches(col, branch, count, start, end, ignore_binary, signoff):
+
+def prepare_patches(col, branch, count, start, end, ignore_binary, signoff,
+ keep_change_id=False):
"""Figure out what patches to generate, then generate them
The patch files are written to the current directory, e.g. 0001_xxx.patch
@@ -35,6 +38,7 @@ def prepare_patches(col, branch, count, start, end, ignore_binary, signoff):
end (int): End patch to use (0=last one in series, 1=one before that,
etc.)
ignore_binary (bool): Don't generate patches for binary files
+ keep_change_id (bool): Preserve the Change-Id tag.
Returns:
Tuple:
@@ -59,11 +63,12 @@ def prepare_patches(col, branch, count, start, end, ignore_binary, signoff):
branch, start, to_do, ignore_binary, series, signoff)
# Fix up the patch files to our liking, and insert the cover letter
- patchstream.fix_patches(series, patch_files)
+ patchstream.fix_patches(series, patch_files, keep_change_id)
if cover_fname and series.get('cover'):
patchstream.insert_cover_letter(cover_fname, series, to_do)
return series, cover_fname, patch_files
+
def check_patches(series, patch_files, run_checkpatch, verbose, use_tree):
"""Run some checks on a set of patches
diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
index f91669a940..4188d0ed35 100644
--- a/tools/patman/patchstream.py
+++ b/tools/patman/patchstream.py
@@ -68,6 +68,7 @@ STATE_PATCH_SUBJECT = 1 # In patch subject (first line of log for a commit)
STATE_PATCH_HEADER = 2 # In patch header (after the subject)
STATE_DIFFS = 3 # In the diff part (past --- line)
+
class PatchStream:
"""Class for detecting/injecting tags in a patch or series of patches
@@ -76,7 +77,7 @@ class PatchStream:
unwanted tags or inject additional ones. These correspond to the two
phases of processing.
"""
- def __init__(self, series, is_log=False):
+ def __init__(self, series, is_log=False, keep_change_id=False):
self.skip_blank = False # True to skip a single blank line
self.found_test = False # Found a TEST= line
self.lines_after_test = 0 # Number of lines found after TEST=
@@ -86,6 +87,7 @@ class PatchStream:
self.section = [] # The current section...END section
self.series = series # Info about the patch series
self.is_log = is_log # True if indent like git log
+ self.keep_change_id = keep_change_id # True to keep Change-Id tags
self.in_change = None # Name of the change list we are in
self.change_version = 0 # Non-zero if we are in a change list
self.change_lines = [] # Lines of the current change
@@ -452,6 +454,8 @@ class PatchStream:
# Detect Change-Id tags
elif change_id_match:
+ if self.keep_change_id:
+ out = [line]
value = change_id_match.group(1)
if self.is_log:
if self.commit.change_id:
@@ -763,7 +767,7 @@ def get_metadata_for_test(text):
pst.finalise()
return series
-def fix_patch(backup_dir, fname, series, cmt):
+def fix_patch(backup_dir, fname, series, cmt, keep_change_id=False):
"""Fix up a patch file, by adding/removing as required.
We remove our tags from the patch file, insert changes lists, etc.
@@ -776,6 +780,7 @@ def fix_patch(backup_dir, fname, series, cmt):
fname (str): Filename to patch file to process
series (Series): Series information about this patch set
cmt (Commit): Commit object for this patch file
+ keep_change_id (bool): Keep the Change-Id tag.
Return:
list: A list of errors, each str, or [] if all ok.
@@ -783,7 +788,7 @@ def fix_patch(backup_dir, fname, series, cmt):
handle, tmpname = tempfile.mkstemp()
outfd = os.fdopen(handle, 'w', encoding='utf-8')
infd = open(fname, 'r', encoding='utf-8')
- pst = PatchStream(series)
+ pst = PatchStream(series, keep_change_id=keep_change_id)
pst.commit = cmt
pst.process_stream(infd, outfd)
infd.close()
@@ -795,7 +800,7 @@ def fix_patch(backup_dir, fname, series, cmt):
shutil.move(tmpname, fname)
return cmt.warn
-def fix_patches(series, fnames):
+def fix_patches(series, fnames, keep_change_id=False):
"""Fix up a list of patches identified by filenames
The patch files are processed in place, and overwritten.
@@ -803,6 +808,7 @@ def fix_patches(series, fnames):
Args:
series (Series): The Series object
fnames (:type: list of str): List of patch files to process
+ keep_change_id (bool): Keep the Change-Id tag.
"""
# Current workflow creates patches, so we shouldn't need a backup
backup_dir = None #tempfile.mkdtemp('clean-patch')
@@ -811,7 +817,8 @@ def fix_patches(series, fnames):
cmt = series.commits[count]
cmt.patch = fname
cmt.count = count
- result = fix_patch(backup_dir, fname, series, cmt)
+ result = fix_patch(backup_dir, fname, series, cmt,
+ keep_change_id=False)
if result:
print('%d warning%s for %s:' %
(len(result), 's' if len(result) > 1 else '', fname))
diff --git a/tools/patman/patman.rst b/tools/patman/patman.rst
index 038b651ee8..a8b317eed6 100644
--- a/tools/patman/patman.rst
+++ b/tools/patman/patman.rst
@@ -371,11 +371,12 @@ Series-process-log: sort, uniq
Separate each tag with a comma.
Change-Id:
- This tag is stripped out but is used to generate the Message-Id
- of the emails that will be sent. When you keep the Change-Id the
- same you are asserting that this is a slightly different version
- (but logically the same patch) as other patches that have been
- sent out with the same Change-Id.
+ This tag is used to generate the Message-Id of the emails that
+ will be sent. When you keep the Change-Id the same you are
+ asserting that this is a slightly different version (but logically
+ the same patch) as other patches that have been sent out with the
+ same Change-Id. The Change-Id tag line is removed from outgoing
+ patches, unless the `keep_change_id` settings is set to `True`.
Various other tags are silently removed, like these Chrome OS and
Gerrit tags::
diff --git a/tools/patman/test_checkpatch.py b/tools/patman/test_checkpatch.py
index a8bb364e42..59a53ef8ca 100644
--- a/tools/patman/test_checkpatch.py
+++ b/tools/patman/test_checkpatch.py
@@ -160,6 +160,22 @@ Signed-off-by: Simon Glass <sjg@chromium.org>
rc = os.system('diff -u %s %s' % (inname, expname))
self.assertEqual(rc, 0)
+ os.remove(inname)
+
+ # Test whether the keep_change_id settings works.
+ inhandle, inname = tempfile.mkstemp()
+ infd = os.fdopen(inhandle, 'w', encoding='utf-8')
+ infd.write(data)
+ infd.close()
+
+ patchstream.fix_patch(None, inname, series.Series(), com,
+ keep_change_id=True)
+
+ with open(inname, 'r') as f:
+ content = f.read()
+ self.assertIn(
+ 'Change-Id: I80fe1d0c0b7dd10aa58ce5bb1d9290b6664d5413',
+ content)
os.remove(inname)
os.remove(expname)
base-commit: f9a47ac8d97da2b3aaf463f268a9a872a8d921df
--
2.41.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2] patman: Add a 'keep_change_id' setting
2023-10-13 1:19 [PATCH] patman: Add a 'keep_change_id' setting Maxim Cournoyer
@ 2023-10-13 3:06 ` Maxim Cournoyer
2023-10-13 16:57 ` Simon Glass
0 siblings, 1 reply; 5+ messages in thread
From: Maxim Cournoyer @ 2023-10-13 3:06 UTC (permalink / raw)
To: u-boot; +Cc: Maxim Cournoyer, Simon Glass
A Change-Id can be useful for traceability purposes, and some projects
may wish to have them preserved. This change makes it configurable
via a new 'keep_change_id' setting.
Signed-off-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
---
Changes in v2:
- Add missing argument to send parser
- Correctly propagate args.keep_change_id
tools/patman/__main__.py | 2 ++
tools/patman/control.py | 12 +++++++++---
tools/patman/patchstream.py | 17 ++++++++++++-----
tools/patman/patman.rst | 11 ++++++-----
tools/patman/test_checkpatch.py | 16 ++++++++++++++++
5 files changed, 45 insertions(+), 13 deletions(-)
diff --git a/tools/patman/__main__.py b/tools/patman/__main__.py
index 8eba5d3486..197ac1aad1 100755
--- a/tools/patman/__main__.py
+++ b/tools/patman/__main__.py
@@ -103,6 +103,8 @@ send.add_argument('--no-signoff', action='store_false', dest='add_signoff',
default=True, help="Don't add Signed-off-by to patches")
send.add_argument('--smtp-server', type=str,
help="Specify the SMTP server to 'git send-email'")
+send.add_argument('--keep-change-id', action='store_true',
+ help='Preserve Change-Id tags in patches to send.')
send.add_argument('patchfiles', nargs='*')
diff --git a/tools/patman/control.py b/tools/patman/control.py
index 916ddf8fcf..b292da9dc2 100644
--- a/tools/patman/control.py
+++ b/tools/patman/control.py
@@ -16,11 +16,14 @@ from patman import gitutil
from patman import patchstream
from u_boot_pylib import terminal
+
def setup():
"""Do required setup before doing anything"""
gitutil.setup()
-def prepare_patches(col, branch, count, start, end, ignore_binary, signoff):
+
+def prepare_patches(col, branch, count, start, end, ignore_binary, signoff,
+ keep_change_id=False):
"""Figure out what patches to generate, then generate them
The patch files are written to the current directory, e.g. 0001_xxx.patch
@@ -35,6 +38,7 @@ def prepare_patches(col, branch, count, start, end, ignore_binary, signoff):
end (int): End patch to use (0=last one in series, 1=one before that,
etc.)
ignore_binary (bool): Don't generate patches for binary files
+ keep_change_id (bool): Preserve the Change-Id tag.
Returns:
Tuple:
@@ -59,11 +63,12 @@ def prepare_patches(col, branch, count, start, end, ignore_binary, signoff):
branch, start, to_do, ignore_binary, series, signoff)
# Fix up the patch files to our liking, and insert the cover letter
- patchstream.fix_patches(series, patch_files)
+ patchstream.fix_patches(series, patch_files, keep_change_id)
if cover_fname and series.get('cover'):
patchstream.insert_cover_letter(cover_fname, series, to_do)
return series, cover_fname, patch_files
+
def check_patches(series, patch_files, run_checkpatch, verbose, use_tree):
"""Run some checks on a set of patches
@@ -166,7 +171,8 @@ def send(args):
col = terminal.Color()
series, cover_fname, patch_files = prepare_patches(
col, args.branch, args.count, args.start, args.end,
- args.ignore_binary, args.add_signoff)
+ args.ignore_binary, args.add_signoff,
+ keep_change_id=args.keep_change_id)
ok = check_patches(series, patch_files, args.check_patch,
args.verbose, args.check_patch_use_tree)
diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
index f91669a940..e2e2a83e67 100644
--- a/tools/patman/patchstream.py
+++ b/tools/patman/patchstream.py
@@ -68,6 +68,7 @@ STATE_PATCH_SUBJECT = 1 # In patch subject (first line of log for a commit)
STATE_PATCH_HEADER = 2 # In patch header (after the subject)
STATE_DIFFS = 3 # In the diff part (past --- line)
+
class PatchStream:
"""Class for detecting/injecting tags in a patch or series of patches
@@ -76,7 +77,7 @@ class PatchStream:
unwanted tags or inject additional ones. These correspond to the two
phases of processing.
"""
- def __init__(self, series, is_log=False):
+ def __init__(self, series, is_log=False, keep_change_id=False):
self.skip_blank = False # True to skip a single blank line
self.found_test = False # Found a TEST= line
self.lines_after_test = 0 # Number of lines found after TEST=
@@ -86,6 +87,7 @@ class PatchStream:
self.section = [] # The current section...END section
self.series = series # Info about the patch series
self.is_log = is_log # True if indent like git log
+ self.keep_change_id = keep_change_id # True to keep Change-Id tags
self.in_change = None # Name of the change list we are in
self.change_version = 0 # Non-zero if we are in a change list
self.change_lines = [] # Lines of the current change
@@ -452,6 +454,8 @@ class PatchStream:
# Detect Change-Id tags
elif change_id_match:
+ if self.keep_change_id:
+ out = [line]
value = change_id_match.group(1)
if self.is_log:
if self.commit.change_id:
@@ -763,7 +767,7 @@ def get_metadata_for_test(text):
pst.finalise()
return series
-def fix_patch(backup_dir, fname, series, cmt):
+def fix_patch(backup_dir, fname, series, cmt, keep_change_id=False):
"""Fix up a patch file, by adding/removing as required.
We remove our tags from the patch file, insert changes lists, etc.
@@ -776,6 +780,7 @@ def fix_patch(backup_dir, fname, series, cmt):
fname (str): Filename to patch file to process
series (Series): Series information about this patch set
cmt (Commit): Commit object for this patch file
+ keep_change_id (bool): Keep the Change-Id tag.
Return:
list: A list of errors, each str, or [] if all ok.
@@ -783,7 +788,7 @@ def fix_patch(backup_dir, fname, series, cmt):
handle, tmpname = tempfile.mkstemp()
outfd = os.fdopen(handle, 'w', encoding='utf-8')
infd = open(fname, 'r', encoding='utf-8')
- pst = PatchStream(series)
+ pst = PatchStream(series, keep_change_id=keep_change_id)
pst.commit = cmt
pst.process_stream(infd, outfd)
infd.close()
@@ -795,7 +800,7 @@ def fix_patch(backup_dir, fname, series, cmt):
shutil.move(tmpname, fname)
return cmt.warn
-def fix_patches(series, fnames):
+def fix_patches(series, fnames, keep_change_id=False):
"""Fix up a list of patches identified by filenames
The patch files are processed in place, and overwritten.
@@ -803,6 +808,7 @@ def fix_patches(series, fnames):
Args:
series (Series): The Series object
fnames (:type: list of str): List of patch files to process
+ keep_change_id (bool): Keep the Change-Id tag.
"""
# Current workflow creates patches, so we shouldn't need a backup
backup_dir = None #tempfile.mkdtemp('clean-patch')
@@ -811,7 +817,8 @@ def fix_patches(series, fnames):
cmt = series.commits[count]
cmt.patch = fname
cmt.count = count
- result = fix_patch(backup_dir, fname, series, cmt)
+ result = fix_patch(backup_dir, fname, series, cmt,
+ keep_change_id=keep_change_id)
if result:
print('%d warning%s for %s:' %
(len(result), 's' if len(result) > 1 else '', fname))
diff --git a/tools/patman/patman.rst b/tools/patman/patman.rst
index 038b651ee8..a8b317eed6 100644
--- a/tools/patman/patman.rst
+++ b/tools/patman/patman.rst
@@ -371,11 +371,12 @@ Series-process-log: sort, uniq
Separate each tag with a comma.
Change-Id:
- This tag is stripped out but is used to generate the Message-Id
- of the emails that will be sent. When you keep the Change-Id the
- same you are asserting that this is a slightly different version
- (but logically the same patch) as other patches that have been
- sent out with the same Change-Id.
+ This tag is used to generate the Message-Id of the emails that
+ will be sent. When you keep the Change-Id the same you are
+ asserting that this is a slightly different version (but logically
+ the same patch) as other patches that have been sent out with the
+ same Change-Id. The Change-Id tag line is removed from outgoing
+ patches, unless the `keep_change_id` settings is set to `True`.
Various other tags are silently removed, like these Chrome OS and
Gerrit tags::
diff --git a/tools/patman/test_checkpatch.py b/tools/patman/test_checkpatch.py
index a8bb364e42..59a53ef8ca 100644
--- a/tools/patman/test_checkpatch.py
+++ b/tools/patman/test_checkpatch.py
@@ -160,6 +160,22 @@ Signed-off-by: Simon Glass <sjg@chromium.org>
rc = os.system('diff -u %s %s' % (inname, expname))
self.assertEqual(rc, 0)
+ os.remove(inname)
+
+ # Test whether the keep_change_id settings works.
+ inhandle, inname = tempfile.mkstemp()
+ infd = os.fdopen(inhandle, 'w', encoding='utf-8')
+ infd.write(data)
+ infd.close()
+
+ patchstream.fix_patch(None, inname, series.Series(), com,
+ keep_change_id=True)
+
+ with open(inname, 'r') as f:
+ content = f.read()
+ self.assertIn(
+ 'Change-Id: I80fe1d0c0b7dd10aa58ce5bb1d9290b6664d5413',
+ content)
os.remove(inname)
os.remove(expname)
base-commit: f9a47ac8d97da2b3aaf463f268a9a872a8d921df
--
2.41.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] patman: Add a 'keep_change_id' setting
2023-10-13 3:06 ` [PATCH v2] " Maxim Cournoyer
@ 2023-10-13 16:57 ` Simon Glass
2023-11-02 22:49 ` Simon Glass
0 siblings, 1 reply; 5+ messages in thread
From: Simon Glass @ 2023-10-13 16:57 UTC (permalink / raw)
To: Maxim Cournoyer; +Cc: u-boot
On Thu, 12 Oct 2023 at 20:06, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> A Change-Id can be useful for traceability purposes, and some projects
> may wish to have them preserved. This change makes it configurable
> via a new 'keep_change_id' setting.
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
> ---
>
> Changes in v2:
> - Add missing argument to send parser
> - Correctly propagate args.keep_change_id
>
> tools/patman/__main__.py | 2 ++
> tools/patman/control.py | 12 +++++++++---
> tools/patman/patchstream.py | 17 ++++++++++++-----
> tools/patman/patman.rst | 11 ++++++-----
> tools/patman/test_checkpatch.py | 16 ++++++++++++++++
> 5 files changed, 45 insertions(+), 13 deletions(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] patman: Add a 'keep_change_id' setting
2023-10-13 16:57 ` Simon Glass
@ 2023-11-02 22:49 ` Simon Glass
2023-11-03 0:30 ` Maxim Cournoyer
0 siblings, 1 reply; 5+ messages in thread
From: Simon Glass @ 2023-11-02 22:49 UTC (permalink / raw)
To: Simon Glass; +Cc: u-boot, Maxim Cournoyer
On Thu, 12 Oct 2023 at 20:06, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> A Change-Id can be useful for traceability purposes, and some projects
> may wish to have them preserved. This change makes it configurable
> via a new 'keep_change_id' setting.
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
> ---
>
> Changes in v2:
> - Add missing argument to send parser
> - Correctly propagate args.keep_change_id
>
> tools/patman/__main__.py | 2 ++
> tools/patman/control.py | 12 +++++++++---
> tools/patman/patchstream.py | 17 ++++++++++++-----
> tools/patman/patman.rst | 11 ++++++-----
> tools/patman/test_checkpatch.py | 16 ++++++++++++++++
> 5 files changed, 45 insertions(+), 13 deletions(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
Applied to u-boot-dm, thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] patman: Add a 'keep_change_id' setting
2023-11-02 22:49 ` Simon Glass
@ 2023-11-03 0:30 ` Maxim Cournoyer
0 siblings, 0 replies; 5+ messages in thread
From: Maxim Cournoyer @ 2023-11-03 0:30 UTC (permalink / raw)
To: Simon Glass; +Cc: u-boot
Hi Simon,
Simon Glass <sjg@chromium.org> writes:
> On Thu, 12 Oct 2023 at 20:06, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>>
>> A Change-Id can be useful for traceability purposes, and some projects
>> may wish to have them preserved. This change makes it configurable
>> via a new 'keep_change_id' setting.
>>
>> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
>> ---
>>
>> Changes in v2:
>> - Add missing argument to send parser
>> - Correctly propagate args.keep_change_id
>>
>> tools/patman/__main__.py | 2 ++
>> tools/patman/control.py | 12 +++++++++---
>> tools/patman/patchstream.py | 17 ++++++++++++-----
>> tools/patman/patman.rst | 11 ++++++-----
>> tools/patman/test_checkpatch.py | 16 ++++++++++++++++
>> 5 files changed, 45 insertions(+), 13 deletions(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> Applied to u-boot-dm, thanks!
And thanks for the timely review! :-)
--
Maxim
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-11-03 0:30 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-13 1:19 [PATCH] patman: Add a 'keep_change_id' setting Maxim Cournoyer
2023-10-13 3:06 ` [PATCH v2] " Maxim Cournoyer
2023-10-13 16:57 ` Simon Glass
2023-11-02 22:49 ` Simon Glass
2023-11-03 0:30 ` Maxim Cournoyer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox