Skip to content
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

- Revamp vsomeip cmake. Initial implementation #15

Closed
wants to merge 1 commit into from

Conversation

heliocastro
Copy link

All cmake buildsystem was redone to get rid of hardcoded config files,
proper subdirectory, generation of namespaces.
For modern projects using vsomeip, only link the proper library is
needed, and cmake provides the necessary extra link and header
includes. This method avois polute the CMakeFiles with include_directory
lines or global link flags.
To link in a modern way, applications developer will just need add
target_link_library( vsomeip::vsomeip)
Install process now uses GNUInstallDirs that is acceptable on both
windows and Linux side and with additions on the generator of config
files, the current generation will be backward compatible older
implementation.
Some of files that are generated on source tree now are generated on
build tree, protecting git clones to be poluted with untracked files.
A second cleanout will ve necessary over the multiple json redefinitions
on testing configuration files.

@gunnarx
Copy link

gunnarx commented Feb 5, 2018

I think it looks like a fantastic contribution! Maintainer will need to review and comment though...

All cmake buildsystem was redone to get rid of hardcoded config files,
proper subdirectory, generation of namespaces.
For modern projects using vsomeip, only link the proper library is
needed, and cmake provides the necessary extra link and header
includes. This method avois polute the CMakeFiles with include_directory
lines or global link flags.
To link in a modern way, applications developer will just need add
target_link_library(<target> vsomeip::vsomeip)
Install process now uses GNUInstallDirs that is acceptable on both
windows and Linux side and with additions on the generator of config
files, the current generation will be backward compatible older
implementation.
Some of files that are generated on source tree now are generated on
build tree, protecting git clones to be poluted with untracked files.
@lutzbichler
Copy link
Collaborator

The changes to the CMakeFiles seem to require changes to the source files (relative pathes to internal header files are replaced by references based on an include path). We cannot accept this.

@gunnarx
Copy link

gunnarx commented Jul 24, 2019

Is this really an all or nothing situation @lutzbichler? I thought you would explain which parts are not OK - for example commenting on the #include statements specifically.

But I do agree that it seems a bit difficult to separate the useful parts from other parts, so therefore I want to ask @heliocastro: It seems to me that this should be split up into smaller changes, and you should clarify if the changes you have made in the source file (that have not only modifying #include statements, but also some other things) are really necessary, in order to "revamp" the cmake? It seems strange to me if the CMakeFile changes also require you to change the #include statements. Is it not possible to modernize the CMakeFiles separately without all the other changes? And maybe separately try to explain what you aim to achieve with the other changes?

Of course I also admit this PR is so extremely old now, that "starting over" is probably an easier approach anyway, if @heliocastro is still around and still willing to contribute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants