ACK / APPLIED[D/Unstable]: [X, B, C, D, unstable][PATCH] UBUNTU: [Packaging] config-check: Add an include directive
Seth Forshee
seth.forshee at canonical.com
Thu Feb 7 17:28:55 UTC 2019
On Wed, Feb 06, 2019 at 09:41:51AM -0200, Marcelo Henrique Cerri wrote:
>
> On Tue, Feb 05, 2019 at 12:45:23PM -0600, Seth Forshee wrote:
> > On Fri, Feb 01, 2019 at 04:03:15PM -0200, Marcelo Henrique Cerri wrote:
> > > BugLink: http://bugs.launchpad.net/bugs/1752072
> > >
> > > Update the config-check script to support a new include directive, that can
> > > be used to override annotations from another file. For instance, with
> > > this change a custom kernel can include the annotation file from
> > > "debian.master/" and override some of it policies.
> > >
> > > The directive is only available when using the file format 3, that
> > > extends format 2.
> > >
> > > The new directive follows the systax:
> > >
> > > include FILEPATH
> > >
> > > Quotes are also accepted:
> > >
> > > include "FILEPATH"
> > >
> > > `FILENAME` is always relative to the current annotations file location.
> > > So, assuming a custom kernel, the following directive will include the
> > > annotations file from the generic kernel:
> > >
> > > include "../../debian.master/config/annotations"
> > >
> > > To avoid mistakes, any reference to a config in the base annotations
> > > file AFTER the include directive will completely override the references
> > > from the included file.
> > >
> > > For instance, the following:
> > >
> > > # FORMAT: 3
> > > include "../../debian.master/config/annotations"
> > > CONFIG_X note<some note>
> > >
> > > Will cause any line related to CONFIG_X in the included annotations file
> > > to be ignored.
> > >
> > > The patch also includes smalls changes to avoid warning due to duplicate
> > > variable declarations.
> > >
> > > Signed-off-by: Marcelo Henrique Cerri <marcelo.cerri at canonical.com>
> > > ---
> > >
> > > That's a patch to replace the patch "config-check: allow overlay annotations files".
> > >
> > > ---
> > > debian/scripts/config-check | 80 +++++++++++++++++++++++++++----------
> > > 1 file changed, 59 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/debian/scripts/config-check b/debian/scripts/config-check
> > > index 224be080514a..843f5999006b 100755
> > > --- a/debian/scripts/config-check
> > > +++ b/debian/scripts/config-check
> > > @@ -3,6 +3,8 @@
> > > # check-config -- check the current config for issues
> > > #
> > > use strict;
> > > +use File::Basename;
> > > +use File::Spec;
> > >
> > > my $P = 'check-config';
> > >
> > > @@ -13,7 +15,7 @@ if ($ARGV[0] eq '--test') {
> > > die "Usage: $P <config> <arch> <flavour> <commonconfig> <warn-only>\n";
> > > }
> > >
> > > -my ($config, $arch, $flavour, $commonconfig, $warn_only) = @ARGV;
> > > +my ($configfile, $arch, $flavour, $commonconfig, $warn_only) = @ARGV;
> > >
> > > my %values = ();
> > >
> > > @@ -25,8 +27,8 @@ $fail_exit = 0 if ($warn_only eq 'true' || $warn_only eq '1');
> > > my $exit_val = 0;
> > >
> > > # Load up the current configuration values -- FATAL if this fails
> > > -print "$P: $config: loading config\n";
> > > -open(CONFIG, "<$config") || die "$P: $config: open failed -- $! -- aborting\n";
> > > +print "$P: $configfile: loading config\n";
> > > +open(CONFIG, "<$configfile") || die "$P: $configfile: open failed -- $! -- aborting\n";
> > > while (<CONFIG>) {
> > > # Pull out values.
> > > /^#*\s*(CONFIG_\w+)[\s=](.*)$/ or next;
> > > @@ -38,38 +40,74 @@ while (<CONFIG>) {
> > > }
> > > 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>) {
> > > +sub read_annotations {
> > > + my ($filename) = @_;
> > > + my %annot;
> > > + my $form = 1;
> > > + my ($config, $value, $options);
> > > +
> > > + # Keep track of the configs that shouldn't be appended because
> > > + # they were include_annot from another annotations file.
> > > + # That's a hash of undefs, aka a set.
> > > + my %noappend;
> > > +
> > > + print "$P: $filename loading annotations\n";
> > > + open(my $fd, "<$filename") ||
> > > + die "$P: $filename: open failed -- $! -- aborting\n";
> > > + while (<$fd>) {
> > > if (/^# FORMAT: (\S+)/) {
> > > - die "$P: $1: unknown annotations format\n" if ($1 != 2);
> > > - $form = $1;
> > > + die "$P: $1: unknown annotations format\n" if ($1 != 2 && $1 != 3);
> > > + $form = $1;
> > > + }
> > > +
> > > + # Format #3 adds the include directive on top of format #2:
> > > + if ($form == 3 && /^\s*include(\s|$)/) {
> > > + # Include quoted or unquoted files:
> > > + if (/^\s*include\s+"(.*)"\s*$/ || /^\s*include\s+(.*)$/) {
> > > + # The include is relative to the current file
> > > + my $include_filename = File::Spec->join(dirname($filename), $1);
> > > + # Append the include files
> > > + my %include_annot = read_annotations($include_filename);
> > > + %annot = ( %annot, %include_annot );
> > > + # And marked them to not be appended:
> > > + my %included_noappend;
> > > + # Discard the values and keep only the keys
> > > + @included_noappend{keys %include_annot} = ();
> > > + %noappend = ( %noappend, %included_noappend );
> > > + next;
> > > + } else {
> > > + die "$P: Invalid include: $_";
> > > + }
> >
> > This handles a definition for a config after an include with that
> > config. I don't think it handles an include after the definition or two
> > includes with a duplicate definition, does it?
>
> Yes. It's sensitive to the order of definitions and includes. You
> could use it, for instance, to provide "default" values (although that
> might not be that useful). So in this example:
>
> # FORMAT: 3
> CONFIG_X policy<{'amd64': 'y'}>
> CONFIG_X mark<ENFORCE> note<reason>
> include "../../debian.master/config/annotations"
>
> If CONFIG_X appears in the included file, everything in the root
> annotations file for CONFIG_X will be ignore. And if the included file
> does not mention CONFIG_X, 'y' will be enforced for CONFIG_X on amd64.
>
> The same is true for multiple include directives. If more than one
> included file defines rules for the same config, the rules in the last
> included file will be used, and everything from the previous includes
> for that config will be ignored.
>
> I explicitly added the restriction that you can't split the definition
> of a single config across several files. For example, you can't have
> the mark for a config in one file and the policy in another.
Okay, my perl is weak so I had to go read about hashes a bit to find out
what happens with this line:
%annot = ( %annot, %include_annot );
if an option already in annot is also in include_annot. It seems the
resulting value for that key will be the one from include_annot, so I
see now that this does work sensibly.
Acked-by: Seth Forshee <seth.forshee at canonical.com>
Applied to disco/master-next and unstable/master, thanks!
>
> >
> > This may not be a show stopper, and I'm sure it doesn't matter for the
> > current use case. I just wanted to make sure I understand what works and
> > what doesn't.
> >
> > > }
> > >
> > > /^#/ && next;
> > > chomp;
> > > /^$/ && next;
> > > -
> > > /^CONFIG_/ || next;
> > >
> > > if ($form == 1) {
> > > - ($config, $value, $options) = split(' ', $_, 3);
> > > - } elsif ($form == 2) {
> > > - ($config, $options) = split(' ', $_, 2);
> > > + ($config, $value, $options) = split(' ', $_, 3);
> > > + } elsif ($form >= 2) {
> > > + ($config, $options) = split(' ', $_, 2);
> > > }
> > >
> > > + if (exists $noappend{$config}) {
> > > + delete $annot{$config};
> > > + delete $noappend{$config};
> > > + }
> > > $annot{$config} = $annot{$config} . ' ' . $options;
> > > + }
> > > + close($fd);
> > > + return %annot;
> > > }
> > > -close(ANNOTATIONS);
> > >
> > > -my $config;
> > > +# ANNOTATIONS: check any annotations marked for enforcement
> > > +my $annotations = "$commonconfig/annotations";
> > > +my %annot = read_annotations($annotations);
> > > +
> > > +my $pass = 0;
> > > +my $total = 0;
> > > +my ($config, $value, $options, $option, $check, $policy);
> > > for $config (keys %annot) {
> > > $check = 0;
> > > $options = $annot{$config};
> > > --
> > > 2.17.1
> > >
> > >
> > > --
> > > kernel-team mailing list
> > > kernel-team at lists.ubuntu.com
> > > https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
>
> --
> Regards,
> Marcelo
More information about the kernel-team
mailing list