Skip to content

Allow configuration of artifact name in app config #4107

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

Merged
merged 1 commit into from
Apr 10, 2017
Merged

Allow configuration of artifact name in app config #4107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions tools/build_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,8 +481,8 @@ def build_project(src_paths, build_path, target, toolchain_name,
build_profile=build_profile)

# The first path will give the name to the library
if name is None:
name = basename(normpath(abspath(src_paths[0])))
name = (name or toolchain.config.name or
basename(normpath(abspath(src_paths[0]))))
toolchain.info("Building project %s (%s, %s)" %
(name, toolchain.target.name, toolchain_name))

Expand Down
10 changes: 9 additions & 1 deletion tools/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,8 @@ class Config(object):
"library": {"name": str, "config": dict, "target_overrides": dict,
"macros": list, "__config_path": str},
"application": {"config": dict, "target_overrides": dict,
"macros": list, "__config_path": str}
"macros": list, "__config_path": str,
"artifact_name": str}
}

__unused_overrides = set(["target.bootloader_img", "target.restrict_size"])
Expand Down Expand Up @@ -797,6 +798,13 @@ def validate_config(self):
return True


@property
def name(self):
if "artifact_name" in self.app_config_data:
Copy link
Contributor

Choose a reason for hiding this comment

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

Not completely familiar with this file yet.... will this work for mbed_lib.json as well as mbed_app.json?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. Just mbed_app.json

Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

@bridadan bridadan Apr 7, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Strangely enough, you can put an mbed_app.json in the root of a library build folder and it will be picked up.

Copy link
Contributor

Choose a reason for hiding this comment

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

What is def name not artifact_name if that is what it modifies or returns?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@theotherjimmy theotherjimmy Apr 10, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

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.

return self.app_config_data["artifact_name"]
else:
return None

def load_resources(self, resources):
""" Load configuration data from a Resources instance and expand it
based on defined features.
Expand Down
18 changes: 8 additions & 10 deletions tools/test/build_api/build_api_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,8 @@ def test_prepare_toolchain_app_config(self, mock_config_init):
mock_target = namedtuple("Target",
"init_hooks name features core")(lambda _, __ : None,
"Junk", [], "Cortex-M3")
mock_config_init.return_value = namedtuple("Config",
"target has_regions")(
mock_target,
False)
mock_config_init.return_value = namedtuple(
"Config", "target has_regions name")(mock_target, False, None)

prepare_toolchain(self.src_paths, None, self.target, self.toolchain_name,
app_config=app_config)
Expand All @@ -106,10 +104,8 @@ def test_prepare_toolchain_no_app_config(self, mock_config_init):
mock_target = namedtuple("Target",
"init_hooks name features core")(lambda _, __ : None,
"Junk", [], "Cortex-M3")
mock_config_init.return_value = namedtuple("Config",
"target has_regions")(
mock_target,
False)
mock_config_init.return_value = namedtuple(
"Config", "target has_regions name")(mock_target, False, None)

prepare_toolchain(self.src_paths, None, self.target, self.toolchain_name)

Expand All @@ -133,7 +129,8 @@ def test_build_project_app_config(self, mock_prepare_toolchain, mock_exists, _,
app_config = "app_config"
mock_exists.return_value = False
mock_prepare_toolchain().link_program.return_value = 1, 2
mock_prepare_toolchain().config = namedtuple("Config", "has_regions")(None)
mock_prepare_toolchain().config = namedtuple(
"Config", "has_regions name")(None, None)

build_project(self.src_paths, self.build_path, self.target,
self.toolchain_name, app_config=app_config)
Expand Down Expand Up @@ -161,7 +158,8 @@ def test_build_project_no_app_config(self, mock_prepare_toolchain, mock_exists,
mock_exists.return_value = False
# Needed for the unpacking of the returned value
mock_prepare_toolchain().link_program.return_value = 1, 2
mock_prepare_toolchain().config = namedtuple("Config", "has_regions")(None)
mock_prepare_toolchain().config = namedtuple(
"Config", "has_regions name")(None, None)

build_project(self.src_paths, self.build_path, self.target,
self.toolchain_name)
Expand Down