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

Generate config.h in build directory not in source #88

Closed

Conversation

hpwxf
Copy link
Collaborator

@hpwxf hpwxf commented Sep 5, 2021

Generating config.h in source directory is not convenient for multiple builds on the same sources and when updating using VCS

I suggest generating it into the binary directory and to add it to include search path.

config.h in source directory is not convenient for multiple builds on same source and when updating using VCS
@hpwxf hpwxf marked this pull request as draft September 5, 2021 10:55
@hpwxf hpwxf marked this pull request as ready for review September 5, 2021 11:04
@RUrlus
Copy link
Owner

RUrlus commented Sep 5, 2021

Hi @hpwxf, happy to make the change regarding the installation on the config header but I want to retain a default version of the header in carma_bits because otherwise the lib is dependent on CMake and not really header only. Someone opened an issue on this a while ago.

@hpwxf
Copy link
Collaborator Author

hpwxf commented Sep 5, 2021

Ok, I understand.

I could do a workaround where there is default config.h in source. This config.h includes a generated_config.h if it is found. Using something like:

#ifdef __has_include // requires C++17 else do nothing
#  if __has_include("generated_config.h")
#   include "generated_config.h"
#   define USE_GENERATED_CONFIG
# endif
#endif

#ifndef USE_GENERATED_CONFIG

/* default config.h content here */

#endif

What do you think about it?

Default config.h can be used for header only configuration.
It switches to a config file generated by CMake in build directory if available.
@hpwxf
Copy link
Collaborator Author

hpwxf commented Sep 7, 2021

I have updated my PR branch using this idea.

@RUrlus
Copy link
Owner

RUrlus commented Sep 20, 2021

Hi @hpwxf, thanks for the updated PR. Sorry, I've been on holiday and haven't been able to pick this up.
I like the idea of the approach but I would prefer to avoid a solution that only works for C++17.

What about setting a definition through CMake and having something like the below in config.h?

#ifdef CARMA_USE_GENERATED_CONFIG
  #include "carma_bits/generated_config.h"
#else
... default config ...
#endif 

@RUrlus
Copy link
Owner

RUrlus commented Oct 28, 2021

Ha @hpwxf, I incorporated the above in #94

@RUrlus RUrlus closed this Oct 28, 2021
@hpwxf
Copy link
Collaborator Author

hpwxf commented Nov 1, 2021

Thank you for

Ha @hpwxf, I incorporated the above in #94

Hi @hpwxf, thanks for the updated PR. Sorry, I've been on holiday and haven't been able to pick this up. I like the idea of the approach but I would prefer to avoid a solution that only works for C++17.

What about setting a definition through CMake and having something like the below in config.h?

#ifdef CARMA_USE_GENERATED_CONFIG
  #include "carma_bits/generated_config.h"
#else
... default config ...
#endif 

Sorry, I was also far far away.
I will check v0.62 as soon as possible.

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.

2 participants