build api: fix release version #12982

0xc0170 · 2020-05-15T15:59:09Z

Summary of changes

This is non trivial fix as the function is being used outside of this repository.
Tools rely on it to return list of targets for 2 or 5. As we removed release_version from many targets,
this broke the logic. To keep the logic as it was, without updating all tools out there now,
lets just return full set of targets - all are supported.

In case for Mbed 2, returning all targets does not make sense, but rather raise an exception here. Not supported. This avoids suprised. If you look at build api functions there are many checks for 2 or 5 so more
clean up needed to actually get release_version out of the tools.

Impact of changes

Migration actions required

Documentation

Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[x] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

0xc0170 · 2020-05-15T16:05:33Z

Run the first series of tests

tools/build_api.py

hugueskamba · 2020-05-15T17:01:24Z

tools/build_api.py

+
+    if version == `2`:
+        raise InvalidReleaseTargetException("Mbed 2 not supported anymore with this version of tools")
+
    mbed_official_release = (


I am not sure the syntax is correct here. Can you run the script with your changes?

I was checking what CI expected and seems to be as it is here. will study CI results once we get some, if all is being tested as it should

hugueskamba · 2020-05-15T17:36:46Z

@evedon
I thought this is script is only used to build Mbed 2. Nothing should be running it anymore since support for Mbed 2 has been dropped.

0xc0170 · 2020-05-15T20:28:19Z

We had previously similar experience, tools are used in expected way - this release version is due to supporting multiple versions so CI had to check for version + target.

0xc0170 · 2020-05-16T08:12:38Z

I fixed travis error

0xc0170 · 2020-05-18T05:39:15Z

CI started

0xc0170 · 2020-05-18T06:41:00Z

ci restarted

0xc0170 · 2020-05-18T07:20:18Z

CI restarted

I changed the approach for mbed 2 in that function, can't afford to raise an exception as that is a breaking change there for now. Rather return an empty tuple, as that is what others expect (CI should not break - if we use exception, I would need to refactor CI config and others).

mbed-ci · 2020-05-18T07:48:07Z

Test run: FAILED

Summary: 2 of 3 test jobs failed
Build number : 4
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-GCC_ARM

0xc0170 · 2020-05-18T07:54:59Z

Ci restarted, can't find the error only jenkins generic exception

0xc0170 · 2020-05-18T07:55:43Z

I found the errors in the log, investigating

This is non trivial fix as the function is being used outside of this repository. Tools rely on it to return list of targets for 2 or 5. As we removed release_version from many targets, this broke the logic. To keep the logic as it was, without updating all tools out there now, lets just return full set of targets - all are supported. In case for Mbed 2, returning all targets does not make sense, but rather raise an exception here. Not supported. This avoids suprised. If you look at build api functions there are many checks for 2 or 5 so more clean up needed to actually get release_version out of the tools.

Co-authored-by: Hugues Kamba <41612201+hugueskamba@users.noreply.github.com>

We cant afford to raise an exception now. As anyone using this out there do not catch it. Rather an empty list, as it can work after this fix - won't do anything.

0xc0170 · 2020-05-18T07:57:53Z

Rebased on top of master to be sure we got all needed (the latest targets removals)

0xc0170 · 2020-05-18T08:00:20Z

CI restarted

@MarceloSalazar The failures might be related to the latest removals (mbed 2 only targets?), will need to check. It might be that CI ignored it previously based on release_version. Lets review CI once it completes after my latest rebase

mbed-ci · 2020-05-18T08:36:58Z

Test run: FAILED

Summary: 2 of 3 test jobs failed
Build number : 6
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-ARM

0xc0170 · 2020-05-18T10:07:44Z

@jeromecoutant would you know why some nucleo fail here:

mbed-os-example-blinky
[Warning] @0,0: L3912W: Option 'legacyalign' is deprecated.
[Error] @0,0: L6218E: Undefined symbol Image$$ARM_LIB_HEAP$$ZI$$Base (referred from BUILD/NUCLEO_F070RB/ARM/mbed-os/rtos/source/TARGET_CORTEX/TOOLCHAIN_ARM_STD/mbed_boot_arm_std.o).

These targets were supported in Mbed OS 5, so should build without errors with this fix we have in here.

0xc0170 · 2020-05-18T11:05:59Z

@jeromecoutant we identified a bug, its in the CI tools (+ this fix), its around baremetal, we are working on fixing that.

jamesbeyond · 2020-05-18T17:46:32Z

After some discussion/investigation, I think we are going to need a way of distinguishing mbed 6 bare-metal profile only targets from the targets support both bare-metal and rtos.

in current targets.json file, "supported_application_profiles": ["bare-metal"] is a bit ambiguity, we can't really claim this target support bare-metal ONLY, unless all other targets have "supported_application_profiles": ["bare-metal","rtos"] .
Maybe we can put this into the top-level target, so every one inherits by default?

So the above context, I'd like to propose the change to this impacted functions to

def get_mbed_official_release(version, profile=None):
    if version == '2':
        return tuple(tuple([]))

    if version in ['5', '6']:
        mbed_official_release = (
            tuple(
                tuple(
                    [
                        TARGET_MAP[target].name,
                        tuple(['ARM', 'GCC_ARM'])
                    ]
                ) for target in TARGET_NAMES \
                if not profile or profile in TARGET_MAP[target].supported_application_profiles
            )
        )

    return mbed_official_release

if the profile parameter not given, it treated as querying for all profiles.
So in the CI scripts, I can use build_api.get_mbed_official_release(6 profile="rtos") to query the targets only support RTOS

@0xc0170 @evedon @MarceloSalazar

jamesbeyond · 2020-05-18T17:56:44Z

tools/build_api.py

    mbed_official_release = (
        tuple(
            tuple(
                [
                    TARGET_MAP[target].name,
-                    tuple(transform_release_toolchains(
-                        TARGET_MAP[target], version))
+                    tuple(['ARM', 'GCC_ARM'])


just wonder why do we use hard-coded tuple here? instead of calling the function? and IAR been dropped?
We can fix the function if need?

I wanted to avoid refactoring at the moment (so changes just to one function to get the functionality needed). transform_release_toolchains does not have much meaning anymore as only ARM or GCC ARM is supported, so this can be hardcoded here

To select supported_application_profiles attribute from targets

Every target is assumed to supported Mbed OS. If its not the case, it supports only baremetal. Thus removing rtos, and adding bare-metal to the app profile.

For only baremetal targets, just drop in replace. Don't need to add/remove.

0xc0170 · 2020-05-18T19:06:12Z

I updated the PR as proposed (adding rtos and baremetal t o the top level Target and overwrite it for some).

0xc0170 · 2020-05-18T19:58:02Z

CI started

0xc0170 · 2020-05-18T20:03:31Z

@jamesbeyond if CI merges, please merge this to test this with nightly asap.

mbed-ci · 2020-05-18T21:19:25Z

Test run: SUCCESS

Summary: 6 of 6 test jobs passed
Build number : 7
Build artifacts

jamesbeyond · 2020-05-18T21:39:55Z

CI passed, merging PR

hugueskamba · 2020-05-19T11:08:46Z

After some discussion/investigation, I think we are going to need a way of distinguishing mbed 6 bare-metal profile only targets from the targets support both bare-metal and rtos.

in current targets.json file, "supported_application_profiles": ["bare-metal"] is a bit ambiguity, we can't really claim this target support bare-metal ONLY, unless all other targets have "supported_application_profiles": ["bare-metal","rtos"] .
Maybe we can put this into the top-level target, so every one inherits by default?

So the above context, I'd like to propose the change to this impacted functions to
def get_mbed_official_release(version, profile=None):
    if version == '2':
        return tuple(tuple([]))

    if version in ['5', '6']:
        mbed_official_release = (
            tuple(
                tuple(
                    [
                        TARGET_MAP[target].name,
                        tuple(['ARM', 'GCC_ARM'])
                    ]
                ) for target in TARGET_NAMES \
                if not profile or profile in TARGET_MAP[target].supported_application_profiles
            )
        )

    return mbed_official_release
if the profile parameter not given, it treated as querying for all profiles.
So in the CI scripts, I can use build_api.get_mbed_official_release(6 profile="rtos") to query the targets only support RTOS

@0xc0170 @evedon @MarceloSalazar

All targets will inherit "supported_application_profiles":["bare-metal", "rtos"] except those that will overwrite the attribute to say "supported_application_profiles":["bare-metal"]. This is because it will indeed be added to the parent of all targets ("Target"). It will therefore be easy to determine which targets only support the bare-metal profile (if not "rtos" in target.supported_application_profiles: pass; ).

0xc0170 requested review from Patater and a team May 15, 2020 15:59

0xc0170 mentioned this pull request May 15, 2020

release_version removal in targets causing tools to not report them as official targets #12980

Closed

evedon requested a review from hugueskamba May 15, 2020 16:09

mergify bot added the needs: work label May 15, 2020

hugueskamba reviewed May 15, 2020

View reviewed changes

0xc0170 added needs: CI and removed needs: work labels May 15, 2020

mergify bot added needs: work and removed needs: CI labels May 15, 2020

0xc0170 added release-version: 6.0.0-rc1 needs: CI and removed needs: work labels May 18, 2020

mergify bot added needs: work and removed needs: CI labels May 18, 2020

0xc0170 and others added 5 commits May 18, 2020 08:57

tools/build_api: fix wording

caf58a7

Co-authored-by: Hugues Kamba <41612201+hugueskamba@users.noreply.github.com>

build: fix error syntax

6e59bdd

tools build: fix toolchain tuple, should be list

810b026

tools build: return empty list for mbed2

59db9f6

We cant afford to raise an exception now. As anyone using this out there do not catch it. Rather an empty list, as it can work after this fix - won't do anything.

0xc0170 added needs: CI and removed needs: work labels May 18, 2020

0xc0170 force-pushed the fix_release_version branch from 5a65d26 to 59db9f6 Compare May 18, 2020 07:57

mergify bot added needs: work and removed needs: CI labels May 18, 2020

RZ_A1XX: public set to false as its base target

5df3c91

jamesbeyond reviewed May 18, 2020

View reviewed changes

0xc0170 added 3 commits May 18, 2020 19:31

tools build: add profile

07084a5

To select supported_application_profiles attribute from targets

targets: add rtos profile by default

d6fbda3

Every target is assumed to supported Mbed OS. If its not the case, it supports only baremetal. Thus removing rtos, and adding bare-metal to the app profile.

targets: enable baremetal by default

9b4cddd

For only baremetal targets, just drop in replace. Don't need to add/remove.

0xc0170 added needs: CI and removed needs: work labels May 18, 2020

jamesbeyond approved these changes May 18, 2020

View reviewed changes

mergify bot added ready for merge and removed needs: CI labels May 18, 2020

jamesbeyond merged commit f478a21 into ARMmbed:master May 18, 2020

mergify bot removed the ready for merge label May 18, 2020

build api: fix release version #12982

build api: fix release version #12982

Uh oh!

Conversation

0xc0170 commented May 15, 2020

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

Uh oh!

0xc0170 commented May 15, 2020

Uh oh!

Uh oh!

hugueskamba May 15, 2020

Choose a reason for hiding this comment

Uh oh!

0xc0170 May 18, 2020

Choose a reason for hiding this comment

Uh oh!

hugueskamba commented May 15, 2020

Uh oh!

0xc0170 commented May 15, 2020

Uh oh!

0xc0170 commented May 16, 2020

Uh oh!

0xc0170 commented May 18, 2020

Uh oh!

0xc0170 commented May 18, 2020

Uh oh!

0xc0170 commented May 18, 2020

Uh oh!

mbed-ci commented May 18, 2020

Test run: FAILED

Uh oh!

0xc0170 commented May 18, 2020

Uh oh!

0xc0170 commented May 18, 2020

Uh oh!

0xc0170 commented May 18, 2020

Uh oh!

0xc0170 commented May 18, 2020

Uh oh!

mbed-ci commented May 18, 2020

Test run: FAILED

Uh oh!

0xc0170 commented May 18, 2020

Uh oh!

0xc0170 commented May 18, 2020

Uh oh!

jamesbeyond commented May 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jamesbeyond May 18, 2020

Choose a reason for hiding this comment

Uh oh!

0xc0170 May 18, 2020

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented May 18, 2020

Uh oh!

0xc0170 commented May 18, 2020

Uh oh!

0xc0170 commented May 18, 2020

Uh oh!

mbed-ci commented May 18, 2020

Test run: SUCCESS

Uh oh!

jamesbeyond commented May 18, 2020

Uh oh!

hugueskamba commented May 19, 2020

Uh oh!

Uh oh!

jamesbeyond commented May 18, 2020 •

edited

Loading