Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMake: Only force Ninja response files on Windows #14656

Merged
merged 1 commit into from
May 13, 2021

Conversation

rwalton-arm
Copy link
Contributor

Summary of changes

We force Ninja to create response files to work around file path length
limitations on Windows. Previously we were forcing the response file
generation for all platforms, unless we were building on Windows using
ARMClang, this was to work around a CMake issue
https://gitlab.kitware.com/cmake/cmake/-/issues/21093

Unfortunately ar on macOS doesn't support Ninja response files. So the
check was incorrect, as forcing a response file to be generated on macOS
causes a build failure when trying to build Mbed's unit tests.

This PR changes the logic so we only generate a response file on Windows
systems if we're not building with ARMClang.

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

[] 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


We force Ninja to create response files to work around file path length
limitations on Windows. Previously we were forcing the response file
generation for all platforms, unless we were building on Windows using
ARMClang, this was to work around a CMake issue
https://gitlab.kitware.com/cmake/cmake/-/issues/21093

Unfortunately `ar` on macOS doesn't support Ninja response files. So the
check was incorrect, as forcing a response file to be generated on macOS
causes a build failure when trying to build Mbed's unit tests.

This PR changes the logic so we only generate a response file on Windows
systems if we're not building with ARMClang.
@mergify mergify bot added the needs: CI label May 11, 2021
@mbed-ci
Copy link

mbed-ci commented May 11, 2021

Jenkins CI Test : ✔️ SUCCESS

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

CLICK for Detailed Summary

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

@0xc0170
Copy link
Contributor

0xc0170 commented May 12, 2021

The initial aim was to have the same behavior on all platforms and exclude the problematic one (if fixed, we can remove the exclusion). As we can see, response files have more issues unfortunately.

The ar issue is even reported on CMake for 4 years without resolution (https://gitlab.kitware.com/cmake/cmake/-/issues/16731).

@0xc0170 0xc0170 requested a review from a team May 12, 2021 07:17
@0xc0170 0xc0170 added the release-type: patch Indentifies a PR as containing just a patch label May 12, 2021
@adbridge adbridge merged commit 117ecd5 into ARMmbed:master May 13, 2021
@mbedmain mbedmain removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels May 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants