Bug#1004345: RFS: ricochet-refresh/3.0.11-1
Hi. I put on my "Debian" hat had a look at this package. I started
with the git branch git@github.com:m-simonelli/ricochet-refresh
885f03138fe6e88006bdb672ce478751d119eff3 and then also ended up
downloading the dsc and comparing it.
Here are my comments. I hope they're not too discouraging...
1. Embedded copy of Tor
I observe that this source package contains a copy of tor. Debian
really doesn't like embedded copies like that. Is there some reason
why you can't use the copy of Tor that's in Debian already ?
Please forgive me if the answer to this is something I ought to
already know. But I think if there is a good answer it ought to be in
some in-tree document, maybe debian/README.source.
(After I noticed this, I stopped my review, since it seemed likely that
changing this would cause substantial other changes.)
2. In debian/rules, there is a bunch of stuff to modify the compile
flags. I think it is good practice to leave a comment next to all
override_dh_* to explain why it is needed, if it is not enitrely
obvious.
In particular,
* Why does dh + cmake not get the install prefix etc. right by
itself ? Is there a Debian bug about that ?
* There are various options like -z now, -D_FORTIFY_SOURCE=2, -g,
etc, which, again, ISTM that dh cmake ought to get right.
* Why are we hand-tuning the parallelism ?
etc.
I'm not very familiar with cmake so perhaps this is all "normal for
cmake" but if so that would be me as contrary to the usual philosophy
of dh(1), which is usually to do the right thing by default.
3. Use of git and tarballs
This is not a blocker. And I'm going to make some comments which will
definitely not be shared by everyone. And I appreciate that much
Debian packaging really likes to think about tarballs. I'll work with
.dscs and tarballs if there isn't a better way.
But:
I see you used git submodules. IMO git submodules are extremely poor.
This message is not the right place for a full explanation, but they
(i) fundamentally break many of git's basic design principles, and
(ii) consequently break many very normal git operations. Separately
(iii) the design and implementation are full of avoidable bugs.
In general, I would recommend git subtree instead - at least if the
subproject is not too large compared to the superproject, which isn't
the case here. But I'm not sure this program ought to have a bundled
copy of Tor anyway. So what precisely ought to be done about this
depends on what to do about the embedded Tor copy.
And, I see that the "uptream tarball" you provided in your RFS is not
identical to (any version of?) the git branch. I'm not sure how it
was produced. AFAICT from the upstream web page, the official source
code is in git. So I would have expected any upstream tarball to be
the result of "git archive".
If you *do* have official upstream tarballs, then the most common
Debian practice nowadays is to use pristine-tar.
Additionally, I see you have a contraption to remove .gitignore and
.gitattributes from your tarballs. I think this is ill-conceived.
There is no reason you can't put a .gitignore into a tarball.
I think the .gitignore is part of the preferred form for modification
(ie, part of the source code.)
Of course I'm happy to talk about everything more (here, irc, or
private email).
Regards,
Ian.
Reply to: