target.requires breaks configuration #162

paul-szczepanek-arm · 2020-12-30T16:24:34Z

Description

Version: 4.2.0
Ubuntu 20.04 LTS

Issue request type

Configuration fails with target override for requires:

    "target_overrides": {
        "*": {
            "target.requires": ["ble-service-link-loss","ble-current-time-service"]
        },

This should work the same as requires in the root of the config. Currently it fails with

Configuring project and generating build system...
Traceback (most recent call last):
  File "/home/paul/.local/bin/mbed-tools", line 8, in <module>
    sys.exit(cli())
  File "/home/paul/.local/lib/python3.8/site-packages/click/core.py", line 829, in __call__
    return self.main(*args, **kwargs)
  File "/home/paul/.local/lib/python3.8/site-packages/click/core.py", line 782, in main
    rv = self.invoke(ctx)
  File "/home/paul/.local/lib/python3.8/site-packages/mbed_tools/cli/main.py", line 38, in invoke
    super().invoke(context)
  File "/home/paul/.local/lib/python3.8/site-packages/click/core.py", line 1259, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/paul/.local/lib/python3.8/site-packages/click/core.py", line 1066, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/paul/.local/lib/python3.8/site-packages/click/core.py", line 610, in invoke
    return callback(*args, **kwargs)
  File "/home/paul/.local/lib/python3.8/site-packages/mbed_tools/cli/build.py", line 96, in build
    generate_config(mbed_target.upper(), toolchain, program)
  File "/home/paul/.local/lib/python3.8/site-packages/mbed_tools/build/config.py", line 27, in generate_config
    config = assemble_config(target_build_attributes, program.root, program.files.app_config_file)
  File "/home/paul/.local/lib/python3.8/site-packages/mbed_tools/build/_internal/config/assemble_build_config.py", line 23, in assemble_config
    return _assemble_config_from_sources_and_lib_files(target_attributes, mbed_lib_files, mbed_app_file)
  File "/home/paul/.local/lib/python3.8/site-packages/mbed_tools/build/_internal/config/assemble_build_config.py", line 47, in _assemble_config_from_sources_and_lib_files
    return Config.from_sources(all_sources)
  File "/home/paul/.local/lib/python3.8/site-packages/mbed_tools/build/_internal/config/config.py", line 119, in from_sources
    _update_config_option(config, key, value, source)
  File "/home/paul/.local/lib/python3.8/site-packages/mbed_tools/build/_internal/config/config.py", line 133, in _update_config_option
    raise ValueError(
ValueError: Can't update option which does not exist. Attempting to set 'target.requires' to '['ble-service-link-loss', 'ble-current-time-service']' in 'File: /home/paul/gh/mbed-os-example-ble/BLE_GattServer_ExperimentalServices/mbed_app.json'.

Enhancement
Bug

The text was updated successfully, but these errors were encountered:

paul-szczepanek-arm · 2020-12-30T16:40:46Z

Note to self so I remember to restart CI when fixed:
blocking ARMmbed/mbed-os-example-ble#349

rwalton-arm · 2021-01-04T10:27:40Z

Thanks for raising this. We had a fix go in for requires in the root of the config in v4.4.0 14a65ef. Looking at that code I don't think it handles the target.requires override, though. We'll look into this issue ASAP.

Apparently the old tools allowed a user to specify a required lib in the "target_overrides" section of the config, even if no root "requires" are defined. This commit adds support for this workflow. Fixes ARMmbed#162

Apparently the old tools allowed a user to specify a required lib in the "target_overrides" section of the config, even if no root "requires" are defined. This commit adds support for this workflow. Fixes #162

paul-szczepanek-arm · 2021-01-21T11:28:25Z

Hi, this still does't work, although the error output is different:

Configuring project and generating build system...
Traceback (most recent call last):
  File "/home/pawszc02/.local/bin/mbed-tools", line 8, in <module>
    sys.exit(cli())
  File "/home/pawszc02/.local/lib/python3.8/site-packages/click/core.py", line 829, in __call__
    return self.main(*args, **kwargs)
  File "/home/pawszc02/.local/lib/python3.8/site-packages/click/core.py", line 782, in main
    rv = self.invoke(ctx)
  File "/home/pawszc02/.local/lib/python3.8/site-packages/mbed_tools/cli/main.py", line 38, in invoke
    super().invoke(context)
  File "/home/pawszc02/.local/lib/python3.8/site-packages/click/core.py", line 1259, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/pawszc02/.local/lib/python3.8/site-packages/click/core.py", line 1066, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/pawszc02/.local/lib/python3.8/site-packages/click/core.py", line 610, in invoke
    return callback(*args, **kwargs)
  File "/home/pawszc02/.local/lib/python3.8/site-packages/mbed_tools/cli/build.py", line 96, in build
    generate_config(mbed_target.upper(), toolchain, program)
  File "/home/pawszc02/.local/lib/python3.8/site-packages/mbed_tools/build/config.py", line 27, in generate_config
    config = assemble_config(target_build_attributes, program.root, program.files.app_config_file)
  File "/home/pawszc02/.local/lib/python3.8/site-packages/mbed_tools/build/_internal/config/assemble_build_config.py", line 29, in assemble_config
    return _assemble_config_from_sources(target_attributes, mbed_lib_files, mbed_app_file)
  File "/home/pawszc02/.local/lib/python3.8/site-packages/mbed_tools/build/_internal/config/assemble_build_config.py", line 60, in _assemble_config_from_sources
    config.update(app_data)
  File "/usr/lib/python3.8/_collections_abc.py", line 832, in update
    self[key] = other[key]
  File "/home/pawszc02/.local/lib/python3.8/site-packages/mbed_tools/build/_internal/config/config.py", line 29, in __setitem__
    self._handle_overrides(item)
  File "/home/pawszc02/.local/lib/python3.8/site-packages/mbed_tools/build/_internal/config/config.py", line 46, in _handle_overrides
    setting = self._find_config_setting(lambda x: x.name == override.name)
  File "/home/pawszc02/.local/lib/python3.8/site-packages/mbed_tools/build/_internal/config/config.py", line 76, in _find_config_setting
    raise ValueError("Could not find element.")
ValueError: Could not find element.

paul-szczepanek-arm · 2021-01-21T11:30:46Z

You can check on ARMmbed/mbed-os-example-ble#349

git clone git@github.com:ARMmbed/mbed-os-example-ble.git
cd mbed-os-example-ble
git remote add paul git@github.com:paul-szczepanek-arm/mbed-os-example-ble.git
git checkout paul/experiment
cd BLE_GattServer_ExperimentalServices
mbed update
mbed-tools compile -t GCC_ARM -m NRF52840_DK

rwalton-arm · 2021-01-21T11:45:56Z

Thanks Paul. I'll take another look :/

rwalton-arm · 2021-01-21T12:07:31Z

target.requires are being handled here. It's failing on the platform.stdio-baud-rate override

DEBUG: Applying override 'platform.stdio-baud-rate'
Traceback (most recent call last):
  File "/Users/robwal02/projects/arm/mbed/repos/mbed-tools/.venv/bin/mbedtools", line 33, in <module>
    sys.exit(load_entry_point('mbed-tools', 'console_scripts', 'mbedtools')())
  File "/Users/robwal02/projects/arm/mbed/repos/mbed-tools/.venv/lib/python3.8/site-packages/click/core.py", line 829, in __call__
    return self.main(*args, **kwargs)
  File "/Users/robwal02/projects/arm/mbed/repos/mbed-tools/.venv/lib/python3.8/site-packages/click/core.py", line 782, in main
    rv = self.invoke(ctx)
  File "/Users/robwal02/projects/arm/mbed/repos/mbed-tools/src/mbed_tools/cli/main.py", line 38, in invoke
    super().invoke(context)
  File "/Users/robwal02/projects/arm/mbed/repos/mbed-tools/.venv/lib/python3.8/site-packages/click/core.py", line 1259, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/Users/robwal02/projects/arm/mbed/repos/mbed-tools/.venv/lib/python3.8/site-packages/click/core.py", line 1066, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/Users/robwal02/projects/arm/mbed/repos/mbed-tools/.venv/lib/python3.8/site-packages/click/core.py", line 610, in invoke
    return callback(*args, **kwargs)
  File "/Users/robwal02/projects/arm/mbed/repos/mbed-tools/src/mbed_tools/cli/build.py", line 96, in build
    generate_config(mbed_target.upper(), toolchain, program)
  File "/Users/robwal02/projects/arm/mbed/repos/mbed-tools/src/mbed_tools/build/config.py", line 27, in generate_config
    config = assemble_config(target_build_attributes, program.root, program.files.app_config_file)
  File "/Users/robwal02/projects/arm/mbed/repos/mbed-tools/src/mbed_tools/build/_internal/config/assemble_build_config.py", line 29, in assemble_config
    return _assemble_config_from_sources(target_attributes, mbed_lib_files, mbed_app_file)
  File "/Users/robwal02/projects/arm/mbed/repos/mbed-tools/src/mbed_tools/build/_internal/config/assemble_build_config.py", line 60, in _assemble_config_from_sources
    config.update(app_data)
  File "/usr/local/opt/python@3.8/bin/../Frameworks/Python.framework/Versions/3.8/lib/python3.8/_collections_abc.py", line 832, in update
    self[key] = other[key]
  File "/Users/robwal02/projects/arm/mbed/repos/mbed-tools/src/mbed_tools/build/_internal/config/config.py", line 29, in __setitem__
    self._handle_overrides(item)
  File "/Users/robwal02/projects/arm/mbed/repos/mbed-tools/src/mbed_tools/build/_internal/config/config.py", line 46, in _handle_overrides
    setting = self._find_config_setting(lambda x: x.name == override.name)
  File "/Users/robwal02/projects/arm/mbed/repos/mbed-tools/src/mbed_tools/build/_internal/config/config.py", line 76, in _find_config_setting
    raise ValueError("Could not find element.")
ValueError: Could not find element.

platform/mbed_lib.json isn't being processed. I guess because it's not set in the requires list.
If requires are specified the tool will only pull in config for required libs. Maybe our understanding of how requires is supposed to work is flawed.

Is target.requires a valid config option when the target doesn't actually set any requires? Most of the other examples I've seen only use requires in the root of the config.

rwalton-arm · 2021-01-21T12:19:06Z

The baremetal-blinky example also uses requires and in its mbed_app.json it requires: [bare-metal].

Then bare_metal/mbed_lib.json requires: ["platform" ...]

Looks like we need a similar requires chain for this ble example.

paul-szczepanek-arm · 2021-01-21T13:29:35Z

yeah, the requires in the root works but the problem with using that one is that it overrides all the deafult mbed-os requires, so it compiles the experimental libraries but not mbed-os itself, the way requires works is wholly unintuitive

paul-szczepanek-arm · 2021-01-21T13:30:04Z

basically, to use the root version of requiers I would need a way to have it add, rather than replace, so something like requires_add

rwalton-arm · 2021-01-21T14:13:23Z

Just to ensure I'm following here: are you saying that target.requires shouldn't filter things that aren't in the target.requires list, and instead pull in the config for all 'default' libs (following directory "label" rules) plus target.requires libs? Conversely root requires will filter any libs not in the requires list, and only pull in the config for explicitly required things. Is that understanding correct?

Is the requires expected behaviour documented anywhere (I couldn't find any docs mentioning it)?

rwalton-arm · 2021-01-21T15:18:02Z

If I just delete target.requires from your mbed_app.json the example seems to configure OK (it doesn't actually build due to a missing CMakeLists.txt in mbed-os-ble-utils). We shouldn't need to require your external utility libraries as they aren't in directories marked with "filter labels" (e.g TARGET_, FEATURE_) so the old tool will suck them in always by default.

The new CMake stuff pulls them in, too. You have an add_subdirectory(ble-example-...) in your top level CMakeLists.txt. The new tools follow the same directory labeling rules as the old tools when scanning for the config files, so those are also pulled in by default.

Here's the memory map output from the old tool using target.requires

Link: BLE_GattServer_ExperimentalServices
Elf2Bin: BLE_GattServer_ExperimentalServices
| Module                                     |      .text |    .data |      .bss |
|--------------------------------------------|------------|----------|-----------|
| [fill]                                     |    502(+0) |   12(+0) |   102(+0) |
| [lib]/c.a                                  |  19464(+0) | 2572(+0) |   127(+0) |
| [lib]/gcc.a                                |    920(+0) |    0(+0) |     0(+0) |
| [lib]/misc                                 |    180(+0) |    4(+0) |    28(+0) |
| [lib]/stdc++.a                             |      0(+0) |    0(+0) |     0(+0) |
| mbed-os-experimental-ble-services/services |   1764(+0) |    0(+0) |     0(+0) |
| mbed-os/cmsis                              |   7836(+0) |  168(+0) |  5956(+0) |
| mbed-os/connectivity                       | 201606(+0) |  286(+0) | 22868(+0) |
| mbed-os/drivers                            |    842(+0) |    0(+0) |     0(+0) |
| mbed-os/events                             |   1464(+0) |    0(+0) |     0(+0) |
| mbed-os/hal                                |   1346(+0) |    8(+0) |   113(+0) |
| mbed-os/platform                           |   6318(+0) |  276(+0) |   446(+0) |
| mbed-os/rtos                               |    382(+0) |    0(+0) |     8(+0) |
| mbed-os/storage                            |     36(+0) |    0(+0) |     4(+0) |
| mbed-os/targets                            |  10318(+0) |   10(+0) |   888(+0) |
| source/main.o                              |   1554(+0) |    0(+0) |   100(+0) |
| Subtotals                                  | 254532(+0) | 3336(+0) | 30640(+0) |
Total Static RAM memory (data + bss): 33976(+0) bytes
Total Flash memory (text + data): 257868(+0) bytes

and without target.requires

[Warning] Gap.h@19,2: #warning "This header is deprecated, please include ble/Gap.h directly" [-Wcpp]
Link: BLE_GattServer_ExperimentalServices
Elf2Bin: BLE_GattServer_ExperimentalServices
| Module                                     |           .text |       .data |          .bss |
|--------------------------------------------|-----------------|-------------|---------------|
| [fill]                                     |       502(+502) |     12(+12) |     102(+102) |
| [lib]/c.a                                  |   19464(+19464) | 2572(+2572) |     127(+127) |
| [lib]/gcc.a                                |       920(+920) |       0(+0) |         0(+0) |
| [lib]/misc                                 |       180(+180) |       4(+4) |       28(+28) |
| [lib]/stdc++.a                             |           0(+0) |       0(+0) |         0(+0) |
| mbed-os-experimental-ble-services/services |     1764(+1764) |       0(+0) |         0(+0) |
| mbed-os/cmsis                              |     7836(+7836) |   168(+168) |   5956(+5956) |
| mbed-os/connectivity                       | 201606(+201606) |   286(+286) | 22868(+22868) |
| mbed-os/drivers                            |       842(+842) |       0(+0) |         0(+0) |
| mbed-os/events                             |     1464(+1464) |       0(+0) |         0(+0) |
| mbed-os/hal                                |     1346(+1346) |       8(+8) |     113(+113) |
| mbed-os/platform                           |     6318(+6318) |   276(+276) |     446(+446) |
| mbed-os/rtos                               |       382(+382) |       0(+0) |         8(+8) |
| mbed-os/storage                            |         36(+36) |       0(+0) |         4(+4) |
| mbed-os/targets                            |   10318(+10318) |     10(+10) |     888(+888) |
| source/main.o                              |     1554(+1554) |       0(+0) |     100(+100) |
| Subtotals                                  | 254532(+254532) | 3336(+3336) | 30640(+30640) |
Total Static RAM memory (data + bss): 33976(+33976) bytes
Total Flash memory (text + data): 257868(+257868) bytes

The old tool apparently lets you put any "target." in "target_overrides" without complaining, and it just seems to ignore it if the parameter you're trying to "override" doesn't exist. The new tools error in this case because the documentation states "It is an error for the application configuration to override an undefined configuration parameter."

So, I think the "fix" here is to remove your target.requires from the config as I don't think it was being used in the old world, and it's not needed in the new world. Hopefully that should get the example to build using the new and old tools.

paul-szczepanek-arm · 2021-01-21T16:02:25Z

thank you, I don't understand how that works, how can mbed see the lib if it's not required by anyone?

target.requires was never a valid target_override. See ARMmbed#162 for details.

target.requires was never a valid target_override. See ARMmbed#162 for details. Partially reverts 6a99b43.

target.requires was never a valid target_override. See #162 for details. Partially reverts 6a99b43.

paul-szczepanek-arm mentioned this issue Dec 30, 2020

Add experimental services example ARMmbed/mbed-os-example-ble#349

Merged

rwalton-arm added this to To do (8) in Mbed Tools Jan 4, 2021

rwalton-arm moved this from To do (8) to In progress (5) in Mbed Tools Jan 8, 2021

rwalton-arm self-assigned this Jan 8, 2021

rwalton-arm mentioned this issue Jan 12, 2021

config: Refactor config processing module #159

Merged

3 tasks

Patater closed this as completed in #159 Jan 15, 2021

Mbed Tools automation moved this from In progress (5) to Done Jan 15, 2021

paul-szczepanek-arm reopened this Jan 21, 2021

paul-szczepanek-arm closed this as completed Jan 21, 2021

rwalton-arm added a commit to rwalton-arm/mbed-tools that referenced this issue Jul 29, 2021

config: Remove support for target.requires

97d3b7e

target.requires was never a valid target_override. See ARMmbed#162 for details.

rwalton-arm added a commit to rwalton-arm/mbed-tools that referenced this issue Jul 29, 2021

config: Remove support for target.requires

b5b7fc2

target.requires was never a valid target_override. See ARMmbed#162 for details.

rwalton-arm added a commit to rwalton-arm/mbed-tools that referenced this issue Jul 29, 2021

config: Remove support for target.requires

e0c5706

target.requires was never a valid target_override. See ARMmbed#162 for details. Partially reverts 6a99b43.

Patater pushed a commit that referenced this issue Jul 29, 2021

config: Remove support for target.requires

89f60b3

target.requires was never a valid target_override. See #162 for details. Partially reverts 6a99b43.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

target.requires breaks configuration #162

target.requires breaks configuration #162

paul-szczepanek-arm commented Dec 30, 2020

paul-szczepanek-arm commented Dec 30, 2020

rwalton-arm commented Jan 4, 2021

paul-szczepanek-arm commented Jan 21, 2021

paul-szczepanek-arm commented Jan 21, 2021 •

edited

Loading

rwalton-arm commented Jan 21, 2021

rwalton-arm commented Jan 21, 2021

rwalton-arm commented Jan 21, 2021 •

edited

Loading

paul-szczepanek-arm commented Jan 21, 2021

paul-szczepanek-arm commented Jan 21, 2021

rwalton-arm commented Jan 21, 2021

rwalton-arm commented Jan 21, 2021

paul-szczepanek-arm commented Jan 21, 2021

target.requires breaks configuration #162

target.requires breaks configuration #162

Comments

paul-szczepanek-arm commented Dec 30, 2020

Description

Issue request type

paul-szczepanek-arm commented Dec 30, 2020

rwalton-arm commented Jan 4, 2021

paul-szczepanek-arm commented Jan 21, 2021

paul-szczepanek-arm commented Jan 21, 2021 • edited Loading

rwalton-arm commented Jan 21, 2021

rwalton-arm commented Jan 21, 2021

rwalton-arm commented Jan 21, 2021 • edited Loading

paul-szczepanek-arm commented Jan 21, 2021

paul-szczepanek-arm commented Jan 21, 2021

rwalton-arm commented Jan 21, 2021

rwalton-arm commented Jan 21, 2021

paul-szczepanek-arm commented Jan 21, 2021

paul-szczepanek-arm commented Jan 21, 2021 •

edited

Loading

rwalton-arm commented Jan 21, 2021 •

edited

Loading