CMake: ninja reponse files disabled for ARMClang on Windows #13827

0xc0170 · 2020-10-28T15:34:28Z

Summary of changes

Issue https://gitlab.kitware.com/cmake/cmake/-/issues/21093

Until this is fixed, we should disable response files for ARMClang + ninja on Windows.
Other toolchains and host systems should benefit from response files thus not disabling them
completely.

This fixes the issue with not finding includes. It's not trivial to find the root cause, it took me a while to figure out why ARMClang can't find the paths.

I hope using host system to check is the proper one. My initial approach was to use WIN32 but this is set to false in my enviroment (using windows with CMder).

Hopefully we won't hit the win path limit anytime soon, otherwise this bug can become a blocker, it should be fixed one day soon.

@MarceloSalazar Please test locally as well if this fixes the issue for you.

Impact of changes

Migration actions required

Documentation

Pull request type

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

Test results

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

Reviewers

0xc0170 · 2020-10-28T15:53:37Z

Wait a minute, I am still testing. It looks like this does not yet complete fixes the issue. Rebuilding the cache and entire tree takes time, so will need more than a minute to test the theory.

ciarmcom · 2020-10-28T16:00:29Z

@0xc0170, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

CMakeLists.txt

MarceloSalazar · 2020-10-28T18:24:51Z

Tested on Windows and I don't see the error about the includes. However, there are two problems:

Link Error: L6218E: Undefined symbol mbed_fault_context (referred from mbed-os\CMakeFiles\mbed-core.dir\platform\source\TARGET_CORTEX_M\TOOLCHAIN_ARM\except.o).
Compile time is huge (but we could live with it for a little while)

0xc0170 · 2020-10-29T08:44:29Z

The linker error I also saw, I'll look at it today.

Compile time is huge (but we could live with it for a little while)

It must be something with the license, as I am testing now again and the time is good again.

0xc0170 · 2020-10-29T10:00:18Z

I found the culprit . We fixed the issue in CMake but has not been released until 3.19.0 (currently rc2). I downloaded it and it links without an error.

I'll update now the version requirements.

Pull request has been modified.

0xc0170 · 2020-10-29T10:07:25Z

Rebased, squashed to two commits.

Using 3.19.0-rc2 ARMClang links without error on Windows with Ninja.

@MarceloSalazar @hugueskamba @rajkan01 Please review

0xc0170 · 2020-10-29T10:34:32Z

CMakeLists.txt

@@ -3,7 +3,7 @@

 # This is the boilerplate for Mbed OS

-cmake_minimum_required(VERSION 3.18.2 FATAL_ERROR)
+cmake_minimum_required(VERSION 3.19.0 FATAL_ERROR)


Travis will fail if it fetches from pip (this version not yet there unfortunately)

I can do:

# Fetch the 3.19.0 - DEPS_DIR="${TRAVIS_BUILD_DIR}/deps" - mkdir ${DEPS_DIR} && cd ${DEPS_DIR} - travis_retry wget --no-check-certificate https://cmake.org/files/v3.19/cmake-3.19.0-rc2-Linux-x86_64.tar.gz - tar -xvf cmake-3.19.0-rc2-Linux-x86_64.tar.gz > /dev/null - mv cmake-3.19.0-rc2-Linux-x86_64 cmake-install - PATH=${DEPS_DIR}/cmake-install:${DEPS_DIR}/cmake-install/bin:$PATH - cd ${TRAVIS_BUILD_DIR}

However this would be just partial fix for now. I'll talk to test team to include 3.19.0 in the jenkins and this can wait for CI update to land

MarceloSalazar · 2020-10-29T11:15:53Z

I confirm the project compiles and links OK when using cmake 3.19.-rc2

0xc0170 · 2020-10-29T11:24:56Z

This will wait for #13477 to land

hugueskamba · 2020-10-29T13:15:19Z

tools/cmake/README.md


 ## How to build an application

 Prerequisities:
- CMake >=3.18.2
+- CMake >=3.19.0
 - mbed-tools >=3.2.0


mbed-tools needs to be at least 3.4.0.

We haven't updated for a while. Updating now!

Issue https://gitlab.kitware.com/cmake/cmake/-/issues/21093 Until this is fixed, we should disable response files for ARMClang + ninja on Windows. Other toolchains and host systems should benefit from response files thus not disabling them completely. This fixes the issue with not finding includes. It's not trivial to find the root cause, it took me a while to figure out why ARMClang can't find the paths. I moved the check to Mbed OS main cmake. It should not be in the toolchain file as it is not related to the toolchain but to generator. We need toolchain properly set up first. Note, I had to protect setting CMAKE_NINJA_FORCE_RESPONSE_FILE. If I set it to 0, ninja would still use rsp files so I rather protected it and define that variable only when required, not always. Co-authored-by: Hugues Kamba <41612201+hugueskamba@users.noreply.github.com>

0xc0170 · 2020-10-30T08:27:02Z

Rebased, Travis should now pass and I'll run Jenkins CI test

Pull request has been modified.

0xc0170 · 2020-10-30T08:29:40Z

CI started

mbed-ci · 2020-10-30T09:03:48Z

Jenkins CI Test : ✔️ SUCCESS

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-test	✔️

0xc0170 requested review from MarceloSalazar, hugueskamba and rajkan01 October 28, 2020 15:34

mergify bot added the release-version: feature branch label Oct 28, 2020

0xc0170 force-pushed the feature-cmake-fix-ninja-armclang branch from 7148f35 to 2e8d4fe Compare October 28, 2020 15:36

0xc0170 added the do not merge label Oct 28, 2020

ciarmcom requested a review from a team October 28, 2020 16:00

ciarmcom added the needs: review label Oct 28, 2020

0xc0170 force-pushed the feature-cmake-fix-ninja-armclang branch from 2e8d4fe to d3351ed Compare October 28, 2020 17:10

hugueskamba reviewed Oct 28, 2020

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

0xc0170 removed the do not merge label Oct 28, 2020

hugueskamba previously approved these changes Oct 28, 2020

View reviewed changes

0xc0170 force-pushed the feature-cmake-fix-ninja-armclang branch from 5d51a57 to ac73e62 Compare October 29, 2020 10:06

0xc0170 commented Oct 29, 2020

View reviewed changes

0xc0170 mentioned this pull request Oct 29, 2020

CMake: add CI testing #13477

Merged

0xc0170 added the needs: preceding PR label Oct 29, 2020

hugueskamba reviewed Oct 29, 2020

View reviewed changes

hugueskamba previously approved these changes Oct 29, 2020

View reviewed changes

0xc0170 and others added 3 commits October 30, 2020 08:26

CMake: 3.19.0 update to fix ARMClang ASM defines

27baeb0

CMake readme: update mbed-tools to 3.4

79329c5

0xc0170 force-pushed the feature-cmake-fix-ninja-armclang branch from 331cdc4 to 79329c5 Compare October 30, 2020 08:26

0xc0170 added needs: CI and removed needs: preceding PR needs: review labels Oct 30, 2020

0xc0170 removed the needs: CI label Oct 30, 2020

0xc0170 merged commit 61e9b7e into ARMmbed:feature-cmake Oct 30, 2020

0xc0170 deleted the feature-cmake-fix-ninja-armclang branch October 30, 2020 13:00

CMake: ninja reponse files disabled for ARMClang on Windows #13827

CMake: ninja reponse files disabled for ARMClang on Windows #13827

Uh oh!

Conversation

0xc0170 commented Oct 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

Uh oh!

0xc0170 commented Oct 28, 2020

Uh oh!

ciarmcom commented Oct 28, 2020

Uh oh!

Uh oh!

MarceloSalazar commented Oct 28, 2020

Uh oh!

0xc0170 commented Oct 29, 2020

Uh oh!

0xc0170 commented Oct 29, 2020

Uh oh!

0xc0170 commented Oct 29, 2020

Uh oh!

0xc0170 Oct 29, 2020

Choose a reason for hiding this comment

Uh oh!

MarceloSalazar commented Oct 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xc0170 commented Oct 29, 2020

Uh oh!

hugueskamba Oct 29, 2020

Choose a reason for hiding this comment

Uh oh!

0xc0170 Oct 29, 2020

Choose a reason for hiding this comment

Uh oh!

0xc0170 Oct 29, 2020

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Oct 30, 2020

Uh oh!

0xc0170 commented Oct 30, 2020

Uh oh!

mbed-ci commented Oct 30, 2020

Jenkins CI Test : ✔️ SUCCESS

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

Uh oh!

Uh oh!

0xc0170 commented Oct 28, 2020 •

edited

Loading

MarceloSalazar commented Oct 29, 2020 •

edited

Loading