Peter Pentchev:
> Package: debhelper
> Version: 11.2.1
> Severity: minor
> Tags: patch
>
> Hi,
>
> Thanks for maintaining and extending debhelper!
>
Hi Peter,
Thanks for testing and the feedback. :)
> When trying to test the new B-D: debhelper-compat (= 11) mechanism on
> my mdk and mdk-doc packages, I stumbled upon a minor issue: if this
> dependency is the first one listed, and it's on a separate line (there
> are no package names on the "Build-Depends:" line itself), then it is
> not recognized because the regular expression used will not match
> the whitespace before the "debhelper-compat" token.
>
Indeed, that sounds like an annoying papercut.
> Attached is a trivial patch. Well, actually the trivial patch is in
> the second commit; the first one adds some debhelper-compat tests and
> the third one adds a test for the bug fixed by the second one.
> Of course, if you feel that adding test-only functions to the Dh_Lib
> module is not really proper, it's your call whether to accept just
> the fix itself without the tests.
>
> Thanks again for taking care of debhelper!
>
> Best regards,
> Peter
>
> [...]
I am happy to apply all the patches with/after some minor tweaks:
> From 4680d18a5125ce6a46afa846e9b4866df0ce7a72 Mon Sep 17 00:00:00 2001
> From: Peter Pentchev <r...@ringlet.net>
> Date: Fri, 4 May 2018 23:59:04 +0300
> Subject: [PATCH 1/3] Lay the groundwork for testing debhelper-compat.
> [...]
> diff --git a/lib/Debian/Debhelper/Dh_Lib.pm b/lib/Debian/Debhelper/Dh_Lib.pm
> index 9c829f0f..9666b7ad 100644
> --- a/lib/Debian/Debhelper/Dh_Lib.pm
> +++ b/lib/Debian/Debhelper/Dh_Lib.pm
> @@ -63,7 +63,7 @@ our (@EXPORT, %dh);
> &compute_doc_main_package &is_so_or_exec_elf_file &hostarch
> &assert_opt_is_known_package &dbgsym_tmpdir &find_hardlinks
> &should_use_root &gain_root_cmd DEFAULT_PACKAGE_TYPE
> - DBGSYM_PACKAGE_TYPE
> + DBGSYM_PACKAGE_TYPE &compat_levels &resetcompat &resetpackages
No need to export these; no tool (except tests) should need them and
they can live with using the fully qualified name.
Example usage:
"""
Debian::Debhelper::Dh_Lib::resetcompat();
"""
Note the separate comment about compat_levels below.
> [...]
> @@ -2386,4 +2399,15 @@ sub dbgsym_tmpdir {
> }
> }
>
> +sub compat_levels {
Let's move this to Test::DH (and use the same method the max as used in
each_compat_from_and_above_subtest). Probably renamed to
"non_deprecated_compat_levels".
> [...]
> +}
> +
> 1
> [...]
> index 00000000..0f711f5f
> --- /dev/null
> +++ b/t/debhelper-compat/syntax.t
> [...]
> +
> +sub test_build_depends {
> + my ($level, $build_depends) = @_;
> + my $dir = Test::DH::tempdir(CLEANUP => 1);
> [...]
That is actually "File::Temp::tempdir" and should be imported/use'd
directly. E.g.
"""
use File::Temp qw(tempdir);
...
my $dir = tempdir(CLEANUP => 1);
"""
> --
> 2.17.0
>
> From 8cc0086067b77be6f3ac130a50e375c6c379e212 Mon Sep 17 00:00:00 2001
> From: Peter Pentchev <r...@ringlet.net>
> Date: Sat, 5 May 2018 00:09:08 +0300
> Subject: [PATCH 2/3] Allow whitespace before the debhelper-compat B-D.
>
> This allows a newline before the debhelper-compat dependency, e.g.
> when all the dependencies are listed on separate rows.
> ---
> lib/Debian/Debhelper/Dh_Lib.pm | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/Debian/Debhelper/Dh_Lib.pm b/lib/Debian/Debhelper/Dh_Lib.pm
> index 9666b7ad..dffd2fad 100644
> --- a/lib/Debian/Debhelper/Dh_Lib.pm
> +++ b/lib/Debian/Debhelper/Dh_Lib.pm
> @@ -1467,7 +1467,7 @@ sub getpackages {
> my $value = join(' ', @{$bd_fields{$field}});
> $value =~ s/\s*,\s*$//;
> for my $dep (split(/\s*,\s*/, $value)) {
> - if ($dep =~
> m/^debhelper-compat\s*[(]\s*=\s*(${PKGVERSION_REGEX})\s*[)]$/) {
> + if ($dep =~
> m/^\s*debhelper-compat\s*[(]\s*=\s*(${PKGVERSION_REGEX})\s*[)]$/) {
Let's add trailing whitespace while we are at it, so we do not get a
repeat for that. I am not sure how to generate that in a real case but
I hadn't seen how to generate leading whitespace either plus the test
case can trivially do it.
Thanks,
~Niels
