Merge ~ahasenack/ubuntu/+source/backuppc:trusty-backuppc-smb-1576187 into ubuntu/+source/backuppc:ubuntu/trusty-devel

Proposed by Andreas Hasenack
Status: Merged
Approved by: Christian Ehrhardt 
Approved revision: 7d660d980d5a1ec9ba52e77025060c039cc673f2
Merged at revision: 79e410c3d44c5783a8e5552e7614c81bb5120800
Proposed branch: ~ahasenack/ubuntu/+source/backuppc:trusty-backuppc-smb-1576187
Merge into: ubuntu/+source/backuppc:ubuntu/trusty-devel
Diff against target: 204 lines (+167/-0)
5 files modified
debian/changelog (+8/-0)
debian/patches/smb-compat-fix.patch (+66/-0)
debian/rules (+1/-0)
debian/tests/control (+3/-0)
debian/tests/smb-backup (+89/-0)
Reviewer Review Type Date Requested Status
Christian Ehrhardt  (community) Approve
Canonical Server Pending
Canonical Server Core Reviewers Pending
Review via email: mp+356883@code.launchpad.net

Description of the change

Same as https://code.launchpad.net/~ahasenack/ubuntu/+source/backuppc/+git/backuppc/+merge/356880, but for trusty

This branch applies the fix that upstream chose for #1576187. This is different from what was applied in debian (which is what we have in bionic and later), but also fixes the problem, and in a better way we believe.

While writing up the testing instructions, I realized these could be easily scripted, and as such I'm also adding a simple samba-based DEP8 test to this package. This test actually exercises the present bug, so if it's run with the package without the fix, it will correctly fail.

The same dep8 test can and will be added to cosmic+1 once it opens (see https://bugs.launchpad.net/ubuntu/+source/backuppc/+bug/1677755).

Testing instructions are provided in the bug, but they are the same as the newly added DEP8 test, which you can run manually if you want.

Bileto ticket: https://bileto.ubuntu.com/#/ticket/3486
PPA: sudo add-apt-repository ppa:ci-train-ppa-service/3486

To post a comment you must log in.
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

I don't understand why the trusty dep8 tests are not running in bileto. They ran locally via autopkgtest in a VM just fine. Is it a bileto restriction or shortcoming?

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

I also updated this branch with the same change I did to the xenial one regarding how the dep8 test populates the share directory.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

But you did not move the dep3 tags up :-/
Not bad but to keep things symetric fix it here as well please or explicitly state that you want to keep as-is.

review: Needs Information
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Indeed, I just pushed a fix for that. No --force was used, so you can check the diff.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

LGTM now - thanks +1

review: Approve
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

This now contains all the latest changes.

Updated bileto ticket: https://bileto.ubuntu.com/#/ticket/3486

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

On trusty bileto still doesn't run your tests, but I know you have local runs that work.
+1 here as well, thanks for sorting out arm.

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Correct, I ran the dep8 tests in trusty locally in a vm.

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Tagged and uploaded.

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 1badf10..90b3286 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,11 @@
6+backuppc (3.3.0-1ubuntu1.1) trusty; urgency=medium
7+
8+ * d/rules, d/p/smb-compat-fix.patch: cope with changes in newer smbclient.
9+ Thanks to Maksym Schipka <maksym@hotmail.com> (LP: #1576187)
10+ * d/t/{control,smb-backup}: simple smb-based DEP8 test (LP: #1677755)
11+
12+ -- Andreas Hasenack <andreas@canonical.com> Tue, 23 Oct 2018 09:06:27 -0300
13+
14 backuppc (3.3.0-1ubuntu1) trusty-proposed; urgency=low
15
16 * Merge from Debian unstable. Remaining changes: (LP: #1201029)
17diff --git a/debian/patches/smb-compat-fix.patch b/debian/patches/smb-compat-fix.patch
18new file mode 100644
19index 0000000..b436970
20--- /dev/null
21+++ b/debian/patches/smb-compat-fix.patch
22@@ -0,0 +1,66 @@
23+From d7a8403b537ed0068e862abc20065e98209527b7 Mon Sep 17 00:00:00 2001
24+From: Maksym Schipka <maksym@hotmail.com>
25+Date: Sun, 14 Aug 2016 09:24:52 +0100
26+Subject: [PATCH] Fixing compatibility with Samba 4.3
27+Origin: https://github.com/backuppc/backuppc/commit/c550528be63c3b66744830e992b908fff3f2e271
28+Bug: https://github.com/backuppc/backuppc/issues/21
29+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/backuppc/+bug/1576187
30+Bug-Debian: https://bugzilla.samba.org/show_bug.cgi?id=11642
31+Last-Update: 2018-10-16
32+
33+---
34+ bin/BackupPC_dump | 2 ++
35+ lib/BackupPC/Xfer/Smb.pm | 13 ++++++++++++-
36+ 2 files changed, 14 insertions(+), 1 deletion(-)
37+diff --git a/bin/BackupPC_dump b/bin/BackupPC_dump
38+index 9491046..e41cb68 100755
39+--- debian/backuppc/usr/share/backuppc/bin/BackupPC_dump
40++++ debian/backuppc/usr/share/backuppc/bin/BackupPC_dump
41+@@ -873,6 +873,8 @@ for my $shareName ( @$ShareNames ) {
42+ # Merge the xfer status (need to accumulate counts)
43+ #
44+ my $newStat = $xfer->getStats;
45++ # MAKSYM 14082016: forcing the right file count if some bytes were transferred; ensures compatibility with at least Samba-4.3
46++ $newStat->{fileCnt} = $nFilesTotal if ( $useTar && $newStat->{fileCnt} == 0 && $xfer->getStats->{byteCnt} > 0 );
47+ if ( $newStat->{fileCnt} == 0 ) {
48+ $noFilesErr ||= "No files dumped for share $shareName";
49+ }
50+diff --git a/lib/BackupPC/Xfer/Smb.pm b/lib/BackupPC/Xfer/Smb.pm
51+index eaf002e..48964cb 100644
52+--- debian/backuppc/usr/share/backuppc/lib/BackupPC/Xfer/Smb.pm
53++++ debian/backuppc/usr/share/backuppc/lib/BackupPC/Xfer/Smb.pm
54+@@ -217,6 +217,8 @@ sub readOutput
55+ # This section is highly dependent on the version of smbclient.
56+ # If you upgrade Samba, make sure that these regexp are still valid.
57+ #
58++ # MAKSYM 14082016: The next regex will never match on Samba-4.3, as
59++ # smbclient doesn't produce output required; keeping it for older Sambas
60+ if ( /^\s*(-?\d+) \(\s*\d+[.,]\d kb\/s\) (.*)$/ ) {
61+ my $sambaFileSize = $1;
62+ my $pcFileName = $2;
63+@@ -230,8 +232,15 @@ sub readOutput
64+ $t->{byteCnt} += $2;
65+ $t->{fileCnt}++;
66+ $t->{XferLOG}->write(\"$_\n") if ( $t->{logLevel} >= 1 );
67+- } elsif ( /^\s*tar: dumped \d+ files/ ) {
68++ } elsif ( /^\s*tar: dumped (\d+) files/) {
69++ # MAKSYM 14082016: Updating file count to the likely number
70+ $t->{xferOK} = 1;
71++ $t->{fileCnt} = $1;
72++ $t->{XferLOG}->write(\"$_\n") if ( $t->{logLevel} >= 0 );
73++ } elsif ( /^\s*tar:\d+\s*Total bytes received: (\d+)/) {
74++ # MAKSYM 14082016: Updating byte count to the likely number
75++ $t->{xferOK} = 1;
76++ $t->{byteCnt} = $1;
77+ $t->{XferLOG}->write(\"$_\n") if ( $t->{logLevel} >= 0 );
78+ } elsif ( /^\s*tar: restored \d+ files/ ) {
79+ $t->{xferOK} = 1;
80+@@ -270,6 +279,8 @@ sub readOutput
81+ } elsif ( /^\s*directory \\/i ) {
82+ $t->{XferLOG}->write(\"$_\n") if ( $t->{logLevel} >= 2 );
83+ } elsif ( /smb: \\>/
84++ || /^\s*tar:\d+/ # MAKSYM 14082016: ignoring 2 more Samba-4.3 specific lines
85++ || /^\s*WARNING:/i
86+ || /^\s*added interface/i
87+ || /^\s*tarmode is now/i
88+ || /^\s*Total bytes written/i
89diff --git a/debian/rules b/debian/rules
90index 3f65022..55e790c 100755
91--- a/debian/rules
92+++ b/debian/rules
93@@ -75,6 +75,7 @@ install: build
94 patch --no-backup-if-mismatch -p0 < debian/patches/config.pl.diff
95 install --mode=644 debian/backuppc/etc/backuppc/config.pl debian/backuppc/usr/share/backuppc/conf
96 rm -rf debian/backuppc/etc/backuppc/config.pl
97+ patch --no-backup-if-mismatch -p0 < debian/patches/smb-compat-fix.patch
98
99 # Build architecture-independent files here.
100 binary-indep: build install
101diff --git a/debian/tests/control b/debian/tests/control
102new file mode 100644
103index 0000000..c07f175
104--- /dev/null
105+++ b/debian/tests/control
106@@ -0,0 +1,3 @@
107+Tests: smb-backup
108+Depends: @, samba, smbclient
109+Restrictions: isolation-container, allow-stderr, needs-root
110diff --git a/debian/tests/smb-backup b/debian/tests/smb-backup
111new file mode 100644
112index 0000000..7ec3f21
113--- /dev/null
114+++ b/debian/tests/smb-backup
115@@ -0,0 +1,89 @@
116+#!/bin/sh
117+
118+set -ex
119+
120+check_status() {
121+ local status_code=$1
122+ local msg="${2:-task}"
123+
124+ if [ "$status_code" -ne "0" ]; then
125+ echo "ERROR: $msg failed, exit status was $status_code"
126+ exit "$status_code"
127+ else
128+ echo "OK: $msg succeeded"
129+ fi
130+}
131+
132+populate_directory() {
133+ local target="$1"
134+ local tempfile
135+
136+ # target must exist and be a directory
137+ test -d "$target" || exit 1
138+
139+ for n in $(seq 1 10); do
140+ tempfile=$(mktemp "$target/tmp.XXXXXX")
141+ # the date command makes each tempfile different
142+ # the seq command just produces a lot of output
143+ # using a "here" document as to not pollute the set -x
144+ # output with large command lines
145+ cat > "$tempfile" <<EOF
146+$(date +%s%N)
147+$(seq 1 81920)
148+EOF
149+ chmod 0644 "$tempfile"
150+ done
151+}
152+
153+add_samba_share() {
154+ if ! testparm -s 2>&1 | grep -qE "^\[public\]"; then
155+ echo "Adding [public] share"
156+ cat >> /etc/samba/smb.conf <<EOFEOF
157+[public]
158+ path = /public
159+ read only = no
160+ guest ok = yes
161+EOFEOF
162+ service smbd reload
163+ else
164+ echo "No need to add [public] share, continuing."
165+ fi
166+ echo "Populating share"
167+ rm -rf /public
168+ mkdir -p /public
169+ chmod 1777 /public
170+ populate_directory /public
171+}
172+
173+add_localhost_backuppc_config() {
174+ cat > /etc/backuppc/localhost.pl <<EOFEOF
175+\$Conf{XferMethod} = 'smb';
176+\$Conf{XferLogLevel} = 1;
177+\$Conf{ClientCharset} = '';
178+\$Conf{ClientCharsetLegacy} = 'iso-8859-1';
179+\$Conf{SmbShareName} = 'public';
180+\$Conf{SmbShareUserName} = '';
181+\$Conf{SmbSharePasswd} = '';
182+\$Conf{PingPath} = '/bin/ping';
183+\$Conf{Ping6Path} = '/bin/ping6';
184+EOFEOF
185+}
186+
187+echo "Adding samba share"
188+add_samba_share
189+
190+echo "Configuring backuppc"
191+add_localhost_backuppc_config
192+
193+result=0
194+echo "Performing a full backup"
195+sudo -u backuppc -H /usr/share/backuppc/bin/BackupPC_dump -v -f localhost || result=$?
196+check_status $result "Full backup"
197+
198+result=0
199+echo "Changing share content and performing an incremental backup"
200+populate_directory /public
201+sudo -u backuppc -H /usr/share/backuppc/bin/BackupPC_dump -v -i localhost || result=$?
202+check_status $result "Incremental backup"
203+
204+echo "Done."

Subscribers

People subscribed via source and target branches