[PATCH] gnu: patman: Fix incomplete get-maintainer patch.

  • Done
  • quality assurance status badge
Details
2 participants
  • Jelle Licht
  • Maxim Cournoyer
Owner
unassigned
Submitted by
Maxim Cournoyer
Severity
normal
M
M
Maxim Cournoyer wrote on 5 Jan 2023 17:59
(address . guix-patches@gnu.org)
20230105165905.12649-1-maxim.cournoyer@gmail.com
The full set of patches already merged into the U-Boot is included for
simplicity and avoiding mistakes like the previous one, where a conflicting
hunk got dropped, causing the following error when running patman:

WARNING: Unknown setting get_maintainer_script

* gnu/packages/bootloaders.scm (u-boot) [source]: Replace the
u-boot-patman-fix-help.patch, u-boot-patman-local-conf.patch and
u-boot-patman-get-maintainer.patch with u-boot-patman-guix-integration.patch.
* gnu/local.mk (dist_patch_DATA): Update patch registrations.
* gnu/packages/patches/u-boot-patman-fix-help.patch: Delete file.
* gnu/packages/patches/u-boot-patman-get-maintainer.patch: Likewise.
* gnu/packages/patches/u-boot-patman-local-conf.patch: Likewise.
* gnu/packages/patches/u-boot-patman-guix-integration.patch: New file.

Reported-by: Jelle Licht <jlicht@fsfe.org>
---

gnu/local.mk | 4 +-
gnu/packages/bootloaders.scm | 4 +-
.../patches/u-boot-patman-fix-help.patch | 40 -
.../u-boot-patman-get-maintainer.patch | 104 --
.../u-boot-patman-guix-integration.patch | 1244 +++++++++++++++++
.../patches/u-boot-patman-local-conf.patch | 176 ---
6 files changed, 1246 insertions(+), 326 deletions(-)
delete mode 100644 gnu/packages/patches/u-boot-patman-fix-help.patch
delete mode 100644 gnu/packages/patches/u-boot-patman-get-maintainer.patch
create mode 100644 gnu/packages/patches/u-boot-patman-guix-integration.patch
delete mode 100644 gnu/packages/patches/u-boot-patman-local-conf.patch

Toggle diff (58 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index bc0309ee87..4afe54a92f 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1916,9 +1916,7 @@ dist_patch_DATA = \
%D%/packages/patches/twinkle-bcg729.patch \
%D%/packages/patches/u-boot-allow-disabling-openssl.patch \
%D%/packages/patches/u-boot-infodocs-target.patch \
- %D%/packages/patches/u-boot-patman-fix-help.patch \
- %D%/packages/patches/u-boot-patman-get-maintainer.patch \
- %D%/packages/patches/u-boot-patman-local-conf.patch \
+ %D%/packages/patches/u-boot-patman-guix-integration.patch \
%D%/packages/patches/u-boot-nintendo-nes-serial.patch \
%D%/packages/patches/u-boot-rockchip-inno-usb.patch \
%D%/packages/patches/u-boot-sifive-prevent-reloc-initrd-fdt.patch \
diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
index 029672721f..e173417855 100644
--- a/gnu/packages/bootloaders.scm
+++ b/gnu/packages/bootloaders.scm
@@ -652,9 +652,7 @@ (define u-boot
%u-boot-sifive-prevent-relocating-initrd-fdt
%u-boot-rk3399-enable-emmc-phy-patch
(search-patch "u-boot-infodocs-target.patch")
- (search-patch "u-boot-patman-fix-help.patch")
- (search-patch "u-boot-patman-local-conf.patch")
- (search-patch "u-boot-patman-get-maintainer.patch")))
+ (search-patch "u-boot-patman-guix-integration.patch")))
(method url-fetch)
(uri (string-append
"https://ftp.denx.de/pub/u-boot/"
diff --git a/gnu/packages/patches/u-boot-patman-fix-help.patch b/gnu/packages/patches/u-boot-patman-fix-help.patch
deleted file mode 100644
index 89bac06c2f..0000000000
--- a/gnu/packages/patches/u-boot-patman-fix-help.patch
+++ /dev/null
@@ -1,40 +0,0 @@
-Upstream status: https://patchwork.ozlabs.org/project/uboot/list/?series=333156
-
-diff --git a/tools/patman/main.py b/tools/patman/main.py
-index 5a7756a221..bf300c6e64 100755
---- a/tools/patman/main.py
-+++ b/tools/patman/main.py
-@@ -7,6 +7,7 @@
- """See README for more information"""
-
- from argparse import ArgumentParser
-+import importlib.resources
- import os
- import re
- import shutil
-@@ -163,11 +164,8 @@ elif args.cmd == 'send':
- fd.close()
-
- elif args.full_help:
-- tools.print_full_help(
-- os.path.join(os.path.dirname(os.path.realpath(sys.argv[0])),
-- 'README.rst')
-- )
--
-+ with importlib.resources.path('patman', 'README.rst') as readme:
-+ tools.print_full_help(str(readme))
- else:
- # If we are not processing tags, no need to warning about bad ones
- if not args.process_tags:
-diff --git a/tools/patman/setup.py b/tools/patman/setup.py
-index 43fdc00ce6..ce9bb4aa63 100644
---- a/tools/patman/setup.py
-+++ b/tools/patman/setup.py
-@@ -7,6 +7,6 @@ setup(name='patman',
- scripts=['patman'],
- packages=['patman'],
- package_dir={'patman': ''},
-- package_data={'patman': ['README']},
-+ package_data={'patman': ['README.rst']},
- classifiers=['Environment :: Console',
- 'Topic :: Software Development'])
Toggle diff (64 lines)
diff --git a/gnu/packages/patches/u-boot-patman-get-maintainer.patch b/gnu/packages/patches/u-boot-patman-get-maintainer.patch
deleted file mode 100644
index 4377f8394e..0000000000
--- a/gnu/packages/patches/u-boot-patman-get-maintainer.patch
+++ /dev/null
@@ -1,104 +0,0 @@
-Upstream status: https://patchwork.ozlabs.org/project/uboot/list/?series=333427
-
-diff --git a/tools/patman/patman.rst b/tools/patman/patman.rst
-index 7828899879..95b6c9c3f0 100644
---- a/tools/patman/patman.rst
-+++ b/tools/patman/patman.rst
-@@ -88,7 +88,7 @@ To add your own, create a file `~/.patman` like this::
- Patman will also look for a `.patman` configuration file at the root
- of the current project git repository, which makes it possible to
- override the `project` settings variable or anything else in a
--project-specific way. The values of this "local" configuration file
-+project-specific way. The values of this "local" configuration file
- take precedence over those of the "global" one.
-
- Aliases are recursive.
-diff --git a/tools/patman/test_settings.py b/tools/patman/test_settings.py
-index 9c14b4aaa3..c768a2fc64 100644
---- a/tools/patman/test_settings.py
-+++ b/tools/patman/test_settings.py
-@@ -6,38 +6,62 @@
- import argparse
- import contextlib
- import os
--import subprocess
-+import sys
- import tempfile
-
- from patman import settings
-+from patman import tools
-
-
- @contextlib.contextmanager
- def empty_git_repository():
- with tempfile.TemporaryDirectory() as tmpdir:
- os.chdir(tmpdir)
-- subprocess.check_call(['git', 'init'])
-+ tools.run('git', 'init', raise_on_error=True)
- yield tmpdir
-
-
-+@contextlib.contextmanager
-+def cleared_command_line_args():
-+ old_value = sys.argv[:]
-+ sys.argv = [sys.argv[0]]
-+ try:
-+ yield
-+ finally:
-+ sys.argv = old_value
-+
-+
- def test_git_local_config():
-- with empty_git_repository():
-- with tempfile.NamedTemporaryFile() as global_config:
-- global_config.write(b'[settings]\n'
-- b'project=u-boot\n')
-- global_config.flush()
-- parser = argparse.ArgumentParser()
-- parser.add_argument('-p', '--project', default='unknown')
--
-- # Test "global" config is used.
-- settings.Setup(parser, 'unknown', global_config.name)
-- args, _ = parser.parse_known_args()
-- assert args.project == 'u-boot'
--
-- # Test local config can shadow it.
-- with open('.patman', 'w', buffering=1) as f:
-- f.write('[settings]\n'
-- 'project=guix-patches\n')
-- settings.Setup(parser, 'unknown', global_config.name)
-- args, _ = parser.parse_known_args([])
-- assert args.project == 'guix-patches'
-+ # Clearing the command line arguments is required, otherwise
-+ # arguments passed to the test running such as in 'pytest -k
-+ # filter' would be processed by _UpdateDefaults and fail.
-+ with cleared_command_line_args():
-+ with empty_git_repository():
-+ with tempfile.NamedTemporaryFile() as global_config:
-+ global_config.write(b'[settings]\n'
-+ b'project=u-boot\n')
-+ global_config.flush()
-+ parser = argparse.ArgumentParser()
-+ parser.add_argument('-p', '--project', default='unknown')
-+ subparsers = parser.add_subparsers(dest='cmd')
-+ send = subparsers.add_parser('send')
-+ send.add_argument('--no-check', action='store_false',
-+ dest='check_patch', default=True)
-+
-+ # Test "global" config is used.
-+ settings.Setup(parser, 'unknown', global_config.name)
-+ args, _ = parser.parse_known_args([])
-+ assert args.project == 'u-boot'
-+ send_args, _ = send.parse_known_args([])
-+ assert send_args.check_patch
-+
-+ # Test local config can shadow it.
-+ with open('.patman', 'w', buffering=1) as f:
-+ f.write('[settings]\n'
-+ 'project: guix-patches\n'
-+ 'check_patch: False\n')
-+ settings.Setup(parser, 'unknown', global_config.name)
-+ args, _ = parser.parse_known_args([])
-+ assert args.project == 'guix-patches'
-+ send_args, _ = send.parse_known_args([])
-+ assert not send_args.check_patch
Toggle diff (239 lines)
diff --git a/gnu/packages/patches/u-boot-patman-guix-integration.patch b/gnu/packages/patches/u-boot-patman-guix-integration.patch
new file mode 100644
index 0000000000..3472656c99
--- /dev/null
+++ b/gnu/packages/patches/u-boot-patman-guix-integration.patch
@@ -0,0 +1,1244 @@
+These changes correspond to commits 9ff7500ace..3154de3dd6 already merged to
+the u-boot-dm custodian repo (at
+https://source.denx.de/u-boot/custodians/u-boot-dm/-/commits/next), scheduled
+to be pulled after the next release.
+
+diff --git a/tools/patman/__init__.py b/tools/patman/__init__.py
+index c9d3e35052..1b98ec7fee 100644
+--- a/tools/patman/__init__.py
++++ b/tools/patman/__init__.py
+@@ -1,6 +1,6 @@
+ # SPDX-License-Identifier: GPL-2.0+
+
+ __all__ = ['checkpatch', 'command', 'commit', 'control', 'cros_subprocess',
+- 'func_test', 'get_maintainer', 'gitutil', 'main', 'patchstream',
++ 'func_test', 'get_maintainer', 'gitutil', '__main__', 'patchstream',
+ 'project', 'series', 'setup', 'settings', 'terminal',
+ 'test_checkpatch', 'test_util', 'tools', 'tout']
+diff --git a/tools/patman/main.py b/tools/patman/__main__.py
+similarity index 89%
+rename from tools/patman/main.py
+rename to tools/patman/__main__.py
+index 8067a288ab..749e6348b6 100755
+--- a/tools/patman/main.py
++++ b/tools/patman/__main__.py
+@@ -7,6 +7,7 @@
+ """See README for more information"""
+
+ from argparse import ArgumentParser
++import importlib.resources
+ import os
+ import re
+ import sys
+@@ -19,6 +20,7 @@ if __name__ == "__main__":
+
+ # Our modules
+ from patman import control
++from patman import func_test
+ from patman import gitutil
+ from patman import project
+ from patman import settings
+@@ -53,7 +55,8 @@ parser.add_argument('-H', '--full-help', action='store_true', dest='full_help',
+ default=False, help='Display the README file')
+
+ subparsers = parser.add_subparsers(dest='cmd')
+-send = subparsers.add_parser('send')
++send = subparsers.add_parser(
++ 'send', help='Format, check and email patches (default command)')
+ send.add_argument('-i', '--ignore-errors', action='store_true',
+ dest='ignore_errors', default=False,
+ help='Send patches email even if patch errors are found')
+@@ -62,6 +65,12 @@ send.add_argument('-l', '--limit-cc', dest='limit', type=int, default=None,
+ send.add_argument('-m', '--no-maintainers', action='store_false',
+ dest='add_maintainers', default=True,
+ help="Don't cc the file maintainers automatically")
++send.add_argument(
++ '--get-maintainer-script', dest='get_maintainer_script', type=str,
++ action='store',
++ default=os.path.join(gitutil.get_top_level(), 'scripts',
++ 'get_maintainer.pl') + ' --norolestats',
++ help='File name of the get_maintainer.pl (or compatible) script.')
+ send.add_argument('-n', '--dry-run', action='store_true', dest='dry_run',
+ default=False, help="Do a dry run (create but don't email patches)")
+ send.add_argument('-r', '--in-reply-to', type=str, action='store',
+@@ -94,9 +103,11 @@ send.add_argument('--smtp-server', type=str,
+
+ send.add_argument('patchfiles', nargs='*')
+
+-test_parser = subparsers.add_parser('test', help='Run tests')
+-test_parser.add_argument('testname', type=str, default=None, nargs='?',
+- help="Specify the test to run")
++# Only add the 'test' action if the test data files are available.
++if os.path.exists(func_test.TEST_DATA_DIR):
++ test_parser = subparsers.add_parser('test', help='Run tests')
++ test_parser.add_argument('testname', type=str, default=None, nargs='?',
++ help="Specify the test to run")
+
+ status = subparsers.add_parser('status',
+ help='Check status of patches in patchwork')
+@@ -113,7 +124,7 @@ status.add_argument('-f', '--force', action='store_true',
+ argv = sys.argv[1:]
+ args, rest = parser.parse_known_args(argv)
+ if hasattr(args, 'project'):
+- settings.Setup(gitutil, parser, args.project, '')
++ settings.Setup(parser, args.project)
+ args, rest = parser.parse_known_args(argv)
+
+ # If we have a command, it is safe to parse all arguments
+@@ -160,11 +171,8 @@ elif args.cmd == 'send':
+ fd.close()
+
+ elif args.full_help:
+- tools.print_full_help(
+- os.path.join(os.path.dirname(os.path.realpath(sys.argv[0])),
+- 'README.rst')
+- )
+-
++ with importlib.resources.path('patman', 'README.rst') as readme:
++ tools.print_full_help(str(readme))
+ else:
+ # If we are not processing tags, no need to warning about bad ones
+ if not args.process_tags:
+diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py
+index d1b902dd96..012c0d895c 100644
+--- a/tools/patman/checkpatch.py
++++ b/tools/patman/checkpatch.py
+@@ -211,7 +211,7 @@ def check_patch(fname, verbose=False, show_types=False, use_tree=False):
+ stdout: Full output of checkpatch
+ """
+ chk = find_check_patch()
+- args = [chk]
++ args = [chk, '--u-boot', '--strict']
+ if not use_tree:
+ args.append('--no-tree')
+ if show_types:
+diff --git a/tools/patman/control.py b/tools/patman/control.py
+index bf426cf7bc..38e98dab84 100644
+--- a/tools/patman/control.py
++++ b/tools/patman/control.py
+@@ -94,8 +94,8 @@ def check_patches(series, patch_files, run_checkpatch, verbose, use_tree):
+
+
+ def email_patches(col, series, cover_fname, patch_files, process_tags, its_a_go,
+- ignore_bad_tags, add_maintainers, limit, dry_run, in_reply_to,
+- thread, smtp_server):
++ ignore_bad_tags, add_maintainers, get_maintainer_script, limit,
++ dry_run, in_reply_to, thread, smtp_server):
+ """Email patches to the recipients
+
+ This emails out the patches and cover letter using 'git send-email'. Each
+@@ -123,6 +123,8 @@ def email_patches(col, series, cover_fname, patch_files, process_tags, its_a_go,
+ ignore_bad_tags (bool): True to just print a warning for unknown tags,
+ False to halt with an error
+ add_maintainers (bool): Run the get_maintainer.pl script for each patch
++ get_maintainer_script (str): The script used to retrieve which
++ maintainers to cc
+ limit (int): Limit on the number of people that can be cc'd on a single
+ patch or the cover letter (None if no limit)
+ dry_run (bool): Don't actually email the patches, just print out what
+@@ -134,7 +136,7 @@ def email_patches(col, series, cover_fname, patch_files, process_tags, its_a_go,
+ smtp_server (str): SMTP server to use to send patches (None for default)
+ """
+ cc_file = series.MakeCcFile(process_tags, cover_fname, not ignore_bad_tags,
+- add_maintainers, limit)
++ add_maintainers, limit, get_maintainer_script)
+
+ # Email the patches out (giving the user time to check / cancel)
+ cmd = ''
+@@ -174,8 +176,8 @@ def send(args):
+ email_patches(
+ col, series, cover_fname, patch_files, args.process_tags,
+ its_a_go, args.ignore_bad_tags, args.add_maintainers,
+- args.limit, args.dry_run, args.in_reply_to, args.thread,
+- args.smtp_server)
++ args.get_maintainer_script, args.limit, args.dry_run,
++ args.in_reply_to, args.thread, args.smtp_server)
+
+ def patchwork_status(branch, count, start, end, dest_branch, force,
+ show_comments, url):
+diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py
+index 7b92bc67be..c25a47bdeb 100644
+--- a/tools/patman/func_test.py
++++ b/tools/patman/func_test.py
+@@ -6,7 +6,9 @@
+
+ """Functional tests for checking that patman behaves correctly"""
+
++import contextlib
+ import os
++import pathlib
+ import re
+ import shutil
+ import sys
+@@ -28,6 +30,21 @@ from patman.test_util import capture_sys_output
+ import pygit2
+ from patman import status
+
++PATMAN_DIR = pathlib.Path(__file__).parent
++TEST_DATA_DIR = PATMAN_DIR / 'test/'
++
++
++@contextlib.contextmanager
++def directory_excursion(directory):
++ """Change directory to `directory` for a limited to the context block."""
++ current = os.getcwd()
++ try:
++ os.chdir(directory)
++ yield
++ finally:
++ os.chdir(current)
++
++
+ class TestFunctional(unittest.TestCase):
+ """Functional tests for checking that patman behaves correctly"""
+ leb = (b'Lord Edmund Blackadd\xc3\xabr <weasel@blackadder.org>'.
+@@ -57,8 +74,7 @@ class TestFunctional(unittest.TestCase):
+ Returns:
+ str: Full path to file in the test directory
+ """
+- return os.path.join(os.path.dirname(os.path.realpath(sys.argv[0])),
+- 'test', fname)
++ return TEST_DATA_DIR / fname
+
+ @classmethod
+ def _get_text(cls, fname):
+@@ -200,6 +216,8 @@ class TestFunctional(unittest.TestCase):
+ text = self._get_text('test01.txt')
+ series = patchstream.get_metadata_for_test(text)
+ cover_fname, args = self._create_patches_for_test(series)
++ get_maintainer_script = str(pathlib.Path(__file__).parent.parent.parent
++ / 'get_maintainer.pl') + ' --norolestats'
+ with capture_sys_output() as out:
+ patchstream.fix_patches(series, args)
+ if cover_fname and series.get('cover'):
+@@ -207,7 +225,7 @@ class TestFunctional(unittest.TestCase):
+ series.DoChecks()
+ cc_file = series.MakeCcFile(process_tags, cover_fname,
+ not ignore_bad_tags, add_maintainers,
+- None)
++ None, get_maintainer_script)
+ cmd = gitutil.email_patches(
+ series, cover_fname, args, dry_run, not ignore_bad_tags,
+ cc_file, in_reply_to=in_reply_to, thread=None)
+@@ -502,6 +520,37 @@ complicated as possible''')
+ finally:
+ os.chdir(orig_dir)
+
++ def test_custom_get_maintainer_script(self):
++ """Validate that a custom get_maintainer script gets used."""
++ self.make_git_tree()
++ with directory_excursion(self.gitdir):
++ # Setup git.
++ os.environ['GIT_CONFIG_GLOBAL'] = '/dev/null'
++ os.environ['GIT_CONF
This message was truncated. Download the full message here.
J
J
Jelle Licht wrote on 6 Jan 2023 13:12
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)(address . 60576@debbugs.gnu.org)
878rifeu9n.fsf@fsfe.org
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

Toggle quote (17 lines)
> The full set of patches already merged into the U-Boot is included for
> simplicity and avoiding mistakes like the previous one, where a conflicting
> hunk got dropped, causing the following error when running patman:
>
> WARNING: Unknown setting get_maintainer_script
>
> * gnu/packages/bootloaders.scm (u-boot) [source]: Replace the
> u-boot-patman-fix-help.patch, u-boot-patman-local-conf.patch and
> u-boot-patman-get-maintainer.patch with u-boot-patman-guix-integration.patch.
> * gnu/local.mk (dist_patch_DATA): Update patch registrations.
> * gnu/packages/patches/u-boot-patman-fix-help.patch: Delete file.
> * gnu/packages/patches/u-boot-patman-get-maintainer.patch: Likewise.
> * gnu/packages/patches/u-boot-patman-local-conf.patch: Likewise.
> * gnu/packages/patches/u-boot-patman-guix-integration.patch: New file.
>
> Reported-by: Jelle Licht <jlicht@fsfe.org>

I don't know the first thing about U-Boot, nor am I a poweruser of patman, but I
can confirm this fixes my issue.

Thanks!

- Jelle
M
M
Maxim Cournoyer wrote on 10 Jan 2023 17:48
(name . Jelle Licht)(address . jlicht@fsfe.org)(address . 60576-done@debbugs.gnu.org)
87y1qajpy7.fsf_-_@gmail.com
Hello,

Jelle Licht <jlicht@fsfe.org> writes:

Toggle quote (22 lines)
> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
>> The full set of patches already merged into the U-Boot is included for
>> simplicity and avoiding mistakes like the previous one, where a conflicting
>> hunk got dropped, causing the following error when running patman:
>>
>> WARNING: Unknown setting get_maintainer_script
>>
>> * gnu/packages/bootloaders.scm (u-boot) [source]: Replace the
>> u-boot-patman-fix-help.patch, u-boot-patman-local-conf.patch and
>> u-boot-patman-get-maintainer.patch with u-boot-patman-guix-integration.patch.
>> * gnu/local.mk (dist_patch_DATA): Update patch registrations.
>> * gnu/packages/patches/u-boot-patman-fix-help.patch: Delete file.
>> * gnu/packages/patches/u-boot-patman-get-maintainer.patch: Likewise.
>> * gnu/packages/patches/u-boot-patman-local-conf.patch: Likewise.
>> * gnu/packages/patches/u-boot-patman-guix-integration.patch: New file.
>>
>> Reported-by: Jelle Licht <jlicht@fsfe.org>
>
> I don't know the first thing about U-Boot, nor am I a poweruser of patman, but I
> can confirm this fixes my issue.

Thanks for the feedback; applied.

Enjoy patman!

--
Thanks,
Maxim
Closed
?