Bug#836903: RFS: qevercloud/3.0.2+ds-1 [ITP] -- Unofficial Evernote Cloud API library for Qt

[Boyuan Yang <073plan@gmail.com>]

Hello Sean,

Sorry for being late. I was having a holiday and turned to something else
in the past few days :p

2016-09-22 10:33 GMT+08:00 Sean Whitton <spwhitton@spwhitton.name>:
> Hello Boyuan,
>
> A few new things:
>
> 1. You dropped --parallel from d/rules without explaining why in the commit
> message.  Does it break the build?  Certainly not essential, but it's
> nice to enable it since our buildds are so overworked.

--parallel is no longer needed to build in parallel since debhelper v10.
I hope the build machines could recognize new debhelper soon.

> 2. Some grammatical errors in package descriptions:
>
> s/documentations/documentation/
>
> s/on Evernote site/on the Evernote site/

done.

> Since 'complete' is an extreme adjective, it is odd to qualify it as
> 'rather complete'.  I would s/rather //.

Sorry but I am not intended to fix (?) it. The function is not complete
due to new releases of Evernote Cloud API recently, and such
usages can be found all around the Internet.

> 3. I observed this:
>
>     hephaestus ~ % objdump -p /usr/bin/nixnote2 | grep NEEDED
>       NEEDED               libhunspell-1.4.so.0
>       NEEDED               libcurl.so.4
>       NEEDED               libpoppler-qt5.so.1
>       NEEDED               libqt5qevercloud.so.3
>       NEEDED               libQt5PrintSupport.so.5
>       NEEDED               libQt5WebKitWidgets.so.5
>       [...]
>
> Maybe the SONAME of qevercloud should be libQt5qevercloud, not
> libqt5evercloud, to match this convention?  Since we can't change this
> in the future, it would be good to get it right now.  Maybe this is
> documented somewhere...

Upstream uses this name intentionally (the name is written in CMakeLists.txt).
I guess he did not want to let his third-party library get confused with
official Qt5 libraries.

> Your `dch -r` is behind HEAD again, and your debian/3.0.2+ds-1 isn't on
> the HEAD of master.

OK now I'm trying to run dch -r on every commit. :)

>> I dropped the tex / pdf / postscript files in the -doc package because
>> they are not that
>> useful when html files are provided as well.
>
> Although HTML is considered the primary format for documentation in
> Debian packages, I would suggest including the PDFs and PS files anyway.
> Someone might want to print the documentation, or they might just prefer
> reading PDFs.  Since it's in a separate -doc package, we don't have to
> worry about cluttering up anyone's system.
>
> If you agree, and install the PDFs and PSs, it would also be a good idea
> to move the html docs into /usr/share/doc/qevercloud-doc/html.

That sounds good even if PDFs are not to be installed. I am putting them
inside the sub-directory html.

> Whether or not you build the PDFs, it would be good to handle this
> error, which could be worrying to someone:
>
>     sh: 1: epstopdf: not found
>     error: Problems running epstopdf. Check your TeX installation!
>
> If you don't want to install the PDFs, maybe you can instruct doxygen
> not to try to run epstopdf, so that the error is not emitted.

I tried to build PDFs, but the dependency is just too large (a lot of
texlive packages). What's more, I met FTBFS with current texlive.

Bug forwarded to https://github.com/d1vanov/QEverCloud/issues/8.

Added a patch to disable the instructions about PDFs in Doxyfile.

> Where you have
>
>     dh_auto_{build,install}
>     dh_auto_{build,install} --builddirectory=$(_QEVERCLOUD_QT5_BUILDDIR)
>
> it's not clear why you have to call it twice.  I suggest adding a
> comment saying what each of the two calls does.

Some more comments are added.

>> > 11. Upstream's README.md seems like a useful file.  Consider installing
>> > it to /usr/share/doc in both your -dev packages.  You should patch it to
>> > remove the stuff about building and installing the library, though.
>>
>> I thought dh_installdocs would install it (as stated in the man page) but it
>> didn't. Wrote it into docs file explicitly and patched now.
>
> I've reported #838538.
>
>> The Git repository on Github has been update, and the new packages are
>> uploaded to mentors and debomatic-amd64. The binary packages on debomatic
>> suggests that nixnote2 successfully recognized the libqt5qevercloud3
>> shlib version
>> requirement.
>
> I did some test builds and everything is looking good :)
>
> However, I did see this output:
>
> dpkg-shlibdeps: warning: package could avoid a useless dependency if
> debian/libqt4qevercloud3/usr/lib/i386-linux-gnu/libqt4qevercloud.so.3
> was not linked against libdl.so.2 (it uses none of the library's
> symbols)
> dpkg-shlibdeps: warning: package could avoid a useless dependency if
> debian/libqt5qevercloud3/usr/lib/i386-linux-gnu/libqt5qevercloud.so.3
> was not linked against libQt5Gui.so.5 (it uses none of the library's
> symbols)
> dpkg-shlibdeps: warning: package could avoid a useless dependency if
> debian/libqt5qevercloud3/usr/lib/i386-linux-gnu/libqt5qevercloud.so.3
> was not linked against libdl.so.2 (it uses none of the library's
> symbols)
>
> Do you know whether those unneeded dependencies may be avoided?

Writing "export DEB_LDFLAGS_MAINT_APPEND = -Wl,--as-needed" in
debian/rules to clean the unneeded links. libQt5Gui is gone now but libdl
still exists.

Well they look harmless and I would just leave them there.

> One more thing -- the .so should be installed as
> libqt?evercloud.so.3.0.0, with a symlink from libqt?evercloud.so.3.  See
> ch. 8 of Debian policy for an explanation of this practice.

Patch added (0010-patch-library-soname-chain.patch).

Issue forwarded: https://github.com/d1vanov/QEverCloud/issues/7

>
> Thanks!  I'm excited to see the completion of this RFS!
>
> --
> Sean Whitton

Thanks. The GitHub repo / debian-mentors / debomatic-amd64 files are
update-to-date now.


Sincerely,
Boyuan Yang