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

[Andreas Hasenack]

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

[Andreas Hasenack (ahasenack)]

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?

[Andreas Hasenack (ahasenack)]

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

[Christian Ehrhardt  (paelzer)]

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

[Andreas Hasenack (ahasenack)]

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

[Christian Ehrhardt  (paelzer)]

LGTM now - thanks +1

review: Approve

[Andreas Hasenack (ahasenack)]

This now contains all the latest changes.

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

[Christian Ehrhardt  (paelzer)]

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.

[Andreas Hasenack (ahasenack)]

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

[Andreas Hasenack (ahasenack)]

Tagged and uploaded.

diff --git a/debian/changelog b/debian/changelog
index 1badf10..90b3286 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+backuppc (3.3.0-1ubuntu1.1) trusty; urgency=medium
+
+  * d/rules, d/p/smb-compat-fix.patch: cope with changes in newer smbclient.
+    Thanks to Maksym Schipka <[e-mail]> (LP: #1576187)
+  * d/t/{control,smb-backup}: simple smb-based DEP8 test (LP: #1677755)
+
+ -- Andreas Hasenack <[e-mail]>  Tue, 23 Oct 2018 09:06:27 -0300
+
 backuppc (3.3.0-1ubuntu1) trusty-proposed; urgency=low
 
   * Merge from Debian unstable. Remaining changes: (LP: #1201029)
diff --git a/debian/patches/smb-compat-fix.patch b/debian/patches/smb-compat-fix.patch
 new file mode 100644
index 0000000..b436970
--- /dev/null
+++ b/debian/patches/smb-compat-fix.patch
@@ -0,0 +1,66 @@
+From d7a8403b537ed0068e862abc20065e98209527b7 Mon Sep 17 00:00:00 2001
+From: Maksym Schipka <[e-mail]>
+Date: Sun, 14 Aug 2016 09:24:52 +0100
+Subject: [PATCH] Fixing compatibility with Samba 4.3
+Origin: https://github.com/backuppc/backuppc/commit/c550528be63c3b66744830e992b908fff3f2e271
+Bug: https://github.com/backuppc/backuppc/issues/21
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/backuppc/+bug/1576187
+Bug-Debian: https://bugzilla.samba.org/show_bug.cgi?id=11642
+Last-Update: 2018-10-16
+
+---
+ bin/BackupPC_dump        |  2 ++
+ lib/BackupPC/Xfer/Smb.pm | 13 ++++++++++++-
+ 2 files changed, 14 insertions(+), 1 deletion(-)
+diff --git a/bin/BackupPC_dump b/bin/BackupPC_dump
+index 9491046..e41cb68 100755
+--- debian/backuppc/usr/share/backuppc/bin/BackupPC_dump
++++ debian/backuppc/usr/share/backuppc/bin/BackupPC_dump
+@@ -873,6 +873,8 @@ for my $shareName ( @$ShareNames ) {
+     # Merge the xfer status (need to accumulate counts)
+     #
+     my $newStat = $xfer->getStats;
++    # MAKSYM 14082016: forcing the right file count if some bytes were transferred; ensures compatibility with at least Samba-4.3
++    $newStat->{fileCnt} = $nFilesTotal if ( $useTar && $newStat->{fileCnt} == 0 && $xfer->getStats->{byteCnt} > 0 );
+     if ( $newStat->{fileCnt} == 0 ) {
+        $noFilesErr ||= "No files dumped for share $shareName";
+     }
+diff --git a/lib/BackupPC/Xfer/Smb.pm b/lib/BackupPC/Xfer/Smb.pm
+index eaf002e..48964cb 100644
+--- debian/backuppc/usr/share/backuppc/lib/BackupPC/Xfer/Smb.pm
++++ debian/backuppc/usr/share/backuppc/lib/BackupPC/Xfer/Smb.pm
+@@ -217,6 +217,8 @@ sub readOutput
+         # This section is highly dependent on the version of smbclient.
+         # If you upgrade Samba, make sure that these regexp are still valid.
+         #
++        # MAKSYM 14082016: The next regex will never match on Samba-4.3, as
++        # smbclient doesn't produce output required; keeping it for older Sambas
+         if ( /^\s*(-?\d+) \(\s*\d+[.,]\d kb\/s\) (.*)$/ ) {
+             my $sambaFileSize = $1;
+             my $pcFileName    = $2;
+@@ -230,8 +232,15 @@ sub readOutput
+             $t->{byteCnt} += $2;
+             $t->{fileCnt}++;
+             $t->{XferLOG}->write(\"$_\n") if ( $t->{logLevel} >= 1 );
+-        } elsif ( /^\s*tar: dumped \d+ files/ ) {
++        } elsif ( /^\s*tar: dumped (\d+) files/) {
++            # MAKSYM 14082016: Updating file count to the likely number
+             $t->{xferOK} = 1;
++            $t->{fileCnt} = $1;
++            $t->{XferLOG}->write(\"$_\n") if ( $t->{logLevel} >= 0 );
++        } elsif ( /^\s*tar:\d+\s*Total bytes received: (\d+)/) {
++            # MAKSYM 14082016: Updating byte count to the likely number
++            $t->{xferOK} = 1;
++            $t->{byteCnt} = $1;
+             $t->{XferLOG}->write(\"$_\n") if ( $t->{logLevel} >= 0 );
+         } elsif ( /^\s*tar: restored \d+ files/ ) {
+             $t->{xferOK} = 1;
+@@ -270,6 +279,8 @@ sub readOutput
+         } elsif ( /^\s*directory \\/i ) {
+             $t->{XferLOG}->write(\"$_\n") if ( $t->{logLevel} >= 2 );
+         } elsif ( /smb: \\>/
++                || /^\s*tar:\d+/ # MAKSYM 14082016: ignoring 2 more Samba-4.3 specific lines
++                || /^\s*WARNING:/i
+                 || /^\s*added interface/i
+                 || /^\s*tarmode is now/i
+                 || /^\s*Total bytes written/i
diff --git a/debian/rules b/debian/rules
index 3f65022..55e790c 100755
--- a/debian/rules
+++ b/debian/rules
@@ -75,6 +75,7 @@ install: build
 	patch --no-backup-if-mismatch -p0 < debian/patches/config.pl.diff
 	install --mode=644 debian/backuppc/etc/backuppc/config.pl debian/backuppc/usr/share/backuppc/conf
 	rm -rf debian/backuppc/etc/backuppc/config.pl
+	patch --no-backup-if-mismatch -p0 < debian/patches/smb-compat-fix.patch
 
 # Build architecture-independent files here.
 binary-indep: build install
diff --git a/debian/tests/control b/debian/tests/control
 new file mode 100644
index 0000000..c07f175
--- /dev/null
+++ b/debian/tests/control
@@ -0,0 +1,3 @@
+Tests: smb-backup
+Depends: @, samba, smbclient
+Restrictions: isolation-container, allow-stderr, needs-root
diff --git a/debian/tests/smb-backup b/debian/tests/smb-backup
 new file mode 100644
index 0000000..7ec3f21
--- /dev/null
+++ b/debian/tests/smb-backup
@@ -0,0 +1,89 @@
+#!/bin/sh
+
+set -ex
+
+check_status() {
+    local status_code=$1
+    local msg="${2:-task}"
+
+    if [ "$status_code" -ne "0" ]; then
+        echo "ERROR: $msg failed, exit status was $status_code"
+        exit "$status_code"
+    else
+        echo "OK: $msg succeeded"
+    fi
+}
+
+populate_directory() {
+    local target="$1"
+    local tempfile
+
+    # target must exist and be a directory
+    test -d "$target" || exit 1
+
+    for n in $(seq 1 10); do
+        tempfile=$(mktemp "$target/tmp.XXXXXX")
+        # the date command makes each tempfile different
+        # the seq command just produces a lot of output
+        # using a "here" document as to not pollute the set -x
+        # output with large command lines
+        cat > "$tempfile" <<EOF
+$(date +%s%N)
+$(seq 1 81920)
+EOF
+    chmod 0644 "$tempfile"
+    done
+}
+
+add_samba_share() {
+    if ! testparm -s 2>&1 | grep -qE "^\[public\]"; then
+        echo "Adding [public] share"
+        cat >> /etc/samba/smb.conf <<EOFEOF
+[public]
+  path = /public
+  read only = no
+  guest ok = yes
+EOFEOF
+        service smbd reload
+    else
+        echo "No need to add [public] share, continuing."
+    fi
+    echo "Populating share"
+    rm -rf /public
+    mkdir -p /public
+    chmod 1777 /public
+    populate_directory /public
+}
+
+add_localhost_backuppc_config() {
+    cat > /etc/backuppc/localhost.pl <<EOFEOF
+\$Conf{XferMethod} = 'smb';
+\$Conf{XferLogLevel} = 1;
+\$Conf{ClientCharset} = '';
+\$Conf{ClientCharsetLegacy} = 'iso-8859-1';
+\$Conf{SmbShareName} = 'public';
+\$Conf{SmbShareUserName} = '';
+\$Conf{SmbSharePasswd} = '';
+\$Conf{PingPath} = '/bin/ping';
+\$Conf{Ping6Path} = '/bin/ping6';
+EOFEOF
+}
+
+echo "Adding samba share"
+add_samba_share
+
+echo "Configuring backuppc"
+add_localhost_backuppc_config
+
+result=0
+echo "Performing a full backup"
+sudo -u backuppc -H /usr/share/backuppc/bin/BackupPC_dump -v -f localhost || result=$?
+check_status $result "Full backup"
+
+result=0
+echo "Changing share content and performing an incremental backup"
+populate_directory /public
+sudo -u backuppc -H /usr/share/backuppc/bin/BackupPC_dump -v -i localhost || result=$?
+check_status $result "Incremental backup"
+
+echo "Done."