Allow configuration of artifact name in app config #4107
Conversation
|
Nice! |
| @@ -797,6 +798,13 @@ def validate_config(self): | |||
| return True | |||
|
|
|||
|
|
|||
| @property | |||
| def name(self): | |||
| if "artifact_name" in self.app_config_data: | |||
There was a problem hiding this comment.
Not completely familiar with this file yet.... will this work for mbed_lib.json as well as mbed_app.json?
There was a problem hiding this comment.
Nope. Just mbed_app.json
There was a problem hiding this comment.
I just thought of that based on seeing related issues in the past (ARMmbed/mbed-cli#229). Not sure if mbed_lib.json is used with mbed compile --library?
Also not sure whether working for both is needed for consistency?
There was a problem hiding this comment.
I would suggest only doing this for apps for now. Not every mbed_lib.json corresponds to an archived library build. In fact, it almost never works like this. At least you know there is only one app, so you can lock down the name of the produced binary.
There was a problem hiding this comment.
Strangely enough, you can put an mbed_app.json in the root of a library build folder and it will be picked up.
There was a problem hiding this comment.
What is def name not artifact_name if that is what it modifies or returns?
There was a problem hiding this comment.
Looking here: https://github.com/theotherjimmy/mbed/blob/6a646eb4b22ad24a42d8533af6c126433496fd08/tools/build_api.py#L484-L485 The default behavior did not change: the directory name of the parent directory of the project is used.
There was a problem hiding this comment.
Also, agreed (with the thumbs down): we should discuss how naming from configs should affect the library name. If we come up with something, I will submit another PR that implements that thing that we came up with.
It would help to provide examples how this is used |
|
Please look at travis failure |
theotherjimmy
force-pushed
the
config-name-app
branch
from
9fae9c4 to
6a646eb
Compare
|
Travis failure a test case API problem. I added to the API, and used the API, but never updated the test cases to mock the API. That's resolved now. |
|
Looks great to me, pending whatever jenkins is unhappy about. It'll need a docs update, is there a procedure for enforcing docs being kept up to date with code functionality changes? |
I restarted it |
|
/morph test |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
This patch allows a user to configure the artifact name through the
mbed_app.jsonfile.Suggested by @andrewleech in #4103
Example
{ "artifact_name" : "FooBar" }This will rename the executable in your application from
<dirname(app-dir)>.bintoFooBar.binThe precedence is (fall through if not specified):
mbed_app.jsonartifact_namekey