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

Re: RFS: added autopkgtest for pplacer



Andreas Tille <andreas@an3as.eu> wrote:
>Hi,
>
>Am Mon, Apr 11, 2022 at 11:23:13AM -0000 schrieb Mohd Bilal:
>&gt; &gt;1. There is no information given where you obtained the data in
>&gt; &gt;   debian/tests/data from.  It is good style to add some script that
>&gt; &gt;   fetches the data from somewhere (may be via wget or whaterver
>&gt; &gt;   tool you prefer.
>&gt; &gt;
>&gt; 
>&gt; I&#39;ve added a script[4] in d/tests/ that fetches the test data 
>
>Looks good.
>
>&gt; &gt;2. The data should be also mentioned in d/copyright.
>&gt; &gt;
>&gt; &gt;3. Since the size of the data exceedes the size of the other files
>&gt; &gt;   it might make sense to create a separate tarball as it is
>&gt; &gt;   described in the Debian Med team policy[3].  Alternatively
>&gt; &gt;   you might consider xz compressing the data files (which needs
>&gt; &gt;   adding them in debian/source/include-binaries) and uncompressing
>&gt; &gt;   them once they are needed.  The compression rate might be
>&gt; &gt;   sufficiently high to tolerate the size of the compressed data
>&gt; &gt;   files.
>&gt; &gt;   It might be also be sensible to create an extra pplacer-examples
>&gt; &gt;   binary package in case the compression rate is not high enough.
>&gt; &gt;   A sign for the issue is also given by lintian:
>&gt; &gt;    I: pplacer: arch-dep-package-has-big-usr-share 12192kB 39%
>&gt; &gt;
>&gt; 
>&gt; I&#39;ve followed the instructions in the med-team policy and moved the tests data to debian-tests-data. The upstream repo[5] for test data  doesn;t state a license . So what should be included in the d/copyright file?
>
>May be its a good idea to ask upstream.  In lots of cases those data are
>in public domain - but this should be clarified.
>
>Also good you created the additional test-data tarball.  Please be so kind
>to push it to pristine-tar branch.  When I try `gbp buildpackage` I get:
>
>/usr/bin/gbp buildpackage --git-pbuilder-options=--source-only-changes 
>gbp:info: Tarballs &#39;pplacer_1.1~alpha19.orig.tar.gz, pplacer_1.1~alpha19.orig-debian-tests-data.tar.gz&#39; not found at &#39;../tarballs/&#39;
>gbp:info: Creating /home/andreas/debian-maintain/salsa/med-team/build-area/pplacer_1.1~alpha19.orig.tar.gz
>gbp:info: Creating /home/andreas/debian-maintain/salsa/med-team/build-area/pplacer_1.1~alpha19.orig-debian-tests-data.tar.gz
>gbp:error: Can not find pristine tar commit for archive &#39;pplacer_1.1~alpha19.orig-debian-tests-data.tar.gz&#39;
>
>&gt; &gt;4. Unfortunately the test does not work in my pbuilder environment.
>&gt; &gt;   I&amp;#39;ve added a `set -x` to run-unit-test and so I&amp;#39;Ve got:
>&gt; &gt; ...
>&gt; &gt;Test 1
>&gt; &gt;make: *** No rule to make target &amp;#39;test&amp;#39;.  Stop.
>&gt; &gt;autopkgtest [07:44:45]: test run-unit-test: -----------------------]
>&gt; &gt;
>&gt; &gt;There is no Makefile provided and if I&amp;#39;m checking the build directory
>&gt; &gt;the Makefile would need a binary _build/tests/tests.native which
>&gt; &gt;contains the according calls to the pplacer binaries.  Thus the test
>&gt; &gt;only works in a full source tree ... and I admit I&amp;#39;m quite baffled
>&gt; &gt;why the Salsa-CI test is passing.  It obviously rebuilds the needed
>&gt; &gt;binary but I have no idea why.
>&gt; &gt;
>&gt; 
>&gt; I&#39;ve removed the part that uses the Makefile for testing and autopkgtest now uses commands from the tutorial[6] and verifies their checksums. So would this be ok for now? 
>
>Sounds good.  I&#39;ll test once you&#39;ve pushed
>pplacer_1.1~alpha19.orig-debian-tests-data.tar.gz  .
>

I've already pushed the tarball to the pristine-tar branch - https://salsa.debian.org/med-team/pplacer/-/tree/pristine-tar

>&gt; Thanks a lot for this detailed review. Please see whether my changes are appropriate or is there anything more to be done.
>
>You are welcome.  We need to make sure to add some paragraph about the
>license of the data since the change in the source package will probably
>require passing the new queue which means license and copright will be
>re-checked.  As I said above an issue in the upstream Github repository
>might be a promising way to find this out.
>

I've opened an issue[7] upstream. Hoping they'll respond soon

>Kind regards
>
>     Andreas.
>

Regards,
rmb

>&gt; &gt; &gt; [1] - https://salsa.debian.org/med-team/pplacer
>&gt; &gt; &gt; [2] - https://fhcrc.github.io/microbiome-demo/
>&gt; &gt;[3] https://med-team.pages.debian.net/policy/#embedding-large-test-data
>&gt; &gt;
>&gt; [4] - https://salsa.debian.org/med-team/pplacer/-/blob/master/debian/tests/get-test-data
>&gt; [5] - https://github.com/fhcrc/microbiome-demo
>&gt; [6] -https://fhcrc.github.io/microbiome-demo/
>&gt; 
[7] - https://github.com/fhcrc/microbiome-demo/issues/7

>&gt; &gt;-- 
>&gt; &gt;http://fam-tille.de
>&gt; 
>&gt; 
>
>-- 
>http://fam-tille.de


Reply to: