Closed 0xc0170 closed 5 years ago
This commit might be the offender: https://github.com/evedon/mbed-os/commit/050a3387715cb383cf8ecdd7b4c6469e23e1cabe. If not, looks like something else had to change on Friday
@evedon Please review
There is a change to tools/export/gnuarmeclipse/.cproject.tmpl
related to the language standard there that I don't immediately understand.
It's possible that it's failing to write the standard to the XML file, so defaulting to something lower than C++14. The errors generated suggest it's in pre-C++14 mode. (std::enable_if_t
appears in C++14).
Is this perhaps related to the new "merging two profile files" trick being applied? Is that combination maybe not fully working (for export only?), so info is lost?
I add @ARMmbed/mbed-os-tools team as well as this is most likely connected to profiles and the change above
I also don't understand the change in that PR (what is fixing and how it broke). Lets wait for @ARMmbed/mbed-os-core team review and fix.
The change to tools/export/gnuarmeclipse/.cproject.tmpl
was made by @madchutney to modify the template to work with partial profile see https://github.com/ARMmbed/mbed-os/commit/3010aaa253a1ee1dd7409d560eb9ec512936df02
Don't see the logic in that change - why is compiler.std
being singled out? Surely all the other settings next to it using the same basic form would have the same problem if this was a generic "run template on profile fragment" issue as described. Aren't all the nearby settings also missing in the fragment?
Maybe this change is just covering over the fact that compiler.std
specifically has gone missing.
I wonder if all the tests should actually be if opts['cpp']['foo'] is defined
- maybe they're all broken, and would fail their entry was missing.
So my suspicion is that somehow the profile combination process is losing compiler.std
, that problem was only revealed by the faulty "if defined" checks in this exporter, and the exporter change is covering it up for this specific entry. The real problem is elsewhere. (As of 5.13.0, not setting the compiler to C++14 is a non-fatal mistake, but current master does need C++14 to be selected).
I'd suggest reverting that change, and digging into whether/why the setting is missing on a complete build.
The change I authored when helping @evedon was workaround the addition of a partial profile, which was causing CI to fail. A partial profile is apparently something that has been supported but not used internally and as such is not handled by CI.
I'm happy to see the change reverted if necessary and a proper implementation of partial profiles included in Mbed CLI and CI system. This was what I recommended at the time but there were insufficient time and resources to complete the task without accruing more technical debt.
A PR to tackle this technical debt has already been proposed for the next PI.
I'm not understanding what the original issue was, and I'm wondering if it was misdiagnosed. What exactly is the CI trying to do with the profile? Is it doing a "for-each" over the "profiles" directory?
Is this enable_if_t
failure we see now arising when attempting a compile with "just minimal-printf" profile? If so, we'll need to stop it trying to do that. If we can't fix the tools, do we need to do something to get that profile fragment out of the way? Put it in a subdirectory?
I'm not understanding what the original issue was, and I'm wondering if it was misdiagnosed. What exactly is the CI trying to do with the profile? Is it doing a "for-each" over the "profiles" directory?
Is this
enable_if_t
failure we see now arising when attempting a compile with "just minimal-printf" profile? If so, we'll need to stop it trying to do that. If we can't fix the tools, do we need to do something to get that profile fragment out of the way? Put it in a subdirectory?
The exporter CI issue arised when attempting a compile with "just minimal-printf" profile. We discussed at the time whether to put the partial profile in a subdirectory or fix CI. If we put the profile in a subdirectory then the command line would become much longer:
mbed compile -m K64F -t GCC_ARM --profile release --profile mbed-os/tools/profiles/extensions/minimal-printf.json
So we decided that it would be better to fix CI, hence the change proposed by @madchutney. I think we need to understand why the change is affecting two targets NRF52840_DK and NUCLEO_L073RZ
specifically.
I am happy to revert the change, but this has the impact described above.
So it won't accept extensions/minimal-printf.json
as relative? Poo.
Really, we we don't want the CI to be wasting its time attempting bogus builds with incomplete profiles.
That template change was a very specific one, and I don't understand why it's sufficient. I can see dozens of the same construct in the same template looking at entries that also aren't set in that partial profile.
I note that as of GCC 6, the default language standard if we don't specify -mstd
is gnu++14
and gnu11
anyway - exactly what we want.
But I wonder what Eclipse does if we leave out that part of the XML file. Does it leave out -mstd
and thus normally get those correct defaults, so it usually works?
Except somehow those two targets select something different? And why didn't we see it on minimal-printf's own PR tests? Some flaw in smart testing w.r.t a profile?
This is blocking PRs to master, so I think we may have to temporarily just move or delete minimal-printf.json
now to stop CI messing with it, pending a proper solution.
Let's move minimal-printf.json
in mbed-os/tools/profiles/extensions/
then.
That's the best option for now.
I will propose a PR.
Internal Jira reference: https://jira.arm.com/browse/MBOCUSTRIA-1605
Description
The failures started on Friday, the first job failing is https://mbed-os.mbedcloudtesting.com/job/mbed-os-ci_exporter-gnuarmeclipse/2550 (internal link).
This are the errors:
Public link: https://github.com/ARMmbed/mbed-os/pull/11021#issuecomment-519974903, contains build logs. It has been failing often and affecting currently master PRs with rate higher than 50 percent of exporters run.
Strange fact, only two targets failing:
NRF52840_DK
andNUCLEO_L073RZ
.It started 9th of August, there were few merges that day: https://github.com/ARMmbed/mbed-os/commits/master. The only generic change is https://github.com/ARMmbed/mbed-os/pull/11051 - this changed gnuarmeclipse files so might be the cause.
cc @evedon @ARMmbed/mbed-os-test @kjbracey-arm @ARMmbed/mbed-os-maintainers
Issue request type