Hi Ben,
I spent some time reviewing the packaging for makemkv-{bin,oss}
today. Here are some thoughts/comments.
General notes:
* The typical pattern for new source packages in Debian is to have an
ITP filed, which is then closed on the first upload. Right now you have
an ITP that's covering both source packages. I'd suggest modifying the
existing ITP you have to be specific to makemkv-oss or makemkv-bin, and
then creating a new ITP for the other. Then you can cleanly close an
ITP for each new source package.
* The changelog entry for a new package should consist solely of a
line similar to "Initial release (Closes: #<ITP bug number>)". All the
work and changes prior to upload aren't specifically called out in the
package's changelog. Also, the version number should always end in "-
1".
* You may want to consider having two separate git repos for the two
different source packages. I realize that they are released in
lockstep, but having different repos may make life easier.
* You may want to look into setting up/using git-buildpackage(1)
(`gbp`), which can help make building packages much easier. Of course,
the trade-off is a bit of a learning curve, but I think it's worth it
in the long run.
makemkv-oss specific notes:
* d/control: Consider setting the Architecture field for the various
lib* binary packages to match that of the makemkv binary package.
* d/copyright: Double-check the listed copyright holders. For
example, the headers in libebml don't match what is listed in
d/copyright. I realize crafting d/copyright can be the most tedious
part of creating a package, but it's important to get it right. Two
things that I do are a case-insensitive grep through the code for
"copyright" and pay special attention to any LICENSE/COPYING/README
files that may have author copyright information. (Note that those
files don't always live at the root of the source, and may have
variations in how they are named.)
* d/makemkv.lintian-overrides: Can probably be deleted. Binaries
should have a manpage; sometimes if one doesn't exist as a packager you
have to write a basic one. Also, this is one of the things noted in the
Reject FAQ for NEW [1], which lists several things that may cause
ftpmaster to reject a package. The other override is desktop-entry-
lacks-keywords-entry; that can easily be fixed with a simple patch to
add some keywords to the .desktop file.
* d/README.source: Add this file to describe the issue(s) requiring
the shipping of bundled copies of libraries. Right off hand I know
about libebml and libmatroska, but there may be others. This informs
others who may review/work on the package in the future.
* d/source/metadata: Add this file; see [2] for more information.
* d/watch: Is incorrectly looking at the makemkv-bin source file.
Also, add a comment that the URL doesn't change, even though it looks
like it might.
makemkv-bin specific notes:
* d/copyright: There is an important difference between code that is
licensed as "GPL-2" and "GPL-2+" (for example). If a license doesn't
have "or any later version" (or similar phrasing), it's incorrect to
describe it with a "+" (or vise-versa). Admittedly, this is an issue in
my original packaging not getting that correct, but it should be fixed
in your packaging work. :)
* d/makemkvcon.lintian-overrides: Same comment as before with
binaries needing a manpage to be provided.
* d/rules: No need to have empty override_* targets.
* d/source/metadata: Add this file.
* d/watch: Similar to before, add a comment about the URL being
correct.
* debconf: I added a simple debconf message that's displayed to the
user to ensure they are aware of the licensing terms when they first
install makemkv-bin. I have pretty much no experience with debconf
hooks; there may be better/more correct ways to handle this.
Phew! That got kind of long, but I hope you find it useful. If you
have any questions or a comment doesn't make sense, please let me know.
Thanks for working on this!
Mathias
[1] -- https://ftp-master.debian.org/REJECT-FAQ.html
[2] -- https://wiki.debian.org/UpstreamMetadata
Attachment:
signature.asc
Description: This is a digitally signed message part