| Message ID | 1408714039-10339-1-git-send-email-trini@ti.com |
|---|---|
| State | RFC |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Tom, On 22 August 2014 07:27, Tom Rini <trini@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@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
On Fri, Aug 22, 2014 at 07:45:42PM -0600, Simon Glass wrote: > Hi Tom, > > On 22 August 2014 07:27, Tom Rini <trini@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@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? Well ble-arg. I thought I had tried this already but apparently not. [make-flags] qemu_mipsel=CONFIG_USE_PRIVATE_LIBGCC=y maltael=CONFIG_USE_PRIVATE_LIBGCC=y pb1000=CONFIG_USE_PRIVATE_LIBGCC=y dbau1550_el=CONFIG_USE_PRIVATE_LIBGCC=y Gets me building all MIPS boards, so I'm just going to stick with that and we can drop env-flags. [snip] > > + 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] Being a middling at best python person, how common is it to condense things down that far?
Hi Tom, On 23 August 2014 06:40, Tom Rini <trini@ti.com> wrote: > On Fri, Aug 22, 2014 at 07:45:42PM -0600, Simon Glass wrote: >> Hi Tom, >> >> On 22 August 2014 07:27, Tom Rini <trini@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@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? > > Well ble-arg. I thought I had tried this already but apparently not. > [make-flags] > qemu_mipsel=CONFIG_USE_PRIVATE_LIBGCC=y > maltael=CONFIG_USE_PRIVATE_LIBGCC=y > pb1000=CONFIG_USE_PRIVATE_LIBGCC=y > dbau1550_el=CONFIG_USE_PRIVATE_LIBGCC=y > > Gets me building all MIPS boards, so I'm just going to stick with that > and we can drop env-flags. > > [snip] >> > + 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] > > Being a middling at best python person, how common is it to condense > things down that far? List comprehension is fairly common - it's a special idiom that I think Python people are quite proud of. I suppose I wasn't in a very Python mood when I wrote GetMakeArguments() originally. Regards, Simon
On Sat, Aug 23, 2014 at 08:09:12AM -0600, Simon Glass wrote: > Hi Tom, > > On 23 August 2014 06:40, Tom Rini <trini@ti.com> wrote: > > On Fri, Aug 22, 2014 at 07:45:42PM -0600, Simon Glass wrote: > >> Hi Tom, > >> > >> On 22 August 2014 07:27, Tom Rini <trini@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@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? > > > > Well ble-arg. I thought I had tried this already but apparently not. > > [make-flags] > > qemu_mipsel=CONFIG_USE_PRIVATE_LIBGCC=y > > maltael=CONFIG_USE_PRIVATE_LIBGCC=y > > pb1000=CONFIG_USE_PRIVATE_LIBGCC=y > > dbau1550_el=CONFIG_USE_PRIVATE_LIBGCC=y > > > > Gets me building all MIPS boards, so I'm just going to stick with that > > and we can drop env-flags. > > > > [snip] > >> > + 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] > > > > Being a middling at best python person, how common is it to condense > > things down that far? > > List comprehension is fairly common - it's a special idiom that I > think Python people are quite proud of. I suppose I wasn't in a very > Python mood when I wrote GetMakeArguments() originally. Then I'm fine with a clean-up to condense things down. Thanks!
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. + + 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] + 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
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@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(+)