[PATCH 6/6] waf: Add configurations with different modules.

Christian Mauderer christian.mauderer at embedded-brains.de
Mon Apr 9 08:03:15 UTC 2018


Am 09.04.2018 um 03:50 schrieb Chris Johns:
> On 06/04/2018 22:26, Christian Mauderer wrote:
>> This allows multiple build configurations (buildsets) with different
>> modules enabled.
>>
>> Update #3351
>> ---
>>  CONTRIBUTING.md      |   7 ++-
>>  README.waf           |   3 +-
>>  builder.py           |  13 ++---
>>  buildset/default.ini |  58 ++++++++++++++++++++++
>>  buildset/sample.ini  |  10 ++++
>>  libbsd.py            |   2 +-
>>  rtems_waf            |   2 +-
>>  waf_libbsd.py        |   9 ----
>>  wscript              | 136 +++++++++++++++++++++++++++++++++++++++++++++------
>>  9 files changed, 203 insertions(+), 37 deletions(-)
>>  create mode 100644 buildset/default.ini
>>  create mode 100644 buildset/sample.ini
>>
>> diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
>> index eddefe98..9e6ee83c 100644
>> --- a/CONTRIBUTING.md
>> +++ b/CONTRIBUTING.md
>> @@ -67,8 +67,7 @@ freebsd-to-rtems.py [args]
>>  ```
>>  
>>  In its default mode of operation, freebsd-to-rtems.py is used to copy code
>> -from FreeBSD to the rtems-libbsd tree and perform transformations.  In forward
>> -mode, the script may be requested to just generate the Waf script.
>> +from FreeBSD to the rtems-libbsd tree and perform transformations.
>>  
>>  In *reverse mode*, this script undoes those transformations and copies
>>  the source code back to the *master* FreeBSD tree. This allows us to do
>> @@ -126,9 +125,9 @@ How to import code from FreeBSD
>>  * Run `./freebsd-to-rtems.py -R`
>>  * Run `./freebsd-to-rtems.py`
>>  * Run `git status` and make sure your working directory is clean.  If you see modified files, then the `freebsd-to-rtems.py` script needs to be fixed first.
>> -* Add the files to import to `libbsd.py`.
>> +* Add the files to import to `libbsd.py` and your intended build set (for example `buildset/default.ini`.
>>  * Run `./freebsd-to-rtems.py`
>> -* Immediately check in the imported files without the changes to `libbsd_waf.py`.  Do not touch the imported files yourself at this point.
>> +* Immediately check in the imported files without the changes to `libbsd.py` and the buildsets.  Do not touch the imported files yourself at this point.
>>  * Port the imported files to RTEMS.  See 'Rules for Modifying FreeBSD Source'.
>>  * Add a test to the testsuite if possible.
>>  * Run `./create-kernel-namespace.sh` if you imported kernel space headers.  Add only your new defines via `git add -p rtemsbsd/include/machine/rtems-bsd-kernel-namespace.h`.
>> diff --git a/README.waf b/README.waf
>> index dddc04a4..485a1899 100644
>> --- a/README.waf
>> +++ b/README.waf
>> @@ -73,7 +73,8 @@ cd rtems-libbsd
>>  git submodule init
>>  git submodule update rtems_waf
>>  waf configure --prefix="$sandbox/rtems-4.12" \
>> -  --rtems-bsps=arm/xilinx_zynq_a9_qemu
>> +  --rtems-bsps=arm/xilinx_zynq_a9_qemu \
>> +  --buildset=buildset/default.ini
> 
> Is this the default and so if you do not add a --buildset option you get this?
> 
> It might pay to highlight this if it is the case.

Yes, you are right. That is the default. I'll add a note regarding that.

> 
>>  waf
>>  waf install
>>  qemu-system-arm -no-reboot -serial null -serial mon:stdio -net none \
>> diff --git a/builder.py b/builder.py
>> index bb5791a8..c968e05c 100755
>> --- a/builder.py
>> +++ b/builder.py
>> @@ -44,6 +44,7 @@ import getopt
>>  import filecmp
>>  import difflib
>>  import codecs
>> +import copy
>>  
>>  #
>>  # Global controls.
>> @@ -487,15 +488,11 @@ class Module(object):
>>      def __init__(self, manager, name, enabled = True):
>>          self.manager = manager
>>          self.name = name
>> -        self.enabled = enabled
>>          self.conditionalOn = "none"
>>          self.files = []
>>          self.cpuDependentSourceFiles = {}
>>          self.dependencies = []
>>  
>> -    def isEnabled(self):
>> -        return self.enabled
>> -
>>      def initCPUDependencies(self, cpu):
>>          if cpu not in self.cpuDependentSourceFiles:
>>              self.cpuDependentSourceFiles[cpu] = []
>> @@ -664,15 +661,19 @@ class ModuleManager(object):
>>              self.modules[m].processSource(direction)
>>  
>>      def setConfiguration(self, config):
>> -        self.configuration = config
>> +        self.configuration = copy.deepcopy(config)
>>  
>>      def getConfiguration(self):
>>          return self.configuration
>>  
>> +    def updateConfiguration(self, config):
>> +        self.configuration.update(config)
>> +
>>      def setModuleConfigiuration(self):
>>          mods = sorted(self.modules.keys())
>>          self.configuration['modules'] = mods
>> -        self.configuration['modules-enabled'] = [m for m in mods if self.modules[m].isEnabled()]
>> +        # Enabled modules are overwritten by config file. Default to all.
>> +        self.configuration['modules-enabled'] = mods
>>  
>>      def generateBuild(self, only_enabled=True):
>>          modules_to_process = self.getEnabledModules()
>> diff --git a/buildset/default.ini b/buildset/default.ini
>> new file mode 100644
>> index 00000000..15b57df4
>> --- /dev/null
>> +++ b/buildset/default.ini
>> @@ -0,0 +1,58 @@
>> +#
>> +# Default configuration.
>> +#
>> +
>> +[general]
>> +name = default
>> +
>> +[modules-enabled]
> 
> Is there a 'modules-disabled' section? I see you have an integer bool in the
> list below so that means we can disable a module in the enable section which is
> confusing. Maybe this should be called 'modules'?

No there is not modules-disabled section. I used boolean values for the
modules so that I can disable them in an extended configuration for
example with

dev_nic_e1000 = 0

So yes, modules is better in that context. I'll change it.

> 
>> +altq = 1
> 
> Can this please be ''enable/disable'?

The python configparser supports the following by default:

"1", "yes", "true", and "on" for true
"0", "no", "false", and "off" for false

So if it is OK for you, I would prefer one of these over introducing a
new "enable" / "disable" that I would have to handle myself. How about
using "on" and "off"?

> 
>> +base = 1
>> +cam = 1
>> +contrib_expat = 1
>> +contrib_libpcap = 1
>> +crypto = 1
>> +crypto_openssl = 1
>> +dev_input = 1
>> +dev_net = 1
>> +dev_nic = 1
>> +dev_nic_broadcomm = 1
>> +dev_nic_dc = 1
>> +dev_nic_e1000 = 1
>> +dev_nic_fxp = 1
>> +dev_nic_re = 1
>> +dev_nic_smc = 1
>> +dev_usb = 1
>> +dev_usb_controller = 1
>> +dev_usb_controller_bbb = 1
>> +dev_usb_input = 1
>> +dev_usb_net = 1
>> +dev_usb_quirk = 1
>> +dev_usb_serial = 1
>> +dev_usb_storage = 1
>> +dev_usb_wlan = 1
>> +dev_wlan_rtwn = 1
>> +dhcpcd = 1
>> +dpaa = 1
>> +evdev = 1
>> +fdt = 1
>> +in_cksum = 1
>> +ipfw = 1
>> +mdnsresponder = 1
>> +mghttpd = 1
>> +mmc = 1
>> +mmc_ti = 1
>> +net = 1
>> +net80211 = 1
>> +netinet = 1
>> +netinet6 = 1
>> +opencrypto = 1
>> +pci = 1
>> +pf = 1
>> +rtems = 1
>> +tests = 1
>> +tty = 1
>> +user_space = 1
>> +user_space_wlanstats = 1
>> +usr_sbin_tcpdump = 1
>> +usr_sbin_wpa_supplicant = 1
>> diff --git a/buildset/sample.ini b/buildset/sample.ini
>> new file mode 100644
>> index 00000000..f30c1491
>> --- /dev/null
>> +++ b/buildset/sample.ini
>> @@ -0,0 +1,10 @@
>> +#
>> +# Currently this is mostly a sample configuration.
>> +#
>> +
>> +[general]
>> +name = sample
>> +extends = default.ini
>> +
>> +[modules-enabled]
>> +dev_nic_broadcomm = 0
>> diff --git a/libbsd.py b/libbsd.py
>> index e3c0a3ac..a256594e 100644
>> --- a/libbsd.py
>> +++ b/libbsd.py
>> @@ -1996,7 +1996,7 @@ class netinet6(builder.Module):
>>  class netipsec(builder.Module):
>>  
>>      def __init__(self, manager):
>> -        super(netipsec, self).__init__(manager, type(self).__name__, enabled = False)
>> +        super(netipsec, self).__init__(manager, type(self).__name__)
>>  
>>      def generate(self):
>>          mm = self.manager
>> diff --git a/rtems_waf b/rtems_waf
>> index d793097d..4ef2f41e 160000
>> --- a/rtems_waf
>> +++ b/rtems_waf
>> @@ -1 +1 @@
>> -Subproject commit d793097d2032ca0ab2d5cf93fa84730865db6c03
>> +Subproject commit 4ef2f41eb7a6b0fd84791eafff9062da1a9d94c4
>> diff --git a/waf_libbsd.py b/waf_libbsd.py
>> index ae8a1ea0..fffdf9fe 100644
>> --- a/waf_libbsd.py
>> +++ b/waf_libbsd.py
>> @@ -295,12 +295,6 @@ class Builder(builder.ModuleManager):
>>              import pprint
>>              pprint.pprint(self.data)
>>  
>> -    def init(self, ctx):
>> -        pass
>> -
>> -    def options(self, opt):
>> -        pass
>> -
> 
> Why remove?
> 

Basically these are not really usable. Waf calls the funtions in this order:

- options
- init
- configure or build

During the options phase, it's not clear which variants are used. The
option parser is just set up at this point. Therefore I can't create the
builders during that phase and I can't use a options function of these
builders. It would be possible to use a @staticmethod for that but then
we would just have an extra waf_libbsd.Builder.options() call which
doesn't really seem useful.

It's slightly different for init: If you take a look at the wscript
changes, I create the builders during that phase. That replaces the
previously used create_builder() function that checked whether the
builder (back then it was only one) is already created and creates it if
not. But with that construction you can just use the standard python
__init__ to do the same what an extra init would do (except for the ctx
but these functionalities). If you really like that function, I think we
could add it again inside of the bsp_init. But in that case, I would
call it bsp_init instead of init. But I don't really see the point of
adding these functions as empty ones if we don't have a use case for them.

>>      def bsp_configure(self, conf, arch_bsp):
>>          if 'configure' in self.data:
>>              for cfg in self.data['configure']:
>> @@ -310,9 +304,6 @@ class Builder(builder.ModuleManager):
>>                                 includes = conf.env.IFLAGS,
>>                                 mandatory = False)
>>  
>> -    def configure(self, conf):
>> -        pass
>> -
> 
> Also why?
> 

Basically this function is a little redundant. We already have one
callback via the bsp_configure().

To be honest: I'm not even sure whether the object oriented approach
with a Builer class is even the right one for the build function:
Basically waf passes the information about how to build one set via the
env which is saved in the build/c4che/arm-rtems5-atsamv_cache.py and
similar scripts. So maybe it would be the more waf-style way to pass the
two parts that we currently have class specific via the env. The two
parts in build are:

    config = self.getConfiguration()

and

    self.data

With that the build would be a standalone function. That leaves the
bsp_configure in that class (which again uses only self.data) and the
generate. In the other patch we already started a discussion whether
generate would be better placed in the general builder.py instead of the
waf_libbsd.py.

Currently I more or less search the right builder out of the builders
list depending on the env and than use the build function of that
object. So the information which data structures are the right ones is
already passed via env.

>>      def build(self, bld):
>>          # This is only necessary for build. But it depends on the waflib which
>>          # is only available in a waf context. But we need the module manager for
>> diff --git a/wscript b/wscript
>> index 6c063965..1dacb3b2 100644
>> --- a/wscript
>> +++ b/wscript
>> @@ -44,23 +44,122 @@ except:
>>  #import libbsd_waf
>>  import libbsd
>>  import waf_libbsd
>> +import os.path
>> +import runpy
>> +import sys
>> +try:
>> +    import configparser
>> +except ImportError:
>> +    import ConfigParser as configparser
>> +import waflib.Options
>> +
>> +builders = {}
>> +
>> +BUILDSET_DIR = "buildset"
>> +BUILDSET_DEFAULT = "buildset/default.ini"
>> +
>> +def load_ini(conf, f):
>> +    ini = configparser.ConfigParser()
>> +    ini.read(f)
>> +    if not ini.has_section('general'):
>> +        conf.fatal("'{}' is missing a general section.".format(f))
>> +    if not ini.has_option('general', 'name'):
>> +        conf.fatal("'{}' is missing a general/name.".format(f))
>> +    if ini.has_option('general', 'extends'):
>> +        extends = ini.get('general', 'extends')
>> +        extendfile = None
>> +        basepath = os.path.dirname(f)
>> +        if os.path.isfile(os.path.join(basepath, extends)):
>> +            extendfile = extends
>> +        elif os.path.isfile(os.path.join(BUILDSET_DIR, extends)):
>> +            extendfile = os.path.join(BUILDSET_DIR, extends)
>> +        else:
>> +            conf.fatal("'{}': Invalid file given for general/extends:'{}'"
>> +                .format(f, extends))
>> +        base = load_ini(conf, extendfile)
>> +        for s in ini.sections():
>> +            if not base.has_section(s):
>> +                base.add_section(s)
>> +            for o in ini.options(s):
>> +                val = ini.get(s, o)
>> +                base.set(s, o, val)
>> +        ini = base
>> +    return ini
>> +
>> +def load_config(conf, f):
>> +    ini = load_ini(conf, f)
>> +    config = {}
>> +
>> +    config['name'] = ini.get('general', 'name')
>>  
>> -builder = None
>> +    config['modules-enabled'] = []
>> +    mods = []
>> +    if ini.has_section('modules-enabled'):
>> +        mods = ini.options('modules-enabled')
>> +    for mod in mods:
>> +        if ini.getboolean('modules-enabled', mod):
>> +            config['modules-enabled'].append(mod)
>> +    return config
>>  
>> -def create_builder():
>> -    global builder
>> -    if builder is None:
>> +def check_buildsets(ctx, buildset_opt):
>> +    buildsets = []
>> +    if buildset_opt == []:
>> +        buildset_opt.append(BUILDSET_DEFAULT)
>> +    for bs in buildset_opt:
>> +        if os.path.isdir(bs):
>> +            for f in os.listdir(bs):
>> +                if f[-4:] == ".ini":
>> +                    buildsets += [os.path.join(bs,f)]
>> +        else:
>> +            for f in bs.split(','):
>> +                buildsets += [f]
>> +    return buildsets
>> +
> 
> Ah ok. Maybe this should be in a separate module with a class so the source
> management and waf build can both share?

The source management shouldn't need to know the ini files. Copying the
sources between FreeBSD and RTEMS libbsd should work on _all_ sources
and not only the enabled ones.

> 
> We are starting to collect a few python modules so I wonder if they should be
> moved into something like 'buildsys', what do you think?

It's possible but I currently don't see any functions in wscript that
would be shared with some other script in libbsd. So it's not really
necessary at that point in time.

> 
>> +def bsp_init(ctx, env, contexts):
>> +    buildsets = []
>> +    if 'buildset' in env.options:
>> +        buildsets = check_buildsets(ctx, env.options['buildset'])
>> +    else:
>> +        buildsets = check_buildsets(ctx, [BUILDSET_DEFAULT])
>> +    bsnames = []
>> +
>> +    global builders
>> +
>> +    for bs in buildsets:
>> +        # create builder objects
>>          builder = waf_libbsd.Builder()
>            buildsets = buildsets.BuildSets()
> ?

I assume that this references the package you mentioned above and should
be a
     buildsets = buildsys.BuildSets()
?

>>          libbsd.load(builder)
>            libbsd.load(builder, buildsets)

I don't think the libbsd.py would need to know about buildsets.

> ?
> Would something like that work?>> Chris
> 
>> +        bsconfig = load_config(ctx, bs)
>> +        bsname = bsconfig['name']
>> +        bsnames += [bsname]
>> +        builder.updateConfiguration(bsconfig)
>>          builder.generate(rtems_version)
>> +        builders[bsname]=builder
>> +
>> +        # Update the contextes for build variants
>> +        for y in contexts:
>> +            newcmd = y.cmd + '-' + bsname
>> +            newvariant = y.variant + '-' + bsname
>> +            class context(y):
>> +                cmd = newcmd
>> +                variant = newvariant
>> +                libbsd_buildset_name = bsname
>> +
>> +    # Transform the commands to per build variant commands
>> +    commands = []
>> +    for cmd in waflib.Options.commands:
>> +        if cmd.startswith(('build', 'clean', 'install')):
>> +            for bsname in bsnames:
>> +                commands += [cmd + '-' + bsname]
>> +        else:
>> +            commands += [cmd]
>> +    waflib.Options.commands = commands
>>  
>>  def init(ctx):
>> -    create_builder();
>> -    rtems.init(ctx, version = rtems_version, long_commands = True)
>> -    builder.init(ctx)
>> +    rtems.init(ctx, version = rtems_version, long_commands = True,
>> +               bsp_init = bsp_init)
>>  
>>  def options(opt):
>> -    create_builder();
>>      rtems.options(opt)
>>      opt.add_option("--enable-auto-regen",
>>                     action = "store_true",
>> @@ -86,20 +185,29 @@ def options(opt):
>>                     default = "2",
>>                     dest = "optimization",
>>                     help = "Set optimization level to OPTIMIZATION (-On compiler flag). Default is 2 (-O2).")
>> -    builder.options(opt)
>> +    opt.add_option("--buildset",
>> +                   action = "append",
>> +                   default = [],
>> +                   dest = "buildset",
>> +                   help = "Select build sets to build. If set to a directory, all .ini file in this directory will be used.")
>>  
>>  def bsp_configure(conf, arch_bsp):
>> -    create_builder();
>>      conf.check(header_name = "dlfcn.h", features = "c")
>>      conf.check(header_name = "rtems/pci.h", features = "c", mandatory = False)
>>      if not rtems.check_posix(conf):
>>          conf.fatal("RTEMS kernel POSIX support is disabled; configure RTEMS with --enable-posix")
>>      if rtems.check_networking(conf):
>>          conf.fatal("RTEMS kernel contains the old network support; configure RTEMS with --disable-networking")
>> -    builder.bsp_configure(conf, arch_bsp)
>> +    env = conf.env.derive()
>> +    for builder in builders:
>> +        ab = conf.env.RTEMS_ARCH_BSP
>> +        variant = ab + "-" + builder
>> +        conf.msg('Configure variant: ', variant)
>> +        conf.setenv(variant, env)
>> +        builders[builder].bsp_configure(conf, arch_bsp)
>> +        conf.setenv(ab)
>>  
>>  def configure(conf):
>> -    create_builder();
>>      if conf.options.auto_regen:
>>          conf.find_program("lex", mandatory = True)
>>          conf.find_program("rpcgen", mandatory = True)
>> @@ -110,9 +218,7 @@ def configure(conf):
>>      conf.env.FREEBSD_OPTIONS =conf.options.freebsd_options
>>      conf.env.OPTIMIZATION = conf.options.optimization
>>      rtems.configure(conf, bsp_configure)
>> -    builder.configure(conf)
>>  
>>  def build(bld):
>> -    create_builder();
>>      rtems.build(bld)
>> -    builder.build(bld)
>> +    builders[bld.libbsd_buildset_name].build(bld)
>>

-- 
--------------------------------------------
embedded brains GmbH
Herr Christian Mauderer
Dornierstr. 4
D-82178 Puchheim
Germany
email: christian.mauderer at embedded-brains.de
Phone: +49-89-18 94 741 - 18
Fax:   +49-89-18 94 741 - 08
PGP: Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.


More information about the devel mailing list