vere icon indicating copy to clipboard operation
vere copied to clipboard

disambiguate header includes

Open barter-simsum opened this issue 2 years ago • 5 comments

  1. Add pkg to all package include paths
  2. For local dependencies, header includes should use an unambiguous path, e.g.
#include "noun/noun.h"
#include "vere/ivory.h"
#include "ur/ur.h"
...

Obviously this avoids name collisions, but I think it also makes the dependency tree of any C file more legible.

barter-simsum avatar Feb 09 '23 21:02 barter-simsum

@mcevoypeter @matthew-levan Side note:

why are external dependencies like sigsegv, openssl, h2o, etc. specified like local dependencies. Can we include these third party dependencies like system headers instead?

Take vere/main.c for instance:

#include "noun.h"
#include "ivory.h"
#include "ur.h"
#include "platform/rsignal.h"
#include "vere.h"
#include "sigsegv.h"
#include "openssl/conf.h"
#include "openssl/engine.h"
#include "openssl/err.h"
#include "openssl/ssl.h"
#include "h2o.h"
#include "curl/curl.h"
#include "db/lmdb.h"
#include "getopt.h"
#include "libgen.h"

#include "ca_bundle.h"
#include "pace.h"
#include "version.h"
#include "whereami.h"

barter-simsum avatar Feb 09 '23 21:02 barter-simsum

We build h2o, openssl, and other third-party dependencies ourselves (you can see them and others in bazel/third_party) so the double-quote syntax is appropriate as we don't want to tell the compiler to look for those header files on the system. The Bazel guidelines on include paths are worth a look.

matthew-levan avatar Feb 09 '23 21:02 matthew-levan

Discussed this with Matt. Above is definitely a separate issue -- just something that occurred to me while messing with the include directives.

It's unclear to me why bazel recommends not using #include <lib.h> style includes if it's running in a chrooted environment. Maybe getting third party dependencies into the include path would be too much hassle. Anyway, it's a separate issue.

barter-simsum avatar Feb 10 '23 00:02 barter-simsum

I'm happy with prepending folders to our own header file paths (like "noun/noun.h" instead of "noun.h" but I'm not sure of the cleanest way to do it. @mcevoypeter any thoughts?

matthew-levan avatar Feb 10 '23 13:02 matthew-levan

The best way to disambiguate the header paths is to use the include_prefix attribute of cc_library. //pkg/c3:c3 actually already does this, allowing us to specify #include "c3.h" (if unambiguous) or #include "c3/c3.h" (if c3.h is ambiguous). The reason all other library targets lack this use of include_prefix was simply a mistake on my part.

mcevoypeter avatar Feb 10 '23 15:02 mcevoypeter