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

Cannot build C++ target in an external that depends on another external. #11293

Closed
tomlu opened this issue May 5, 2020 · 5 comments
Closed
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request

Comments

@tomlu
Copy link
Contributor

tomlu commented May 5, 2020

Description of the problem / feature request:

Cannot build C++ target in an external that depends on another external.

When building the externals separately it works, but when in the context of the superproject then subprojects cannot #include externals.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Situation: three workspaces, baz -> bar -> foo

$ foo/WORKSPACE
<empty>

$ foo/BUILD
cc_library(
    name = "foo",
    srcs = ["foo.cc"],
    hdrs = ["foo.h"],
)

$ foo/foo.h
int foo_fn();

$ foo/foo.cc
#include "foo.h"
int foo_fn() { return 2; }

$ bar/WORKSPACE
local_repository(
    name = "foo",
    path = "../foo",
)

$ bar/BUILD
cc_library(
    name = "bar",
    srcs = ["bar.cc"],
    hdrs = ["bar.h"],
    deps = ["@foo//:foo"],
)

$ bar/bar.h
int bar_fn();

$ bar/bar.cc
#include "bar.h"
#include "external/foo/foo.h"
int bar_fn() { return foo_fn(); }

$ baz/WORKSPACE

local_repository(
    name = "bar",
    path = "../bar",
)

local_repository(
    name = "foo",
    path = "../foo",
)

$ baz/BUILD
cc_library(
    name = "baz",
    srcs = ["baz.cc"],
    deps = ["@bar//:bar"],
)

$ baz/baz.cc
#include "external/bar/bar.h"
int baz_fn() { return bar_fn(); }

$ cd $HOME/bar
$ blaze build :bar
  <success>

$ cd $HOME/baz
$ blaze build :baz
  failure: bar.cc cannot find "external/foo/foo.h"

The exec root for bar no longer includes the external
repo 'foo' when build in the context of 'baz'.

Note that 'baz' supplies all transitive workspace dependencies.

What operating system are you running Bazel on?

Windows 10.

What's the output of bazel info release?

release 3.1.0

If bazel info release returns "development version" or "(@non-git)", tell us how you built Bazel.

I tried building Bazel from HEAD, but it doesn't build out-of-the box @Head on Windows.

What's the output of git remote get-url origin ; git rev-parse master ; git rev-parse HEAD ?

N/A

Have you found anything relevant by searching the web?

Not exactly relevant.

@tomlu
Copy link
Contributor Author

tomlu commented May 5, 2020

The direct cause is that only the superproject has the externals in its execroot. Subprojects (when compiled in the context of a superproject) do not add the execroot to the C++ action's include paths.

The relevant line is here: https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java#L957

@jin
Copy link
Member

jin commented May 6, 2020

The direct cause is that only the superproject has the externals in its execroot.

This is WAI without recursive workspaces, since the superproject (baz) has its own output base (and therefore execroot), with direct subprojects / external repos symlinked into execroot/__main__/external. It doesn't interact with previously created output bases of subprojects (bar, foo). Instead, subprojects are siblings of each other. I tried changing all the imports from #include "external/foo/foo.h" to #include "../foo/foo.h", and it worked.

(I understand that uplevel includes can be questionable, but I'm using this to illustrate the current design of the repo symlink layout. Until we have recursive workspaces, I think subprojects would remain as single-level sibling directories in the execroot of the superproject)

File: bar/BUILD
cc_library(
    name = "bar",
    srcs = ["bar.cc"],
    hdrs = ["bar.h"],
    deps = ["@foo//:foo"],
    visibility = ["//visibility:public"],
)

File: bar/WORKSPACE
local_repository(
    name = "foo",
    path = "../foo",
)

File: bar/bar.cc
#include "bar.h"
#include "../foo/foo.h"
int bar_fn() { return foo_fn(); }

File: bar/bar.h
int bar_fn();

File: baz/BUILD
cc_library(
    name = "baz",
    srcs = ["baz.cc"],
    deps = ["@bar//:bar"],
)

File: baz/WORKSPACE
local_repository(
    name = "bar",
    path = "../bar",
)

local_repository(
    name = "foo",
    path = "../foo",
)

File: baz/baz.cc
#include "../bar/bar.h"
int baz_fn() { return bar_fn(); }

File: foo/BUILD
cc_library(
    name = "foo",
    srcs = ["foo.cc"],
    hdrs = ["foo.h"],
    visibility = ["//visibility:public"],
)

File: foo/WORKSPACE   <EMPTY>

File: foo/foo.cc
#include "foo.h"
$ tree $(bazel info execution_root) -L 3
/usr/local/google/home/jingwen/.cache/bazel/_bazel_jingwen/e41b65c7fff48ca337e095867d8b6a75/execroot/__main__
├── baz.cc -> /tmp/externals/baz/baz.cc
├── bazel-out
│   ├── _actions
│   ├── k8-fastbuild
│   │   ├── bin
│   │   └── testlogs
│   ├── stable-status.txt
│   ├── _tmp
│   │   └── actions
│   └── volatile-status.txt
├── BUILD -> /tmp/externals/baz/BUILD
├── external
│   ├── bar -> /usr/local/google/home/jingwen/.cache/bazel/_bazel_jingwen/e41b65c7fff48ca337e095867d8b6a75/external/bar
│   ├── bazel_tools -> /usr/local/google/home/jingwen/.cache/bazel/_bazel_jingwen/e41b65c7fff48ca337e095867d8b6a75/external/bazel_tools
│   ├── foo -> /usr/local/google/home/jingwen/.cache/bazel/_bazel_jingwen/e41b65c7fff48ca337e095867d8b6a75/external/foo
│   └── local_config_cc -> /usr/local/google/home/jingwen/.cache/bazel/_bazel_jingwen/e41b65c7fff48ca337e095867d8b6a75/external/local_config_cc
└── WORKSPACE -> /tmp/externals/baz/WORKSPACE

12 directories, 5 files
$ for F in foo bar baz; do cd $F; bazel build //:$F; cd ..; done
INFO: Analyzed target //:foo (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //:foo up-to-date:
  bazel-bin/libfoo.a
  bazel-bin/libfoo.so
INFO: Elapsed time: 0.162s, Critical Path: 0.02s
INFO: 0 processes.
INFO: Build completed successfully, 1 total action
INFO: Analyzed target //:bar (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //:bar up-to-date:
  bazel-bin/libbar.a
  bazel-bin/libbar.so
INFO: Elapsed time: 0.167s, Critical Path: 0.02s
INFO: 0 processes.
INFO: Build completed successfully, 1 total action
INFO: Analyzed target //:baz (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //:baz up-to-date:
  bazel-bin/libbaz.a
  bazel-bin/libbaz.so
INFO: Elapsed time: 0.158s, Critical Path: 0.02s
INFO: 0 processes.
INFO: Build completed successfully, 1 total action

You might also wanna try --experimental_sibling_repository_layout, which places the subprojects repo as siblings to the superproject repo in the superproject's execroot, instead of the external directory in __main__. This would make the ../ include clearer.

$ bazel build //:baz --experimental_sibling_repository_layout
$ tree $(bazel info execution_root)/.. -L 2
/usr/local/google/home/jingwen/.cache/bazel/_bazel_jingwen/e41b65c7fff48ca337e095867d8b6a75/execroot/__main__/..
├── bar -> /usr/local/google/home/jingwen/.cache/bazel/_bazel_jingwen/e41b65c7fff48ca337e095867d8b6a75/external/bar
├── bazel_tools -> /usr/local/google/home/jingwen/.cache/bazel/_bazel_jingwen/e41b65c7fff48ca337e095867d8b6a75/external/bazel_tools
├── DO_NOT_BUILD_HERE
├── foo -> /usr/local/google/home/jingwen/.cache/bazel/_bazel_jingwen/e41b65c7fff48ca337e095867d8b6a75/external/foo
├── local_config_cc -> /usr/local/google/home/jingwen/.cache/bazel/_bazel_jingwen/e41b65c7fff48ca337e095867d8b6a75/external/local_config_cc
└── __main__
    ├── baz.cc -> /tmp/externals/baz/baz.cc
    ├── bazel-out
    ├── BUILD -> /tmp/externals/baz/BUILD
    └── WORKSPACE -> /tmp/externals/baz/WORKSPACE

@jin jin added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. untriaged labels May 6, 2020
@tomlu
Copy link
Contributor Author

tomlu commented May 6, 2020

We have a subproject that is perfectly standalone. If anyone writes code and tests it, it'll work, but when included in a superproject it'll fail in that context. That's not great.

Worse, if any subproject that we want to consume take advantage of this we would have to convince upstream authors to change their "external" references to "../", because we don't have free edit access to those projects.

I saw that you did --experimental_sibling_repository_layout, and I gave it a spin. I thought that was done to allow people to have an "external" root directory themselves. How does it make things more clear for people?

But overall, I think what you are saying is that you know this isn't desirable, and recursive workspaces is what is needed to address this?

@philwo philwo added the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Jun 15, 2020
@philwo philwo added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) type: feature request and removed untriaged labels Dec 8, 2020
@philwo philwo removed the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Nov 29, 2021
@github-actions
Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label May 24, 2023
@github-actions
Copy link

github-actions bot commented Jun 8, 2023

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please reach out to the triage team (@bazelbuild/triage). Thanks!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request
Projects
None yet
Development

No branches or pull requests

3 participants