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

Including headers from the build directory is not the best practice #217

Open
ehein6 opened this issue Jan 11, 2017 · 1 comment
Open

Including headers from the build directory is not the best practice #217

ehein6 opened this issue Jan 11, 2017 · 1 comment

Comments

@ehein6
Copy link
Contributor

ehein6 commented Jan 11, 2017

There are a few headers like stinger_defs.h.in that are generated at configure time. So the header file stinger_defs.h is actually a generated build artifact that doesn't belong in the source directory. Source files still need to know where it is. So the current build system copies the file to an include subdirectory in the build dir and adds this to the includes.

For better or for worse, ALL headers are currently handled this way. They are copied to the build directory and included from there. This has several consequences, most of which I consider negatives.

  1. The include path is not the same as the location of the file in the source tree. For example, pagerank.h is located in lib/stinger_alg/inc/, but in the build directory it is include/stinger_alg/pagerank.h
  2. When using "goto definition" in an IDE, the IDE will locate definitions in the build directory copy of the header, but edits to this copy will be lost before the next build.
  3. CMake loses the ability to track dependencies between libraries. If stinger_alg needs a header from stinger_net (but not a link dependency) we have to ensure that the headers get copied before stinger_alg is built or the build will fail. CMake will figure out the dependency DAG with linking, but it assumes headers will just "be there" unless you explicitly tell it that you depend on a header file that doesn't exist yet

The only advantage I see is in allowing libraries like stinger_core to control what their public interface looks like, possibly not publishing all their headers. But this would be better handled by an "install" target anyways.

Instead, all headers should simply be included from the source directory. A custom solution can be implemented for the few headers that need to be generated during the build.

@jpfairbanks
Copy link

Yes I agree that all of these things are negatives. Perhaps a more permanent solution is to reduce the number of things that are compile time constants that need to be generated. Is there an easy way to find out what is preventing that?

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

No branches or pull requests

2 participants