Re: [metaphlan] Added autopkgtes t and distutils patch — please review

Étienne Mollier <[email protected]> Mon, 5 May 2025 22:34:38 +0200

Newsgroups	gmane.linux.debian.devel.medical
Message-ID	<[email protected]>

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 <[email protected]>
: :' : pgp: 8f91 b227 c7d6 f2b1 948c 8236 793c f67e 8f0d 11da
`. `' sent from /dev/pts/1, please excuse my verbosity
`-

signature.asc (application/pgp-signature, 833 B)