Re: .changes files as first-class objects
On Fri, 2010-01-15 at 01:48 -0600, Raphael Geissert wrote:
> > diff --git a/lib/Lab.pm b/lib/Lab.pm
> > index cc319ce..565d38d 100644
> > --- a/lib/Lab.pm
> > +++ b/lib/Lab.pm
> > @@ -50,6 +50,7 @@ sub is_lab {
> > return -d "$self->{dir}/binary"
> > && -d "$self->{dir}/udeb"
> > && -d "$self->{dir}/source"
> > + && -d "$self->{dir}/changes"
> > && -d "$self->{dir}/info";
> > }
> >
> Although I understand the reason for this change there's a subtle problem
> with it: the frontend checks the return status of is_lab() and dies on
> failure.
> This would make lintian complain about any existing lab and require either
> the creation of $labdir/changes by hand or the complete recreation of the
> lab. Although the latter is pretty much what happens because of the lab
> version bump, the version change is handled gracefully.
>
> So, I think that change should not be made.
Having is_lab() return success for something that doesn't match the
current lab format seemed wrong. I'm happy to skip this change if the
consensus is that it shouldn't be included, as we've bumped the format
version at the same time.
> > There are a few areas I think are worth highlighting:
> >
> > * The functionality of the malformed-changes-file test has changed.
> > Although a "malformed" .changes file (i.e. one missing a Format field)
> > will cause the remaining tests for changes files to be skipped, it is
> > possible that the file was sufficiently well-formed for Lintian to find
> > a Files stanza within it, in which case any properly formatted entries
> > therein will already have been scheduled.
>
> I'm not sure there's really a point on doing that; but what I'm more
> concerned about is the amount of code duplication used to parse the control
> files in multiple places.
> I guess we could wait until the .dsc-related changes are made before
> refactoring anything.
I wasn't entirely clear which part of the above you meant by "that". If
it was the scanning of files included in a .changes file despite it not
having a valid Format field, then it's a side-effect of other changes
rather than a change in itself.
Previously the frontend would read the .changes directly and only
schedule the contained files if the .changes passed basic validation
(which basically amounted to "has a Format field"). With .changes being
promoted to checkable objects in their own right, so long as the file
can be parsed and files found listed in it, they will be scheduled at
the same time as the .changes file itself.
If that's not what you meant, then apologies for the waffle and please
let me know what you did mean. ;-)
> Other than the is_lab() issue I think it's good.
Thanks for the review.
Adam
Reply to: