[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#851756: RFS: telegram-desktop/1.0.6-1 [ITP]



On Fri, Feb 03, 2017 at 01:33:40AM +0800, Boyuan Yang wrote:
> Hi,

Hi Boyuan!

I do not assume the sponsoree is subscribed to either the bug or
debian-mentors, so I suggest you always CC him.

> I just heard that someone filed an RFS for telegram-desktop. Good job! Wish 
> that you could make this package into good shape and put it into Debian.

Yeah :)

> > I can't understand how pristine-tar works. Should I manually download [...]
> 
> You should use upstream tarball when applicable. If we use the tarball which 
> is *identical* with the tarball released by upstream, we can have confidence 
> that no source code is modified in the distribution version of this software. 

Exactly.
You need not to manually commit the pristine-tar data.  If you have
'pristine-tar = True' in either your debian/gbp.conf or ~/.gbp.conf then
when you do `gbp import-orig ../bla.tar.gz` gbp will commit the data by
itself.

> Downloading manually can be unnecessary. If you carefully write debian/watch 
> file, the "uscan" tool can do it for you according to information in d/watch 
> file. If you don't really want to hack into d/watch, then manual download 
> should be needed.

IME, having a debian/Watch is important, not just because of the
download thing, but to have automatic tools detect new upstream releases
(besides, then you can do `gbp import-orig --uscan` to do everything…)


Although, the above points are moot, because you are working from the
upstream git tree, and not importing tarballs.  I have very little
experience in working with those, and indeed for them I usually do
    uscan --download
    git merge vXX
    pristine-tar commit ../foo_XX.tar.gz vXX
And I haven't yet found a way to have gbp be nice and do the work for
me (nor I know whether there is a way to do that).
(FTR, I do that with src:dehydrated)

> A big trouble is that upstream usually bundles lots of third-party sources 
> into its release. You will need to write detailed d/copyright file for those 
> files.

Once we make sure those bundled libs are not used, they can stay there,
and just dump info in d/copyright; which I prefer over a +ds repacking.

> * I really don't recommend using "ronn" tool to generate man page. Even we 
> have ronn in Debian, ronn is already dead upstream [1] and we shouldn't use a 
> dead tool in build toolchain. Writing man pages manually won't take up too 
> much time, or at least we can consider tools other than ronn (yes, there are 
> other tools available).

like pandoc?
Then again, I agree that this manpage is very short that it is not so
hard to write it in groff from the start.
Anyhow, I don't find particularly important this point.


> * Please consider explicitly enable (full) hardening flags in d/rules and test 
> if the build can pass.
> 
> * Is the hard Depends: to fcitx-frontend-qt5 necessary? Your instruction would 
> make everyone who installs telegram-desktop to install fcitx, which is an 
> Input Method Framework. I recommend you downgrade it to Suggests.
> 
> * Build-depends fcitx-frontend-qt5 seems very wrong. Could you please explain 
> why you add this one?
> 
> [1] https://github.com/rtomayko/ronn/

Good point; Коля, please have a look at them.


Then:

In d/copyright, the Apache-2.0 license text should point to the thing in
/usr/share/common-license; it's not enough to point to an external
website, per Debian Policy everything should be available locally.

> According to their contributing policy [1], I put my patches into public
> domain. Is it right?

Well, it's bullshit if you ask me; I wouldn't be particularly happy to
put my work in the public domain exactly because I don't want the
"Telegram team needs to use full Telegram Desktop source code with some
different license" as they put it.  But yep, it's right.


dpkg-shlibdeps reports a lot of libraries that are linked but the binary
doesn't use any symbol: can you try to build with -Wl,--as-needed ?


lintian reports several mispelling errors, including one in the manpage
that you wrote..


You patched Telegram/gyp/Telegram.gyp but are you sure you don't have to
also patch Telegram/gyp/telegram_linux.gypi ?  At the very least I
noticed a
    -L/build/foobar/out/Release/../../../Libraries/libexif-0.6.20/libexif/.libs
in the final linking that I didn't expect to see, and there is no
libexif-dev in Build-Depends.




In other news, I now also installed the package, nice work :)

-- 
regards,
                        Mattia Rizzolo

GPG Key: 66AE 2B4A FCCF 3F52 DA18  4D18 4B04 3FCD B944 4540      .''`.
more about me:  https://mapreri.org                             : :'  :
Launchpad user: https://launchpad.net/~mapreri                  `. `'`
Debian QA page: https://qa.debian.org/developer.php?login=mattia  `-

Attachment: signature.asc
Description: PGP signature


Reply to: