[elbe-devel] [PATCH] preprocess: always set --variant=default

[Manuel Traut]

Hi,

On 16:50 Tue 04 Dec     , Manuel Traut wrote:
> currently is not posible to set a default value for an element, e.g.
> 
> <hostname>test</hostname>
> <hostname variant="ng">test-ng</hostname>
> 
> will result in
> 
> <hostname>test</hostname>
> 
> if no --variant parameter was given, but it will result in
> 
> <hostname>test</hostname>
> <hostname variant="ng">test-ng</hostname>
> 
> if --variant="ng" was specified.
> 
> With this change elbe uses --variant=default as default. So
> 
> <hostname variant="default">test</hostname>
> <hostname variant="ng">test-ng</hostname>
> 
> will work for both calls described above.

while trying to cleanup the example directory using variants
(with this patch) i found a big limitation:

<buildtype variant="default">amd64</buildtype>
<buildtype variant="arm">armhf</buildtype>
..
<hostname variant="default">desktop</hostname>
<hostname variant="specialname">specialname</hostname>
..

If i now specify --variant=arm i get an invalid XML because
hostname is not set. If a third OR element will be used it
becomes even worse..

I have several ideas to solve this:

a) Specifying a list of variants as default in XML
--------------------------------------------------

<project default="amd64,desktop">
..
<buildtype variant="amd64">amd64</buildtype>
<buildtype variant="arm">armhf</buildtype>
..
<hostname variant="desktop">desktop</hostname>
<hostname variant="specialname">specialname</hostname>
..

Pro: Naming in XML is more clear
Con: For building an ARM image, deeper knowledge of the XML is needed.

b) Manually defining sets of allowed variants in XML
----------------------------------------------------

Instead of a list of variants only a single set could be given per cmd-line
to elbe-preprocess:

<project>
	<sets>
		<set name='default'>amd64,desktop</set>
		<set name='arm-desktop'>arm,desktop</set>
		<set name='amd64-specialname'>amd64,specialname</set>
	</sets>
	..
	<buildtype variant="amd64">amd64</buildtype>
	<buildtype variant="arm">armhf</buildtype>
	..
	<hostname variant="desktop">desktop</hostname>
	<hostname variant="specialname">specialname</hostname>
	..

Pro: Invalid combinations of variants like amd64,arm can be forbidden if only
       sets can be specified on the commandline.

     A default set of variants can be specified

Con: All allowed combinations need to be listed

     The same could be done with the current implementation:
        <buildtype variant="amd64-desktop,amd64-specialname">amd64</buildtype>
        <buildtype variant="arm-desktop">armhf</buildtype>
        ..
        <hostname variant="amd64-desktop,arm-desktop">desktop</hostname>
        <hostname variant="amd64-specialname">specialname</hostname>
       However it becomes hard to read.

c) Defining allowed or forbidden combinations and auto generate sets
--------------------------------------------------------------------

<project>
	<sets default=amd64-desktop>
		<allowed>amd64,desktop</allowed>
		<allowed>amd64,specialname</allowed>
		<allowed>arm,desktop</allowed>
	</sets>

<project>
	<sets default=amd64-desktop>
		<forbidden>arm,amd64</forbidden>
	</sets>


This could be used to:
 * Provide the default set to build
 * Specify all allowed sets manually
 * Calculate all allowed sets by listing forbidden combinations




However all ideas i currently have make variant handling more complex than it
is at the moment. I'm still not sure, if we should just keep the implementation
as is in devel/elbe-3.0 atm.

What do you think?

  Manu


> Signed-off-by: Manuel Traut <manut at linutronix.de>
> ---
>  docs/elbe-preprocess.txt        | 3 +++
>  elbepack/commands/preprocess.py | 6 ++----
>  elbepack/xmlpreprocess.py       | 7 ++-----
>  3 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/docs/elbe-preprocess.txt b/docs/elbe-preprocess.txt
> index 1a562398..17c25549 100644
> --- a/docs/elbe-preprocess.txt
> +++ b/docs/elbe-preprocess.txt
> @@ -23,6 +23,9 @@ only be used if 'elbe preprocess' is called with --variant=audio or
>  --variant=video or --variant=audio,video. If no --variant is given, the tag
>  will be dropped.
>  
> +A tag with variant="default" is only used if no --variant parameter was
> +specified.
> +
>  OPTIONS
>  -------
>  
> diff --git a/elbepack/commands/preprocess.py b/elbepack/commands/preprocess.py
> index 44e298c4..18f56cdf 100644
> --- a/elbepack/commands/preprocess.py
> +++ b/elbepack/commands/preprocess.py
> @@ -14,7 +14,7 @@ from elbepack.xmlpreprocess import XMLPreprocessError, xmlpreprocess
>  
>  def add_pass_through_options(oparser):
>      oparser.add_option("-v", "--variants", dest="variant",
> -                       default=None,
> +                       default="default",
>                         help="enable only tags with empty or given variant")
>  
>  
> @@ -35,9 +35,7 @@ def run_command(argv):
>          print("%s doesn't exist" % args[0], file=sys.stderr)
>          sys.exit(20)
>  
> -    variants = []
> -    if opt.variant:
> -        variants = opt.variant.split(',')
> +    variants = opt.variant.split(',')
>  
>      try:
>          xmlpreprocess(args[0], opt.output, variants)
> diff --git a/elbepack/xmlpreprocess.py b/elbepack/xmlpreprocess.py
> index 2a20445a..f2966357 100644
> --- a/elbepack/xmlpreprocess.py
> +++ b/elbepack/xmlpreprocess.py
> @@ -29,16 +29,13 @@ class XMLPreprocessError(Exception):
>      pass
>  
>  
> -def xmlpreprocess(fname, output, variants=None):
> +def xmlpreprocess(fname, output, variants):
>  
>      # pylint: disable=too-many-locals
>      # pylint: disable=too-many-branches
>  
>      # first convert variants to a set
> -    if not variants:
> -        variants = set([])
> -    else:
> -        variants = set(variants)
> +    variants = set(variants)
>  
>      schema_file = "https://www.linutronix.de/projects/Elbe/dbsfed.xsd"
>      parser = XMLParser(huge_tree=True)
> -- 
> 2.19.2
>