[U-Boot] [RFC PATCH] tools/buildman: Add env-flags section support

[Simon Glass]

Hi Tom,

On 22 August 2014 07:27, Tom Rini <trini at ti.com> wrote:
> In some cases (such as building little endian MIPS with the ELDK
> toolchain) you need to set a variable in the environment in order for
> the build to complete.  Add env-flags, similar to make-flags, to allow
> for this.
>
> Signed-off-by: Tom Rini <trini at ti.com>
> ---
>
> I have this as RFC since GetEnvSettings is a copy/paste of
> GetMakeArguments and that tells me there must be a pythonic way of
> extending / renaming GetMakeArguments to take the dict to work from as
> an argument.  And as per my other email, ${variable-board} doesn't work
> so the README is actually incorrect.
>
>  tools/buildman/README           |   24 +++++++++++++++++++++++
>  tools/buildman/builderthread.py |    6 ++++++
>  tools/buildman/toolchain.py     |   40 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 70 insertions(+)
>
> diff --git a/tools/buildman/README b/tools/buildman/README
> index d4e8404..b513b81 100644
> --- a/tools/buildman/README
> +++ b/tools/buildman/README
> @@ -676,6 +676,30 @@ It is expected that any variables added are dealt with in U-Boot's
>  config.mk file and documented in the README.
>
>
> +Providing key/value pairs to the environment
> +============================================
> +
> +U-Boot's build system supports a few environment variables (such as
> +CONFIG_USE_PRIVATE_LIBGCC) which affect the build product. These flags can be
> +specified in the buildman settings file. They can also be useful when building
> +U-Boot against other open source software.
> +
> +[env-flags]
> +mipsel-flags=CONFIG_USE_PRIVATE_LIBGCC=y
> +qemu_mipsel=${mipsel-flags}
> +maltael=${mipsel-flags}
> +pb1000=${mipsel-flags}
> +dbau1550_el=${mipsel-flags}
> +
> +This will set 'CONFIG_USE_PRIVATE_LIBGCC' to 'y' in the environment for
> +qemu_mipsel, maltael, pb100 and dbau1550_el.  A special variable ${target} is
> +available to access the target name (qemu_mipsel, maltael, pb1000 and
> +dbau1550_el in this case). Variables are resolved recursively.
> +
> +It is expected that any variables added are dealt with in U-Boot's
> +config.mk file and documented in the README.

This should be equivalent to adding them on the 'make' command line.
Is there somethign else?

> +
> +
>  Quick Sanity Check
>  ==================
>
> diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
> index 8214662..920752c 100644
> --- a/tools/buildman/builderthread.py
> +++ b/tools/buildman/builderthread.py
> @@ -174,6 +174,12 @@ class BuilderThread(threading.Thread):
>
>                  # Set up the environment and command line
>                  env = self.toolchain.MakeEnvironment()
> +
> +                # Add additional flags
> +                for flag in self.builder.toolchains.GetEnvSettings(brd):
> +                    key_val = flag.split('=')
> +                    env[key_val[0]] = key_val[1]
> +

I would say move this code into the MakeEnvironment() function
mentioned above - that way the toolchain environment is all in one
place.

>                  Mkdir(out_dir)
>                  args = []
>                  cwd = work_dir
> diff --git a/tools/buildman/toolchain.py b/tools/buildman/toolchain.py
> index 1b9771f..1ab59b3 100644
> --- a/tools/buildman/toolchain.py
> +++ b/tools/buildman/toolchain.py
> @@ -111,6 +111,7 @@ class Toolchains:
>              else:
>                  self.paths.append(value)
>          self._make_flags = dict(bsettings.GetItems('make-flags'))
> +        self._env_flags = dict(bsettings.GetItems('env-flags'))
>
>      def Add(self, fname, test=True, verbose=False):
>          """Add a toolchain to our list
> @@ -245,3 +246,42 @@ class Toolchains:
>              else:
>                  i += 1
>          return args
> +
> +    def GetEnvSettings(self, board):
> +        """Returns key=value pairs to set in the environment prior to 'make'
> +
> +        The flags are in a section called 'env-flags'. Flags are named
> +        after the target they represent, for example snapper9260=TESTING=1
> +        will set TESTING to 1 in the environment prior to invoking make when
> +        building the snapper9260 board.
> +
> +        References to other boards can be added in the string also. For
> +        example:
> +
> +        [env-flags]
> +        at91-boards=ENABLE_AT91_TEST=1
> +        snapper9260=${at91-boards} BUILD_TAG=442
> +        snapper9g45=${at91-boards} BUILD_TAG=443
> +
> +        This will return 'ENABLE_AT91_TEST=1 BUILD_TAG=442' for snapper9260
> +        and 'ENABLE_AT91_TEST=1 BUILD_TAG=443' for snapper9g45.
> +
> +        A special 'target' variable is set to the board target.
> +
> +        Args:
> +            board: Board object for the board to check.
> +        Returns:
> +            key=value parts to set in the environment for that board, or ''
> +            if none
> +        """
> +        self._env_flags['target'] = board.target
> +        arg_str = self.ResolveReferences(self._env_flags,
> +                           self._env_flags.get(board.target, ''))
> +        args = arg_str.split(' ')
> +        i = 0
> +        while i < len(args):
> +            if not args[i]:
> +                del args[i]
> +            else:
> +                i += 1
> +        return args

Yes this could go in a common function. You can pass the dictionary to
it, and updates within the function will update the dictionary.

BTW I suppose we can replace the last 8 lines with something like:

return [arg for arg in arg_str.split(' ') if arg]

Regards,
Simon