[X, B, C, D, unstable][PATCH] UBUNTU: [Packaging] config-check: allow overlay annotations files

[Thadeu Lima de Souza Cascardo]

On Thu, Jan 31, 2019 at 05:10:25PM +0000, Andy Whitcroft wrote:
> On Thu, Jan 31, 2019 at 10:17:36AM -0600, Seth Forshee wrote:
> > On Thu, Jan 31, 2019 at 01:46:17PM -0200, Marcelo Henrique Cerri wrote:
> > > BugLink: http://bugs.launchpad.net/bugs/1752072
> > > 
> > > Update the config-check script to parse additional annotations files
> > > that can overlay the original annotations on a config basis. The overlay
> > > files are read from "$DEBIAN/config/*.annotations".
> > > 
> > > Since the overlay works on a config basis, any mention to a config in
> > > the overlay file will cause the entry for that config on the original
> > > file to be ignore.
> > > 
> > > For instance, the following line in the overlay file:
> > > 
> > > CONFIG_X note<some note>
> > > 
> > > Will cause any line related to CONFIG_X in the original annotations file
> > > to be ignored.
> > > 
> > > Signed-off-by: Marcelo Henrique Cerri <marcelo.cerri at canonical.com>
> > > ---
> > > 
> > > I intend to use this patch to better handle config annotations for
> > > custom, and specially cloud, kernels. That way we can keep
> > > "debian.$BRANCH/config/annotations" as a symlink to the base kernel
> > > (usually "debian.master/config/annotations") and keep the
> > > customizations for the custom kernel on a separate file (ie:
> > > "debian.$BRANCH/config/$BRANCH.annotations").
> > > 
> > > With this patch applied I'm planning to extract all the configs
> > > requirements for the cloud kernel on a separate file. That should help
> > > us to have a clear perspect of the the customizations made to a kernel.
> > > 
> > > ---
> > >  debian/scripts/config-check | 45 ++++++++++++++++++++-----------------
> > >  1 file changed, 25 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/debian/scripts/config-check b/debian/scripts/config-check
> > > index 07958556e58e..e28d28153c4b 100755
> > > --- a/debian/scripts/config-check
> > > +++ b/debian/scripts/config-check
> > > @@ -41,33 +41,38 @@ close(CONFIG);
> > >  # ANNOTATIONS: check any annotations marked for enforcement
> > >  my $pass = 0;
> > >  my $total = 0;
> > > -my $annotations = "$commonconfig/annotations";
> > >  my ($config, $value, $options, $option, $value, $check, $policy);
> > > -print "$P: $annotations loading annotations\n";
> > > +
> > >  my %annot;
> > > -my $form = 1;
> > > -open(ANNOTATIONS, "<$annotations") || die "$P: $annotations: open failed -- $! -- aborting\n";
> > > -while (<ANNOTATIONS>) {
> > > -	if (/^# FORMAT: (\S+)/) {
> > > -		die "$P: $1: unknown annotations format\n" if ($1 != 2);
> > > -		$form = $1;
> > > -	}
> > > +my @annotations_list = ("$commonconfig/annotations", glob("$commonconfig/*.annotations"));
> > 
> > I don't think we should glob here, then we're creating the ability to
> > layer multiple annotations overlays but without some good criteria for
> > which ones take precedence (I assume it would turn out to be ascii sort
> > order).
> 
> Which is likely an order affected by LANG= settings; which sounds all
> rather unreliable.
> 
> > 
> > I'd prefer if this were maybe $branch.annotations, or even just
> > something like annotations.overlay. Otherwise I think this looks fine.
> 
> Is there some reason it could not be in $DEBIAN so it is branch
> specific?  Or are we hoping to check it into the master branch?
> 
> -apw

Or if he is planning on always using the debian.master version of it, why not
have an "include" directive and include it, maybe with a format=3?

Cascardo.