Merge ~bryce/ubuntu/+source/logwatch:eoan-merge-7.5.0-2 into ubuntu/+source/logwatch:debian/sid

Proposed by Bryce Harrington
Status: Merged
Merge reported by: Andreas Hasenack
Merged at revision: bc128c9093fbe21ae8e223183c04f39881a86e7e
Proposed branch: ~bryce/ubuntu/+source/logwatch:eoan-merge-7.5.0-2
Merge into: ubuntu/+source/logwatch:debian/sid
Diff against target: 387 lines (+251/-2)
2 files modified
debian/changelog (+247/-0)
debian/control (+4/-2)
Reviewer Review Type Date Requested Status
Andreas Hasenack (community) Approve
Canonical Server Pending
Christian Ehrhardt  Pending
Review via email: mp+367812@code.staging.launchpad.net

This proposal supersedes a proposal from 2019-05-18.

Description of the change

Merge with debian's logwatch package.

Two changes can be dropped, one remains.

See prior merge proposal for full details, but in short, the postfix mta override is no longer needed, and the deletion of mail.log.0 from the logwatch config file is superfluous since it's ignored anyway. The remaining change of moving some packages from Recommends to Suggests, probably wouldn't be taken by Debian so needs to remain in our packaging.

PPA available for testing is at
https://launchpad.net/~bryce/+archive/ubuntu/logwatch-merge-7.5.0-2

Usual git ubuntu tags are pushed.

To post a comment you must log in.
Revision history for this message
Andreas Hasenack (ahasenack) wrote : Posted in a previous version of this proposal

I tried to track down then the logrotation change was added, but that is very old. I think this is when it was introduced:
logwatch (7.3.6.cvs20090906-1ubuntu3) maverick; urgency=low

  * conf/logfiles/*, debian/dist.conf/logfiles/*:
    Due to migration to rsyslog, the first rotation is now .1 and not .0
    (fixes LP: #606715)

 -- Thierry Carrez (ttx) <email address hidden> Mon, 06 Sep 2010 14:57:14 +0200

Can you please check if that's also the case in debian, i.e., if they are also affected by it, and then submit this change to them? Salsa may not be the best place, given https://salsa.debian.org/debian/logwatch/merge_requests/1 is still unanswered, so maybe a debian bug that we can "Closes: #xxxxxx" in our d/changelog.

Revision history for this message
Andreas Hasenack (ahasenack) wrote : Posted in a previous version of this proposal

I think we can drop the default-mta delta. It is provided by postfix at least since precise, as far as I can see:

precise:
root@precise:~# aptitude search '~Pdefault-mta'
p postfix - High-performance mail transport agent
p postfix:i386 - High-performance mail transport agent

trusty:
root@trusty:~# aptitude search '~Pdefault-mta'
p postfix - High-performance mail transport agent

xenial:
root@xenial:~# aptitude search '~Pdefault-mta'
p postfix - High-performance mail transport agent

bionic:
root@bionic:~# aptitude search '~Pdefault-mta'
p postfix - High-performance mail transport agent
p postfix:i386 - High-performance mail transport agent

cosmic:
root@cosmic:~# aptitude search '~Pdefault-mta'
p postfix - High-performance mail transport agent

disco:
root@disco:~# aptitude search '~Pdefault-mta'
p postfix - High-performance mail transport agent

I tracked it down to this change in 2009:
postfix (2.6.2~rc1-1) unstable; urgency=low

  [Wietse Venema]

  * New upstream release: 2.6.2~rc1

  [LaMont Jones]

  * move postfix-add-{filter,policy} manpages to section 8, and deliver
  * provide: default-mta on ubuntu

 -- LaMont Jones <email address hidden> Wed, 03 Jun 2009 14:17:08 -0600

review: Needs Fixing
Revision history for this message
Bryce Harrington (bryce) wrote : Posted in a previous version of this proposal

Good suggestions Andreas. I dug into debian and discovered they were indeed aware of the problem, incl. the specific bug that spawned our patch:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=612634

However, they appear to have chosen the approach of listing both the .0 and .1 in their dist.conf/logfiles/maillog.conf file.

Presumably if there is no .0, it's just ignored? If so, perhaps this change could simply be dropped from our changeset?

Revision history for this message
Bryce Harrington (bryce) wrote : Posted in a previous version of this proposal

Regarding the default-mta, that makes sense in relation to Christian's commit message. I agree with your analysis and will drop this change item.

Revision history for this message
Andreas Hasenack (ahasenack) wrote : Posted in a previous version of this proposal

> Good suggestions Andreas. I dug into debian and discovered they were indeed
> aware of the problem, incl. the specific bug that spawned our patch:
>
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=612634
>
> However, they appear to have chosen the approach of listing both the .0 and .1
> in their dist.conf/logfiles/maillog.conf file.
>
> Presumably if there is no .0, it's just ignored? If so, perhaps this change
> could simply be dropped from our changeset?

Something to check, I don't know the behavior. If you can check a few logrotate runs, see what happens, and turns out it's a no-op, then +1 for dropping that delta as well.

Revision history for this message
Bryce Harrington (bryce) wrote : Posted in a previous version of this proposal

I read through the logwatch docs yesterday, and studied things a bit further. I've also set up an lxc install and manually ran through a few forced logrotate runs and logwatch cycles.

The behavior in question is described here:

    You can have as many LogFile entries as you wish. All the files specified
    will be merged into one input stream for any filters that use this logfile
    group. You can also use standard wildcards when you specify the filename.
    -- https://sourceforge.net/p/logwatch/git/ci/master/tree/HOWTO-Customize-LogWatch

IOW, listing mail.log.0 as well as mail.log.1 in /usr/share/logwatch/dist.conf/logfiles/maillog.conf will have no effect if the mail.log.0 is missing. I've verified this in the sut: http://paste.ubuntu.com/p/Pvwc6kfYRM/

So, we can indeed drop the delta.

Revision history for this message
Bryce Harrington (bryce) wrote : Posted in a previous version of this proposal

I also looked through the other config files in logwatch/*.conf/logfiles/ for any irregularities. Everything looks ok as far as I understand, except for /usr/share/logwatch/default.conf/logfiles/vdr.conf. It lists "LogFile = vdr.log.0", but iiuc that should more correctly be "LogFile = vdr.log.1". I'll file a bug report on this; I don't think it's necessary to be addressed in this merge.

Revision history for this message
Bryce Harrington (bryce) wrote : Posted in a previous version of this proposal
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Looks good, +1.

Just one tiny change perhaps, that you can do before uploading, is the changelog/commit message about the postfix delta drop.

Instead of:
    - debian/control: depend on postfix rather than exim4.
      + This is provided by postfix as of precise at least.

use:
    - debian/control: depend on postfix rather than exim4.
      + default-mta is provided by postfix as of precise at least.

And, this is a matter of personal choice, when explaining why something is being dropped (or added, or changed, or ...), we tend to use square brackets [] and reserve the indentation levels (+, -, +) for existing commit messages. So if you like that too, you could use:

    - debian/control: depend on postfix rather than exim4.
      [default-mta is provided by postfix as of precise at least]

Just to be clear, the above changes would be in the commit message, then the merge finish action would regenerate the changelog correctly.

review: Approve
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches