[pkg] New package: wcc
Le 16/06/2017 ? 15:55, Raphael Hertzog a ?crit :
> Hello Philippe,
Hello Rapha?l, thanks for the review :)
> On Thu, 25 May 2017, Philippe Thierry wrote:
>> I've uploaded the wcc package to pkg-security repository:
>> https://anonscm.debian.org/cgit/pkg-security/wcc.git
> Here are my comments:
>
> 1/ the debian/copyright file looks strange to me, you should not put all
> files under the same block when they have different licenses applied.
> Createe one default block for the main code (MIT) and then supplementary
> blocks for third-party dependencies that are embedded and that have
> different licenses.
Yes this project is pretty a mess in term of copyrights. I've updated
the copyright file to separate the different blocks. There is nearly no
more License mismatch, but i haven't finished the copyright part. As you
can see, it's really hard to write this file, because there is nearly
all the licenses in all the files...
>
> 2/ you have put names in debian/copyright that I am not able to find in
> the sources... where do they come from? Also I see one name
> ("Dag-Erling Co?dan Sm?rgrav") which has certainly been badly
> copy-pasted... read in UTF-8 when it was Latin1 or something like this.
Seen it. I'm currently updating the copyright owner, it will be corrected.
>
> => I see that third-party dependencies are managed as git submodule,
> and I don't have those submodules on a fresh clone obviously
I'm using --git-submodules option from gbp. It's my first package with
submodules, i don't know if there is a "standard" way to manage it.
>
> 3/ there are PDF files in "doc", I think that ftpmasters want sources
> for PDF files. See https://ftp-master.debian.org/REJECT-FAQ.html ("Source
> missing")
I've updated the watch file and how uscan work to clean the pdf files.
Yet i don't know how to tell uscan how to build an orig tarball with the
submodules checkouted in it :-/ Its content only contains the master git
repo content without the pdf files.
This make gbp fails with:
dpkg-source: info: local changes detected, the modified files are:
[...] (all submodules content)
If you know the way to build a dfsg tarball properly with submodules in
it.... ?
>
> 4/ debian/compat is 9, but you have debhelper >= 10 in Build-Depends, you
> probably want to bump it.
Corrected.
>
> 5/ debian/control contains hardcoded dependencies on C shared libraries:
> libcapstone3, libelf1, libreadline7, libgsl2. You don't need those as
> they are generated as part of ${shlibs:Depends}.
Ok i didn't know. Corrected.
>
> 6/ debian/control containes "Testsuite: autopkgtest", you don't need that
> as dpkg will add this field automatically when it finds
> debian/tests/control
Idem. Didn't know. Corrected.
>
> 7/ I don't see any pristine-tar branch, how am I supposed to get
> the upstream tarball that you are using ? => you should be using
> --git-pristine-tar-commit too.
Ok. Updated.
>
> 8/ debian/source/lintian-overrides are too generic, you are ignoring all
> instances of the source-contains-prebuilt-doxygen-documentation
> source-contains-prebuilt-binary and source-is-missing tags... you should
> only ignore them for the specific cases that you have analyzed. You
> can use patterns to match multiple files e.g.
> wcc source: source-contains-prebuilt-doxygen-documentation src/tex/*
> Same for debian/lintian-overrides (which should be renamed
> debian/wcc.lintian-overrides to match other debian/wcc.* files).
Didn't know it was possible to reduce the scope. My tests failed. I've
updated with your syntax.
>
> 9/ in debian/source/options, you should escape the dots in
> extend-diff-ignore otherwise the dots are matching any character (and not
> only a dot)
Updated.
>
> 10/ the way you use gbp buildpackage and its submodule support (I wasn't
> even aware of this feature) should probably be documented in
> debian/README.source as this is not really a widely known practice yet.
Done.
>
> 11/ debian/gbp.conf overrides the upstream tag name so that it points
> to a real upstream tag... I'm not sure that this is a very good idea.
> What happens when you run "gbp import-orig --uscan" with a new upstream
> version? It will create a Debian specific tag that will conflict with
> the upstream tag. And the associated tarball will lack the submodules I
> guess. You probably don't expect to use gbp import-orig but this is still
> what people are used to use by default and it will create problems. =>
> again very important to document how to work with your package
Corrected.
>
> 12/ drop debian/README.Debian, it doesn't add anything over the current
> description in debian/control
Done.
>
> 13/ drop debian/wcc.dirs, it's not needed, those dirs are created by
> dh_install implicitly
Done.
>
> Cheers,
Cheers,
Reply to: