Skip to content

enh: Move CMake defines to separate header, unused by Bazel #235

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.

Sign up for GitHub

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

Jump to bottom
Merged
schuhschuh merged 1 commit into from
Sep 1, 2017
Merged

enh: Move CMake defines to separate header, unused by Bazel #235

schuhschuh merged 1 commit into from
Sep 1, 2017

Conversation

@schuhschuh
Copy link
Member

@schuhschuh schuhschuh commented Aug 31, 2017

This change avoids having to configure a private .h.in file during the Bazel build such that these files are not available to dependent projects in the GENDIR of the external gflags project.

Alternative change which fixes #233 instead of PR #234.

For no good reason, Bazel seems to add the GENDIR to the include path of dependent projects, which cannot be fixed easily. A solution would be to place private generated header files in a subdirectory as suggested in #234, but unfortunately the path of this directory cannot be referenced with a macro such as $(GENDIR) which differs when gflags is used as external dependency in another project. A possible solution I explored was to use $(location :config_h), but this gives the full file path, not the directory path. Using $(location :config_h)/.. seemed to work at some point, but isn't very pretty either.

So, instead of trying to get things right for Bazel, simply avoid having to have a genrule for config.h in the first place, as this rule is mainly a dummy to get rid of unused #cmakedefines.

@schuhschuh
enh: Move CMake defines to separate header, unused by Bazel
99df065 
This change avoids having to configure a private .h.in file during the Bazel build such that these files are not available to dependent projects in the GENDIR of the external gflags project.
@schuhschuh schuhschuh added this to the v3.0.0 milestone Aug 31, 2017
@schuhschuh schuhschuh mentioned this pull request Aug 31, 2017
Closed
@drigz drigz mentioned this pull request Aug 31, 2017
Merged
@drigz
Copy link

drigz commented Aug 31, 2017

LGTM.

For no good reason, Bazel seems to add the GENDIR to the include path of dependent projects

I think it does this so that the original approach to includes (repository relative paths, eg #include "third_party/gflags/gflags.h") works with generated headers too. Unfortunately, this conflicts unpleasantly with the structure of many open-source projects as we see here.

Interestingly, even though the -iquote path to the generated files without the prefix is still passed to the compiler (wrapper), I couldn't include gflags.h without the gflags/ prefix

This appears to be the sandbox doing its job:

➜  config_h bazel build -s :main --spawn_strategy=standalone  
INFO: Analysed target //:main (0 packages loaded).
INFO: Found 1 target...
SUBCOMMAND: # //:main [action 'Compiling main.cc']
(cd /usr/local/google/home/rodrigoq/.cache/bazel/_bazel_rodrigoq/e1b1a7dc54e5baf9bf675d7aafb0181c/execroot/__main__ && \
  exec env - \
    PWD=/proc/self/cwd \
  /usr/bin/gcc -U_FORTIFY_SOURCE -fstack-protector -Wall -B/usr/bin -B/usr/bin -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer '-std=c++0x' -MD -MF bazel-out/local-fastbuild/bin/_objs/main/main.pic.d '-frandom-seed=bazel-out/local-fastbuild/bin/_objs/main/main.pic.o' -fPIC -iquote . -iquote bazel-out/local-fastbuild/genfiles -iquote external/com_github_gflags_gflags -iquote bazel-out/local-fastbuild/genfiles/external/com_github_gflags_gflags -iquote external/bazel_tools -iquote bazel-out/local-fastbuild/genfiles/external/bazel_tools -Ibazel-out/local-fastbuild/bin/external/com_github_gflags_gflags/_virtual_includes/gflags -isystem external/bazel_tools/tools/cpp/gcc3 -fno-canonical-system-headers -Wno-builtin-macro-redefined '-D__DATE__="redacted"' '-D__TIMESTAMP__="redacted"' '-D__TIME__="redacted"' -c main.cc -o bazel-out/local-fastbuild/bin/_objs/main/main.pic.o)
ERROR: /usr/local/google/home/rodrigoq/git/bazeltest/config_h/BUILD:1:1: undeclared inclusion(s) in rule '//:main':
this rule is missing dependency declarations for the following files included by 'main.cc':
  'bazel-out/local-fastbuild/genfiles/external/com_github_gflags_gflags/gflags_declare.h'
Target //:main failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 0.419s, Critical Path: 0.18s
FAILED: Build did NOT complete successfully

@schuhschuh schuhschuh merged commit 57ceb0e into gflags:master Sep 1, 2017
@schuhschuh schuhschuh deleted the fix-no-config_h-genrule-for-bazel branch September 1, 2017 09:10
@schuhschuh schuhschuh modified the milestones: v3.0.0, v2.2.2 Sep 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bazel cmake

Projects

None yet

Milestone

v2.2.2

Development

Successfully merging this pull request may close these issues.

Bazel rule leaks config.h into projects that depend on it

2 participants

@schuhschuh @drigz