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

CLI2: copy mapfile for diff statistics #15110

Closed
wants to merge 4 commits into from

Conversation

JojoS62
Copy link
Contributor

@JojoS62 JojoS62 commented Sep 27, 2021

Summary of changes

copy the linker map file to target.elf.map.old. The memap.py statistics generator looks for a given filename +'.old'. If it exists, it generates the difference to previous module sizes as with CLI1.
Current behaviour is not copy the map file, so this feature is not yet available in CLI2 builds.

Impact of changes

The file(COPY_FILE, ...) function in cmake requires cmake 3.21, mbed is using currently 3.19. This PR maybe hold back until mbed upgrades to require cmake >= 3.21.

Migration actions required

Documentation


Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[x] 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)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers


@ciarmcom
Copy link
Member

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

@ladislas
Copy link
Contributor

Ah that's great! I really missed this feature and ended up implementing a workaround copying both old and new map files and then using diff. It works but not as nice.

@JojoS62
Copy link
Contributor Author

JojoS62 commented Sep 27, 2021

yes, the only problem maybe that it requires a relative new cmake version. I have tried with other existing function, but without the RESULT 0 feature, the functions throw an error if the file not exists or wants to overwrite an existing one.

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 27, 2021

cmake_minimum_required(VERSION 3.19.0 FATAL_ERROR)

As it requires a new version, it should update the version in the main CMakeLists. As I know, we provided at least 2 fixes in CMake for armclang, it would be nice to have them. 3.21.3 should have them

@JojoS62
Copy link
Contributor Author

JojoS62 commented Sep 27, 2021

a quick search shows that it will affect >100 CMakeLists.txt, a lot in the tests. I guess there is no common setting and all need to be updated?

I will try.

@mergify mergify bot added the needs: work label Sep 28, 2021
@mergify
Copy link

mergify bot commented Sep 28, 2021

This PR cannot be merged due to conflicts. Please rebase to resolve them.

@mergify mergify bot removed the needs: review label Sep 28, 2021
@JojoS62
Copy link
Contributor Author

JojoS62 commented Sep 28, 2021

closing because of problems with rebase, I will create a new PR

@JojoS62
Copy link
Contributor Author

JojoS62 commented Sep 28, 2021

replaced by #15117 due to rebase problems

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants