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

Make variable substitution in local_defines in C++ rule does not accept labels from data #13930

Closed
joelwilliamson opened this issue Sep 1, 2021 · 3 comments
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-Rules-CPP Issues for C++ rules type: feature request

Comments

@joelwilliamson
Copy link
Contributor

joelwilliamson commented Sep 1, 2021

Description of the problem / feature request:

In C++ rules, local_defines allow "Make" variable substitution, including execpath: List of defines to add to the compile line. Subject to "Make" variable substitution and Bourne shell tokenization. (https://docs.bazel.build/versions/main/be/c-cpp.html#cc_binary.local_defines)

In "Make" variable substitution, C++ rules are documented to be able to use labels referenced in the data attribute: All referenced labels must appear in the consuming rule's srcs, output files, or deps. Otherwise the build fails. C++ rules can also reference labels in data. (https://docs.bazel.build/versions/main/be/make-variables.html)

However, if a local_defines rule tries to do "VAR=\"$(execpath :data_label)\"", the expansion fails with the error "label ':data_label' in $(location) expression is not a declared prerequisite of this rule."

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

filegroup(
  name = "data_label",
  srcs  = ["data_file"],
)

cc_library(
  name = "lib",
  srcs = ["lib.cc"],
  hdrs = ["lib.hh"],
  data = [":data_label"],
  local_defines = ["VAR=\"$(execpath :data_label)\""],
)

What operating system are you running Bazel on?

Linux

What's the output of bazel info release?

2021/08/31 23:23:18 Using unreleased version at commit 662cf54
development version

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

Have you found anything relevant by searching the web?

cfb6989#diff-dded2426bd501014f754eeb69f0eb986a5db2a8908ff219106af4c76a5da09e2R531

    ImmutableMap.Builder<Label, ImmutableCollection<Artifact>> builder = ImmutableMap.builder();

    if (ruleContext.attributes().has("deps", LABEL_LIST)) {
      for (TransitiveInfoCollection current : ruleContext.getPrerequisites("deps")) {
        builder.put(
            AliasProvider.getDependencyLabel(current),
            current.getProvider(FileProvider.class).getFilesToBuild().toList());
      }
    }

I think this should also get do getPrerequisites("data")

Any other information, logs, or outputs that you want to share?

@aiuto aiuto added team-Rules-CPP Issues for C++ rules untriaged labels Sep 5, 2021
@oquenchil oquenchil 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 Feb 14, 2022
bazel-io pushed a commit that referenced this issue Feb 24, 2022
This is a fix for #13930

Closes #13938.

PiperOrigin-RevId: 430677775
@garymm
Copy link

garymm commented Sep 19, 2022

Is there a work-around available for this?

If not, can this be pack-ported to bazel 5?
#13938

I'm still seeing this on bazel 5.3.1.

garymm added a commit to garymm/vulkan that referenced this issue Sep 19, 2022
The previous attempt to use a hermetic copy of it didn't work in all
cases. The dylib was linked properly, but some Vulkan libraries at
runtime access it through the "loader driver interface", which requires
an `icd.d` directory to be present in a particular path.

See
https://vulkan.lunarg.com/doc/view/1.3.211.0/mac/LoaderDriverInterface.html#user-content-driver-discovery-on-macos

So for now we'll just rely on the user to install MoltenVK
(probably via the LunarG Vulkan SDK), but I hope that maybe this is
fixable once bazelbuild/bazel#13930 is fixed.
@garymm
Copy link

garymm commented Mar 10, 2023

This seems fixed on Bazel 6. I think it should be closed.

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 90 days unless any other activity occurs. If you think this issue is still relevant and should stay open, please post any comment here and the issue will no longer be marked as stale.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label May 14, 2024
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-Rules-CPP Issues for C++ rules type: feature request
Projects
None yet
Development

No branches or pull requests

5 participants