Merge ~liushuyu-011/ubuntu/+source/spamassassin:ubuntu/devel into ubuntu/+source/spamassassin:ubuntu/devel

Proposed by Zixing Liu
Status: Needs review
Proposed branch: ~liushuyu-011/ubuntu/+source/spamassassin:ubuntu/devel
Merge into: ubuntu/+source/spamassassin:ubuntu/devel
Diff against target: 99 lines (+77/-0)
3 files modified
debian/changelog (+7/-0)
debian/patches/series (+1/-0)
debian/patches/t-adapt-to-new-dns-module-behavior.patch (+69/-0)
Reviewer Review Type Date Requested Status
Benjamin Drung (community) Approve
Review via email: mp+462834@code.launchpad.net

Description of the change

This MP fixes the DNS unit tests in Spamassassin, where the built-in DNS server failed to start due to an API change in the Net::DNS::Nameserver module.

To post a comment you must log in.
Revision history for this message
Zixing Liu (liushuyu-011) wrote :
Revision history for this message
Benjamin Drung (bdrung) wrote :

This change was sponsored by Michael Hudson-Doyle.

review: Approve
Revision history for this message
Bryce Harrington (bryce) wrote :
Download full text (4.9 KiB)

Hi Zixing,

Thanks for fixing this problem, but in the future please can you also
ensure there is a bug report associated with the change? That is
important not just for tracking, but also for documenting what the
original problem was that necessitated the change (I am guessing a dep8
test failure?) and to explain the approach used for fixing it.

In particular, this is important for fixes originating from the Ubuntu
side, in order to facilitate forwarding them upstream at some point. In
particular, this patch in particular looks like it should go upstream,
so I'd like to know if you didn't forward it upstream yourself, what the
reasons were?

Thanks again for this contribution, it looks like a solid fix and I
appreciate that you caught it during -devel.

Bryce

----- Forwarded message from Zixing Liu <email address hidden> -----

Date: Thu, 21 Mar 2024 04:22:20 -0000
From: Zixing Liu <email address hidden>
To: <email address hidden>
Subject: [Merge] ~liushuyu-011/::/spamassassin:ubuntu/devel into ::/spamassassin:ubuntu/devel
Reply-To: <email address hidden>

Zixing Liu has proposed merging ~liushuyu-011/ubuntu/+source/spamassassin:ubuntu/devel into ubuntu/+source/spamassassin:ubuntu/devel.

Requested reviews:
  Ubuntu Sponsors (ubuntu-sponsors)

For more details, see:
https://code.launchpad.net/~liushuyu-011/ubuntu/+source/spamassassin/+git/spamassassin/+merge/462834

This MP fixes the DNS unit tests in Spamassassin, where the built-in DNS server failed to start due to an API change in the Net::DNS::Nameserver module.
--
Your team git-ubuntu developers is subscribed to branch ubuntu/+source/spamassassin:ubuntu/devel.

diff --git a/debian/changelog b/debian/changelog
index 406f4ee..1430ee5 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+spamassassin (4.0.0-8ubuntu3~ppa1) noble; urgency=medium
+
+ * debian/patches/t-adapt-to-new-dns-module-behavior.patch: Adapt to
+ new Net::DNS::Nameserver behavior.
+
+ -- Zixing Liu <email address hidden> Wed, 20 Mar 2024 22:19:39 -0600
+
 spamassassin (4.0.0-8ubuntu2) noble; urgency=medium

   * No-change rebuild against libssl3t64
diff --git a/debian/patches/series b/debian/patches/series
index 991acfd..994a39f 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -12,3 +12,4 @@ spamd_disable_ai_addrconfig_for_ipv4_literals.patch
 bug1041759-deprecated-method.patch
 bug1039960-deprecated-method-rdstring.patch
 bug_1056001-reread-prefs.patch
+t-adapt-to-new-dns-module-behavior.patch
diff --git a/debian/patches/t-adapt-to-new-dns-module-behavior.patch b/debian/patches/t-adapt-to-new-dns-module-behavior.patch
new file mode 100644
index 0000000..dd2d435
--- /dev/null
+++ b/debian/patches/t-adapt-to-new-dns-module-behavior.patch
@@ -0,0 +1,69 @@
+Description: Adapt to new Net::DNS::Nameserver behavior
+ The newer version will do fork() itself and will break
+ if doing a double-fork from the tests.
+Author: Zixing Liu <email address hidden>
+Forwarded: no
+Last-Update: 2024-03-21
+
+--- spamassassin-4.0.0.orig/t/dnsbl_subtests.t
++++ spamassassin-4.0.0/t/dnsbl_subtests.t
+@@ -221,7 +221,8 @@ sub dns_server($$) {
+ LocalAddr => $l...

Read more...

Unmerged commits

480b153... by Zixing Liu

d/p/t-adapt-to-new-dns-module-behavior.patch: add a patch to fix DNS tests ...

... where the built-in DNS server might be stuck due to an API change

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 406f4ee..1430ee5 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,10 @@
6+spamassassin (4.0.0-8ubuntu3~ppa1) noble; urgency=medium
7+
8+ * debian/patches/t-adapt-to-new-dns-module-behavior.patch: Adapt to
9+ new Net::DNS::Nameserver behavior.
10+
11+ -- Zixing Liu <zixing.liu@canonical.com> Wed, 20 Mar 2024 22:19:39 -0600
12+
13 spamassassin (4.0.0-8ubuntu2) noble; urgency=medium
14
15 * No-change rebuild against libssl3t64
16diff --git a/debian/patches/series b/debian/patches/series
17index 991acfd..994a39f 100644
18--- a/debian/patches/series
19+++ b/debian/patches/series
20@@ -12,3 +12,4 @@ spamd_disable_ai_addrconfig_for_ipv4_literals.patch
21 bug1041759-deprecated-method.patch
22 bug1039960-deprecated-method-rdstring.patch
23 bug_1056001-reread-prefs.patch
24+t-adapt-to-new-dns-module-behavior.patch
25diff --git a/debian/patches/t-adapt-to-new-dns-module-behavior.patch b/debian/patches/t-adapt-to-new-dns-module-behavior.patch
26new file mode 100644
27index 0000000..dd2d435
28--- /dev/null
29+++ b/debian/patches/t-adapt-to-new-dns-module-behavior.patch
30@@ -0,0 +1,69 @@
31+Description: Adapt to new Net::DNS::Nameserver behavior
32+ The newer version will do fork() itself and will break
33+ if doing a double-fork from the tests.
34+Author: Zixing Liu <zixing.liu@canonical.com>
35+Forwarded: no
36+Last-Update: 2024-03-21
37+
38+--- spamassassin-4.0.0.orig/t/dnsbl_subtests.t
39++++ spamassassin-4.0.0/t/dnsbl_subtests.t
40+@@ -221,7 +221,8 @@ sub dns_server($$) {
41+ LocalAddr => $local_addr, LocalPort => $local_port,
42+ ReplyHandler => \&reply_handler, Verbose => 0);
43+ $ns or die "Cannot create a nameserver object";
44+- $ns->main_loop;
45++ $ns->start_server(10);
46++ return $ns;
47+ }
48+
49+ sub find_free_port($) {
50+@@ -308,16 +309,7 @@ if ($sock_tcp) {
51+ $sock_tcp->close() or die "Error closing TCP socket: $!";
52+ }
53+
54+-# detach a DNS server process
55+-my $pid = fork();
56+-defined $pid or die "Cannot fork: $!";
57+-if (!$pid) { # child
58+- dns_server($dns_server_localaddr, $dns_server_localport);
59+- exit;
60+-}
61+-
62+-# parent
63+-# print STDERR "Forked a DNS server process [$pid]\n";
64++my $ns = dns_server($dns_server_localaddr, $dns_server_localport);
65+ sleep 1;
66+
67+ $spamassassin_obj = Mail::SpamAssassin->new({
68+@@ -353,28 +345,9 @@ test_samples(
69+ # X_URIBL_Y_FFD no longer hits intentionally (not in the 127.0.0.0/8 range),
70+ # see Bug 6803
71+
72+-if ($pid) {
73+- kill('TERM',$pid) or die "Cannot stop a DNS server [$pid]: $!";
74+-
75+-# Bug 7000: Seems like a DNS server process can't be terminated. [...]
76+-# Reason is "waitpid($pid,0)". If commented out, it does not hang.
77+-# There are no extra processes after end of this test.
78+-#
79+-# perlfunc: waitpid - waiting for a particular pid with FLAGS of 0 is
80+-# implemented everywhere
81+-#
82+-# perlport: (Win32) waitpid Can only be applied to process handles returned
83+-# for processes spawned using "system(1, ...)" or pseudo processes created
84+-# with "fork()".
85+-#
86+-# so ... waitpid($pid,0) should work on Windows, but it doesn't - nevermind:
87+-
88+- waitpid($pid,0) unless $RUNNING_ON_WINDOWS;
89+-
90+- undef $pid;
91+-}
92+-
93+ END {
94+ $spamassassin_obj->finish if $spamassassin_obj;
95+- kill('KILL',$pid) if $pid; # ignoring status
96++ if ($ns) {
97++ $ns->stop_server();
98++ }
99+ }

Subscribers

People subscribed via source and target branches