Hello here! On Sun, Apr 05, 2020 at 10:38:30AM +0300, Shlomi Fish wrote: > Hi Mattia! > > Thanks for the update. > > On Fri, Apr 3, 2020 at 5:30 PM Mattia Rizzolo <[email protected]> wrote: > > > just a private mail to mention that I didn't forget of this. > > Sorry for my science, I hope to get to this soon :\
I said this, but it turned out that I forgot. Fortunately I didn't archive the thread and it popped up now. > > https://salsa.debian.org/shlomif-guest/fortune-mod So, as a review of this. I see the git repo effectly is quite a mess, with way too generic commit messages like "debian update files for upstream version" or "fix lintian warnings" or "debian package" that don't really describe what and WHY you did such change. Plus the botchered upstream import at ee5dc32a2910a3a8411d2a528afd68b3383eb879 that was later on fixed. I guess I'll just look at the status at the current HEAD rather than the history... That said: * debian/menu: + remove the file: Policy v4.0.1 says to not install them anymore * debian/changelog: + squash together those 3 UNRELEASED chagnes, plus it's not an NMU anymore if you are adopting it (this is an ITS after all) + I can't believe you did one and a half screenful of commits with just those changes... And indeed looking at the diff is obvious that you didn't document basically any change. * debian/compat + bump to version 13, plus use the new debhelper-compat notation in B-D, so drop this. * debian/control: + bump standards-version + add Rules-Requires-Root * debian/copyright: + it would be best to rewrite the whole file using https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/ though since we are tight with the bullseye release if you wish to do so at the next upload I would understand it * debian/*.lintian-overrides: + you are the new upstream, right? then at least set Homepage to point to your github repo.. * debian/patches: + fort.patch: an empty patch?? + you cleaned out all patches. did you really integrate them all?? * debian/fortune-mod.manpages: + what happened with this file? * debian/rules: + I honestly can't believe such package has such a complicated rules file -.- even more as since you are upstream I kind of expected to make your own life easier + override_dh_auto_build-indep: pretty sure that's the default anyway + this: override_dh_auto_install-arch: dh_auto_install -- prefix=$(CURDIR)/debian/tmp \ - install-fortune install-util install-man + install + $(call MYCHRPATH) I'm pretty sure `dh_auto_install` already calls `install`. you should set DH_VERBOSE=1 so to clearly see what dh invokes. + you are adding all that handling to call `chrpath`. can't you fix the build system to just not save RPATH at all? + why skipping the tests? at least a comment on top is needed And this is only a "quick" look, I didn't even try to pressure myself into nitpicking as I usually do. Trying to build the sources: dpkg-source: info: local changes detected, the modified files are: fortune-mod/INSTALL fortune-mod/fortune/fortune-man.part1 fortune-mod/fortune/fortune-man.part2 fortune-mod/fortune/fortune.6 fortune-mod/fortune/gen-fortune-man-page.pl fortune-mod/util/strfile.8 dpkg-source: info: you can integrate the local changes with dpkg-source --commit dpkg-source: error: aborting due to unexpected upstream changes, see /tmp/fortune-mod_3.4.1-0.1.diff.yV3FX6 Basically, all those files are new, drop them from git. And even `debian/rules clean` doesn't delete them. Where are they coming from? Then I went on to build it and it failed right away: dh_auto_configure -- \ -D LOCALDIR=/usr/share/games/fortunes \ -D LOCALODIR=/usr/share/games/fortunes/off dh_auto_configure: warning: Compatibility levels before 10 are deprecated (level 9 in use) install -d obj-x86_64-linux-gnu cd obj-x86_64-linux-gnu && cmake -DCMAKE_INSTALL_PREFIX=/usr -DCMAKE_BUILD_TYPE=None -DCMAKE_INSTALL_SYSCONFDIR=/etc -DCMAKE_INSTALL_LOCALSTATEDIR=/var -DCMAKE_EXPORT_NO_PACKAGE_REGISTRY=ON -DCMAKE_FIND_PACKAGE_NO_PACKAGE_REGISTRY=ON "-GUnix Makefiles" -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_INSTALL_LIBDIR=lib/x86_64-linux-gnu -D LOCALDIR=/usr/share/games/fortunes -D LOCALODIR=/usr/share/games/fortunes/off .. Can't exec "cmake": No such file or directory at /usr/share/perl5/Debian/Debhelper/Dh_Lib.pm line 522. .... -.- → missing Build-Dep After installing cmake it went on. lintian also has quite a lot to say: W: fortune-mod: groff-message usr/share/man/man1/strfile.1.gz 10: warning: macro 'Aq' not defined W: fortune-mod: menu-command-not-in-package usr/share/menu/fortune-mod:5 while W: fortune-mod source: package-uses-deprecated-debhelper-compat-version 9 W: fortune-mod source: patch-file-present-but-not-mentioned-in-series fort.patch I: fortune-mod source: out-of-date-standards-version 4.5.0 (released 2020-01-20) (current is 4.5.1) P: fortune-mod source: silent-on-rules-requiring-root (note how the last 4 are things I also mentioned in my manual review above. which is to say: you could have spotted these yourself) I: fortune-mod source: unused-override debian-watch-file-is-missing I: fortune-mod: unused-override no-homepage-field I: fortunes: unused-override no-homepage-field I: fortunes-min: unused-override no-homepage-field I: fortunes-off: unused-override no-homepage-field P: fortune-mod source: no-homepage-field (that's because the taggin went from bianries to source… still) I: fortune-mod source: vcs-field-uses-insecure-uri Vcs-Git git://salsa.debian.org/shlomif-guest/fortune-mod.git (I didn't even notice this… that url doesn't even work, so ... -.-) P: fortune-mod source: trailing-whitespace debian/changelog Honestly, this is way too much to fix for me to even consider uploading. And the fact that there are these many flags makes me want to check it very very very deeply before even considering that, which then includes making sure all the patches you dropped were applied upstream and whatever else I may think of, which then means reviewing this would take me forever. Also, I noticed you wrote in the ITS bug Nov 16th, without CCing anything. I guess you are not quite in the know about how the Debian bug tracker works, since that mail likely reached Adrea only, who is sure to not look at it. A package maintainer is expected to know how to interact with the BTS, so please read https://www.debian.org/Bugs/ and subpages. The proper process to ask for package sponsorships is delined, for example, here: https://mentors.debian.net/sponsors/ -- regards, Mattia Rizzolo GPG Key: 66AE 2B4A FCCF 3F52 DA18 4D18 4B04 3FCD B944 4540 .''`. More about me: https://mapreri.org : :' : Launchpad user: https://launchpad.net/~mapreri `. `'` Debian QA page: https://qa.debian.org/developer.php?login=mattia `-
signature.asc
Description: PGP signature
