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

When using the bazel module, how to define the refresh_command in BUILD #189

Open
Lynskylate opened this issue May 18, 2024 · 13 comments
Open

Comments

@Lynskylate
Copy link

For the document, Open a BUILD file—we'd recommend using (or creating) //BUILD—and add something like:

load("@hedron_compile_commands//:refresh_compile_commands.bzl", "refresh_compile_commands")

refresh_compile_commands(
    name = "refresh_compile_commands",

    # Specify the targets of interest.
    # For example, specify a dict of targets and any flags required to build.
    targets = {
      "//:my_output_1": "--important_flag1 --important_flag2=true",
      "//:my_output_2": "",
    },
    # No need to add flags already in .bazelrc. They're automatically picked up.
    # If you don't need flags, a list of targets is also okay, as is a single target string.
    # Wildcard patterns, like //... for everything, *are* allowed here, just like a build.
      # As are additional targets (+) and subtractions (-), like in bazel query https://docs.bazel.build/versions/main/query.html#expressions
    # And if you're working on a header-only library, specify a test or binary target that compiles it.
)

Hessian2-codec is the bazel-compile-commands-extractor‘s user. Recently the pr convert the repo to bazel module. But we have met some question.

As a library, we are meant to be referenced by other projects. Due to the use of the bazel module, bazel regards the repository with git_override as a dev_dependency. As a result, it cannot be used by users who depend on hessian2-codec. It will throw the exception like ERROR: error loading package '@@hessian2-codec~//': Unable to find package for @@[unknown repo 'hedron_compile_commands' requested from @@hessian2-codec~]//:refresh_compile_commands.bzl: The repository '@@[unknown repo 'hedron_compile_commands' requested from @@hessian2-codec~]' could not be resolved: No repository visible as '@hedron_compile_commands' from repository '@@hessian2-codec~'.

Failure will occur when loading the build file for hessian2-codec.

Regarding defining refresh_command in the bazel module, are there any best practices?

@Lynskylate
Copy link
Author

A workaround fix is add bazel-compile-commands-extractor to bzr, and we treat it as a full dependency.

@mmorel-35
Copy link

@cpsauer ,

Even with the dev_dependency as in here
bazelbuild/bazel-central-registry#2054
There are some errors looking for hedron_compile_commands

See https://buildkite.com/bazel/bcr-presubmit/builds/5552#018f8b08-67ea-42d6-86f9-3476a05b8de6

@cpsauer
Copy link
Contributor

cpsauer commented May 18, 2024

Thanks so much for reporting, guys. I think I get it--and I've got an idea.

@cpsauer
Copy link
Contributor

cpsauer commented May 18, 2024

Would it work to break out the refresh_compile_commands call into a separate BUILD file? Say //dev_tools:BUILD?

@cpsauer
Copy link
Contributor

cpsauer commented May 18, 2024

To explain: I think this is an instance of more general trickiness around dev_dependencies in bzlmod--with the general case being discussed here.

@cpsauer
Copy link
Contributor

cpsauer commented May 18, 2024

I agree that it's a bit janky, but making it a non-dev-dependency is probably not something you want either, for the reasons in this issue. It's likely that we'll end up depending on rules_python again soon and better to not bring in a ton of unneeded deps for folks. And I'm loathe to do the BCR approach sinc it's (AFAIK) irreversible and would meaningfully complicate releasing.

@cpsauer
Copy link
Contributor

cpsauer commented May 18, 2024

Please let me know if that alternative approach might work. And sorry about the pain here. This isn't a case I'd considered or heard about before--and I really appreciate your taking the time to explore and explain

@cpsauer
Copy link
Contributor

cpsauer commented May 18, 2024

If that does fix things, please don't close just yet. We should update the readme to guide people around this.

@mmorel-35
Copy link

mmorel-35 commented May 19, 2024

It seems like the same problem was already identified several month ago here : bazelbuild/bazel-central-registry#1503

@fmeum, @alexeagle, do you have an opinion about this issue ?

@fmeum
Copy link

fmeum commented May 20, 2024

Yes, #189 (comment) sounds right to me. Dev tooling for a module ideally should not live in packages that are consumed by module users.

@mmorel-35
Copy link

mmorel-35 commented May 20, 2024

I believe that at least one version shall be published in the BCR, and be used as dev_dependency.
Then the module can use git_override(which is always local to the module) to point to the latest version of hedron_compile_commands.
This way the test module inside hessian2-codec that depends on hessian2-codec won't be failing with a missing dependency

@cpsauer
Copy link
Contributor

cpsauer commented May 23, 2024

^ What do you think about the potential solution above, also recommended by fmeum?

@peakschris
Copy link

This is what I'm using in my MODULE.bazel file. We have converted to a 100% module based build:

bazel_dep(name = "hedron_compile_commands", dev_dependency = True)
HEDRON_COMPILE_COMMANDS_TAG="204aa593e002cbd177d30f11f54cff3559110bb9"
HEDRON_COMPILE_COMMANDS_INTEGRITY="sha256-mXK5x4jhnGgPGTo6oGoiUxK6MQcFmnWnKiv3k/qnCsk="
archive_override(
    module_name = "hedron_compile_commands",
    strip_prefix = "bazel-compile-commands-extractor-"+HEDRON_COMPILE_COMMANDS_TAG,
    urls = ["https://github.com/hedronvision/bazel-compile-commands/archive/"+HEDRON_COMPILE_COMMANDS_TAG+".zip"],
    integrity = HEDRON_COMPILE_COMMANDS_INTEGRITY,
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants