Fix treatment of subcomponents

[PenguinLemma]

As seen in issue #69, evoke is not handling corretly subcomponents. They are always taken to be system components. Also, the cmake generation is not working properly as the directories are added as parent_subomponent instead of parent/subcomponent.

This pull request does the following to fix that:

• Add hierarchical_name to Component, that will contain a hierarchically correct name (i.e., with / instead of _ between parent and subomponent).
• Use this name to search for a component in the project (this happens in Project::IsSystemComponent).
• Use this name when adding subdirectories (evoke-generated cmake files) so that IDEs will correctly recognize the structure of the project
• Add some tests

Note that Component::name will contain the name that the resulting lib/obj/bin will have, i.e. "parent_subcomponent". We could drop this member variable (as it can be obtained by substituting slashes by underscores from hierarchical_name, but the underscode version of the name of a component is required in different parts of evoke and making the substitution every time could affect performance. Also, since underscore might be used as part of a component's name by the user, hierarchical_name is not easily recoverable from name.

[hl4]

There are maybe some misunderstands.

First, the name of submodules are designed to concat the parts of folder name, separated with underscore instead using subfolder name, in that case we may use -L options to specify the path of library.

Let's say, we have two component, located the path is some/user/name/component and other/user/component we build it to a library named lib/libsome_user_name_component.a instead of lib/some/user/name/libcomponent.a. we may link it to some executable with option -lsome_user_name_component and -Llib, even if this is also a library named component.

I think it is not necessary relation that one component is parent of other.

Second, the member root of component class is a relative path to project, it can be used as hierarchical name, simply called root.string().

In my option, you may only fixed the generator of the cmakelist. It should be an issue.

[PenguinLemma]

Hi @hl4

Thank you for your response and for looking into the request. Let's see if we can walk through the process and make sure the behaviour I saw and intended to change had to indeed be changed.

First, about the component library names, that would stay unchanged. They will still be libcomponent_subcomponent.a and not lib/component/libsubcomponent.a.

Now, I realised with a couple of different examples that there were never libraries created inside my project for subcomponents. And reading the CMakeLists confirmed it: subcomponents were treated as system libraries.

As far as I understand, to decide whether a component is a project component or a system component we are checking whether the name of that component is in project.components or not. And components are added to project.component in lines 180 and 183 in Project.cpp. In the case of a component that is not unit tests, line 180.

The exact command is:

components.emplace(it->path().string(), it->path());

So the name of the component inside components comes from the path to it. That means that a subcomponent will be saved in std::unordered_map<std::string, Component> components with key "component/subcomponent", and not "component_subcomponent". But again, when we check for a component later on to decide whether it's the project's component or system's component, we are looking for "component_subcomponent". And that is why all subcomponents were treated, for my understanding, as system components.

My proposed solution consists on including the slash-separated path-like name of the component inside a Component, and use that name both when using add_subdirectories and when checking in a component is in project.components. As far as I could see when re-runing evoke containing the proposed changes in several projects, now subcomponents are treated as project components when they are indeed and any other case is left intact.

Please, let me know if anything is unclear or if indeed there is a mismatch between my understanding of this part of evoke or its intended behaviour and the reality of it.

[hl4]

It's clever to use the prefix of component to distinguish system component from project components. I agree with that. but those components without a prefix, which is direct sub-folder of the project root folder, would be an issue. Should we consider ./ a prefix? The project.components member field would be an issue, too. I don't find any code use the field for component, except the CMakeProjectExportor. I think you can make this much better.

[PenguinLemma]

For those components that are directly in the root folder, both names will coincide, so there is no change needed. At least I can see no issue.

About the second part, I am not sure I follow you. You are right that the changes only affect the CMake generation, but there were no changes done to project.components. Only Component class was added a member variable and the rest of the changes are in CMakeProjectExporter.

As for making it much better, the main problem we have is that we need to be able to recover path-like names in CMakeProjectExporter. Deducing them from the underscore-separated names is out of the question because I could involved lots of comparisons, searches and most likely conflicts, given the freedom of component naming that the user has.

What other options do we have? These are the two that come to mind that wouldn't imply doing extensive searches for possible prefixes:

1. Proposed solution (keep that name together with the underscore version inside component).
2. In CMakeProjectExporter add a member variable std::unordered_map< std::string, std::string> hierarchical_names, and then add

   for(auto& comp : project.components)
   {
   hierarchical_names.emplace(comp.second.GetName(), comp.first);
   }

   to the beginning of CMakeProjectExporter::createCMakeListsFiles. Then use hierarchical_names[comp.second.GetName()] every time the dash-separated path-like name of a component is needed (i.e., in those places in CMakeProjectExporter in which GetHierarchicalName() was used in my first proposed solution.

In terms of memory, solution 1 looks to me more efficient, but the impact would be minimum (unless you have hundreds of components). In terms of run-time, probably solution 1 is better, as it accesses directly those names instead of checking in an std::unordered_map, but solution 2 can be implemented so that run-time is virtually the same. In terms of isolation of changes, the second solution is better, as all necessary changes will be in CMakeProjectExporter, which is the component that presented the problem.

I didn't think initially of solution 2 as I thought the issue could be propagated to other parts of evoke, so sorry for that and thank you for helping me give it a good second thought.

Do you think it would be best to implement approach 2, then?