On Tuesday, March 05 2024, Guido Günther wrote: > Hi,
Hey, Thanks for the review. > On Mon, Mar 04, 2024 at 05:14:03PM -0500, Sergio Durigan Junior wrote: >> Some Debian packages (namely QEMU, although it has been "fixed" >> recently) keep patches using the "unified=0" format, i.e., patches >> that were generated without context). For an example of such patch, >> see: >> >> https://salsa.debian.org/qemu-team/qemu/-/blob/4112b90d8217c618aec5050c2059223486150cc0/debian/patches/openbios-spelling-endianess.patch >> >> git-apply(1) supports the "--unidiff-zero" option, which can handle >> these patches. This commit exposes this option to gbp users. > > What happens to patches that have context and you add --unidiff-zero? They still apply fine, but with a caveat, as git-apply(1) says: By default, git apply expects that the patch being applied is a unified diff with at least one line of context. This provides good safety measures, but breaks down when applying a diff generated with --unified=0. To bypass these checks use --unidiff-zero. Note, for the reasons stated above, the usage of context-free patches is discouraged. So this option should probably only be used when necessary. > Can you please add a test to 13_test_gbp_pq.py and update the manpages > too? Ah, right, sorry about that. For some reason I thought that the manpages were auto-generated. Here's an updated patch. Thanks, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible https://sergiodj.net/ From 0e417fa2ccbfa8b74dc4e4e6ca77fc9497688ac3 Mon Sep 17 00:00:00 2001 From: Sergio Durigan Junior <[email protected]> Date: Mon, 4 Mar 2024 17:05:29 -0500 Subject: [PATCH] Implement 'unidiff-zero' option Some Debian packages (namely QEMU, although it has been "fixed" recently) keep patches using the "unified=0" format, i.e., patches that were generated without context). For an example of such patch, see: https://salsa.debian.org/qemu-team/qemu/-/blob/4112b90d8217c618aec5050c2059223486150cc0/debian/patches/openbios-spelling-endianess.patch git-apply(1) supports the "--unidiff-zero" option, which can handle these patches. This commit exposes this option to gbp users. Signed-off-by: Sergio Durigan Junior <[email protected]> --- docs/manpages/gbp-pq.xml | 21 +++++++++++++++++++++ gbp/config.py | 5 +++++ gbp/git/repository.py | 5 ++++- gbp/scripts/common/pq.py | 13 ++++++++----- gbp/scripts/pq.py | 13 +++++++++---- gbp/scripts/pq_rpm.py | 8 ++++++-- tests/13_test_gbp_pq.py | 15 +++++++++++++++ tests/data/unidiff-1.patch | 11 +++++++++++ tests/data/unidiff-2.patch | 7 +++++++ 9 files changed, 86 insertions(+), 12 deletions(-) create mode 100644 tests/data/unidiff-1.patch create mode 100644 tests/data/unidiff-2.patch diff --git a/docs/manpages/gbp-pq.xml b/docs/manpages/gbp-pq.xml index 2bc9a20c..4948e248 100644 --- a/docs/manpages/gbp-pq.xml +++ b/docs/manpages/gbp-pq.xml @@ -34,6 +34,7 @@ <arg><option>--pq-from=</option><replaceable>[DEBIAN|TAG]</replaceable></arg> <arg><option>--upstream-tag=</option><replaceable>tag-format</replaceable></arg> <arg><option>--[no-]ignore-new</option></arg> + <arg><option>--[no-]unidiff-zero</option></arg> <group choice="plain"> <arg><option>drop</option></arg> <arg><option>export</option></arg> @@ -291,6 +292,26 @@ </para> </listitem> </varlistentry> + <varlistentry> + <term><option>--[no-]unidiff-zero</option> + </term> + <listitem> + <para> + Apply the patches using git-apply(1)'s + <option>--unidiff-zero</option> option. This option is + useful when dealing with patches that were generated using + <option>--unified=0</option>, i.e., patches that do not + have any lines of context. + </para> + <para> + Note that, for security reasons, using context-free + patches is discouraged. Use this option only when + necessary. The default is to + <replaceable>not</replaceable> use + <option>--unidiff-zero</option> when applying patches. + </para> + </listitem> + </varlistentry> </variablelist> </refsect1> <refsect1> diff --git a/gbp/config.py b/gbp/config.py index bad59cde..054074c2 100644 --- a/gbp/config.py +++ b/gbp/config.py @@ -195,6 +195,7 @@ class GbpOptionParser(OptionParser): 'time-machine': 1, 'track': 'True', 'track-missing': 'False', + 'unidiff-zero': 'False', 'upstream-branch': 'upstream', 'upstream-tag': 'upstream/%(version)s', 'upstream-tree': 'TAG', @@ -384,6 +385,10 @@ class GbpOptionParser(OptionParser): 'bare': "whether to create a bare repository on the remote side. " "'Default is '%(bare)s'.", + 'unidiff-zero': + "whether to apply patches using git-apply(1)'s '--unidiff-zero' " + "option, for patches that don't have any context (i.e., were " + "generated with '--unified=0'). Default is '%(unidiff-zero)s'.", 'urgency': "Set urgency level, default is '%(urgency)s'", 'repo-user': diff --git a/gbp/git/repository.py b/gbp/git/repository.py index 972a058f..1c5750c4 100644 --- a/gbp/git/repository.py +++ b/gbp/git/repository.py @@ -1781,7 +1781,8 @@ class GitRepository(object): output, ret = self._git_getoutput('format-patch', options.args) return [line.strip() for line in output] - def apply_patch(self, patch, index=True, context=None, strip=None, fix_ws=False): + def apply_patch(self, patch, index=True, context=None, strip=None, fix_ws=False, + unidiff_zero=False): """Apply a patch using git apply""" args = [] if context: @@ -1792,6 +1793,8 @@ class GitRepository(object): args.append("--whitespace=fix") if strip is not None: args += ['-p', str(strip)] + if unidiff_zero: + args += ['--unidiff-zero'] args.append(patch) self._git_command("apply", args) diff --git a/gbp/scripts/common/pq.py b/gbp/scripts/common/pq.py index 1a80ec05..2591b875 100644 --- a/gbp/scripts/common/pq.py +++ b/gbp/scripts/common/pq.py @@ -305,13 +305,16 @@ def switch_to_pq_branch(repo, branch): repo.set_branch(pq_branch) -def apply_single_patch(repo, branch, patch, fallback_author, topic=None): +def apply_single_patch(repo, branch, patch, fallback_author, topic=None, + unidiff_zero=False): switch_to_pq_branch(repo, branch) - apply_and_commit_patch(repo, patch, fallback_author, topic) + apply_and_commit_patch(repo, patch, fallback_author, topic, + unidiff_zero=unidiff_zero) gbp.log.info("Applied %s" % os.path.basename(patch.path)) -def apply_and_commit_patch(repo, patch, fallback_author, topic=None, name=None): +def apply_and_commit_patch(repo, patch, fallback_author, topic=None, name=None, + unidiff_zero=False): """apply a single patch 'patch', add topic 'topic' and commit it""" author = {'name': patch.author, 'email': patch.email, @@ -330,10 +333,10 @@ def apply_and_commit_patch(repo, patch, fallback_author, topic=None, name=None): gbp.log.warn("Patch '%s' has no authorship information" % patch_fn) try: - repo.apply_patch(patch.path, strip=patch.strip) + repo.apply_patch(patch.path, strip=patch.strip, unidiff_zero=unidiff_zero) except GitRepositoryError: gbp.log.warn("Patch %s failed to apply, retrying with whitespace fixup" % patch_fn) - repo.apply_patch(patch.path, strip=patch.strip, fix_ws=True) + repo.apply_patch(patch.path, strip=patch.strip, unidiff_zero=unidiff_zero, fix_ws=True) tree = repo.write_tree() msg = "%s\n\n%s" % (patch.subject, patch.long_desc) if topic: diff --git a/gbp/scripts/pq.py b/gbp/scripts/pq.py index 78f479ca..02411967 100755 --- a/gbp/scripts/pq.py +++ b/gbp/scripts/pq.py @@ -267,7 +267,7 @@ def safe_patches(series, repo): def import_quilt_patches(repo, branch, series, tries, force, pq_from, - upstream_tag): + upstream_tag, unidiff_zero=False): """ apply a series of quilt patches in the series file 'series' to branch the patch-queue branch for 'branch' @@ -282,6 +282,7 @@ def import_quilt_patches(repo, branch, series, tries, force, pq_from, DEBIAN indicates the current branch, TAG indicates that the corresponding upstream tag should be used. @param upstream_tag: upstream tag template to use + @param unidiff_zero: whether to apply the patch using '--unidiff-zero' """ tmpdir = None series = os.path.join(repo.path, series) @@ -332,7 +333,8 @@ def import_quilt_patches(repo, branch, series, tries, force, pq_from, gbp.log.debug("Applying %s" % patch.path) try: name = os.path.basename(patch.path) - apply_and_commit_patch(repo, patch, maintainer, patch.topic, name) + apply_and_commit_patch(repo, patch, maintainer, patch.topic, name, + unidiff_zero=unidiff_zero) except Exception as e: gbp.log.err("Failed to apply '%s': %s" % (patch.path, e)) repo.force_head('HEAD', hard=True) @@ -371,7 +373,8 @@ def import_pq(repo, branch, options): tries = options.time_machine if (options.time_machine > 0) else 1 num = import_quilt_patches(repo, branch, series, tries, options.force, options.pq_from, - options.upstream_tag) + options.upstream_tag, + options.unidiff_zero) gbp.log.info("%d patches listed in '%s' imported on '%s'" % (num, series, repo.get_branch())) @@ -447,6 +450,7 @@ def build_parser(name): parser.add_config_file_option(option_name="pq-from", dest="pq_from", choices=['DEBIAN', 'TAG']) parser.add_config_file_option(option_name="upstream-tag", dest="upstream_tag") parser.add_boolean_config_file_option(option_name="ignore-new", dest="ignore_new") + parser.add_boolean_config_file_option(option_name="unidiff-zero", dest="unidiff_zero") return parser @@ -504,7 +508,8 @@ def main(argv): elif action == "apply": patch = Patch(patchfile) maintainer = get_maintainer_from_control(repo) - apply_single_patch(repo, current, patch, maintainer, options.topic) + apply_single_patch(repo, current, patch, maintainer, options.topic, + options.unidiff_zero) elif action == "switch": switch_pq(repo, current, options) except KeyboardInterrupt: diff --git a/gbp/scripts/pq_rpm.py b/gbp/scripts/pq_rpm.py index fcefbb6e..6ff3980d 100755 --- a/gbp/scripts/pq_rpm.py +++ b/gbp/scripts/pq_rpm.py @@ -327,7 +327,8 @@ def import_spec_patches(repo, options): (base, upstream_commit)) for patch in queue: gbp.log.debug("Applying %s" % patch.path) - apply_and_commit_patch(repo, patch, packager) + apply_and_commit_patch(repo, patch, packager, + unidiff_zero=options.unidiff_zero) except (GbpError, GitRepositoryError) as err: repo.set_branch(base) repo.delete_branch(pq_branch) @@ -406,6 +407,8 @@ def build_parser(name): parser.add_config_file_option(option_name="spec-file", dest="spec_file") parser.add_config_file_option(option_name="packaging-dir", dest="packaging_dir") + parser.add_boolean_config_file_option(option_name="unidiff-zero", + dest="unidiff_zero") return parser @@ -465,7 +468,8 @@ def main(argv): rebase_pq(repo, options) elif action == "apply": patch = Patch(patchfile) - apply_single_patch(repo, current, patch, fallback_author=None) + apply_single_patch(repo, current, patch, fallback_author=None, + unidiff_zero=options.unidiff_zero) elif action == "switch": switch_pq(repo, current) except KeyboardInterrupt: diff --git a/tests/13_test_gbp_pq.py b/tests/13_test_gbp_pq.py index dc53c260..bb3e69f0 100644 --- a/tests/13_test_gbp_pq.py +++ b/tests/13_test_gbp_pq.py @@ -77,6 +77,21 @@ class TestApplyAndCommit(testutils.DebianGitTestRepo): info = self.repo.get_commit_info('HEAD') self.assertIn('Gbp-Pq: Name foobar', info['body']) + def test_apply_unidiff_path(self): + """Test if applying a unidiff=0 patch works""" + patch = gbp.patch_series.Patch(_patch_path('unidiff-1.patch')) + pq.apply_and_commit_patch(self.repo, patch, None, name='unidiff-1') + + newpatch = gbp.patch_series.Patch(_patch_path('unidiff-2.patch')) + pq.apply_and_commit_patch(self.repo, newpatch, None, name='unidiff-2', + unidiff_zero=True) + + with open(os.path.join(self.repo.path, 'unidiff-test'), 'r', + encoding='UTF-8') as f: + line = '-'.join(f.read().splitlines()) + + self.assertEqual('1-2-6-3-4-5', line) + @testutils.skip_without_cmd('dpkg') def test_debian_missing_author(self): """ diff --git a/tests/data/unidiff-1.patch b/tests/data/unidiff-1.patch new file mode 100644 index 00000000..1571079d --- /dev/null +++ b/tests/data/unidiff-1.patch @@ -0,0 +1,11 @@ +Author: Sergio Durigan Junior <[email protected]> +Subject: unidiff-test + +--- /dev/null 2023-12-30 16:32:58.028000015 -0500 ++++ unidiff-test 2024-03-06 21:35:53.210519700 -0500 +@@ -0,0 +1,5 @@ ++1 ++2 ++3 ++4 ++5 diff --git a/tests/data/unidiff-2.patch b/tests/data/unidiff-2.patch new file mode 100644 index 00000000..b7dac307 --- /dev/null +++ b/tests/data/unidiff-2.patch @@ -0,0 +1,7 @@ +Author: Sergio Durigan Junior <[email protected]> +Subject: bla + +--- unidiff-test.old 2024-03-06 21:35:53.210519700 -0500 ++++ unidiff-test 2024-03-06 21:36:18.746123981 -0500 +@@ -2,0 +3 @@ ++6 -- 2.43.0 _______________________________________________ git-buildpackage mailing list [email protected] http://lists.sigxcpu.org/mailman/listinfo/git-buildpackage
