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

remote_download_outputs=toplevel doesn't download libraries in cc_import needed to run. #11532

Closed
wesleyw72 opened this issue Jun 2, 2020 · 3 comments
Assignees
Labels
team-Rules-CPP Issues for C++ rules untriaged

Comments

@wesleyw72
Copy link

Description of the problem / feature request:

I have a cc_binary target which depends on a DSO in the following way.

cc_binary(
  name = "my_binary",
  deps = [":stuff", ":import_lib"],
)

cc_library(
  name = "import_lib",
  deps = [":libfoo_lib", ":another_thing"]
)

cc_import(
  name = "libfoo_lib",
  shared_library = ":libfoo.so",
)

cc_binary(
  name = "libfoo.so",
  srcs = ["blah.cpp"],
  linkshared = 1,
  linkstatic = 1,
)

When a machine does bazel run :my_binary, it's not downloading libfoo.so and then producing an error like:

error while loading shared libraries: libfoo.so: cannot open shared object file: No such file or directory

It seems that it doesn't recognise that libfoo.so is necessary for execution.

What operating system are you running Bazel on?

Ubuntu 16.04

What's the output of bazel info release?

Replace this line with your answer.

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

release 1.2.1

@ulfjack
Copy link
Contributor

ulfjack commented Jun 2, 2020

I can reproduce the issue at HEAD. The reason is that libfoo.so in the cc_binary runfiles is a symlink to the actual libfoo.so file, and the code implementing the flag isn't aware of symlinks. I have a fix for this.

ulfjack added a commit to ulfjack/bazel that referenced this issue Jun 2, 2020
Top-level output files can be symlinks to non-top-level output files, such as
in the case of a cc_binary linking against a .so built in the same build.

I reuse the existing mayInsensitivelyPropagateInputs() method, which was
originally introduced for action rewinding, but seems to have exactly the
intended semantics here as well.

Unfortunately, this requires a small change to the BlazeModule API to pass in
the full analysis result (so we can access the action graph, so we can read
the aformentioned flag).

I considered using execution-phase information on whether an output is a
symlink. However, at that point it's too late! The current design only allows
pulling an output file *when its generating action runs*. It would be better if
this was changed to pull output files independently of action execution. That
would avoid a lot of problems with the current design, such as bazelbuild#10902.

Fixes bazelbuild#11532.

Change-Id: Iaf1e48895311fcf52d9e1802d53598288788a921
@ulfjack
Copy link
Contributor

ulfjack commented Jun 2, 2020

@buchgr

@wesleyw72
Copy link
Author

Thanks @ulfjack for looking at this and proposing a fix so quickly!

ulfjack added a commit to ulfjack/bazel that referenced this issue Aug 6, 2020
Top-level output files can be symlinks to non-top-level output files, such as
in the case of a cc_binary linking against a .so built in the same build.

I reuse the existing mayInsensitivelyPropagateInputs() method, which was
originally introduced for action rewinding, but seems to have exactly the
intended semantics here as well.

Unfortunately, this requires a small change to the BlazeModule API to pass in
the full analysis result (so we can access the action graph, so we can read
the aformentioned flag).

I considered using execution-phase information on whether an output is a
symlink. However, at that point it's too late! The current design only allows
pulling an output file *when its generating action runs*. It would be better if
this was changed to pull output files independently of action execution. That
would avoid a lot of problems with the current design, such as bazelbuild#10902.

Fixes bazelbuild#11532.

Change-Id: Iaf1e48895311fcf52d9e1802d53598288788a921
ulfjack added a commit to ulfjack/bazel that referenced this issue Aug 19, 2020
Top-level output files can be symlinks to non-top-level output files, such as
in the case of a cc_binary linking against a .so built in the same build.

I reuse the existing mayInsensitivelyPropagateInputs() method, which was
originally introduced for action rewinding, but seems to have exactly the
intended semantics here as well.

Unfortunately, this requires a small change to the BlazeModule API to pass in
the full analysis result (so we can access the action graph, so we can read
the aformentioned flag).

I considered using execution-phase information on whether an output is a
symlink. However, at that point it's too late! The current design only allows
pulling an output file *when its generating action runs*. It would be better if
this was changed to pull output files independently of action execution. That
would avoid a lot of problems with the current design, such as bazelbuild#10902.

Fixes bazelbuild#11532.

Change-Id: Iaf1e48895311fcf52d9e1802d53598288788a921
ulfjack added a commit to ulfjack/bazel that referenced this issue Sep 16, 2020
Top-level output files can be symlinks to non-top-level output files, such as
in the case of a cc_binary linking against a .so built in the same build.

I reuse the existing mayInsensitivelyPropagateInputs() method, which was
originally introduced for action rewinding, but seems to have exactly the
intended semantics here as well.

Unfortunately, this requires a small change to the BlazeModule API to pass in
the full analysis result (so we can access the action graph, so we can read
the aformentioned flag).

I considered using execution-phase information on whether an output is a
symlink. However, at that point it's too late! The current design only allows
pulling an output file *when its generating action runs*. It would be better if
this was changed to pull output files independently of action execution. That
would avoid a lot of problems with the current design, such as bazelbuild#10902.

Fixes bazelbuild#11532.

Change-Id: Iaf1e48895311fcf52d9e1802d53598288788a921
ulfjack added a commit to ulfjack/rules_pkg that referenced this issue Sep 22, 2020
Using a shell script is not portable and also causes issues with
build-without-the-bytes, which can handle local symlink actions (at least in
principle), but cannot handle symlinks returned from remote execution.

Also see bazelbuild/bazel#11532.
aiuto pushed a commit to bazelbuild/rules_pkg that referenced this issue Sep 24, 2020
Using a shell script is not portable and also causes issues with
build-without-the-bytes, which can handle local symlink actions (at least in
principle), but cannot handle symlinks returned from remote execution.

Also see bazelbuild/bazel#11532.
Yannic pushed a commit to Yannic/bazel that referenced this issue Oct 5, 2020
Top-level output files can be symlinks to non-top-level output files, such as
in the case of a cc_binary linking against a .so built in the same build.

I reuse the existing mayInsensitivelyPropagateInputs() method, which was
originally introduced for action rewinding, but seems to have exactly the
intended semantics here as well.

Unfortunately, this requires a small change to the BlazeModule API to pass in
the full analysis result (so we can access the action graph, so we can read
the aformentioned flag).

I considered using execution-phase information on whether an output is a
symlink. However, at that point it's too late! The current design only allows
pulling an output file *when its generating action runs*. It would be better if
this was changed to pull output files independently of action execution. That
would avoid a lot of problems with the current design, such as bazelbuild#10902.

Fixes bazelbuild#11532.

Change-Id: Iaf1e48895311fcf52d9e1802d53598288788a921

Closes bazelbuild#11536.

Change-Id: Ie1bf49a8d08f0b2422426ecd95fe79b3686f8427
PiperOrigin-RevId: 332939828
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Rules-CPP Issues for C++ rules untriaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants