Re: .changes files as first-class objects
Hi Adam :),
Adam D. Barratt wrote:
> Hi,
>
> Apologies for not noting the fact in private/TODO (I had intended to,
> but it must have slipped my mind :-/) but I've been having a look at the
> changes required to promote .changes files to first-class checkable
> objects; it turned out to be relatively easy to achieve.
I guess we all need to get used to updating that file, so you should not
worry ;)
> 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.
> 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.
>
> * The "package" name for .changes files is now the filename with the
> trailing ".changes" removed. This is consistent with the current
> functionality for .changes files, but does produce some odd results (as
> demonstrated in a later point).
>
> I debated using the Source field from the .changes instead which would
> also work but means we need an extra piece of special-case code in
> Schedule.pm for files without a Source field.
>
I think using the file name is fine, for the same reasons.
> * The output format for changes tags has changed subtly as it is no
> longer special cased; by way of an example:
>
> -E: changed-by-no-name.changes: changed-by-name-missing
> someone@example.com +E: changed-by-no-name
> changes: changed-by-name-missing
> someone@example.com
>
> I think the new format is correct and is what should have been used all
> along.
Agreed.
Other than the is_lab() issue I think it's good.
Cheers,
--
Raphael Geissert - Debian Developer
www.debian.org - get.debian.net
Reply to: