Re: Refactoring unpack/list-${type}pkg and Read_pkglists
On 2011-07-13 01:07, Russ Allbery wrote:
> Niels Thykier <niels@thykier.net> writes:
>
> [...]
>> Looking at the parser (and unpack/list-* scripts) I see that aside from
>> the header, the format of the binary and the udeb looks identical. Do
>> anyone see a reason /not/ to merge the parser/writers for these two formats?
>> If we merge them I can almost throw out list-udeb and one third of
>> Read_pkglists, so I am a bit biased here.
>
> Yup, they should be identical formats.
>
Done, I let the Read_pkglists::read_bin_list accept the udeb header for
now (should save an archive-wide recheck of all udebs).
>> Secondly... Are there any use for the "architecture" and the "files"
>> (not to be confused with the "file") field in the source package?
>> Architecture does not appear to be referenced and the "files" field
>> appears to be corrupted in all existing files, so I doubt we use it.
>
> Yeah, files looks like it's not used. The goal was originally to
> accumulate all of the files that a *.dsc file refers to, but it looks like
> we're getting the size instead. My guess is that there's no point to that
> field.
>
> I don't think either architecture or standards-version are used. I
> suspect we could drop both of those unless we wanted to generate reports
> on the architecture and standards version used in the archive for some
> reason. (And we could always add them back in later.)
>
I am not sure architecture would make sense for that, since it is in the
source file. Especially because we already extract the field (like the
others) directly from the dsc in collection/fields.
That being said, I am not objecting to getting rid of
collection/fields and just caching the fields in Lintian::Collect. Half
the fields are already cached in three other different modules.
>> Thirdly, html_reports appears to be referring to a "maintainer" field in
>> the binary/udeb files (if it cannot find the source). As far as I can
>> tell, this will always fail since the field not present. That being
>> said, I might have overlooked something.
>
> I think this is left over from an earlier refactoring. I think there may
> have been a maintainer field there at some point in the past. This can go
> now; it's just a complicated way of setting it to undef.
>
Refactored it to a simple way of setting it to undef. :P
>> I see there is a reporting entry in private/TODO; are/were there any
>> specific plans/ideas not written down in that TODO? Personally I think
>> this could go hand in hand with providing a mirror-sync possibility for
>> (a future) liblintian-perl.
>
> The only other thing that occurs to me is that the current templating done
> for html_report is really not very good. I tried to keep the HTML page
> generation logic separate, but so much code had to go into the template
> that they're almost hard-to-read Perl modules at this point. I don't know
> if a different templating language like Template Toolkit might let more of
> the logic move into either the templating language or the actual page.
>
I think it might help to change to Template toolkit/libtemplate-perl. I
think its simpler language would force us to move the complicated code
out of the template itself. As an added bonus it seems to be handling
the text parts we have to generate ourselves[1].
I don't suppose we have a sandbox setup we can use for testing the code?
[1]
http://template-toolkit.org/docs/tutorial/Datafile.html#section_Producing_XML
>> Finally, did I miss something or are there any "magic" parts I should be
>> aware of, if I start messing with this (other than if I break it, I
>> break the website)?
>
> The way the config file is parsed is weird and annoying, and it would be
> nice to integrate that better with the way we're doing configuration in
> general for Lintian.
>
The require './config' part? Sure; I was already considering to make a
module for the config parsing in frontend/lintian (LINTIAN_ROOT is
always set at that point anyway)
~Niels
Reply to: