[Needs Packaging] DPF-Plugins for Eoan

Lintian returns no errors.

*** This is an automated message ***

This bug is tagged needs-packaging which identifies it as a request for a new package in Ubuntu. As a part of the managing needs-packaging bug reports specification, https://wiki.ubuntu.com/QATeam/Specs/NeedsPackagingBugs, all needs-packaging bug reports have Wishlist importance. Subsequently, I'm setting this bug's status to Wishlist.

Please link where we can find the package and resubscribe sponsors.

Links added to description.

For the record of other sponsors, I'm in the process of reviewing this (slowly) both from a general code perspective as well as a packaging perspective. This is currently assigned to me as such, during my review.

FOR THE RECORD: I've been taking my time working on this review for about a week now, so as not to overwork vs. the other work I do outside Ubuntu.

Static Code Analysis Tools Results (CodeWarrior)

Static code analysis of the C/C++ driven files present shows some TOCTOU risk and potential command injection but the command injection doesn't *appear* to be actually command injection in the manner it suggests (therefore code injection is a false-positive match). TOCTOU risks are potentially present in how it handles "Time of check" vs. "Time of use" of searched files, but that's an upstream issue and as it has other mechanisms to handle such errors written into the code around the potential TOCTOU violations, I don't believe there's a major code issue blocking inclusion of the plugins.

------

Packaging Review:

* Copyright file is mostly complete, but it seems there are some redundant overlaps, possibly because of multiple licenses being used.

* Rules file looks clean.

* Install files look clean.

* debian/control specifically with package descriptors of built binaries needs some work, in my opinion:

 - ALL packages share the same description. This includes the metapackage and -common, and may lead to some confusion if someone is not in the know about what each package specifically provides (and if they are only reading package descriptions). Therefore, the short AND long descriptions could use some tweaking to provide a simple one-line description in the description about the specifics of what each package does. This may be me being overly specific, but adding that extra little bit into the description would go a long way to making this more acceptable. Specifics below.

 - As dsp-plugins-{lv2, ladspa, etc.} all provide *specific formats* of the packaged plugins, it is my belief each of those packages should in turn include a description that they contain those specific packaging formats of those packages for use in their corresponding compatible software that can use those formats of the same plugins. This includes in the short description (you could add " (LV2 format)" to the end of the -lv2 package for example.

 - The -common package should contain something to indicate it contains files common across *all* the varying packaged formats of the plugins.

 - The metapackage does not detail that it pulls in all packaged flavors of the same plugins. A simple description stating this about the metapackage would be preferable,

While it may not be directly blocking sponsorship regarding the lacking of these slightly extra descriptive lines in the package descriptions, I would prefer to see them in the specific 'packaging' formats of the plugins packages and the common package.

Looks like some lines got blended togehter, the individual formats are defined in the first lines, and that ended up being missed. I blame myself for that.

Digging further, I've made revisions to my assessment.

dpf-plugins-common says: " Collection of DPF-based plugins in LADSPA, DSSI, LV2 and VST2 formats." This doesn't indicate that it's common files, and that the actual format of the plugins are stored in the individual packages themselves.

dpf-plugins metapackage doesn't state it's a metapackage that pulls in the collection of all formats of the plugins.

The other concerns are no longer valid, these two remain. (I missed a few lines in reading, oops)

Fix for control file pushed.

NOTE: This was fixed by Eickmeyer in the code as well as the invalid changelog target. https://git.launchpad.net/dpf-plugins/commit/?id=db45f68223a5461d2de4ebe1efdde418d76ee4fd fixes the description concerns.

From my perspective this package *looks* clean, so I'll sponsor it into NEW. It will still need AA review.

Hello Erich! Thank you for preparing the new package and thank you Thomas for sponsoring it. The package looks good, but there are still a few things I'd like to have addressed before we can accept it.

1) Copyright needs a bit more work, I suppose? So the worst thing about packages that batch up multiple projects together is the copyright hell. Due to copyright reasons we should always try to get as close to the actual copyright state of the source as possible. After glancing through the source with licensecheck -r, it still feels to me that there are a few parts missing in debian/copyright. For example:

 - dpf/dgl/src/nanovg is copyrighted separately and is licensed under the zlib license.
 - dpf/dgl/src/resources/LICENSE-DejaVuSans.ttf.txt - looks like a separate license for the attached font?
 - dpf/dgl/src/sofd/libsofd* - those files seem to be having an MIT header, so this should be mirrored to the copyright file.
 - dpf/distrho/src/dssi/ and some other parts seem to be LGPL
 - dpf/distrho/src/vestige/vestige.h GPL here as well?
 - plugins/common/ - these files are not mentioned anywhere but are reported as MIT

Could you do a licensecheck run on the source and see what can be done?

2) I see in debian/control that you have a versioned dependency on libprojectm-dev (>= 2.0.1+dfsg-10~bpo60+1). The 2.0.1+dfsg-10~bpo60+1 is very very old and even precise had a higher version available (not to mention eoan having 2.1.0+dfsg-4build2 already). I think we should just drop the version in the dependency in this case.

3) There is no debian/watch file, but I also don't see any releases shared on the homepage of the project. This makes me wonder - how are the new upstream versions prepared and tagged? Where does the 1.2 version number come from? (I didn't see it anywhere in the source). It's rather important for us to be able to match new upstream releases pushed to Ubuntu with releases done on the home project. We all trust eachother as we know eachother, but to the outside this might feel unsafe, as no one knows if the upstream tarball only consists of changes approved by upstream.

How does this situation look here? Could you explain the process?

Ok, I guess this is most of it. I will reject the source for now - let's try iterating on it some more. Thanks!

I can answer the source tarball question. Tagged releases on GH are where the tar ball version numbers originate from. Source tarballs are derived from GH's automatic source code tarballing for the repo on new version tags. Horrid, I know, but... that's what I discerned when asking that question to Erich before.

Erich indicated to me they did not know how to structure the debian/watch file. I assisted them in that regard, using https://wiki.debian.org/debian/watch#GitHub as a 'starting source' as the 'source' they were pulling from is at https://github.com/DISTRHO/DPF-Plugins/

The source code in the debian/watch I just added to their code repository with their permission (https://git.launchpad.net/dpf-plugins/commit/?id=5b6c3f2d5c0dae3bd4887cf3b9b294f4459e3f4d) includes a working debian/watch that properly pulls from the upstream tags and source code downloads. (according to uscan anyways)

1) Looks like I missed those before, added now.
2) Fixed.
3) Thomas made a nice watch file. Forgive my ignorance, but I didn't know how (or even if) to add that. Thomas has since shown me where to find that information.

Thanks for all the fixes! Looking good packaging-wise I must say, but sadly the package is FTBFS on all architectures besides amd64 and i386. I suppose those two architectures make the most sense, right? In this case, should we restrict the number of build architectures to amd64 and i386 for now? Is there any sense in building it for arches like s390x, arm64 etc.?

There's really no point in building for s390x, arm64, or powerpc. These plugins are simply not created for those architectures.