Re: Bug#716905: RFS: goodbye/0.3-1 [ITP]
On Mon, Jul 15, 2013 at 05:42:41PM +0200, Gergely Nagy wrote:
> Adam Borowski <kilobyte@angband.pl> writes:
>
> > Let's add some WTF to FTPmasters' day! :)
>
> This package is a perfect fit for a personal archive, but if anyone
> wants to upload it: you're in for a race for the fastest REJECT ever.
I'm afraid you managed to find an error. One. Which is a terrible shame,
as the point of perversing the rules means one should have a spotless
record. Fixed and reuploaded.
So, let's go through what you pointed out:
> * debian/rules without arguments does not behave sanely:
>
> make: *** No targets. Stop.
>
> It should at least do something, but "no targets" is unexpected.
There is no requirement for it to do anything, and failing immediately
is a sane thing to do.
As for consistency, let's compare with a bunch of packages:
9base: does most of the build, returns 0
aalib: calls configure, returns 0
dadadodo: says "make: *** No targets. Stop.", returns 2
dh-exec: says "dh_auto_configure -- --libexecdir=/usr/lib", returns 0
dpatch: patches itself, returns 0
edbrowse: attempts compilation with no earlier targets called, fails
returning 2
git-flow: says "dh override_dh_auto_build", returns 0
ivykis: tries to chmod some non-existant files (no target dependency),
fails returning 2
libmikmod: fails due to some unset variables, returns 2
libmongo-client: says "make: Nothing to be done for `build'.",
returns 0
mikmod: fails configuration, returns 2
I'd say, "goodbye" behaves best of these (same as "dadadodo"). The error
message accurately describes what's wrong (you specified no targets).
> It says the same when tcc is not installed too, instead of failing
> violently for build-deps not being present.
/bin/sh: 142: tcc: not found
make: *** [binary] Error 127
The error message is informative, again.
> * It unconditionally rewrites debian/control
>
> This is REJECT reason alone. You simply don't do that.
Thus I don't. Please tell me in what circumstances would it overwrite
debian/control.
> * debian/rules with an invalid target name still works:
>
> debian/rules binary == debian/rules yranib
>
> It should produce an error. This kind of unexpected behaviour is
> confusing, and not something we want in the archive. We already have
> CDBS for that[1], thank you.
Policy 4.9 explicitely allows extra targets. Using any not defined in
the policy is undefined behaviour; a comment in "goodbye" mentions that.
> * There is no error checking:
>
> Trying to build the package in a chroot that doesn't have enough
> space: it will result in an empty, invalid package.
>
> The return value of mkdir(), system() and all that needs to be
> checked, or you're not complying with Policy 4.6.
You got me here.
While this is a rather popular error, it is unacceptable in a packaging
example.
> * d/rules can be exploited by an overly long version string
There's no buffer overflow (scanf("%127"), snprintf), merely truncation.
I'd call a limit of 127 characters in a version number reasonable. I can
use asprintf if you wish, but I prefer it this way, with a sanity check.
In any case, you can already run arbitrary code like, say, "rm -rf ~" in a
makefile. Breaking a build system by editing said build system is not an
accomplishment.
> * The binary created by d/rules is invalid:
>
> algernon@hadhodrond:/tmp/b$ dget -x -u http://mentors.debian.net/debian/pool/main/g/goodbye/goodbye_0.3-1.dsc
> dget: retrieving http://mentors.debian.net/debian/pool/main/g/goodbye/goodbye_0.3-1.dsc
> % Total % Received % Xferd Average Speed Time Time Time Current
> Dload Upload Total Spent Left Speed
> 100 1440 100 1440 0 0 342k 0 --:--:-- --:--:-- --:--:-- 703k
> dget: retrieving http://mentors.debian.net/debian/pool/main/g/goodbye/goodbye_0.3.orig.tar.xz
> % Total % Received % Xferd Average Speed Time Time Time Current
> Dload Upload Total Spent Left Speed
> 100 1824 100 1824 0 0 471k 0 --:--:-- --:--:-- --:--:-- 1781k
> dget: retrieving http://mentors.debian.net/debian/pool/main/g/goodbye/goodbye_0.3-1.debian.tar.gz
> % Total % Received % Xferd Average Speed Time Time Time Current
> Dload Upload Total Spent Left Speed
> 100 3133 100 3133 0 0 133k 0 --:--:-- --:--:-- --:--:-- 145k
> dpkg-source: info: extracting goodbye in goodbye-0.3
> dpkg-source: info: unpacking goodbye_0.3.orig.tar.xz
> dpkg-source: info: unpacking goodbye_0.3-1.debian.tar.gz
> algernon@hadhodrond:/tmp/b$ cd goodbye-0.3/
> algernon@hadhodrond:/tmp/b/goodbye-0.3$ debian/rules binary
> ar: creating ../goodbye_0.3-1_all.deb
> doing: [binary]\n
> algernon@hadhodrond:/tmp/b/goodbye-0.3$ dpkg-deb -c ../goodbye_0.3-1_all.deb
> dpkg-deb: error: archive has no newlines in header
>
> And indeed, debian-binary contains "2.0\n", and no newline.
Uh?
dpkg-deb -c ../goodbye_0.3-1_all.deb
drwxr-xr-x root/root 0 2013-07-15 22:42 usr/
drwxr-xr-x root/root 0 2013-07-15 22:42 usr/share/
drwxr-xr-x root/root 0 2013-07-15 22:42 usr/share/doc/
drwxr-xr-x root/root 0 2013-07-15 22:42 usr/share/doc/goodbye/
-rw-r--r-- root/root 476 2011-07-29 01:45 usr/share/doc/goodbye/copyright
-rw-r--r-- root/root 203 2013-07-15 22:42 usr/share/doc/goodbye/changelog.gz
-rw-r--r-- root/root 322 2013-07-15 22:42 usr/share/doc/goodbye/changelog.Debian.gz
drwxr-xr-x root/root 0 2013-07-15 22:42 usr/share/man/
drwxr-xr-x root/root 0 2013-07-15 22:42 usr/share/man/man1/
-rw-r--r-- root/root 285 2013-07-15 22:42 usr/share/man/man1/goodbye.1.gz
drwxr-xr-x root/root 0 2013-07-15 22:42 usr/bin/
-rwxr-xr-x root/root 126 2011-07-29 01:45 usr/bin/goodbye
Could you please tell me how did you get that effect? That would imply
make suddenly failing to unescape "\\"; any version of make in Debian
follows the same rules.
> * Debugging d/rules is unnecessarily hard
>
> I can' easily attach gdb to figure out what's wrong. stracing it is
> painful too.
set follow-fork-mode
strace -f
tcc (or gcc, or clang ...) to a file
On the other hand, Make is one of worst languages where it comes to
debugging. debhelper at least sabotages it by using mostly perl and
shell, but try to debug a CDBS package...
> * root:root owner is not enforced in the control.tar.gz:
>
> algernon@hadhodrond:/tmp/b/goodbye-0.3$ tar tzvf control.tar.gz
> -rw-r--r-- algernon/algernon 723 2013-07-15 17:35 control
>
> This is reject worthy too.
I see no such requirement anywhere; only files in data.tar.gz need to
have a specific user. Common sense says they should be readable, which
they are, and that's it. You would need to go quite out of your way:
unar it by hand, then untar using a version of tar that defaults to
--same-user, as root. This is not what dpkg-deb does.
This package has two aims:
* educational: to show people the absolutely[1] minimal low-level way of
creating a correct binary package
* fun: perverting the policy while obeying every word, and being as
readable as possible
Thus criticism that points to a rule it breaks is valid, criticism that
accuses it about merely breaking common sense misses the point.
[1]. Any step can be implemented some other way, none can't be skipped.
--
ᛊᚨᚾᛁᛏᚣ᛫ᛁᛊ᛫ᚠᛟᚱ᛫ᚦᛖ᛫ᚹᛖᚨᚲ
Reply to: