debian/changelog

@@ -27,6 +27,9 @@ debhelper (11.2) UNRELEASED; urgency=medium
   * dh_installxfonts: Fix typo that causes a misc:Depends on
     non-existing xfont-utils.  (Closes: #894835)
   * dh_installwm.1: Document --all.  (Closes: #895011)
+  * dh_usrlocal: Fix bug where the generated prerm script generated
+    by dh_usrlocal could remove a directory directly in /usr/local.
+    (Closes: #894549)
 
   [ Mattia Rizzolo ]
   * Lower the version restrictions on dpkg and dpkg-dev.  They are not

dh_usrlocal

@@ -77,54 +77,49 @@ foreach my $package (@{$dh{DOPACKAGES}}) {
 
 	if (-d "$tmp/usr/local") {
 		my (@dirs, @justdirs);
-		find({bydepth => 1,
-		      no_chdir => 1,
-		      wanted => sub {
-			my $fn = $File::Find::name;
-			if (-d $fn) {
-				my $user = 'root';
-				my $group = 'staff';
-				my $mode = '02775';
-				if (should_use_root()) {
-					my $stat = stat $fn;
-					$user = getpwuid $stat->uid;
-					$group = getgrgid $stat->gid;
-					$mode = sprintf "%04lo", ($stat->mode & 07777);
-					if ($stat->uid == 0 && $stat->gid == 0) {
-						$group = 'staff';
-						$mode = '02775';
-					}
-				}
-
-
-
-				$fn =~ s!^\Q$tmp\E!!;
-				return if $fn eq '/usr/local';
-				
-				# @dirs is in parents-first order for dir creation...
-				unshift @dirs, "$fn $mode $user $group";
-				# ...whereas @justdirs is depth-first for removal.
-				push @justdirs, $fn;
-				doit('rmdir', $_);
-			}
-			else {
-				warning("$fn is not a directory");
-			}
+		find({no_chdir => 1,
+		      preprocess => sub {
+				  # Ensure a reproducible traversal.
+				  return sort @_;
+		      },
+		      postprocess => sub {
+				  # Uninstall, unless a direct child of /usr/local.
+				  $_ = $File::Find::dir;
+				  s!^\Q$tmp\E!!;
+				  push @justdirs, $_ if m!/usr/local/.*/!;
+				  # Remove a directory after its childs.
+				  doit('rmdir', $File::Find::dir);
+		      },
+			  wanted => sub {
+				  # rmdir would fail later anyways.
+				  error("${File::Find::name} is not a directory")
+					  if not -d $File::Find::name;
+				  # Install directory before its childs.
+				  my $fn = $File::Find::name;
+				  $fn =~ s!^\Q$tmp\E!!;
+				  return if $fn eq '/usr/local';
+				  if (should_use_root()) {
+					  my $stat = stat $File::Find::dir;
+					  if ($stat->uid == 0 && $stat->gid == 0) {
+						  push @dirs, "$fn 02775 root staff";
+					  } else {
+						  my $user = getpwuid $stat->uid;
+						  my $group = getgrgid $stat->gid;
+						  my $mode = sprintf "%04lo", ($stat->mode & 07777);
+						  push @dirs, "$fn $mode $user $group";
+					  }
+				  } else {
+					  push @dirs, "$fn 02775 root staff";
+				  }
 		      }}, "$tmp/usr/local");
-		doit('rmdir', "$tmp/usr/local");
-	
-		my $bs = "\\";     # A single plain backslash
-		my $ebs = $bs x 2; # Escape the backslash from the shell
+
 		# This constructs the body of a 'sed' c\ expression which
 		# is parsed by the shell in double-quotes
-		my $dirs = join("$ebs\n", sort @dirs);
-		pop @justdirs; # don't remove directories directly in /usr/local
-		my $justdirs = join("$ebs\n", reverse sort @justdirs);
 		if (! $dh{NOSCRIPTS}) { 
 			autoscript($package,"postinst", "postinst-usrlocal",
-				   "/#DIRS#/ c${ebs}\n${dirs}");
+					   { 'DIRS' => join ("\n", @dirs)}) if @dirs;
 			autoscript($package,"prerm", "prerm-usrlocal",
-				   "/#JUSTDIRS#/ c${ebs}\n${justdirs}") if length $justdirs;
+					   { 'JUSTDIRS' => join ("\n", @justdirs)}) if @justdirs;
 		}
 	}
 }

t/dh_usrlocal/01-basic.t

+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Test::More;
+plan(tests => 1);
+
+use File::Path qw(remove_tree);
+use File::Basename qw(dirname);
+use lib dirname(dirname(__FILE__));
+use Test::DH;
+use Debian::Debhelper::Dh_Lib qw(!dirname);
+
+sub extract_generated_lines {
+	my ($file) = @_;
+	my (@lines, $marker);
+	return if not -f $file;
+	open(my $fd, '<', $file) or error("open($file) failed: $!");
+	while (my $line = <$fd>) {
+		chomp($line);
+		if (defined($marker)) {
+			last if $line eq $marker;
+			push(@lines, $line);
+			next;
+		}
+		if ($line =~ m{\s*<<\s*(\S+)\s*$}) {
+			$marker = $1;
+		}
+	}
+	close($fd);
+	return @lines;
+}
+
+each_compat_subtest {
+
+	my (@postinst, @prerm);
+	my @scripts = qw(
+        debian/debhelper.postinst.debhelper
+        debian/debhelper.prerm.debhelper
+    );
+
+	rm_files(@scripts);
+	remove_tree('debian/debhelper');
+	install_dir('debian/debhelper/usr/local/foo');
+	install_dir('debian/debhelper/usr/local/bar');
+
+	ok(run_dh_tool('dh_usrlocal'));
+	@postinst = extract_generated_lines("debian/debhelper.postinst.debhelper");
+	@prerm = extract_generated_lines("debian/debhelper.prerm.debhelper");
+
+	is_deeply(\@postinst, [
+				  '/usr/local/bar 02775 root staff',
+				  '/usr/local/foo 02775 root staff',
+			  ], "Correct dir creation")
+		or do { diag("postinst: $_") for @postinst; };
+	is_deeply(\@prerm, [], "No removal of top level dirs #894549")
+		or do { diag("prerm: $_") for @prerm; };
+	
+	remove_tree('debian/debhelper');
+	rm_files(@scripts);
+	install_dir('debian/debhelper/usr/local/foo/dir/somewhere');
+	install_dir('debian/debhelper/usr/local/bar/another-dir/elsewhere');
+
+	ok(run_dh_tool('dh_usrlocal'));
+
+	@postinst = extract_generated_lines("debian/debhelper.postinst.debhelper");
+	@prerm = extract_generated_lines("debian/debhelper.prerm.debhelper");
+
+	is_deeply(\@postinst, [
+				  '/usr/local/bar 02775 root staff',
+				  '/usr/local/bar/another-dir 02775 root staff',
+				  '/usr/local/bar/another-dir/elsewhere 02775 root staff',
+				  '/usr/local/foo 02775 root staff',
+				  '/usr/local/foo/dir 02775 root staff',
+				  '/usr/local/foo/dir/somewhere 02775 root staff',
+			  ], "Correct dir creation")
+		or do { diag("postinst: $_") for @postinst; };
+	is_deeply(\@prerm, [
+				  '/usr/local/bar/another-dir/elsewhere',
+				  '/usr/local/bar/another-dir',
+				  '/usr/local/foo/dir/somewhere',
+				  '/usr/local/foo/dir',
+			  ], "Correct dir removal")
+		or do { diag("prerm: $_") for @prerm; };
+};
+