Hello Boyuan,
On Fri, Oct 07, 2016 at 02:42:05PM +0800, Boyuan Yang wrote:
> Sorry for being late. I was having a holiday and turned to something else
> in the past few days :p
No worries -- so long as we don't miss the stretch freeze!
> > 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.
I learned of this change shortly after I wrote to you :) Cool.
> > 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.
Fair enough, you can keep 'rather complete' if you prefer it. If I were
writing the description, I would have used 'almost complete' or 'mostly
complete'.
Also, 'presents' is odd. Maybe: "QEverCloud is an almost complete
Evernote SDK for Qt."
That's just my preference though. Not a big deal.
> > 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.
You're right: https://github.com/d1vanov/QEverCloud/issues/9
> > 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.
Upstream has made a new release 3.0.3 incorporating a fix o/
Could you try building the docs with this new release, please?
Btw, the Forwarded: header should point at patches submitted, not bug
reports without patches. You should include the issue URI in the patch
description, instead. (Doesn't matter if you're able to drop the
patch.)
> > 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.
Great, those make it really clear.
> > 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.
Yes, libdl is from glibc, so that's fine. It's good to remove the
libqt5gui because that adds an actual binary package dependency.
> > 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
In addition, the symlink in the -dev package conventionally points at
libqt?evercloud.so.3.0.0, rather than at libqt?evercloud.so.3 (again,
see ch. 8 of Policy which specifies that this should be a "symlink
pointing to the shared library", though I suppose that could be
interpreted to include pointing via another symlink).
--
Sean Whitton
Attachment:
signature.asc
Description: PGP signature