Hi, I have now subscribed to the debian-dpkg list so we can communicate directly through the list. > There are several problems with your patch. Let's go through them: > > diff --git a/scripts/dpkg-shlibdeps.pl b/scripts/dpkg-shlibdeps.pl > > index 91ea4e5..f96669d 100755 > > --- a/scripts/dpkg-shlibdeps.pl > > +++ b/scripts/dpkg-shlibdeps.pl > > @@ -163,29 +163,38 @@ foreach my $file (keys %exec) { > > my %altlibfiles; > > my %soname_notfound; > > my %alt_soname; > > + > > + # Used to remember to quit if an error > > + my $soname_error_found; > > First of all, there are indentations problems. A tabulations is always > equivalent to 8 spaces. (And you seem to use a setting where a tab means > 4 spaces). And due to this, you completely broke the indentation. I don't > know what editors you use, but be sure to fix this. Sorry, using gedit, I think I have fixed this now. > You might better name this variable $error_count and you need to > initialize to 0 (and this probably must be done outside the main loop > (see comments about where you return the failure). Done. > > - unless (defined $lib) { > > - $soname_notfound{$soname} = 1; > > - $global_soname_notfound{$soname} = 1; > > - my $msg = _g("couldn't find library %s needed by %s (ELF format: '%s'; RPATH: '%s').\n" . > > - "Note: libraries are not searched in other binary packages " . > > - "that do not have any shlibs or symbols file.\nTo help dpkg-shlibdeps " . > > - "find private libraries, you might need to set LD_LIBRARY_PATH."); > > - if (scalar(split_soname($soname))) { > > - error($msg, $soname, $file, $obj->{format}, join(":", @{$obj->{RPATH}})); > > - } else { > > - warning($msg, $soname, $file, $obj->{format}, join(":", @{$obj->{RPATH}})); > > + my $lib = my_find_library($soname, $obj->{RPATH}, $obj->{format}, $file); > > + if (defined $lib) { > > You changed an "unless" test in a "if" test. You reordered two > blocks of code without a good reason. It makes it difficult to see what is > the core of your change. If you want to make miscellaneous improvements > while fixing the bug, please create a dedicated git branch and commit each > change separately so that it can be better reviewed. I have removed my changes to the flow. I just like using stacked if commands instead of "next;" to perform flow control, as I think its neater and clearer. > > + $libfiles{$lib} = $soname; > > + my $reallib = realpath($lib); > > + if ($reallib ne $lib) { > > + $altlibfiles{$reallib} = $soname; > > + } > > + print "Library $soname found in $lib\n" if $debug; > > + } else { > > + $soname_notfound{$soname} = 1; > > + $global_soname_notfound{$soname} = 1; > > + my $msg = _g("couldn't find library %s needed by %s (ELF format: '%s'; RPATH: '%s').\n" . > > + "Note: libraries are not searched in other binary packages " . > > + "that do not have any shlibs or symbols file.\nTo help dpkg-shlibdeps " . > > + "find private libraries, you might need to set LD_LIBRARY_PATH."); > > + if (scalar(split_soname($soname))) { > > + errormsg($msg, $soname, $file, $obj->{format}, join(":", @{$obj->{RPATH}})); > > + $soname_error_found = $soname_error_found + 1; > > I prefer one of the shorter forms: > $soname_error_found++; > $soname_error_found += 1; Done. > > + } else { > > + warning($msg, $soname, $file, $obj->{format}, join(":", @{$obj->{RPATH}})); > > + } > > } > > - next; > > - } > > - $libfiles{$lib} = $soname; > > - my $reallib = realpath($lib); > > - if ($reallib ne $lib) { > > - $altlibfiles{$reallib} = $soname; > > - } > > - print "Library $soname found in $lib\n" if $debug; > > } > > + if ($soname_error_found == 1) { > > + error("Cannot continue due to the error above."); > > + } elsif ($soname_error_found > 1) { > > + error("Cannot continue due to the errors listed above."); > > + } > > Failing here is not the best place. You are still in the main loop > (foreach my $file (keys %exec)) and it means you still have other files to > analyze... so you will not achieve the goal of #596841 which is to see > all errors before failing. You should really move the failure outside of > the main loop (just after it) and so you need to move the variable > counting the errors outside of the loop scope as well. I half achieved it, I was testing my changes using one binary, so I saw more errors, but not as many as I should have. I have now moved the $error_count variable to before the loop, and the failure to after the main loop, and after the package level warning code. > The error message here must be translated and you should use ngettext > to retrieve the correct error message (ngettext is wrapped in P_() > by Dpkg::Gettext): > P_("Cannot continue due to the error above.", > "Cannot continue due to the errors listed above.", > $soname_error_found); > > > my $file2pkg = find_packages(keys %libfiles, keys %altlibfiles); > > my $symfile = Dpkg::Shlibs::SymbolFile->new(); > > my $dumplibs_wo_symfile = Dpkg::Shlibs::Objdump->new(); Done. > I hope you can provide an updated patch based on my comments. > > Did you try your change by the way? Yes, I tested it by using a binary for the fgrun program, with and without the required simgear libraries. I have attached the updated patch file. Thanks, Chris
--- dpkg-shlibdeps-old.pl 2010-12-08 17:12:10.000000000 +0000 +++ dpkg-shlibdeps.pl 2010-12-08 17:11:03.000000000 +0000 @@ -150,6 +150,9 @@ my %objdump_cache; my %symfile_has_soname_cache; +# Used to count errors due to missing libraries +my $error_count = 0; + my $cur_field; foreach my $file (keys %exec) { $cur_field = $exec{$file}; @@ -173,7 +176,8 @@ "that do not have any shlibs or symbols file.\nTo help dpkg-shlibdeps " . "find private libraries, you might need to set LD_LIBRARY_PATH."); if (scalar(split_soname($soname))) { - error($msg, $soname, $file, $obj->{format}, join(":", @{$obj->{RPATH}})); + errormsg($msg, $soname, $file, $obj->{format}, join(":", @{$obj->{RPATH}})); + $error_count++; } else { warning($msg, $soname, $file, $obj->{format}, join(":", @{$obj->{RPATH}})); } @@ -423,6 +427,13 @@ } } +# Quit now if any missing libraries +if ($error_count >= 1) { + error(P_("Cannot continue due to the error above.", + "Cannot continue due to the errors listed above.", + $error_count)); +} + # Open substvars file my $fh; if ($stdout) {
Attachment:
signature.asc
Description: This is a digitally signed message part