Skip to content

Migrate old memap file handling into toolchains #8779

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 30, 2018
Merged

Migrate old memap file handling into toolchains #8779

merged 1 commit into from
Nov 30, 2018

Conversation

theotherjimmy
Copy link
Contributor

Description

This PR moves the old memap file handling required for differential
memap from within memap to the toolchain object. This has the
advantage that we can do the mv <app>.map <app>.map.old the moment
before it is overwritten by the linker. This should allow multiple
reruns of memap without modifying your build directory.

Pull request type

[ ] Fix
[x] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

@0xc0170 0xc0170 requested a review from a team November 16, 2018 16:27
bridadan
bridadan approved these changes Nov 16, 2018
View reviewed changes
Copy link
Contributor

@bridadan bridadan left a comment

Choose a reason for hiding this comment

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

Bueno 🎉

@cmonr
Copy link
Contributor

cmonr commented Nov 16, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Nov 16, 2018

Build : SUCCESS

Build number : 3650
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8779/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Nov 17, 2018

Exporter Build : SUCCESS

Build number : 3257
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/8779/

@mbed-ci
Copy link

mbed-ci commented Nov 17, 2018

Test : FAILURE

Build number : 3430
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/8779/3430

@cmonr
Copy link
Contributor

cmonr commented Nov 17, 2018

NRF test issue.

Will restart in rollup.

@0xc0170 0xc0170 removed the rollup PR label Nov 19, 2018
@0xc0170
Copy link
Contributor

0xc0170 commented Nov 19, 2018

As noted earlier, this conflics with 8607 that is already part of one rollup. This should go alone as it is and might need conflict resolution after 8607 lands.

@theotherjimmy
Copy link
Contributor Author

@0xc0170 There was no prior mention of #8607 in this thread. Perhaps you're thinking of another PR.

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 19, 2018

Correct, another PR. Anyway, had a problem with a first rollup of the day. will do second and this will be in there

@cmonr
Copy link
Contributor

cmonr commented Nov 22, 2018

Starting CI

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 22, 2018

@theotherjimmy Can you rebase , there was tools PR integrated today that is causing now conflict.

@theotherjimmy
Migrate old memap file handling into toolchains
38d8bb6 
### Description

This PR moves the old memap file handling required for differential
memap from within memap to the toolchain object. This has the
advantage that we can do the `mv <app>.map <app>.map.old` the moment
before it is overwritten by the linker. This should allow multiple
reruns of memap without modifying your build directory.

### Pull request type

    [ ] Fix
    [x] Refactor
    [ ] Target update
    [ ] Functionality change
    [ ] Docs update
    [ ] Test update
    [ ] Breaking change
@bulislaw
Copy link
Member

All the PRs need to be engineering ready (marked as "needs: CI") by the end of the day (Austin). Otherwise it won't make 5.11 and will need to come in the next patch release (5.11.1).

@theotherjimmy
Copy link
Contributor Author

@0xc0170 Rebased.

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 26, 2018

@theotherjimmy This would be fine for the next patch release, Shall we move this to 5.11.1 ? 5.11rc1 is still having quite number of PR to get through

@theotherjimmy
Copy link
Contributor Author

@0xc0170 Feel free to do that.

@cmonr
Copy link
Contributor

cmonr commented Nov 29, 2018

CI started

@mbed-ci
Copy link

mbed-ci commented Nov 29, 2018

Test run: SUCCESS

Summary: 4 of 4 test jobs passed
Build number : 6
Build artifacts
Build logs

@cmonr cmonr mentioned this pull request Nov 30, 2018
Closed
@0xc0170
Copy link
Contributor

0xc0170 commented Nov 30, 2018

Not related failure, see referenced PR above. We will restart once it is fixed

@0xc0170 0xc0170 closed this Nov 30, 2018
@0xc0170 0xc0170 reopened this Nov 30, 2018
@0xc0170 0xc0170 removed the needs: CI label Nov 30, 2018
@0xc0170
Copy link
Contributor

0xc0170 commented Nov 30, 2018

Wrong button, reopened :-) Will restart once master is green

@mbed-ci
Copy link

mbed-ci commented Nov 30, 2018

Build : SUCCESS

Build number : 3704
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8779/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 30, 2018

This restarted old CI, aborting now.

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 30, 2018

@cmonr Can you take care of the statuses for morph here please?

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 30, 2018

CI started

@mbed-ci
Copy link

mbed-ci commented Nov 30, 2018

Test run: SUCCESS

Summary: 4 of 4 test jobs passed
Build number : 7
Build artifacts
Build logs

@cmonr cmonr merged commit 2b46b1d into ARMmbed:master Nov 30, 2018
@theotherjimmy theotherjimmy mentioned this pull request Jan 17, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xc0170 0xc0170 0xc0170 approved these changes

@bridadan bridadan bridadan approved these changes

Assignees
No one assigned
Labels
release-version: 5.11.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@theotherjimmy @cmonr @mbed-ci @0xc0170 @bulislaw @bridadan @NirSonnenschein