Skip to content

Change CMAKE_BINARY_DIR to CMAKE_CURRENT_BINARY_DIR #14272

New issue

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

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

Already on GitHub? # to your account

Merged

Conversation

ladislas
Copy link
Contributor

Summary of changes

Following comment #14265 (comment), this PR changes the use of CMAKE_BINARY_DIR to CMAKE_CURRENT_BINARY_DIR.

This will put target specific files (.elf, .map, .bin, .hex, etc.) in their respective target directories, like this:

image

This change paves the way to "multiple target support" in the near future.

Impact of changes

None

Migration actions required

None

Documentation

None


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 @hugueskamba @rwalton-arm @Patater @rajkan01


@ciarmcom ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Feb 11, 2021
@ciarmcom
Copy link
Member

@ladislas, thank you for your changes.
@rajkan01 @0xc0170 @rwalton-arm @Patater @hugueskamba @ARMmbed/mbed-os-tools @ARMmbed/mbed-os-maintainers please review.

Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder where we would use CMAKE_BINARY_DIR as we always related to the current target.
It looks like the current changes in this pull request should be current binary dir.

@mergify mergify bot added needs: CI and removed needs: review labels Feb 12, 2021
@ladislas
Copy link
Contributor Author

I wonder where we would use CMAKE_BINARY_DIR as we always related to the current target.
It looks like the current changes in this pull request should be current binary dir.

@0xc0170 I'm not sure I understand what you mean :)

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 12, 2021

I might have not clearly stated, just that this change is all good. I am only wondering if the CMAKE_BINARY_DIR has any use for us in Mbed OS (if we ever need to use it). hypothetical question

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 12, 2021

CI started

@mbed-ci
Copy link

mbed-ci commented Feb 12, 2021

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_build-greentea-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-test ✔️
jenkins-ci/mbed-os-ci_greentea-test ✔️

@0xc0170 0xc0170 merged commit ba1e7b5 into ARMmbed:master Feb 14, 2021
@mergify mergify bot removed the ready for merge label Feb 14, 2021
@mbedmain mbedmain added release-version: 6.8.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Feb 16, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants