[buildd-tools-devel] Bug#763635: Bug#763635: sbuild: sbuild-update writes a stray line to stderr

Wookey wookey at wookware.org
Wed Oct 8 01:59:32 UTC 2014


+++ Wookey [2014-10-03 14:56 +0100]:
> +++ Adam Borowski [2014-10-01 21:45 +0200]:
> > On Wed, Oct 01, 2014 at 07:32:26PM +0100, Wookey wrote:
> > > The issue is this line in lib/Sbuild/ResolverBase.pm:
> > >  $self->log("Initial foreign arches: '@existing_foreign_arches'\n");
> > > 
> > > Which is a boring $self->log function like many many others. 
> 
> > Running with $debug set to 1 confirms that indeed $logfile is not set up
> > yet.  So it appears you're calling log() too early.
> 
> Thanks for that debug. You are correct. 
> 
> This is slightly harder to fix than I was hoping as the original patch
> did not properly separate reporting and setup. I hope to upload a
> fixed version in the next day or two.

OK. This took me some time due to having to learn some more about
sbuild's/perl object inheritance structure, and thus scoping, getting
confused about what should be visible where and wondering why on earth
it was so hard to print the keys of a hash.

Attached is a patch that fixes the stderr issue by determining the
initial foreign arches later (dpkg setup instead of resolver object
instantiation), moving the log notifications from low-level functions
into the higher-level Build.pm code, and properly recording the status
in the generate_stats function. I think all this is is an improvement.

I will upload this once I've got it tidy in git.

Wookey
-- 
Principal hats:  Linaro, Emdebian, Wookware, Balloonboard, ARM
http://wookware.org/
-------------- next part --------------
diff --git a/HACKING b/HACKING
index 0c50452..7889c2f 100644
--- a/HACKING
+++ b/HACKING
@@ -66,10 +66,15 @@ New upstream releases:
 
 ? Run ./configure and then "make dist" to generate the release tarball.
 
+
 New Debian releases:
 
-? Copy the release tarball out of the build dir, renaming to
-  sbuild_<ver>.orig.tar.gz then unpack it and use it to build the Debian release.
+? Run debian/git-tag-debian in the git source to tag the debian
+  release.
+
+? Copy the upstream (make dist) release tarball out of the build dir,
+  rename it to sbuild_<ver>.orig.tar.gz then unpack it and use it to
+  build the Debian release (with the usual tools such as
+  dpkg-buildpackage, or sbuild).
 
-? Run debian/git-tag-debian to tag the release, then dpkg-buildpackage.
 
diff --git a/debian/changelog b/debian/changelog
index 31952d4..403c972 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+sbuild (0.64.3-2) UNRELEASED; urgency=medium
+
+  * Ensure new messages go to stdout, not stderr (Closes: #763635)
+
+ -- Wookey <wookey at debian.org>  Wed, 01 Oct 2014 17:32:42 +0100
+
 sbuild (0.64.3-1) unstable; urgency=medium
 
   * Team upload
diff --git a/lib/Sbuild/Build.pm b/lib/Sbuild/Build.pm
index cd49c90..ac6ca4a 100644
--- a/lib/Sbuild/Build.pm
+++ b/lib/Sbuild/Build.pm
@@ -804,6 +804,7 @@ sub fetch_source_files {
     my $pkg = $self->get('Package');
     my $ver = $self->get('OVersion');
     my $host_arch = $self->get('Host Arch');
+    my $resolver = $self->get('Dependency Resolver');
 
     my ($dscarchs, $dscpkg, $dscver, @fetched);
 
@@ -964,8 +965,9 @@ sub fetch_source_files {
     $build_conflicts_indep =~ s/\n\s+/ /g if defined $build_conflicts_indep;
 
 
+    $self->log_subsubsection("Check foreign Arches");
     # Check for cross-arch dependencies
-    # parse $build_depends* for explicit :arch and add the foreign arch, as needed
+    # parse $build_depends* for explicit :arch and add the foreign arches, as needed
     sub get_explicit_arches
     {
         my $visited_deps = pop;
@@ -974,7 +976,7 @@ sub fetch_source_files {
         my %set;
         for my $dep (@deps)
         {
-            # I make sure to break any recursion in the deps data structure
+            # Break any recursion in the deps data structure (is this overkill?)
             next if !defined $dep;
             my $id = ref($dep) ? refaddr($dep) : "str:$dep";
             next if $visited_deps->{$id};
@@ -1008,13 +1010,17 @@ sub fetch_source_files {
     my $added_any_new;
     for my $foreign_arch(@foreign_arches)
     {
-        my $resolver = $self->get('Dependency Resolver');
         $resolver->add_foreign_architecture($foreign_arch);
         $added_any_new = 1;
     }
     $self->run_chroot_update() if $added_any_new;
 
+    my @keylist=keys %{$resolver->get('Initial Foreign Arches')};
+    $self->log( 'Initial Foreign Architectures: ');
+    $self->log( join ' ', @keylist, "\n");
 
+    $self->log('Foreign Architectures in build-deps: ');
+    $self->log( join ' ', @foreign_arches, "\n\n");
 
 
 
@@ -1042,6 +1048,7 @@ sub fetch_source_files {
 
     debug("Arch check ok ($host_arch included in $dscarchs)\n");
 
+
     $self->set('Build Depends', $build_depends);
     $self->set('Build Depends Arch', $build_depends_arch);
     $self->set('Build Depends Indep', $build_depends_indep);
@@ -1961,6 +1968,7 @@ sub add_stat {
 
 sub generate_stats {
     my $self = shift;
+    my $resolver = $self->get('Dependency Resolver');
 
     $self->add_stat('Job', $self->get('Job'));
     $self->add_stat('Package', $self->get('Package'));
@@ -1969,6 +1977,10 @@ sub generate_stats {
     $self->add_stat('Machine Architecture', $self->get_conf('ARCH'));
     $self->add_stat('Host Architecture', $self->get('Host Arch'));
     $self->add_stat('Build Architecture', $self->get('Build Arch'));
+    my @keylist=keys %{$resolver->get('Initial Foreign Arches')};
+    push @keylist, keys %{$resolver->get('Added Foreign Arches')};
+    my $foreign_arches = join ' ', @keylist;
+    $self->add_stat('Foreign Architectures', $foreign_arches );
     $self->add_stat('Distribution', $self->get_conf('DISTRIBUTION'));
     $self->add_stat('Space', $self->get('This Space'));
     $self->add_stat('Build-Time',
diff --git a/lib/Sbuild/ResolverBase.pm b/lib/Sbuild/ResolverBase.pm
index 531685a..a62a739 100644
--- a/lib/Sbuild/ResolverBase.pm
+++ b/lib/Sbuild/ResolverBase.pm
@@ -58,7 +58,7 @@ sub new {
     # Typically set by Sbuild::Build, but not outside a build context.
     $self->set('Host Arch', $self->get_conf('HOST_ARCH'));
     $self->set('Build Arch', $self->get_conf('BUILD_ARCH'));
-    $self->set('Initial Foreign Arches', $self->get_foreign_architectures());
+    $self->set('Initial Foreign Arches', {});
     $self->set('Added Foreign Arches', {});
 
     my $dummy_archive_list_file = $session->get('Location') .
@@ -94,8 +94,8 @@ sub setup {
 	print $F "APT::Install-Recommends false;\n";
 
 	if ($self->get('Host Arch') ne $self->get('Build Arch')) {
-	    print $F "APT::Architecture=".$self->get('Host Arch');
-	    $self->log("Adding APT::Architecture ".$self->get('Host Arch')." to the apt config");
+	    print $F 'APT::Architecture=' . $self->get('Host Arch');
+	    $self->log('Adding APT::Architecture ' . $self->get('Host Arch') . ' to the apt config');
 	}
 	if ($self->get('Split')) {
 	    print $F "Dir \"$chroot_dir\";\n";
@@ -161,8 +161,6 @@ sub get_foreign_architectures {
         return {};
     }
 
-    $self->log("Initial foreign arches: '@existing_foreign_arches'\n");
-
     my %set;
     foreach (@existing_foreign_arches) { $set{$_} = 1; }
     return \%set;
@@ -193,8 +191,9 @@ sub add_foreign_architecture {
         $self->log_error("Failed to set dpkg foreign-architecture config\n");
         return 0;
     }
+    debug("Added foreign arch: $arch\n") if $arch;
+
     $added_foreign_arches->{$arch} = 1;
-    $self->log("Adding dpkg foreign-architecture $arch\n");
     return 1;
 }
 
@@ -223,6 +222,9 @@ sub setup_dpkg {
 
     my $session = $self->get('Session');
 
+    # Record initial foreign arch state so it can be restored
+    $self->set('Initial Foreign Arches', $self->get_foreign_architectures());
+
     if ($self->get('Host Arch') ne $self->get('Build Arch')) {
 	add_foreign_architecture($session, $self->get('Host Arch'))
     }
@@ -738,8 +740,8 @@ sub setup_apt_archive {
 	  USER => 'root',
 	  DIR => '/' });
     if ($?) {
-	$self->log_error("E: Failed to set " . $self->get_conf('BUILD_USER') .
-			 ":sbuild ownership on $dummy_gpghome\n");
+	$self->log_error('E: Failed to set ' . $self->get_conf('BUILD_USER') .
+			 ':sbuild ownership on $dummy_gpghome\n');
 	return 0;
     }
     if (!(-d $dummy_archive_dir || mkdir $dummy_archive_dir, 0775)) {


More information about the Buildd-tools-devel mailing list