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.
2. Some grammatical errors in package descriptions:
s/documentations/documentation/
s/on Evernote site/on the Evernote site/
Since 'complete' is an extreme adjective, it is odd to qualify it as
'rather complete'. I would s/rather //.
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...
On Sun, Sep 18, 2016 at 09:02:42PM +0800, Boyuan Yang wrote:
> > 2. We need to test building something against your new shared library,
> > and we'll need to confirm that the right dependencies get generated for
> > it by dpkg-shlibdeps. If you haven't done so already, could you update
> > your nixnote packaging to use your new qevercloud shlib, please?
>
> The new version (2.0~beta9+dfsg-1) was pushed to GitHub, mentors
> and debomatic-amd64.
> Source package changed (ds -> dfsg) due to unclear license status (as
> discussed previously in nixnote2 RFS).
Your `dch -r` is behind HEAD again, and your debian/3.0.2+ds-1 isn't on
the HEAD of master.
> > 3. In a recent commit you renamed debian/{*.symbols =>
> > *.symbols.amd64}. So now you are not providing any symbols files
> > for other architectures. But for a shared library, you need to
> > provide a symbols or shlibs file for all the reasons described in
> > ch. 8 of the Debian Policy Manual.
> >
> > I assume that you did this rename because the symbols files is
> > architecture-dependent. That probably means it's very fragile: changes
> > which don't break the ABI might change the symbols file. This is a
> > known issue with C++ shared libs.[1]
>
> It was indeed because of architecture-dependent symbols of C++.
>
> >
> > You need to make the symbols file less fragile (some suggestions
> > involving sed(1) here[2]) so that it will work for all archs, or switch
> > to a shlibs file instead. README.md suggests that upstream is aware of
> > the issue of ABI compatibility, so shlibs files might be enough.
>
> Anyway I switched to shlibs using dh_makeshlibs. It is reasonable for
> this package to depend on upstream to keep ABI comaptibility, but I
> will also try to test symbol files on each release (even it is not included
> in the tarball, it can be stored in the git history).
Okay, sounds reasonable.
> The new qevercloud-doc added.
>
> 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.
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.
> > 7. In your rules file you make a lot of explicit calls to dh_auto_*.
> > When you do something like this
> >
> > override_dh_auto_clean:
> > dh_auto_clean
> > rm -rf $(_QEVERCLOUD_QT5_BUILDDIR)
> >
> > it's fine, because it's clear to a reader that you are appending to an
> > automatic command. However, when you do this:
> >
> > custom_regenerate_source_code:
> > (cd $(_QEVERCLOUD_GENERATOR_DIR); cmake .;)
> > dh_auto_build --buildsystem=makefile -- -C $(_QEVERCLOUD_GENERATOR_DIR)
> > mkdir -p QEverCloud/src/generated
> > etc.
> >
> > the dh_auto_build line is quite hard to understand -- someone would need
> > to look up the makefile buildsystem. It would be better to replace that
> > with an explicit call to $(MAKE). In dh_auto_build(1) it says "If it
> > doesn't work, you're encouraged to skip using dh_auto_build at all, and
> > just run the build process manually."
>
> That is reasonable indeed. Switched to call $(MAKE).
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.
> > 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?
Thanks! I'm excited to see the completion of this RFS!
--
Sean Whitton
Attachment:
signature.asc
Description: PGP signature