Debian Bug report logs - #608930
please provide a tool to parse dpkg.log* files

Package: dpkg; Maintainer for dpkg is Dpkg Developers <debian-dpkg@lists.debian.org>; Source for dpkg is src:dpkg.

Reported by: Mike Hommey <mh+reportbug@glandium.org>

Date: Sat, 26 Sep 2009 08:42:02 UTC

Severity: wishlist

Blocking fix for 548415: reportbug: Package upgrade information in bug reports

Reply or subscribe to this bug.

Toggle useless messages

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to debian-bugs-dist@lists.debian.org, Reportbug Maintainers <reportbug-maint@lists.alioth.debian.org>:
Bug#548415; Package reportbug. (Sat, 26 Sep 2009 08:42:05 GMT) Full text and rfc822 format available.

Acknowledgement sent to Mike Hommey <mh+reportbug@glandium.org>:
New Bug report received and forwarded. Copy sent to Reportbug Maintainers <reportbug-maint@lists.alioth.debian.org>. (Sat, 26 Sep 2009 08:42:05 GMT) Full text and rfc822 format available.

Message #5 received at submit@bugs.debian.org (full text, mbox):

From: Mike Hommey <mh+reportbug@glandium.org>
To: Debian Bug Tracking System <submit@bugs.debian.org>
Subject: reportbug: Package upgrade information in bug reports
Date: Sat, 26 Sep 2009 10:29:13 +0200
Package: reportbug
Version: 4.8
Severity: wishlist

Hi,

I was just writing a bug report on the new nautilus, looking in dpkg.log
to know what version I had before the upgrade, and then I got wondering
if that wouldn't be an interesting information to have by default in
bug reports, for both the package the user reports the bug against, and
all its dependencies, indicating both the previous version and when the
upgrade happened.

If space is considered a problem, I'd say this information could replace
the packages descriptions, which are IMHO pretty useless to the package
maintainers.

What do you think?

Mike




Information forwarded to debian-bugs-dist@lists.debian.org, Reportbug Maintainers <reportbug-maint@lists.alioth.debian.org>:
Bug#548415; Package reportbug. (Tue, 04 Jan 2011 17:03:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Sandro Tosi <morph@debian.org>:
Extra info received and forwarded to list. Copy sent to Reportbug Maintainers <reportbug-maint@lists.alioth.debian.org>. (Tue, 04 Jan 2011 17:03:03 GMT) Full text and rfc822 format available.

Message #10 received at 548415@bugs.debian.org (full text, mbox):

From: Sandro Tosi <morph@debian.org>
To: Mike Hommey <mh+reportbug@glandium.org>, 548415@bugs.debian.org
Subject: Re: Bug#548415: reportbug: Package upgrade information in bug reports
Date: Tue, 4 Jan 2011 17:59:17 +0100
Hi Mike,
ok, I know, it's a rather old report... :)

On Sat, Sep 26, 2009 at 10:29, Mike Hommey <mh+reportbug@glandium.org> wrote:
> Package: reportbug
> Version: 4.8
> Severity: wishlist
>
> Hi,
>
> I was just writing a bug report on the new nautilus, looking in dpkg.log
> to know what version I had before the upgrade, and then I got wondering
> if that wouldn't be an interesting information to have by default in
> bug reports, for both the package the user reports the bug against, and
> all its dependencies, indicating both the previous version and when the
> upgrade happened.
>
> If space is considered a problem, I'd say this information could replace
> the packages descriptions, which are IMHO pretty useless to the package
> maintainers.
>
> What do you think?

I'm just wondering... by default dpkg.log is rotated monthly, so if
you're so unlucky to install the package on 31st and report the bug on
1st of the next month you don't get any information, if not searching
in all the rotated files, which then required maybe to go back in time
a long ago, and so also to report the date of the upgrade, and so on

also, there are several cases where the upgrade information (if
available) is useless, f.e. in reports like "documentation for
function aaa() is missing".

I understand it can be nice to have that info in some circumstances,
but I don't see a way to balance that with an over-complicated
solutions.

any ideas?

cheers,
-- 
Sandro Tosi (aka morph, morpheus, matrixhasu)
My website: http://matrixhasu.altervista.org/
Me at Debian: http://wiki.debian.org/SandroTosi




Information forwarded to debian-bugs-dist@lists.debian.org, Reportbug Maintainers <reportbug-maint@lists.alioth.debian.org>:
Bug#548415; Package reportbug. (Tue, 04 Jan 2011 17:18:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Mike Hommey <mh@glandium.org>:
Extra info received and forwarded to list. Copy sent to Reportbug Maintainers <reportbug-maint@lists.alioth.debian.org>. (Tue, 04 Jan 2011 17:18:03 GMT) Full text and rfc822 format available.

Message #15 received at 548415@bugs.debian.org (full text, mbox):

From: Mike Hommey <mh@glandium.org>
To: Sandro Tosi <morph@debian.org>
Cc: 548415@bugs.debian.org
Subject: Re: Bug#548415: reportbug: Package upgrade information in bug reports
Date: Tue, 4 Jan 2011 18:15:32 +0100
On Tue, Jan 04, 2011 at 05:59:17PM +0100, Sandro Tosi wrote:
> Hi Mike,
> ok, I know, it's a rather old report... :)
> 
> On Sat, Sep 26, 2009 at 10:29, Mike Hommey <mh+reportbug@glandium.org> wrote:
> > Package: reportbug
> > Version: 4.8
> > Severity: wishlist
> >
> > Hi,
> >
> > I was just writing a bug report on the new nautilus, looking in dpkg.log
> > to know what version I had before the upgrade, and then I got wondering
> > if that wouldn't be an interesting information to have by default in
> > bug reports, for both the package the user reports the bug against, and
> > all its dependencies, indicating both the previous version and when the
> > upgrade happened.
> >
> > If space is considered a problem, I'd say this information could replace
> > the packages descriptions, which are IMHO pretty useless to the package
> > maintainers.
> >
> > What do you think?
> 
> I'm just wondering... by default dpkg.log is rotated monthly, so if
> you're so unlucky to install the package on 31st and report the bug on
> 1st of the next month you don't get any information, if not searching
> in all the rotated files, which then required maybe to go back in time
> a long ago, and so also to report the date of the upgrade, and so on
> 
> also, there are several cases where the upgrade information (if
> available) is useless, f.e. in reports like "documentation for
> function aaa() is missing".
> 
> I understand it can be nice to have that info in some circumstances,
> but I don't see a way to balance that with an over-complicated
> solutions.
> 
> any ideas?

Maybe a helper that the maintainer could ask the bug reporter to run. I
sometimes as them to run reportbug --template $somepackage, that could
be something similar.

Mike




Information forwarded to debian-bugs-dist@lists.debian.org, Reportbug Maintainers <reportbug-maint@lists.alioth.debian.org>:
Bug#548415; Package reportbug. (Tue, 04 Jan 2011 17:24:09 GMT) Full text and rfc822 format available.

Acknowledgement sent to Sandro Tosi <morph@debian.org>:
Extra info received and forwarded to list. Copy sent to Reportbug Maintainers <reportbug-maint@lists.alioth.debian.org>. (Tue, 04 Jan 2011 17:24:09 GMT) Full text and rfc822 format available.

Message #20 received at 548415@bugs.debian.org (full text, mbox):

From: Sandro Tosi <morph@debian.org>
To: Mike Hommey <mh@glandium.org>
Cc: 548415@bugs.debian.org
Subject: Re: Bug#548415: reportbug: Package upgrade information in bug reports
Date: Tue, 4 Jan 2011 18:20:46 +0100
On Tue, Jan 4, 2011 at 18:15, Mike Hommey <mh@glandium.org> wrote:
> Maybe a helper that the maintainer could ask the bug reporter to run. I
> sometimes as them to run reportbug --template $somepackage, that could
> be something similar.

mh, that would be nice! but do you think that's something up to
reportbug to provide, or more fittingly something dpkg can ship?

-- 
Sandro Tosi (aka morph, morpheus, matrixhasu)
My website: http://matrixhasu.altervista.org/
Me at Debian: http://wiki.debian.org/SandroTosi




Information forwarded to debian-bugs-dist@lists.debian.org, Reportbug Maintainers <reportbug-maint@lists.alioth.debian.org>:
Bug#548415; Package reportbug. (Tue, 04 Jan 2011 17:24:13 GMT) Full text and rfc822 format available.

Acknowledgement sent to Mike Hommey <mh@glandium.org>:
Extra info received and forwarded to list. Copy sent to Reportbug Maintainers <reportbug-maint@lists.alioth.debian.org>. (Tue, 04 Jan 2011 17:24:13 GMT) Full text and rfc822 format available.

Message #25 received at 548415@bugs.debian.org (full text, mbox):

From: Mike Hommey <mh@glandium.org>
To: Sandro Tosi <morph@debian.org>
Cc: 548415@bugs.debian.org
Subject: Re: Bug#548415: reportbug: Package upgrade information in bug reports
Date: Tue, 4 Jan 2011 18:23:40 +0100
On Tue, Jan 04, 2011 at 06:20:46PM +0100, Sandro Tosi wrote:
> On Tue, Jan 4, 2011 at 18:15, Mike Hommey <mh@glandium.org> wrote:
> > Maybe a helper that the maintainer could ask the bug reporter to run. I
> > sometimes as them to run reportbug --template $somepackage, that could
> > be something similar.
> 
> mh, that would be nice! but do you think that's something up to
> reportbug to provide, or more fittingly something dpkg can ship?

I really don't have a preference.

Mike




Information forwarded to debian-bugs-dist@lists.debian.org, Reportbug Maintainers <reportbug-maint@lists.alioth.debian.org>:
Bug#548415; Package reportbug. (Tue, 04 Jan 2011 17:39:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Sandro Tosi <morph@debian.org>:
Extra info received and forwarded to list. Copy sent to Reportbug Maintainers <reportbug-maint@lists.alioth.debian.org>. (Tue, 04 Jan 2011 17:39:03 GMT) Full text and rfc822 format available.

Message #30 received at 548415@bugs.debian.org (full text, mbox):

From: Sandro Tosi <morph@debian.org>
To: Mike Hommey <mh@glandium.org>, control@bugs.debian.org
Cc: 548415@bugs.debian.org
Subject: Re: Bug#548415: reportbug: Package upgrade information in bug reports
Date: Tue, 4 Jan 2011 18:35:00 +0100
clone 548415 -1
reassign -1 dpkg
retitle -1 please provide a tool to parse dpkg.log* files
block  548415 by -1
thanks

On Tue, Jan 4, 2011 at 18:23, Mike Hommey <mh@glandium.org> wrote:
> On Tue, Jan 04, 2011 at 06:20:46PM +0100, Sandro Tosi wrote:
>> On Tue, Jan 4, 2011 at 18:15, Mike Hommey <mh@glandium.org> wrote:
>> > Maybe a helper that the maintainer could ask the bug reporter to run. I
>> > sometimes as them to run reportbug --template $somepackage, that could
>> > be something similar.
>>
>> mh, that would be nice! but do you think that's something up to
>> reportbug to provide, or more fittingly something dpkg can ship?
>
> I really don't have a preference.

Ok, so I clone this bug to ask dpkg maintainer to provide a tool to
parse dpkg.log* files. for example it would be cool to have an option
to have the last upgrade of a package like

  <date> <previous version> <upgrade version>

and probably a list of all the "relevant" events of a given package
(f.e., install, remove, reinstall, purge, etc etc). Mike something I'm
missing here?

Cheers,
-- 
Sandro Tosi (aka morph, morpheus, matrixhasu)
My website: http://matrixhasu.altervista.org/
Me at Debian: http://wiki.debian.org/SandroTosi




Bug 548415 cloned as bug 608930. Request was from Sandro Tosi <morph@debian.org> to control@bugs.debian.org. (Tue, 04 Jan 2011 17:39:03 GMT) Full text and rfc822 format available.

Bug reassigned from package 'reportbug' to 'dpkg'. Request was from Sandro Tosi <morph@debian.org> to control@bugs.debian.org. (Tue, 04 Jan 2011 17:39:06 GMT) Full text and rfc822 format available.

Bug No longer marked as found in versions reportbug/4.8. Request was from Sandro Tosi <morph@debian.org> to control@bugs.debian.org. (Tue, 04 Jan 2011 17:39:06 GMT) Full text and rfc822 format available.

Changed Bug title to 'please provide a tool to parse dpkg.log* files' from 'reportbug: Package upgrade information in bug reports' Request was from Sandro Tosi <morph@debian.org> to control@bugs.debian.org. (Tue, 04 Jan 2011 17:39:07 GMT) Full text and rfc822 format available.

Added indication that bug 608930 blocks 548415 Request was from Sandro Tosi <morph@debian.org> to control@bugs.debian.org. (Tue, 04 Jan 2011 17:39:08 GMT) Full text and rfc822 format available.

Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Developers <debian-dpkg@lists.debian.org>:
Bug#608930; Package dpkg. (Wed, 05 Jan 2011 08:03:05 GMT) Full text and rfc822 format available.

Acknowledgement sent to Raphael Hertzog <hertzog@debian.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Developers <debian-dpkg@lists.debian.org>. (Wed, 05 Jan 2011 08:03:05 GMT) Full text and rfc822 format available.

Message #45 received at 608930@bugs.debian.org (full text, mbox):

From: Raphael Hertzog <hertzog@debian.org>
To: Sandro Tosi <morph@debian.org>
Cc: Mike Hommey <mh@glandium.org>, 608930@bugs.debian.org, enrico@debian.org, 548415@bugs.debian.org
Subject: Re: Bug#548415: reportbug: Package upgrade information in bug reports
Date: Wed, 5 Jan 2011 08:58:53 +0100
Hi,

On Tue, 04 Jan 2011, Sandro Tosi wrote:
> On Tue, Jan 4, 2011 at 18:23, Mike Hommey <mh@glandium.org> wrote:
> > On Tue, Jan 04, 2011 at 06:20:46PM +0100, Sandro Tosi wrote:
> >> On Tue, Jan 4, 2011 at 18:15, Mike Hommey <mh@glandium.org> wrote:
> >> > Maybe a helper that the maintainer could ask the bug reporter to run. I
> >> > sometimes as them to run reportbug --template $somepackage, that could
> >> > be something similar.
> >>
> >> mh, that would be nice! but do you think that's something up to
> >> reportbug to provide, or more fittingly something dpkg can ship?
> >
> > I really don't have a preference.
> 
> Ok, so I clone this bug to ask dpkg maintainer to provide a tool to
> parse dpkg.log* files. for example it would be cool to have an option
> to have the last upgrade of a package like
> 
>   <date> <previous version> <upgrade version>
>
> and probably a list of all the "relevant" events of a given package
> (f.e., install, remove, reinstall, purge, etc etc). Mike something I'm
> missing here?

I don't think we're going to ship anything like this in the near future.

But Enrico Zini was interested in adding this information in the
apt-xapian-index database. We have added a feature in the master branch so
that he can get the status information in real time.

Maybe you could reuse the information stored there once it's implemented?
Ccing Enrico so that he can comment too.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Follow my Debian News ▶ http://RaphaelHertzog.com (English)
                      ▶ http://RaphaelHertzog.fr (Français)




Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Developers <debian-dpkg@lists.debian.org>:
Bug#608930; Package dpkg. (Wed, 05 Jan 2011 08:12:05 GMT) Full text and rfc822 format available.

Acknowledgement sent to Mike Hommey <mh@glandium.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Developers <debian-dpkg@lists.debian.org>. (Wed, 05 Jan 2011 08:12:05 GMT) Full text and rfc822 format available.

Message #50 received at 608930@bugs.debian.org (full text, mbox):

From: Mike Hommey <mh@glandium.org>
To: Raphael Hertzog <hertzog@debian.org>
Cc: Sandro Tosi <morph@debian.org>, 608930@bugs.debian.org, enrico@debian.org, 548415@bugs.debian.org
Subject: Re: Bug#548415: reportbug: Package upgrade information in bug reports
Date: Wed, 5 Jan 2011 09:08:22 +0100
On Wed, Jan 05, 2011 at 08:58:53AM +0100, Raphael Hertzog wrote:
> Hi,
> 
> On Tue, 04 Jan 2011, Sandro Tosi wrote:
> > On Tue, Jan 4, 2011 at 18:23, Mike Hommey <mh@glandium.org> wrote:
> > > On Tue, Jan 04, 2011 at 06:20:46PM +0100, Sandro Tosi wrote:
> > >> On Tue, Jan 4, 2011 at 18:15, Mike Hommey <mh@glandium.org> wrote:
> > >> > Maybe a helper that the maintainer could ask the bug reporter to run. I
> > >> > sometimes as them to run reportbug --template $somepackage, that could
> > >> > be something similar.
> > >>
> > >> mh, that would be nice! but do you think that's something up to
> > >> reportbug to provide, or more fittingly something dpkg can ship?
> > >
> > > I really don't have a preference.
> > 
> > Ok, so I clone this bug to ask dpkg maintainer to provide a tool to
> > parse dpkg.log* files. for example it would be cool to have an option
> > to have the last upgrade of a package like
> > 
> >   <date> <previous version> <upgrade version>
> >
> > and probably a list of all the "relevant" events of a given package
> > (f.e., install, remove, reinstall, purge, etc etc). Mike something I'm
> > missing here?
> 
> I don't think we're going to ship anything like this in the near future.
> 
> But Enrico Zini was interested in adding this information in the
> apt-xapian-index database. We have added a feature in the master branch so
> that he can get the status information in real time.
> 
> Maybe you could reuse the information stored there once it's implemented?
> Ccing Enrico so that he can comment too.

Except if we make all users have apt-xapian-index installed, this is not
going to be helpful to maintainers.

Mike




Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Developers <debian-dpkg@lists.debian.org>:
Bug#608930; Package dpkg. (Wed, 05 Jan 2011 10:42:08 GMT) Full text and rfc822 format available.

Acknowledgement sent to Raphael Hertzog <hertzog@debian.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Developers <debian-dpkg@lists.debian.org>. (Wed, 05 Jan 2011 10:42:08 GMT) Full text and rfc822 format available.

Message #55 received at 608930@bugs.debian.org (full text, mbox):

From: Raphael Hertzog <hertzog@debian.org>
To: Mike Hommey <mh@glandium.org>
Cc: Sandro Tosi <morph@debian.org>, 608930@bugs.debian.org, enrico@debian.org, 548415@bugs.debian.org
Subject: Re: Bug#548415: reportbug: Package upgrade information in bug reports
Date: Wed, 5 Jan 2011 11:39:40 +0100
On Wed, 05 Jan 2011, Mike Hommey wrote:
> Except if we make all users have apt-xapian-index installed, this is not
> going to be helpful to maintainers.

It's a recommends of aptitude and is installed by default.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Follow my Debian News ▶ http://RaphaelHertzog.com (English)
                      ▶ http://RaphaelHertzog.fr (Français)




Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Developers <debian-dpkg@lists.debian.org>:
Bug#608930; Package dpkg. (Thu, 06 Jan 2011 04:33:04 GMT) Full text and rfc822 format available.

Acknowledgement sent to Enrico Zini <enrico@debian.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Developers <debian-dpkg@lists.debian.org>. (Thu, 06 Jan 2011 04:33:04 GMT) Full text and rfc822 format available.

Message #60 received at 608930@bugs.debian.org (full text, mbox):

From: Enrico Zini <enrico@debian.org>
To: Mike Hommey <mh@glandium.org>
Cc: Raphael Hertzog <hertzog@debian.org>, Sandro Tosi <morph@debian.org>, 608930@bugs.debian.org, 548415@bugs.debian.org
Subject: Re: Bug#548415: reportbug: Package upgrade information in bug reports
Date: Thu, 6 Jan 2011 12:28:32 +0800
[Message part 1 (text/plain, inline)]
On Wed, Jan 05, 2011 at 09:08:22AM +0100, Mike Hommey wrote:

> > But Enrico Zini was interested in adding this information in the
> > apt-xapian-index database. We have added a feature in the master branch so
> > that he can get the status information in real time.
> > 
> > Maybe you could reuse the information stored there once it's implemented?
> > Ccing Enrico so that he can comment too.
> 
> Except if we make all users have apt-xapian-index installed, this is not
> going to be helpful to maintainers.

I haven't implemented anything on the apt-xapian-index side of things
yet.

I'd consider it entirely reasonable to use the data extracted with this
dpkg hook into some data file independent from apt-xapian-index.

In fact, the index itself should not be used as a primary data store, as
it should be possible to delete it and recreate it without loss of
information. For this reason, since the upgrade information is extracted
incrementally from the hook, it needs to be collected outside of the
index, like we currently do with the cataloged_times plugin:
/var/lib/apt-xapian-index/cataloged_times.p.

We should work out how this storage should be:
 - a flat file is not efficient when the operation is "updating only one
   value of only one package".
 - A log file keeps growing.
 - If you don't want apt-xapian-index as a dependency, then I imagine
   you want to keep the number of dependencies as small as possible: a
   sqlite3 file maybe? Some simple key-value store à-la gdbm?


Ciao,

Enrico

-- 
GPG key: 4096R/E7AD5568 2009-05-08 Enrico Zini <enrico@enricozini.org>
[signature.asc (application/pgp-signature, inline)]

Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Developers <debian-dpkg@lists.debian.org>:
Bug#608930; Package dpkg. (Thu, 06 Jan 2011 07:06:05 GMT) Full text and rfc822 format available.

Acknowledgement sent to Mike Hommey <mh@glandium.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Developers <debian-dpkg@lists.debian.org>. (Thu, 06 Jan 2011 07:06:05 GMT) Full text and rfc822 format available.

Message #65 received at 608930@bugs.debian.org (full text, mbox):

From: Mike Hommey <mh@glandium.org>
To: Enrico Zini <enrico@debian.org>
Cc: Raphael Hertzog <hertzog@debian.org>, Sandro Tosi <morph@debian.org>, 608930@bugs.debian.org, 548415@bugs.debian.org
Subject: Re: Bug#548415: reportbug: Package upgrade information in bug reports
Date: Thu, 6 Jan 2011 08:03:43 +0100
On Thu, Jan 06, 2011 at 12:28:32PM +0800, Enrico Zini wrote:
> On Wed, Jan 05, 2011 at 09:08:22AM +0100, Mike Hommey wrote:
> 
> > > But Enrico Zini was interested in adding this information in the
> > > apt-xapian-index database. We have added a feature in the master branch so
> > > that he can get the status information in real time.
> > > 
> > > Maybe you could reuse the information stored there once it's implemented?
> > > Ccing Enrico so that he can comment too.
> > 
> > Except if we make all users have apt-xapian-index installed, this is not
> > going to be helpful to maintainers.
> 
> I haven't implemented anything on the apt-xapian-index side of things
> yet.
> 
> I'd consider it entirely reasonable to use the data extracted with this
> dpkg hook into some data file independent from apt-xapian-index.
> 
> In fact, the index itself should not be used as a primary data store, as
> it should be possible to delete it and recreate it without loss of
> information. For this reason, since the upgrade information is extracted
> incrementally from the hook, it needs to be collected outside of the
> index, like we currently do with the cataloged_times plugin:
> /var/lib/apt-xapian-index/cataloged_times.p.
> 
> We should work out how this storage should be:
>  - a flat file is not efficient when the operation is "updating only one
>    value of only one package".
>  - A log file keeps growing.
>  - If you don't want apt-xapian-index as a dependency, then I imagine
>    you want to keep the number of dependencies as small as possible: a
>    sqlite3 file maybe? Some simple key-value store à-la gdbm?

I, for one, don't care very much about the dependency, but more about
the fact that not having it installed could mean not being able to pull
the information, even by installing afterwards. A dpkg.log parser would
always work, except when the logs are rotated and old enough to have
been removed.

Mike




Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Developers <debian-dpkg@lists.debian.org>:
Bug#608930; Package dpkg. (Tue, 11 Jan 2011 00:15:05 GMT) Full text and rfc822 format available.

Acknowledgement sent to Sandro Tosi <morph@debian.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Developers <debian-dpkg@lists.debian.org>. (Tue, 11 Jan 2011 00:15:05 GMT) Full text and rfc822 format available.

Message #70 received at 608930@bugs.debian.org (full text, mbox):

From: Sandro Tosi <morph@debian.org>
To: Mike Hommey <mh@glandium.org>
Cc: Enrico Zini <enrico@debian.org>, Raphael Hertzog <hertzog@debian.org>, 608930@bugs.debian.org, 548415@bugs.debian.org
Subject: Re: Bug#548415: reportbug: Package upgrade information in bug reports
Date: Tue, 11 Jan 2011 01:11:16 +0100
On Thu, Jan 6, 2011 at 08:03, Mike Hommey <mh@glandium.org> wrote:
> I, for one, don't care very much about the dependency, but more about
> the fact that not having it installed could mean not being able to pull
> the information, even by installing afterwards. A dpkg.log parser would
> always work, except when the logs are rotated and old enough to have
> been removed.

I tend to agree with Mike here: a log parser would be very nice, like something

dpkg-log last_ops <package>

<N> <op_N, like purge install remove etc> <package> <version>
<N-1> <op_N-1> <package> <version> (<prev_version> if needed)
<N-2> <op_N-2> <package> <version>
<N-3> <op_N-3> <package> <version>
<N-4> <op_N-4> <package> <version>

or a series of other interesting commands.

Default logrotate configuration keeps a year of log, so we have quite
a bit of room even for the very lazy bug reporters :)

What I'd like to avoid (with the reportbug maint hat on) is to let
reportbug parse a log file (either dpkg or xapian) to extract the info
it needs.

Cheers,
-- 
Sandro Tosi (aka morph, morpheus, matrixhasu)
My website: http://matrixhasu.altervista.org/
Me at Debian: http://wiki.debian.org/SandroTosi




Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Developers <debian-dpkg@lists.debian.org>:
Bug#608930; Package dpkg. (Fri, 25 Feb 2011 11:33:08 GMT) Full text and rfc822 format available.

Acknowledgement sent to Mike Hommey <mh@glandium.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Developers <debian-dpkg@lists.debian.org>. (Fri, 25 Feb 2011 11:33:08 GMT) Full text and rfc822 format available.

Message #75 received at 608930@bugs.debian.org (full text, mbox):

From: Mike Hommey <mh@glandium.org>
To: Sandro Tosi <morph@debian.org>
Cc: Enrico Zini <enrico@debian.org>, Raphael Hertzog <hertzog@debian.org>, 608930@bugs.debian.org, 548415@bugs.debian.org
Subject: Re: Bug#548415: reportbug: Package upgrade information in bug reports
Date: Fri, 25 Feb 2011 12:32:13 +0100
On Tue, Jan 11, 2011 at 01:11:16AM +0100, Sandro Tosi wrote:
> On Thu, Jan 6, 2011 at 08:03, Mike Hommey <mh@glandium.org> wrote:
> > I, for one, don't care very much about the dependency, but more about
> > the fact that not having it installed could mean not being able to pull
> > the information, even by installing afterwards. A dpkg.log parser would
> > always work, except when the logs are rotated and old enough to have
> > been removed.
> 
> I tend to agree with Mike here: a log parser would be very nice, like something
> 
> dpkg-log last_ops <package>
> 
> <N> <op_N, like purge install remove etc> <package> <version>
> <N-1> <op_N-1> <package> <version> (<prev_version> if needed)
> <N-2> <op_N-2> <package> <version>
> <N-3> <op_N-3> <package> <version>
> <N-4> <op_N-4> <package> <version>
> 
> or a series of other interesting commands.
> 
> Default logrotate configuration keeps a year of log, so we have quite
> a bit of room even for the very lazy bug reporters :)
> 
> What I'd like to avoid (with the reportbug maint hat on) is to let
> reportbug parse a log file (either dpkg or xapian) to extract the info
> it needs.

Looks like things are happening
http://justimho.blogspot.com/2011/02/let-me-introduce-dpkglog-and-dpkg.html

Mike




Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Developers <debian-dpkg@lists.debian.org>:
Bug#608930; Package dpkg. (Tue, 01 Mar 2011 08:09:04 GMT) Full text and rfc822 format available.

Acknowledgement sent to Patrick Schoenfeld <schoenfeld@debian.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Developers <debian-dpkg@lists.debian.org>. (Tue, 01 Mar 2011 08:09:04 GMT) Full text and rfc822 format available.

Message #80 received at 608930@bugs.debian.org (full text, mbox):

From: Patrick Schoenfeld <schoenfeld@debian.org>
To: debian-dpkg@lists.debian.org, 608930@bugs.debian.org
Subject: Merging DPKG::Log into dpkg codebase
Date: Tue, 1 Mar 2011 09:07:52 +0100
[.. Resending the mail as it seemingly did not reach the list and BTS ..]

Hi dpkg maintainers,

in a reply to my blog post about DPKG::Log, Raphael Hertzog, commented:

> I would suggest you to submit Dpkg::Log to dpkg
> itself... to be included in libdpkg-perl.
>
> I wonder why you did not ask in the first place.
> That way we can keep the code in sync in case dpkg starts outputting different stuff in the log file.

and I must confess, the only reason why I didn't ask is:
I just did not think about it.

Well, I've written DPKG::Log because I had a need for it and thought
it could be useful for others. Merging it into the dpkg codebase is
probably a good idea and so I'm revisiting that idea with this mail.
I see one problem, however.
My library, DPKG::Log, is written purely in Perl. I didn't see a big
need to implement it in C, because after all log processing
isn't something you do on an embedded system, I guess.
Now, AFAICT, it is one of the dpkg maintainers goals, to implement
dpkg-tools in C, isn't it?
Would this be a problem?

Apart from that: I'm dependend on that tool and therefore I'd
hate to submit and forget. So would it still be possible to
take care for DPKG::Log/dpkg-report, if it would get merged
into the dpkg codebase?

Best Regards,
Patrick




Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Developers <debian-dpkg@lists.debian.org>:
Bug#608930; Package dpkg. (Tue, 01 Mar 2011 08:42:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Raphael Hertzog <hertzog@debian.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Developers <debian-dpkg@lists.debian.org>. (Tue, 01 Mar 2011 08:42:03 GMT) Full text and rfc822 format available.

Message #85 received at 608930@bugs.debian.org (full text, mbox):

From: Raphael Hertzog <hertzog@debian.org>
To: Patrick Schoenfeld <schoenfeld@debian.org>, 608930@bugs.debian.org
Cc: debian-dpkg@lists.debian.org
Subject: Re: Bug#608930: Merging DPKG::Log into dpkg codebase
Date: Tue, 1 Mar 2011 09:38:24 +0100
Hi,

On Tue, 01 Mar 2011, Patrick Schoenfeld wrote:
> Well, I've written DPKG::Log because I had a need for it and thought
> it could be useful for others. Merging it into the dpkg codebase is
> probably a good idea and so I'm revisiting that idea with this mail.
> I see one problem, however.
> My library, DPKG::Log, is written purely in Perl. I didn't see a big
> need to implement it in C, because after all log processing
> isn't something you do on an embedded system, I guess.
> Now, AFAICT, it is one of the dpkg maintainers goals, to implement
> dpkg-tools in C, isn't it?
> Would this be a problem?

It would be a problem if we target this for the "dpkg" package but
we could introduce a "dpkg-utils" package where Perl would not be
a problem. Furthermore Dpkg::Log itself has its place in libdpkg-perl.

There's no reason for this tool to be integrated in the "dpkg" binary
package since it's not at all required to perform package installations.

> Apart from that: I'm dependend on that tool and therefore I'd
> hate to submit and forget. So would it still be possible to
> take care for DPKG::Log/dpkg-report, if it would get merged
> into the dpkg codebase?

Sure, you're more than welcome to take care of it!

Now, I have not yet looked into your code. But merging it supposes
that you follow our conventions and reuse our existing Perl libraries
when it makes sense.

I suggest you look into some of the existing Dpkg::* module, that you read
doc/coding-style.txt and that you try submitting a Dpkg::Log::Status
module (yes there could be Log modules to parse other files like the
alternatives log file so it's best to use a dedicated module from the
start).

If you have any question, feel free to ask.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Follow my Debian News ▶ http://RaphaelHertzog.com (English)
                      ▶ http://RaphaelHertzog.fr (Français)




Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Developers <debian-dpkg@lists.debian.org>:
Bug#608930; Package dpkg. (Wed, 02 Mar 2011 07:36:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Patrick Schoenfeld <schoenfeld@debian.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Developers <debian-dpkg@lists.debian.org>. (Wed, 02 Mar 2011 07:36:03 GMT) Full text and rfc822 format available.

Message #90 received at 608930@bugs.debian.org (full text, mbox):

From: Patrick Schoenfeld <schoenfeld@debian.org>
To: Raphael Hertzog <hertzog@debian.org>
Cc: 608930@bugs.debian.org, debian-dpkg@lists.debian.org
Subject: Re: Bug#608930: Merging DPKG::Log into dpkg codebase
Date: Wed, 2 Mar 2011 08:31:52 +0100
Hi,

On Tue, Mar 01, 2011 at 09:38:24AM +0100, Raphael Hertzog wrote:
> Hi,
> 
> On Tue, 01 Mar 2011, Patrick Schoenfeld wrote:
> > Well, I've written DPKG::Log because I had a need for it and thought
> > it could be useful for others. Merging it into the dpkg codebase is
> > probably a good idea and so I'm revisiting that idea with this mail.
> > I see one problem, however.
> > My library, DPKG::Log, is written purely in Perl. I didn't see a big
> > need to implement it in C, because after all log processing
> > isn't something you do on an embedded system, I guess.
> > Now, AFAICT, it is one of the dpkg maintainers goals, to implement
> > dpkg-tools in C, isn't it?
> > Would this be a problem?
> 
> It would be a problem if we target this for the "dpkg" package but
> we could introduce a "dpkg-utils" package where Perl would not be
> a problem. Furthermore Dpkg::Log itself has its place in libdpkg-perl.

Ok, makes sense.

> There's no reason for this tool to be integrated in the "dpkg" binary
> package since it's not at all required to perform package installations.

Agreed.

> > Apart from that: I'm dependend on that tool and therefore I'd
> > hate to submit and forget. So would it still be possible to
> > take care for DPKG::Log/dpkg-report, if it would get merged
> > into the dpkg codebase?
> 
> Sure, you're more than welcome to take care of it!
> 
> Now, I have not yet looked into your code. But merging it supposes
> that you follow our conventions and reuse our existing Perl libraries
> when it makes sense.

Well, I have not looked into the coding guidelines, yet. I'll look into
that. Re-Using existing libraries, where it makes sense, however is
always the way to go (for that reason I'm already using Dpkg::Version ;)

> I suggest you look into some of the existing Dpkg::* module, that you read
> doc/coding-style.txt and that you try submitting a Dpkg::Log::Status
> module (yes there could be Log modules to parse other files like the
> alternatives log file so it's best to use a dedicated module from the
> start).

Hmm. I'm not really sure, if ::Status would be the right name, but
on the OTOH you, as a dpkg maintainer, know better.
Besides that: The idea in general is good. I guess I'll rewrite DPKG::Log as
Dpkg::Log to be a common class, implementing the common interface for
dpkg logfiles and Dpkg::Log::Status extending that.

Best Regards,
Patrick




Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Developers <debian-dpkg@lists.debian.org>:
Bug#608930; Package dpkg. (Sat, 13 Aug 2011 17:24:05 GMT) Full text and rfc822 format available.

Acknowledgement sent to Sandro Tosi <morph@debian.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Developers <debian-dpkg@lists.debian.org>. (Sat, 13 Aug 2011 17:24:05 GMT) Full text and rfc822 format available.

Message #95 received at 608930@bugs.debian.org (full text, mbox):

From: Sandro Tosi <morph@debian.org>
To: Mike Hommey <mh@glandium.org>
Cc: Enrico Zini <enrico@debian.org>, Raphael Hertzog <hertzog@debian.org>, 608930@bugs.debian.org, 548415@bugs.debian.org
Subject: Re: Bug#548415: reportbug: Package upgrade information in bug reports
Date: Sat, 13 Aug 2011 19:21:17 +0200
On Fri, Feb 25, 2011 at 12:32, Mike Hommey <mh@glandium.org> wrote:
> On Tue, Jan 11, 2011 at 01:11:16AM +0100, Sandro Tosi wrote:
>> On Thu, Jan 6, 2011 at 08:03, Mike Hommey <mh@glandium.org> wrote:
>> > I, for one, don't care very much about the dependency, but more about
>> > the fact that not having it installed could mean not being able to pull
>> > the information, even by installing afterwards. A dpkg.log parser would
>> > always work, except when the logs are rotated and old enough to have
>> > been removed.
>>
>> I tend to agree with Mike here: a log parser would be very nice, like something
>>
>> dpkg-log last_ops <package>
>>
>> <N> <op_N, like purge install remove etc> <package> <version>
>> <N-1> <op_N-1> <package> <version> (<prev_version> if needed)
>> <N-2> <op_N-2> <package> <version>
>> <N-3> <op_N-3> <package> <version>
>> <N-4> <op_N-4> <package> <version>
>>
>> or a series of other interesting commands.
>>
>> Default logrotate configuration keeps a year of log, so we have quite
>> a bit of room even for the very lazy bug reporters :)
>>
>> What I'd like to avoid (with the reportbug maint hat on) is to let
>> reportbug parse a log file (either dpkg or xapian) to extract the info
>> it needs.
>
> Looks like things are happening
> http://justimho.blogspot.com/2011/02/let-me-introduce-dpkglog-and-dpkg.html

Sorry to chime in so late; I've reported some bugs against
libdpkg-log-perl, let's see how it goes. If I missed some, please
integrated :)

Regards,
-- 
Sandro Tosi (aka morph, morpheus, matrixhasu)
My website: http://matrixhasu.altervista.org/
Me at Debian: http://wiki.debian.org/SandroTosi




Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Developers <debian-dpkg@lists.debian.org>:
Bug#608930; Package dpkg. (Fri, 13 Jan 2012 15:51:11 GMT) Full text and rfc822 format available.

Acknowledgement sent to Patrick Schoenfeld <schoenfeld@debian.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Developers <debian-dpkg@lists.debian.org>. (Fri, 13 Jan 2012 15:51:11 GMT) Full text and rfc822 format available.

Message #100 received at 608930@bugs.debian.org (full text, mbox):

From: Patrick Schoenfeld <schoenfeld@debian.org>
To: debian-dpkg@lists.debian.org, 608930@bugs.debian.org
Subject: Dpkg::Log - log file parsing support for dpkg log files
Date: Fri, 13 Jan 2012 16:50:09 +0100
[Message part 1 (text/plain, inline)]
Hi,

we had the topic quiet a while ago and I must confess I haven't made a
lot of progress on bringing forward the merge of DPKG::Log in the dpkg
code base.

However, I feel, I should somehow try to get this forward and so I'm
sending a patch, which could be reviewed, so actually *some* progress
is starting to happen.

There are some notes,  I have to make about it:

- It includes a dpkg-report script, but its probably not yet useful
  for anyone, because its missing a template and a manpage.
  With regard to the template: I'm unsure if dpkg maintainers would be
  ok to stay with libtemplate-perl, which is used in the script so far.
  Feedback needed.

- The patch does not yet include any packaging updates.

- It also does not yet incorporate any fixes for the bugs reported
  against libdpkg-log-perl.

Please find the patch attached.

Kindly asking for feedback,
best Regards,
Patrick
[dpkg-log.patch (text/x-diff, attachment)]

Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Developers <debian-dpkg@lists.debian.org>:
Bug#608930; Package dpkg. (Wed, 25 Jan 2012 10:33:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Raphael Hertzog <hertzog@debian.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Developers <debian-dpkg@lists.debian.org>. (Wed, 25 Jan 2012 10:33:09 GMT) Full text and rfc822 format available.

Message #105 received at 608930@bugs.debian.org (full text, mbox):

From: Raphael Hertzog <hertzog@debian.org>
To: Patrick Schoenfeld <schoenfeld@debian.org>, 608930@bugs.debian.org
Subject: Re: Bug#608930: Dpkg::Log - log file parsing support for dpkg log files
Date: Wed, 25 Jan 2012 11:28:35 +0100
Hi,

On Fri, 13 Jan 2012, Patrick Schoenfeld wrote:
> we had the topic quiet a while ago and I must confess I haven't made a
> lot of progress on bringing forward the merge of DPKG::Log in the dpkg
> code base.
> 
> However, I feel, I should somehow try to get this forward and so I'm
> sending a patch, which could be reviewed, so actually *some* progress
> is starting to happen.

Thanks for this!

Here's my summary (after the review):

- the patch is much too big for a simple functionality like this one, you
  have to cut some code away, there are useless checks (code will end up
  failing if users submit something that's not expected, you don't have
  to hardcode checks for all possible mistakes that user might make),
  there are too many classes and intermediary objects, etc.
  Do your best to be "concise" and "readable".

- the amount of submitted code is so big that you should split the patch
  in multiple patches: first add the "base" module
  (Dpkg::Log/Dpkg::Log::Entry), then add one type of derived module after
  another.

  Start with a basic design that does only parsing and storage of
  information, come back to me with that and with a few iterations we'll
  get something clean as a base to build upon.

  We'll add the query features later on.

- You should not need so many modules,let me suggest:

  Dpkg::Log (introductory doc + maybe helper functions)
  Dpkg::Log::Base (your actual Dpkg::Log + Dpkg::Log::Analyse)
  Dpkg::Log::Base::Entry (your actual Dpkg::Log::Entry + 
  Dpkg::Log::Status (
  Dpkg::Log::Status::Entry

  i.e. somehow we'll have to find a way to merge your "Analyse*" modules
  into the basic modules.


You'll find belowe many more comments noted while I was doing the review.
There might be more to say but honestly I don't want to be too nitpicky
while the basic design doesn't suit me for now.

> - It includes a dpkg-report script, but its probably not yet useful
>   for anyone, because its missing a template and a manpage.
>   With regard to the template: I'm unsure if dpkg maintainers would be
>   ok to stay with libtemplate-perl, which is used in the script so far.
>   Feedback needed.

What kind of output formatting do you expect people to need? What
advanced feature of libtemplate-perl do you expect people to use?

If there's nothing more than what some plain substitutions can do, then
I don't think that the dependency is warranted.

In any case, it's probably best to keep this for later and deal with the
modules first.

> +  # Return all entries as <ENTRY PACKAGE NAME> objects
> +  @entries = $dpkg_log->entries;
> +  
> +  # Loop over entries
> +  while ($entry = $dpkg_log->next_entry) {
> +      ...
> +  }

I don't think that this integrated iterator brings anything (in particular
since it can't be manually reset apparently).

> +  # Get datetime from logfile or object, depending on weither
> +  # object stores from/to values or not
> +  ($from, $to) = $dpkg_log->get_datetime_info();

It's not clear what those "$from/$to" are used for. Is it to keep only a
subset of the entries parsed?

> +use Carp;

[...]

> +    croak "odd number of arguments" if not $entry_class;
> +    croak "wrong argument type: argument '$entry_class' should be a module"
> +        unless UNIVERSAL::can($entry_class, 'new');

We have Dpkg::ErrorHandling and you should use that and not croak directly.
Maybe you want to extend Dpkg::ErrorHandling so that we have a standard
function that croaks... not sure how to best name it. Maybe "caller_error"
to show that it's to be used when we want to fail because the caller of
the function made a mistake.

> +    my $self = {
> +        entry_class => $entry_class,
> +        entries => [],
> +        invalid_lines => [],
> +        from => undef,
> +        to => undef,
> +        offset => 0,
> +        filename => undef,
> +        parse => 0,
> +        time_zone => 'local',
> +        timestamp_pattern => '%F %T',
> +        from => 0,
> +        to => 0,
> +        %params
> +    };

from and to are listed twice here.

> +=item $dpkg_log->parse()
> +
> +This is a stub method which has to be implemented by an inherting class.
> +
> +=back
> +=cut

I think that you need an empty line between back and cut. Check with
podchecker. There are other similar problems from what I saw.

> +sub parse {
> +    croak "Not yet implemented."
> +}

internerr() instead of croak and the messages should explain what's not
implemented.

> +=head2 Working with the logfile
> +
> +These methods are to retrieve entries from the parsed logfile. All these
> +methods require that the parse method has been run.
> +
> +Be aware that if the object does not store entries these methods will die.
> +This is also true, if a parsed logfile is empty or contains invalid lines only.
> +
> +=over 4
> +
> +=item @entries = $dpkg_log->entries();
> +
> +Returns all entries, stored in the object, as entry objects. The identity of
> +an entry object is defined by the parameter passed to the object constructor.
> +
> +This method accepts a hash with params, which can contain the keys B<from>
> +and B<to>, which, if specified, will be used to filter the entries by date and
> +time.
> +=cut
> +sub entries {
> +    my ($self, %params) = @_;
> +    my $package = $self->{entry_class};
> +
> +    croak "Object does not store entries. Eventually parse function were not" .
> +        " run or log is empty." if (not @{$self->{entries}});

Why is it a problem if the log is empty ? I would not fail and let the
function return something empty...

> +
> +    if (not ($params{from} or $params{to})) {
> +        return map { $package->new($_) } @{$self->{entries}};
> +    } else {
> +        return $self->filter_by_time($package, %params);
> +    }

Invert the clauses to get rid of the not() in the test.

> +}
> +
> +=item $entry = $dpkg_log->next_entry;
> +
> +Returns the next entry, stored in the object, as entry object.
> +The identity of an entry object is defined by the parameter
> +passed to the object constructor.
> +
> +=cut
> +sub next_entry {
> +    my ($self) = @_;
> +    my $package = $self->{entry_class};
> +   
> +    croak "Object does not store entries. Eventually parse function were not" .
> +        " run or log is empty." if (not @{$self->{entries}});
> +
> +    my $offset = $self->{offset}++;
> +    if (not defined(@{$self->{entries}}[$offset])) {
> +        return undef;
> +    } else {
> +        return $package->new(@{$self->{entries}}[$offset]);
> +    }
> +}

As I said, I would get rid of that.

> +
> +=item @entries = $dpkg_log->filter_by_time(from => ts, to => ts)
> +
> +=item @entries = $dpkg_log->filter_by_time(from => ts)
> +
> +=item @entries = $dpkg_log->filter_by_time(to => ts)
> +
> +=item @entries = $dpkg_log->filter_by_time(from => ts, entry_ref => $entry_ref)

Use a single line like the one below and explain %params in the comment:

=item @entries = $dpkg_log->filter_by_time(%params)

Listing all combinations is error-prone (and you're lacking some).

> +This method filters entries by a certain date/time range and returns
> +entry object. The identity of an entry object is defined by the parameter
> +passed to the object constructor.
> +
> +The range can be specified by passing B<from> and B<to> arguments,
> +otherwise it will use whatever is stored in the object.

This fallback to values stored within the object seems counter-intuitive
to me. I don't see why the object should store such values...

> +If for any value no explicit parameter is given the timestamp used for filtering
> +will be measured by the first and last entry in the logfile.

Why do they have to be measured? If I don't specify one of both values, I
expect to have an unbounded interval (on one side)... it will give the
same result but it's clearer IMO.

> +
> +If the given from and to values are not DateTime objects, they will be
> +interpreted as strings and will be passed to DateTime::Format::Strptime.
> +
> +=cut
> +sub filter_by_time {
> +    my ($self, %params) = @_;
> +    my $package = $self->{entry_class};
> +
> +    my $entry_ref;
> +    if (not defined $params{entry_ref}) {
> +        $entry_ref = $self->{entries};
> +    } else {
> +        $entry_ref = $params{entry_ref};
> +    }
> +
> +    croak "Object does not store entries. Eventually parse function were not" .
> +        " run or log is empty." if (not @{$entry_ref});

Same here, filtering an empty set should not fail and return something
empty too.

> +
> +    my $ts_parser = DateTime::Format::Strptime->new(
> +        pattern => $self->{timestamp_pattern},
> +        time_zone => $self->{time_zone}
> +    );
> +
> +    for my $field qw(from to) {
> +        if (not defined $params{$field} and not defined $self->{$field}) {
> +            my $e_index = ($field eq "from") ? 0 : -1;
> +            $params{$field} = $package->new($entry_ref->[$e_index])->timestamp;
> +        } elsif (defined $self->{$field} and not defined $params{$field}) {
> +            $params{$field} = $self->{$field};
> +        } elsif (defined $params{$field} and not reftype($params{$field})) {
> +            $params{$field} = $ts_parser->parse_datetime($params{$field});
> +        }
> +    }
> +    my ($from, $to) = ($params{from}, $params{to});
> +
> +    my @entries = map { $package->new($_) } @{$entry_ref};
> +    return grep { $_->timestamp >= $from and $_->timestamp <= $to } @entries;
> +}

The are ways to simplify this code (and make it more readable) IMO.

> +
> +=item ($from, $to) = $dpkg_log->get_datetime_info()
> +
> +Returns the date/time period of the logfile or the one
> +defined in the object.
> +If object is initialized with from and/or to parameters it should return
> +these parameters, otherwise the timestamp of the first and the last entry
> +are returned.

I don't see (yet) what this function is used for. It's doesn't look like
a useful feature at least.

> --- /dev/null
> +++ b/scripts/Dpkg/Log/Analyse.pm
> @@ -0,0 +1,93 @@
> +package Dpkg::Log::Analyse;

Analyser or Analyzer. Not sure if we have picked en-US or en-GB by default
in dpkg...

> +use Dpkg::Log::Analyse;
> +
> +my $analyser = Dpkg::Log::Analyse->new('filename' => 'dpkg.log');
> +$analyser->analyse;
> +
> +=head1 DESCRIPTION
> +
> +This module is used to analyse a dpkg log.

I don't see what this module is useful for. Is it just to create a
Dkpg::Log and call parse on it?

> +Dpkg::Log::Analyse::Package - Describe a package as analysed from a dpkg.log
> +

What's the purpose of this module?

[...]
> +use overload (
> +    '""' => 'as_string',
> +    'eq' => 'equals',
> +    'cmp' => 'compare',
> +    '<=>' => 'compare'
> +);
[...]

> +        if (defined $params{$v}) {
> +            croak "wrong argument type: argument '$v' should be a Dpkg::Version"
> +                unless reftype($params{$v} eq "Dpkg::Version");
> +        }

Your usage of reftype here (and probably elsewhere in the code) is wrong.

$ perl -MDpkg::Version -M'Scalar::Util qw(reftype blessed)' -e '$v=Dpkg::Version->new("1.2"); print reftype($v) . "\n"; print blessed($v)'
HASH
Dpkg::Version

But it might be better to use $obj->isa("Dpkg::Version").

> +    for my $s qw(package status) {
> +        if (defined $params{$s}) {
> +            croak "wrong argument type: argument '$s' must not be a ref"
> +                if reftype($params{$s});
> +        }
> +    }

Are those kind of checks really necessary ?

> +    my ($self, $version) = @_;
> +    if ($version) {

What if we have version 0 ? (use defined() in the check)

> +        $self->_set_version('version', $version);
> +    } else {
> +        $version = $self->{version};
> +    }
> +    return $version;
> +}
> +
> +=item $package->previous_version
> +
> +Return or set the previous version of this package.
> +
> +=cut
> +sub previous_version {
> +    my ($self, $previous_version) = @_;
> +    if ($previous_version) {

Likewise.

> +        $self->_set_version('previous_version', $previous_version);

Those _set_version are really useless. You can do the affectation
immediately 

$self->{previous_version} = Dpkg::Version->new($previous_version);

It will directly with it correctly even if we already have a Dpkg::Version.

> +=item equals($package1, $package2);

Don't document internal methods, instead just document the operator (eq, ==).

> +=item print "equal" if $package1 eq $package2
> +
> +Compares two packages in their string representation.
> +
> +=cut
> +sub equals {
> +    my ($first, $second) = @_;
> +    return ($first->as_string eq $second->as_string);
> +}
> +
> +
> +=item compare($package1, $package2)
> +
> +=item print "greater" if $package1 > $package2
> +
> +Compare two packages. See B<OVERLOADING> for details on how
> +the comparison works.
> +=cut
> +sub compare {
> +    my ($first, $second) = @_;
> +    return -1 if ($first->name ne $second->name);

That's wrong. You want to return "$first->name cmp $second->name" if you
want to be able to order anything based on your comparison operator...

> +    if ((not $first->previous_version) and (not $second->previous_version)) {
> +        return ($first->version <=> $second->version);
> +    } elsif ((not $first->previous_version) or (not $second->previous_version)) {
> +        return -1;
> +    } elsif ($first->previous_version != $second->previous_version) {
> +        return -1;
> +    }
> +    
> +    return (($first->version <=> $second->version));
> +
> +}

And what if the versions are equal ? The same version of the same package
might be listed multiple times in a log file

> +
> +=item $package_str = $package->as_string
> +
> +=item printf("Package name: %s", $package);

I simply used “=item "$package"” to document the conversion to string.
Don't put sample code that is unrelated and might confuse people
(likewise with the “print "equals"” and similar stuff you used before).
Put only the operator and explains what's the return value.

> +Return this package as a string. This will return the package name
> +and the version (if set) in the form package_name/version.
> +If version is not set, it will return the package name only.
> +
> +=cut
> +sub as_string {
> +    my $self = shift;
> +
> +    my $string = $self->{package};
> +    if ($self->version) {

Need a defined() again?

> +        $string = $string . "/" . $self->version;
> +    }
> +    return $string;
> +}
> +
> +sub _set_version {

As I said, this should not be required.

> +++ b/scripts/Dpkg/Log/Analyse/Status.pm
> @@ -0,0 +1,235 @@
> +package Dpkg::Log::Analyse::Status;

Again, I don't see the need for this module. It's doing basic queries
on Dpkg::Log in a way that's not generic enough to be suitable for all use
cases that applications that might have.

You should better have good querying capabilities integrated in your 
Dpkg::Log::Status.pm and let the application decide what it wants to
extract and how.

> +package Dpkg::Log::Entry;
> +
> +use strict;
> +use warnings;
> +
> +use overload ( '""' => 'line' );
> +
> +=head1 METHODS
> +
> +=over 4
> +
> +=item $dpkg_log_entry = PACKAGE->new($line, $lineno, %params)
> +
> +Returns a new PACKAGE object.
> +The arguments B<line> and B<lineno> are mandatory.
> +They store the complete line as stored in the log and the line number.

Documentation should not be a template. Use Dpkg::Log::Entry and not
"PACKAGE". People who are writing derived classes know how it works
and will know how to instantiate their own class.

> +Call the parser.
> +
> +The B<time_zone> parameter is optional and specifies in which time zone
> +the dpkg log timestamps are.  If its omitted it will use the default
> +local time zone.
> +Its possible to specify either a DateTime::TimeZone object or a string.
> +=cut

You should fix your design so that the timestamp is parsed by
Dpkg::Log since all lines of dpkg log files will start with a timestamp.
It's also logical since you have put the timestamp attribute in the
base class Dpkg::Log::Entry.

> +sub parse {
> +    my ($self, %params) = @_;
> +    open(my $log_fh, "<", $self->{filename})
> +        or croak("unable to open logfile for reading: $!");
> + 
> +    my $params = {
> +        'from' => $self->{from},
> +        'to' => $self->{to}, 
> +        'time_zone' => $self->{time_zone},
> +        'timestamp_pattern' => $self->{timestamp_pattern},
> +        %params
> +    };
> +
> +    # Determine system timezone
> +    my $tz;
> +    if (ref($params->{time_zone}) and (ref($params->{time_zone}) eq "DateTime::TimeZone")) {
> +        $tz = $params->{time_zone};
> +    } elsif (ref($params->{time_zone})) {
> +        croak "time_zone argument has to be a string or a DateTime::TimeZone object";
> +    } else {
> +        $tz =  DateTime::TimeZone->new( 'name' => $params->{time_zone} );
> +    }
> +    my $ts_parser = DateTime::Format::Strptime->new( 
> +        pattern => $params->{timestamp_pattern},
> +        time_zone => $params->{time_zone}
> +    );
> +
> +    my $lineno = 0;
> +    while  (my $line = <$log_fh>) {
> +        $lineno++;
> +        chomp $line;
> +        next if $line =~ /^$/;
> +

You could put the content of the loop in a parse_line sub.
And it would also call $self->SUPER::parse_line() to let the
parent class extract the timestamp and fill the object that
you pass it.

(And it would return the remaining of the line to be parsed in case of
success, undef otherwise)

> +        if ($entry[2] eq "update-alternatives:") {
> +            next;

update-alternatives no longer writes to dpkg.log.

> +++ b/scripts/Dpkg/Log/Status/Entry.pm
> @@ -0,0 +1,297 @@
> +=head1 NAME
> +
> +Dpkg::Log::Status::Entry - Describe a log entry in a dpkg.log

All the handling of attributes on *::Entry could be generalized
and stuffed into the base class if it's really only a storage
class. Using dedicated methods like $entry->type() doesn't bring anything
compared to $entry->attribute("type").

I'm skipping dpkg-report.pl and the test-suite for now. There enough work
to do on the basic design of the modules already.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Pre-order a copy of the Debian Administrator's Handbook and help
liberate it: http://debian-handbook.info/liberation/




Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Developers <debian-dpkg@lists.debian.org>:
Bug#608930; Package dpkg. (Mon, 30 Jan 2012 12:24:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Patrick Schoenfeld <schoenfeld@debian.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Developers <debian-dpkg@lists.debian.org>. (Mon, 30 Jan 2012 12:24:46 GMT) Full text and rfc822 format available.

Message #110 received at 608930@bugs.debian.org (full text, mbox):

From: Patrick Schoenfeld <schoenfeld@debian.org>
To: Raphael Hertzog <hertzog@debian.org>
Cc: Patrick Schoenfeld <schoenfeld@debian.org>, 608930@bugs.debian.org
Subject: Re: Bug#608930: Dpkg::Log - log file parsing support for dpkg log files
Date: Mon, 30 Jan 2012 13:15:17 +0100
Hi Raphael,

thanks for sharing your opinion with me.

On Wed, Jan 25, 2012 at 11:28:35AM +0100, Raphael Hertzog wrote:
> - the patch is much too big for a simple functionality like this one, you
>   have to cut some code away, there are useless checks (code will end up
>   failing if users submit something that's not expected, you don't have
>   to hardcode checks for all possible mistakes that user might make),
>   there are too many classes and intermediary objects, etc.
>   Do your best to be "concise" and "readable".

I will not comment the "en detail" critique for now, since I figure we
have different design conceivabilities. We should discus about this first.

So lets go:
When I originally designed the library (and dpkg-report) I had the following queries in
mind:

- Which actions happened in a given logfile?
- Which actions (...) in a given timerange?
- Which actions happened to a certain package?
- What is the final state (installed, half-configured) of a package at
  the end of the logfile?
- What is the installed version of a package at the end of the logfile?
- What was the installed version of the papckage at the beginning of the
  logfile?
- What time range does a logfile cover?

When analysing the format of the existing logfiles of systems where I
needed this, I figured that a logfile contains several entries
describing either the status of the dpkg run at a whole, the status of
an affected package or an action happening on a given package.

To answer the queries I figured that I need a lot of information about
different entities, like entries and packages (and conffiles) with
different attributes and different methods to work with. For example,
one who wants to analyse a logfile with different queries - which I can not
know in advance - will want to work with a line-oriented module like
Dpkg::Log::Status, which will return parameterized objects of each line,
while somebody who wants to do common queries (like those above), is
better off with something already doing the tracking work this
requires.

Ulimately I think Dpkg::Log and Dpkg::Log::Analyse are logically for different
tasks (low- and high-levell) and therefore need to be separate.
You seem to have another opinion, but I'm missing a rational.
Just reducing the number of modules does not seem to cut it.

> Again, I don't see the need for this module. It's doing basic queries
> on Dpkg::Log in a way that's not generic enough to be suitable for all use
> cases that applications that might have.

I stronly disagree on this. Yes, it is doing "basic" queries (in the
sense of queries most typical use cases will involve), yet those
queries are not simple (states need to be tracked) and so applications
shouldn't have to-reinvent them everytime.

> > +        if ($entry[2] eq "update-alternatives:") {
> > +            next;
> 
> update-alternatives no longer writes to dpkg.log.

Maybe. But older logfiles exist and might get processed for whatever
reason.

> All the handling of attributes on *::Entry could be generalized
> and stuffed into the base class if it's really only a storage
> class. Using dedicated methods like $entry->type() doesn't bring anything
> compared to $entry->attribute("type").

Well, it at least brings a well-defined API, IMHO.

-Patrick




Information forwarded to debian-bugs-dist@lists.debian.org, Dpkg Developers <debian-dpkg@lists.debian.org>:
Bug#608930; Package dpkg. (Mon, 30 Jan 2012 13:54:05 GMT) Full text and rfc822 format available.

Acknowledgement sent to Raphael Hertzog <hertzog@debian.org>:
Extra info received and forwarded to list. Copy sent to Dpkg Developers <debian-dpkg@lists.debian.org>. (Mon, 30 Jan 2012 13:54:15 GMT) Full text and rfc822 format available.

Message #115 received at 608930@bugs.debian.org (full text, mbox):

From: Raphael Hertzog <hertzog@debian.org>
To: Patrick Schoenfeld <schoenfeld@debian.org>
Cc: 608930@bugs.debian.org
Subject: Re: Bug#608930: Dpkg::Log - log file parsing support for dpkg log files
Date: Mon, 30 Jan 2012 14:50:46 +0100
Hi,

On Mon, 30 Jan 2012, Patrick Schoenfeld wrote:
> So lets go:
> When I originally designed the library (and dpkg-report) I had the following queries in
> mind:
> 
> - Which actions happened in a given logfile?
> - Which actions (...) in a given timerange?
> - Which actions happened to a certain package?
> - What is the final state (installed, half-configured) of a package at
>   the end of the logfile?
> - What is the installed version of a package at the end of the logfile?
> - What was the installed version of the papckage at the beginning of the
>   logfile?
> - What time range does a logfile cover?

This is fine with me. I can understand the need behind all those queries.

> When analysing the format of the existing logfiles of systems where I
> needed this, I figured that a logfile contains several entries
> describing either the status of the dpkg run at a whole, the status of
> an affected package or an action happening on a given package.

True.

> To answer the queries I figured that I need a lot of information about
> different entities, like entries and packages (and conffiles) with
> different attributes and different methods to work with. For example,
> one who wants to analyse a logfile with different queries - which I can not
> know in advance - will want to work with a line-oriented module like
> Dpkg::Log::Status, which will return parameterized objects of each line,
> while somebody who wants to do common queries (like those above), is
> better off with something already doing the tracking work this
> requires.

I'm not sure I follow you here. In all cases, the only thing that I
want is a list of lines and the associated metadata extracted from this
line.

If I use a high level object like Dpkg::Log::Status I want to have access
to many details (action type, package, version, etc.). If I use the
generic parent that Dpkg::Log should be I will have access only to generic
parameters (timestamp, remaining of the line in the log file).

> Ulimately I think Dpkg::Log and Dpkg::Log::Analyse are logically for different
> tasks (low- and high-levell) and therefore need to be separate.

Spell me explicitely out the tasks that you put in low-level and the tasks
that you put in the high level.

Hopefully you will see that reading lines in an array or a hash is not
worth a module that is only doing this.

And you will see that whatever you're doing in Dpkg::Log::Analyse::Foo should
just be methods of Dpkg::Log::Foo. Right now your Dpkg::Log::Analyse::Foo
is doing one time batch processing of Dpkg::Log::Foo and stores results in
attributes.

It means you're computing stuff that the user might not need. And it means
that your results will be stale as soon as the underlying object will be
updated.

With a tighter integration, you'd not have those kind of downsides.

> You seem to have another opinion, but I'm missing a rational.
> Just reducing the number of modules does not seem to cut it.

The rationale is that the split is artificial and only confuses the
design. It found it very difficult to review your work, mostly due to
this and the big size of the patch.

You seem to believe that the queries require a very complex processing and
that it would make sense to split this off. I would answer that you should
improve your parsing and the way that you store your underlying data if
your queries are too difficult to write as simple methods currently.


> > Again, I don't see the need for this module. It's doing basic queries
> > on Dpkg::Log in a way that's not generic enough to be suitable for all use
> > cases that applications that might have.
> 
> I stronly disagree on this. Yes, it is doing "basic" queries (in the
> sense of queries most typical use cases will involve), yet those
> queries are not simple (states need to be tracked) and so applications
> shouldn't have to-reinvent them everytime.

I don't want applications to re-invent those queries. But applications
should call appropriate methods of Dpkg::Log::Status directly.

That said sometimes your API could return a list and you can leave it up to the
application to extract the last item of the list if it only needs that.

For example:
$log->get_status($pkg)
$log->get_last_action($pkg)
$log->get_packages("status=foo")
$log->get_packages("status=foo", "action=remove")
$log->get_newly_installed_packages()
$log->get_upgraded_packages()
etc.

> > > +        if ($entry[2] eq "update-alternatives:") {
> > > +            next;
> > 
> > update-alternatives no longer writes to dpkg.log.
> 
> Maybe. But older logfiles exist and might get processed for whatever
> reason.

Right, then you can add a comment explaining why you ignore it. :)

> > All the handling of attributes on *::Entry could be generalized
> > and stuffed into the base class if it's really only a storage
> > class. Using dedicated methods like $entry->type() doesn't bring anything
> > compared to $entry->attribute("type").
> 
> Well, it at least brings a well-defined API, IMHO.

$entry->attribute("type") is not a well defined API?

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Pre-order a copy of the Debian Administrator's Handbook and help
liberate it: http://debian-handbook.info/liberation/




Send a report that this bug log contains spam.


Debian bug tracking system administrator <owner@bugs.debian.org>. Last modified: Wed Apr 16 13:35:11 2014; Machine Name: beach.debian.org

Debian Bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.