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

Ignore build directory from scan resources #3852

Merged
merged 4 commits into from Mar 9, 2017

Conversation

Projects
None yet
4 participants
@theotherjimmy
Contributor

theotherjimmy commented Feb 28, 2017

This is a bug fix for the following bug (Github issue ARMmbed/mbed-cli#437):

If two builds were run specifying a non-default build folder, the second
build would fail to link with duplicate symbols and may not fit on the
device. The root of this problem is that these non-default build folders
are not ignored by scan-resources, and therefore included in the build.

We fix this bug by ignoring the build directory passed into the tools.

@screamerbg

This comment has been minimized.

Member

screamerbg commented Feb 28, 2017

Can't we have build for as param to scan resources and defaulting to None if not specified?

This will make it backwards compatible and also won't throw nasty python error if build dir is not assigned before scan resources

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Mar 1, 2017

Build dir is assigned in the constructor

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Mar 6, 2017

Build dir is assigned in the constructor

Does that answer your question @screamerbg ?

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Mar 6, 2017

Hey @0xc0170 New implementation coming soon.

@theotherjimmy theotherjimmy force-pushed the theotherjimmy:ignore-build-dir branch 5 times, most recently Mar 6, 2017

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Mar 6, 2017

@screamerbg Could you review again?

@theotherjimmy theotherjimmy force-pushed the theotherjimmy:ignore-build-dir branch 4 times, most recently Mar 6, 2017

Ignore build directory from scan resources
This is a bug fix for the following bug (Github issue #437):

If two builds were run specifying a non-default build folder, the second
build would fail to link with duplicate symbols and may not fit on the
device. The root of this problem is that these non-default build folders
are not ignored by scan-resources, and therefore included in the build.

We fix this bug by ignoring the build directory passed into the tools.

@theotherjimmy theotherjimmy force-pushed the theotherjimmy:ignore-build-dir branch to fbb6f71 Mar 6, 2017

theotherjimmy added some commits Mar 7, 2017

Re-factor mbed2 lib builds to use prepare_toolchain
The prior patch in this series makes the assumption that any building
will go through `build_api.prepare_toolchain`. This was not a valid
assumption for the mbed2 build process. So, instead of maintaining 2
ways of using the toolchain classes, I elected to unify on
`prepare_toolchain`.
Remove static analysis scan
An earlier patch in this series relies on the assumption that all
toolchain construction goes through `prepare_toolchain`. This is still
not the case. The only remaining user of the `mbedToolchain` object that
does not go through `prepare_toolchain` is the static analysis scanner.
It's basically dead-code at this point. I say we remove it. So this
patch removes it.
Update test_api call to prepare_toolchain
An earlier patch in this series changed the API for
`build_api.prepare_toolchain`. This commit updates the `find_test`
function to call `prepare_toolchain` correctly.
@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Mar 7, 2017

This snowballed pretty good.

@screamerbg

Looks great! And you took care of some legacy mbed 2.0 routines. Sweet!

@screamerbg

This comment has been minimized.

Member

screamerbg commented Mar 7, 2017

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Mar 7, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1650

All builds and test passed!

@0xc0170 0xc0170 merged commit e9158f4 into ARMmbed:master Mar 9, 2017

4 checks passed

Cam-CI uvisor Build & Test Success
Details
ci/morph-test Job has started
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@0xc0170 0xc0170 referenced this pull request Mar 16, 2017

Merged

Don't set the build_dir to anything on export #3924

1 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment