Bug#554612: /usr/bin/dpkg-source: include-binaries doesn't work

[Mike Hommey-2]

Package: dpkg-dev
Version: 1.15.4.1
Severity: normal
File: /usr/bin/dpkg-source

Disclaimer: I've only tried to add binaries in a subdirectory of debian/. Maybe
it only doesn't work there.

I tried several different things, including:
- Putting full path in debian/source/include-binaries (starting with debian/)
- Putting path after debian/ in debian/source/include-binaries (i.e.
  extras/Throbber-small.png for debian/extras/Throbber-small.png)
- Putting nothing in debian/source/include-binaries and use the
  --include-binaries option to dpkg-source.

None of the above works.

The first two end up saying:
dpkg-source: error: unwanted binary file: ../Throbber-small.png

The last one ends up saying:
dpkg-source: info: adding ../Throbber-small.gif to debian/source/include-binaries
dpkg-source: info: building iceape in iceape_2.0-1.debian.tar.gz
tar: Removing leading `../' from member names
tar: ../Throbber-small.png: Cannot stat: No such file or directory
tar: Exiting with failure status due to previous errors
dpkg-source: error: tar -cf - gave error exit status 2


-- System Information:
Debian Release: squeeze/sid
  APT prefers unstable
  APT policy: (500, 'unstable'), (1, 'experimental')
Architecture: amd64 (x86_64)

Kernel: Linux 2.6.31-1-amd64 (SMP w/2 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash

Versions of packages dpkg-dev depends on:
ii  binutils                      2.20-2     The GNU assembler, linker and bina
ii  bzip2                         1.0.5-3    high-quality block-sorting file co
ii  dpkg                          1.15.4.1   Debian package management system
ii  libtimedate-perl              1.1900-1   Time and date functions for Perl
ii  lzma                          4.43-14    Compression method of 7z format in
ii  make                          3.81-7     An utility for Directing compilati
ii  patch                         2.5.9-5    Apply a diff file to an original
ii  perl [perl5]                  5.10.1-6   Larry Wall's Practical Extraction
ii  perl-modules                  5.10.1-6   Core Perl modules

Versions of packages dpkg-dev recommends:
ii  build-essential           11.4           Informational list of build-essent
ii  fakeroot                  1.14.3         Gives a fake root environment
ii  gcc [c-compiler]          4:4.3.3-9+nmu1 The GNU C compiler
ii  gcc-4.1 [c-compiler]      4.1.2-27       The GNU C compiler
ii  gcc-4.2 [c-compiler]      4.2.4-6        The GNU C compiler
ii  gcc-4.3 [c-compiler]      4.3.4-6        The GNU C compiler
ii  gcc-4.4 [c-compiler]      4.4.2-2        The GNU C compiler
ii  gnupg                     1.4.10-2       GNU privacy guard - a free PGP rep
ii  gpgv                      1.4.10-2       GNU privacy guard - signature veri

Versions of packages dpkg-dev suggests:
ii  debian-keyring [debian-mainta 2009.08.27 GnuPG (and obsolete PGP) keys of D

-- no debconf information



--
To UNSUBSCRIBE, email to debian-bugs-dist-REQUEST@...
with a subject of "unsubscribe". Trouble? Contact listmaster@...

[Mike Hommey]

On Thu, Nov 05, 2009 at 05:24:40PM +0100, Mike Hommey wrote:

> Package: dpkg-dev
> Version: 1.15.4.1
> Severity: normal
> File: /usr/bin/dpkg-source
>
> Disclaimer: I've only tried to add binaries in a subdirectory of debian/. Maybe
> it only doesn't work there.
>
> I tried several different things, including:
> - Putting full path in debian/source/include-binaries (starting with debian/)
> - Putting path after debian/ in debian/source/include-binaries (i.e.
>   extras/Throbber-small.png for debian/extras/Throbber-small.png)
> - Putting nothing in debian/source/include-binaries and use the
>   --include-binaries option to dpkg-source.
>
> None of the above works.
>
> The first two end up saying:
> dpkg-source: error: unwanted binary file: ../Throbber-small.png
>
> The last one ends up saying:
> dpkg-source: info: adding ../Throbber-small.gif to debian/source/include-binaries
> dpkg-source: info: building iceape in iceape_2.0-1.debian.tar.gz
> tar: Removing leading `../' from member names
> tar: ../Throbber-small.png: Cannot stat: No such file or directory
> tar: Exiting with failure status due to previous errors
> dpkg-source: error: tar -cf - gave error exit status 2

A bit more information. It *hum*hum* works, actually, and the error
message above was due to remaining stuff in
debian/source/include-binaries.

So, with nothing in debian/source/include-binaries it works, provided
you give the --include-binaries option, except that the binary files
end up both at the root and in the debian/ (sub)directory both in the
debian.tar.gz and once extracted.

But then, doing dpkg-source -b without the --include-binaries option on
the directory you get from dpkg-source -x'ing the resulting .dsc from
above, complains about the modified file that is in the root directory.

dpkg-source: info: using source format `3.0 (quilt)'
dpkg-source: info: building iceape using existing ./iceape_2.0.orig.tar.bz2
dpkg-source: error: cannot represent change to iceape-2.0/preview.png: binary file contents changed
dpkg-source: error: add preview.png in debian/source/include-binaries if you want to store the modified binary in the debian tarball
dpkg-source: error: unrepresentable changes to source

Trying again with --include-binaries works, but puts the file twice at
the root level:

$ tar -ztvf iceape_2.0-1.debian.tar.gz|grep preview.png
-rw-r--r-- 0/0            9474 2009-11-05 17:29 debian/extras/preview.png
-rw-r--r-- 0/0            9474 2009-11-05 15:35 preview.png
-rw-r--r-- 0/0            9474 2009-11-05 15:35 preview.png

(I really should try with something smaller than iceape, because
dpkg-source -b with the quilt format is slower than the old 1.0 format,
which was not fast either)



--
To UNSUBSCRIBE, email to debian-bugs-dist-REQUEST@...
with a subject of "unsubscribe". Trouble? Contact listmaster@...

[Raphael Hertzog-3]

On Thu, 05 Nov 2009, Mike Hommey wrote:
> A bit more information. It *hum*hum* works, actually, and the error
> message above was due to remaining stuff in
> debian/source/include-binaries.

No, it really doesn't work as expected. It works in lenny but doesn't work
properly in sid. I think it might be due to a behavioural difference
in perl's File::Find...

With --include-binaries if you should really get messages like this:
dpkg-source: info: adding debian/test.png to debian/source/include-binaries
and not:
dpkg-source: info: adding ../test.png to debian/source/include-binaries

debian/source/include-binaries should contain paths relative to the root
directory of the source package.

Effectively find() behaves differently but it's also due to a lack of a
parameter that guaranteed that it would not change the current directory.
So it's a bug of dpkg-source and not of File::Find.

That said, it should work for binaries outside of the debian directory as
it's not the same code path and in that case I get proper messages.

Attached is the suggested patch. Tell me if you have other problems.

Cheers,
--
Raphaël Hertzog

diff --git a/scripts/Dpkg/Source/Package/V2.pm b/scripts/Dpkg/Source/Package/V2.pm
index 613dff9..b7fe76d 100644
--- a/scripts/Dpkg/Source/Package/V2.pm
+++ b/scripts/Dpkg/Source/Package/V2.pm
@@ -349,8 +349,8 @@ sub do_build {
         }
         return @result;
     };
-    find({ wanted => $check_binary, preprocess => $filter_ignore },
-         File::Spec->catdir($dir, "debian"));
+    find({ wanted => $check_binary, preprocess => $filter_ignore,
+           no_chdir => 1 }, File::Spec->catdir($dir, "debian"));
     error(_g("detected %d unwanted binary file(s) " .
         "(add them in debian/source/include-binaries to allow their " .
         "inclusion)."), $unwanted_binaries) if $unwanted_binaries;

[Mike Hommey]

On Thu, Nov 05, 2009 at 09:04:45PM +0100, Raphael Hertzog wrote:

> On Thu, 05 Nov 2009, Mike Hommey wrote:
> > A bit more information. It *hum*hum* works, actually, and the error
> > message above was due to remaining stuff in
> > debian/source/include-binaries.
>
> No, it really doesn't work as expected. It works in lenny but doesn't work
> properly in sid. I think it might be due to a behavioural difference
> in perl's File::Find...
>
> With --include-binaries if you should really get messages like this:
> dpkg-source: info: adding debian/test.png to debian/source/include-binaries
> and not:
> dpkg-source: info: adding ../test.png to debian/source/include-binaries
>
> debian/source/include-binaries should contain paths relative to the root
> directory of the source package.
>
> Effectively find() behaves differently but it's also due to a lack of a
> parameter that guaranteed that it would not change the current directory.
> So it's a bug of dpkg-source and not of File::Find.
>
> That said, it should work for binaries outside of the debian directory as
> it's not the same code path and in that case I get proper messages.
>
> Attached is the suggested patch. Tell me if you have other problems.

It seems to work properly with the patch. Would you know when this would
posible hit unstable ? ;)

Thanks,

Mike

PS: I do wonder, however, how dpkg-source can be *so much* slower than
tar -jxf ; diff -ruN (15 seconds for tar and 2 s for diff vs. 3'30 for
dpkg-source in quilt mode and 3' in format 1.0)



--
To UNSUBSCRIBE, email to debian-bugs-dist-REQUEST@...
with a subject of "unsubscribe". Trouble? Contact listmaster@...

[Raphael Hertzog-3]

On Thu, 05 Nov 2009, Mike Hommey wrote:
> > No, it really doesn't work as expected. It works in lenny but doesn't work
> > properly in sid. I think it might be due to a behavioural difference
> > in perl's File::Find...

In fact no, it's a regression that I introduced in
3525f79792cff51757d99d45f51c96a15e6fa779.

> > Attached is the suggested patch. Tell me if you have other problems.
>
> It seems to work properly with the patch. Would you know when this would
> posible hit unstable ? ;)

I don't know when we're going to upload the next version, I certainly hope
in the next few weeks, it's been 2 months already since the last upload.

Note however that you only need the fix to build the source package so you
could in theory upload the resulting package... it can be unpacked everywhere
(and rebuilt provided that they are not rebuilding the source package).

> PS: I do wonder, however, how dpkg-source can be *so much* slower than
> tar -jxf ; diff -ruN (15 seconds for tar and 2 s for diff vs. 3'30 for
> dpkg-source in quilt mode and 3' in format 1.0)

Check Dpkg::Source::Patch::add_diff_*() to understand why. AFAIK it's
there that most of the time is spent. It does a manual comparison of both
trees with many checks.

To verify run:
perl -d:Profile /usr/bin/dpkg-source -b <yourpackagedir>
And look at the first lines of prof.out.

Cheers,
--
Raphaël Hertzog




--
To UNSUBSCRIBE, email to debian-dpkg-bugs-REQUEST@...
with a subject of "unsubscribe". Trouble? Contact listmaster@...

[Mike Hommey]

On Thu, Nov 05, 2009 at 10:13:15PM +0100, Raphael Hertzog wrote:

> On Thu, 05 Nov 2009, Mike Hommey wrote:
> > > No, it really doesn't work as expected. It works in lenny but doesn't work
> > > properly in sid. I think it might be due to a behavioural difference
> > > in perl's File::Find...
>
> In fact no, it's a regression that I introduced in
> 3525f79792cff51757d99d45f51c96a15e6fa779.
>
> > > Attached is the suggested patch. Tell me if you have other problems.
> >
> > It seems to work properly with the patch. Would you know when this would
> > posible hit unstable ? ;)
>
> I don't know when we're going to upload the next version, I certainly hope
> in the next few weeks, it's been 2 months already since the last upload.
>
> Note however that you only need the fix to build the source package so you
> could in theory upload the resulting package... it can be unpacked everywhere
> (and rebuilt provided that they are not rebuilding the source package).

True, I forgot that buildds don't rebuild the source package.

> > PS: I do wonder, however, how dpkg-source can be *so much* slower
> > than
> > tar -jxf ; diff -ruN (15 seconds for tar and 2 s for diff vs. 3'30
> > for
> > dpkg-source in quilt mode and 3' in format 1.0)
>
> Check Dpkg::Source::Patch::add_diff_*() to understand why. AFAIK it's
> there that most of the time is spent. It does a manual comparison of
> both
> trees with many checks.
>
> To verify run:
> perl -d:Profile /usr/bin/dpkg-source -b <yourpackagedir>
> And look at the first lines of prof.out.

Oh my...

%Time    Sec.     #calls   sec/call  F  name
32.52   69.8110    44702   0.001562
Dpkg::Source::Patch::add_diff_file
29.95   64.2837    44829   0.001434     Dpkg::IPC::fork_and_exec
10.22   21.9370    44829   0.000489     Dpkg::IPC::wait_child
 7.14   15.3309    97591   0.000157     File::Spec::Unix::abs2rel
 4.25    9.1206   585681   0.000016     File::Spec::Unix::canonpath

forking diff for each file definitely doesn't help here. But that's
another story.

Cheers,

Mike



--
To UNSUBSCRIBE, email to debian-bugs-dist-REQUEST@...
with a subject of "unsubscribe". Trouble? Contact listmaster@...