Bug#884697: Progress
Hello Christian Göttsche, On Fri, Jun 29, 2018 at 12:34:44PM +0200, Christian Göttsche wrote: > ping > > I uploaded a new version (lintian fixes, new std version, updated vcs > fields) to mentors (https://mentors.debian.net/package/logrotate). > Someone any ideas about the piuparts issues I mentioned? > > Otherwise I think the package is stable; its working for me on several > machines. I've looked over the 3.14.0-1 package version and generally everything looks very good to me. I'm appending my review notes about minor things below which might be useful for future improvements none the less. Please tell me if you want me to go ahead with further testing and uploading of the package, or if you already have someone else in mind for this task. Please also mention if you've contacted and what their response have been for the people offering mentorship (like in https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=887151#17 ). # logrotate WATCH OUT / HEADS UP: - I'm not sure about the current state but this has bitten me in the past. The debhelper systemd integration in the past had no particular knowledge about timer units. That resulted in the service unit for the respective timer unit being both enabled and *started* (or restarted, depending on if the package is newly installed or upgraded) at package installation/configure time. You likely do not want to trigger a logrotate at package installation/upgrade time and delay the entire dpkg operation until it completes. (I imagine some people might have massive logs that might take a very long time.) Please verify if current dh-systemd has improved on this front or if you need to add overrides for logrotate.service to not be started/enabled. I suspect this might very well be fixed now to just not start or enable services which don't have any [Install] section (like logrotate.service, but adding a build-time check to verify upstream doesn't slip one in there might be a useful safety for the future?). Minor things I reacted on that you might want to consider for future package versions: debian/upstream/metadata: - Repository url should have '.git' appended instead of last '/', right? - I think Bug-Database is more revelant for listing issues url instead of using Contact. debian/control: - I'm not sure using github project url in Homepage field is appropriate. It's supposed to be an url relevant for end users AFAIK. eg. github pages url would be suitable (if available, which it doesn't seem to be for logrotate). debian/logrotate.preinst: - how old is this? There are no version checks and I didn't look, but maybe it can be dropped now? The less manual written maintainerscript code the better. debian/logrotate.README.Debian: - this seems pretty outdated info as well considering for buster. Maybe it should also be dropped? debian/rules: - neat, but even better would be line-wrapping configure override using a backslash to put each config option on a separate line for easier reading. debian/tests: - given existance of tests reduces unstable->testing migration delay, this might just be a bit too trivial test to exist alone??? (while at the same time an existing test might be better than no test at all) debian/patches/manpage.patch: - why is this information relevant to put in the manpage? A more general solution would be to describe that apt-file can be used to search for which package contains something. Not suitable to document in specialized manpages like logrotate IMHO. Oh, I see this patch is not listed in series file so not applied. Well, might be another reason to drop it. debian/patches: - I see you've already upstreamed some of your work. I hope you will continue that trend with the remaining patches as well. Regards, Andreas Henriksson
Bug#884697: Progress
ping I uploaded a new version (lintian fixes, new std version, updated vcs fields) to mentors (https://mentors.debian.net/package/logrotate). Someone any ideas about the piuparts issues I mentioned? Otherwise I think the package is stable; its working for me on several machines.
Bug#884697: Progress
> PS: I just saw that you prefixed your branches with cgzones...
> That means that I can just do the fixes without stepping on your toes..
> So I've now imported all previous versions into the repository under the
> non-prefixed naming scheme.
Thanks for importing the history and applying my changes.
Can you change the default branch of the repository from cgzones to master?
When running piuparts, it fails with:
[...]
0m27.5s DEBUG: Starting command: ['chroot', '/tmp/tmphBJakN', 'dpkg',
'--purge', 'cron', 'libpopt0:amd64', 'logrotate']
0m29.0s DUMP:
(Reading database ... 12167 files and directories currently installed.)
Removing logrotate (3.11.0-0.1) ...
Purging configuration files for logrotate (3.11.0-0.1) ...
Removing cron (3.0pl1-130) ...
invoke-rc.d: could not determine current runlevel
invoke-rc.d: policy-rc.d denied execution of stop.
Purging configuration files for cron (3.0pl1-130) ...
Removing libpopt0:amd64 (1.16-10+b2) ...
Processing triggers for libc-bin (2.27-2) ...
0m29.0s DEBUG: Command ok: ['chroot', '/tmp/tmphBJakN', 'dpkg',
'--purge', 'cron', 'libpopt0:amd64', 'logrotate']
0m29.0s DEBUG: Starting command: ['chroot', '/tmp/tmphBJakN', 'dpkg',
'--purge', 'cron', 'libpopt0:amd64']
0m29.0s DUMP:
dpkg: warning: ignoring request to remove cron which isn't installed
dpkg: warning: ignoring request to remove libpopt0 which isn't installed
0m29.0s DEBUG: Command ok: ['chroot', '/tmp/tmphBJakN', 'dpkg',
'--purge', 'cron', 'libpopt0:amd64']
0m29.0s DEBUG: Starting command: ['chroot', '/tmp/tmphBJakN', 'dpkg',
'--purge', 'logrotate']
0m29.0s DUMP:
dpkg: warning: ignoring request to remove logrotate which isn't installed
0m29.0s DEBUG: Command ok: ['chroot', '/tmp/tmphBJakN', 'dpkg',
'--purge', 'logrotate']
0m29.0s DEBUG: Starting command: ['chroot', '/tmp/tmphBJakN', 'dpkg',
'--purge', '--pending']
0m29.1s DEBUG: Command ok: ['chroot', '/tmp/tmphBJakN', 'dpkg',
'--purge', '--pending']
0m29.1s DEBUG: Starting command: ['chroot', '/tmp/tmphBJakN', 'dpkg',
'--remove', '--pending']
0m29.1s DEBUG: Command ok: ['chroot', '/tmp/tmphBJakN', 'dpkg',
'--remove', '--pending']
0m29.1s DEBUG: Starting command: ['lsof', '-w', '+D', '/tmp/tmphBJakN']
0m29.5s DEBUG: Command failed (status=1), but ignoring error: ['lsof',
'-w', '+D', '/tmp/tmphBJakN']
0m30.4s ERROR: WARN: Broken symlinks:
/etc/systemd/system/timers.target.wants/logrotate.timer ->
/lib/systemd/system/logrotate.timer
0m30.4s DEBUG: Starting command: ['chroot', '/tmp/tmphBJakN',
'dpkg-divert', '--list']
0m30.4s DUMP:
diversion of /usr/share/man/man1/sh.1.gz to
/usr/share/man/man1/sh.distrib.1.gz by dash
diversion of /bin/sh to /bin/sh.distrib by dash
0m30.4s DEBUG: Command ok: ['chroot', '/tmp/tmphBJakN', 'dpkg-divert', '--list']
0m30.4s DEBUG: Starting command: ['chroot', '/tmp/tmphBJakN',
'apt-get', 'clean']
0m30.4s DEBUG: Command ok: ['chroot', '/tmp/tmphBJakN', 'apt-get', 'clean']
0m30.4s DEBUG: Recording chroot state
0m32.3s ERROR: FAIL: Package purging left files on system:
/etc/systemd/system/logrotate.timer -> /dev/null not owned
/etc/systemd/system/timers.target.wants/logrotate.timer ->
/lib/systemd/system/logrotate.timer not owned
/var/lib/systemd/deb-systemd-helper-enabled/logrotate.timer.dsh-also
not owned
/var/lib/systemd/deb-systemd-helper-enabled/timers.target.wants/logrotate.timer
not owned
/var/lib/systemd/deb-systemd-helper-masked/not owned
/var/lib/systemd/deb-systemd-helper-masked/logrotate.timer not owned
0m32.3s ERROR: FAIL: Installation and purging test.
0m32.7s DEBUG: Starting command: ['umount', '/tmp/tmphBJakN/dev/shm']
0m32.7s DEBUG: Command ok: ['umount', '/tmp/tmphBJakN/dev/shm']
0m32.7s DEBUG: Starting command: ['umount', '/tmp/tmphBJakN/dev/console']
0m32.7s DEBUG: Command ok: ['umount', '/tmp/tmphBJakN/dev/console']
0m32.7s DEBUG: Starting command: ['umount', '/tmp/tmphBJakN/dev/ptmx']
0m32.8s DEBUG: Command ok: ['umount', '/tmp/tmphBJakN/dev/ptmx']
0m32.8s DEBUG: Starting command: ['umount', '/tmp/tmphBJakN/dev/pts']
0m32.8s DEBUG: Command ok: ['umount', '/tmp/tmphBJakN/dev/pts']
0m32.8s DEBUG: Starting command: ['umount', '/tmp/tmphBJakN/proc']
0m32.8s DEBUG: Command ok: ['umount', '/tmp/tmphBJakN/proc']
0m32.8s DEBUG: Starting command: ['rm', '-rf', '--one-file-system',
'/tmp/tmphBJakN']
0m33.0s DEBUG: Command ok: ['rm', '-rf', '--one-file-system', '/tmp/tmphBJakN']
0m33.0s DEBUG: Removed directory tree at /tmp/tmphBJakN
0m33.0s ERROR: piuparts run ends.
Even when installing the timer/service by supplying them as
debian/logrotate.{timer|service} this issue persists.
Any idea how to fix this?
Bug#884697: Progress
On Sun, Mar 25, 2018 at 02:33:54PM +0200, Tobias Frost wrote: > However, it is missing the packaging history (it should have been imported > using git-build-package's import-dscs --debsnap) > Do you mind to recreate it (I can also do it, just let me know) PS: I just saw that you prefixed your branches with cgzones... That means that I can just do the fixes without stepping on your toes.. So I've now imported all previous versions into the repository under the non-prefixed naming scheme. -- tobi
Bug#884697: Progress
On Fri, Mar 09, 2018 at 10:24:44PM +0100, Christian Göttsche wrote: > There is now a salsa project for logrotate: > https://salsa.debian.org/debian/logrotate > > I also pushed my current packaging progress of version 3.14.0 to the > branch cgzones_master. Thanks! However, it is missing the packaging history (it should have been imported using git-build-package's import-dscs --debsnap) Do you mind to recreate it (I can also do it, just let me know) -- tobi
Bug#884697: Progress
There is now a salsa project for logrotate: https://salsa.debian.org/debian/logrotate I also pushed my current packaging progress of version 3.14.0 to the branch cgzones_master.
