Hi Harish, > Hi team , i have write autopkgtest for metaphlan [0] and also added a patch to replace the deprecated distutils.version.LooseVersion with packaging.version.parse [1]. > kindly review , I'd really appreciate your feedback or suggestions if anything looks off. Thank you for your work on getting an autopkgtest implementation for metaphlan, I took some time to have a look. Please consider my comments below. I first focus on your commit[0], which adds the autopkgtest: > [0] https://salsa.debian.org/med-team/metaphlan/-/commit/c9b7630a9521eff9b5ae59fb1e0774ce8b50de0a First of all, please, can you document the origin of all the test data files that you added in debian/tests/data/ in the d/copyright file? That would alleviate any questions about copyright issues and give an idea how to get the necessary test data to make adjustments if need be. See the machine readable copyright file format[2] to see how to wrap up the necessary paragraphs. Or, if they happen to be autogenerated by a simpler procedure, then it is preferable to reconstruct them, either at build time or at autopkgtest time, than to embed the dataset, especially given its size. [2]: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/ Then, when trying to reconstruct the necessary source package from the git tree, my attempt to trigger a build resulted in the following error: $ gbp buildpackage gbp:info: Performing the build dpkg-source: info: using source format '3.0 (quilt)' dpkg-source: error: unwanted binary file: debian/tests/data/mpa_vFeb24_CDIFF_CHOCOPhlAnSGB_20240910.1.bt2l dpkg-source: error: unwanted binary file: debian/tests/data/mpa_vFeb24_CDIFF_CHOCOPhlAnSGB_20240910.2.bt2l dpkg-source: error: unwanted binary file: debian/tests/data/mpa_vFeb24_CDIFF_CHOCOPhlAnSGB_20240910.3.bt2l dpkg-source: error: unwanted binary file: debian/tests/data/mpa_vFeb24_CDIFF_CHOCOPhlAnSGB_20240910.4.bt2l dpkg-source: error: unwanted binary file: debian/tests/data/mpa_vFeb24_CDIFF_CHOCOPhlAnSGB_20240910.pkl dpkg-source: error: unwanted binary file: debian/tests/data/mpa_vFeb24_CDIFF_CHOCOPhlAnSGB_20240910.rev.1.bt2l dpkg-source: error: unwanted binary file: debian/tests/data/mpa_vFeb24_CDIFF_CHOCOPhlAnSGB_20240910.rev.2.bt2l dpkg-source: error: detected 7 unwanted binary files (add them in debian/source/include-binaries to allow their inclusion). It looks like filing the debian/source/include-binaries file was required for those files after all. I'm slightly concerned that the size of the debian/ directory exceeds the size of metaphlan by a factor five. Chance are we might be asked to remove the data files from the debian directory to put them somewhere else. To avoid such situation, it may be necessary to consider using the embedding method described in the Debian Med policy[3]. [3]: https://med-team.pages.debian.net/policy/#embedding-large-test-data Note that there seems to be stray patch queue files hidden below d/tests/.pc. They are raising lintian warnings. You may wish to remove this directory. Now I focus on you commit[1] which adds the patch to remove the dependency on distutils: > [1] https://salsa.debian.org/med-team/metaphlan/-/commit/03c0cb6fdba60293db3ae47c809a002b4d4bb226 The quilt patch as such looks good. As far as I can see, upstream is still using distutils in their code[4], so it might be beneficial to inform them of the existence of your patch. [4]: https://github.com/biobakery/MetaPhlAn/blob/master/metaphlan/metaphlan.py Note that we store quilt patches non-applied in git, thus, only the addition of debian/patches/ should have occurred in the git commit. However, 03c0cb6fdba60293db3ae47c809a002b4d4bb226 resulted in addition of the .pc/ directory and the modification of the file metaphlan/metaphlan.py. I have pushed a commit to restore the upstream tree to its pristine condition. (Note: I understand this is not specifically Debian Med policy, but I used the hint documented in Debian Perl policy[5] to avoid exposing the .pc/ directory to the git tree, to avoid that kind of issue in my day-to-day packaging activity. $ cat ~/.config/git/ignore .pc/ [5]: https://perl-team.pages.debian.net/git.html ) I hope I haven't been too overwhelming. If that helps, I think the autopkgtest script d/t/run-unit-test might be sufficient to make sure that the program has no obvious issues. Have a nice day, :) -- .''`. Étienne Mollier <emollier@debian.org> : :' : pgp: 8f91 b227 c7d6 f2b1 948c 8236 793c f67e 8f0d 11da `. `' sent from /dev/pts/1, please excuse my verbosity `-
Attachment:
signature.asc
Description: PGP signature