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

load() doesn't work inside function #1550

Closed
jart opened this issue Jul 22, 2016 · 8 comments
Closed

load() doesn't work inside function #1550

jart opened this issue Jul 22, 2016 · 8 comments
Labels
team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.

Comments

@jart
Copy link
Contributor

jart commented Jul 22, 2016

If a WORKSPACE file calls a function containing native.load() then the imported symbol will not be available to the function namespace in subsequent statements.

Further explanation

Bazel projects oftentimes specify a foo_repositories() function so dependent repositories can easily schlep in transitive dependencies. (Here's the convention used by Closure Rules.) But what if one of those dependencies is another Bazel project following the same convention?

Assume I wanted to define the following:

def io_bazel_rules_closure():
  native.local_repository(
      name = "io_bazel_rules_closure",
      path = "/usr/local/google/home/jart/code/rules_closure",
  )
  load("@io_bazel_rules_closure//closure:defs.bzl", "closure_repositories")
  closure_repositories(
      omit_gson = True,
      omit_guava = True,
      omit_icu4j = True,
      omit_json = True,
      omit_jsr305 = True,
      omit_jsr330_inject = True,
      omit_protobuf_java = True,
  )

Then I get the following error:

function 'load' does not exist.

If I change load to native.load then I get a different error:

function 'closure_repositories' does not exist.

The workaround is that all repositories providing a foo_repositories() function must be explicitly specified in the WORKSPACE file. This makes the external dependency convention that Closure Rules has adopted less useful.

@damienmg
Copy link
Contributor

This is WAI. Bazel needs to knows the dependencies before analysis. This could be lifted a little but with a lot of engineering. Unless we have a strong use case for it, it is not going to be implemented.

Closing but feel free to argue :)

@davidzchen
Copy link
Member

Do we have a way to make this less cumbersome yet? Skydoc has a similar problem since it depends on rules_saas, and currently, users would have to copy and paste a rather significant chunk of workspace rules:

git_repository(
    name = "io_bazel_rules_sass",
    remote = "https://github.com/bazelbuild/rules_sass.git",
    tag = "0.0.1",
)
load("@io_bazel_rules_sass//sass:sass.bzl", "sass_repositories")
sass_repositories()

git_repository(
    name = "io_bazel_skydoc",
    remote = "https://github.com/bazelbuild/skydoc.git",
    tag = "0.0.3",
)
load("@io_bazel_skydoc//skylark:skylark.bzl", "skydoc_repositories")
skydoc_repositories()

@damienmg
Copy link
Contributor

Why can't you put the sass repository in the skydoc_repositories macro? If that's because you are loading them from skylark:skylark.bzl maybe splitting it up is the correct answer.

@davidzchen
Copy link
Member

I can move the git_repository for sass into skydoc_repositories, but then it is still not possible to load() and call sass_repositories() from within the skydoc_repositories macro:

ERROR: /Users/dzc/Projects/bazelbuild/skydoc/skylark/skylark.bzl:267:3: function 'sass_repositories' does not exist
ERROR: com.google.devtools.build.lib.packages.BuildFileContainsErrorsException: error loading package '': Extension 'skylark/skylark.bzl' has errors
INFO: Elapsed time: 0.135s
FAILED: Build did NOT complete successfully (0 packages loaded)

@damienmg
Copy link
Contributor

It's a really complex issue, making load available from macro make cycle inside a macro possible:
e.g.:

def macro():
   native.local_repository('foo', ...)
   native.load("@foo//:bar.bzl", "baz")

Then calling macro would require foo to be defined because of the load statement, However this whole statement is evaluated inside the same sky function. There is probably a solution to this but it would make the code really complex and take a long time to make right.

@jart
Copy link
Contributor Author

jart commented Aug 22, 2016

Please help me understand why that's a cycle, because I'm very curious about why this issue is so complicated. Please keep in mind I know very little about the Bazel core.

I would have assumed that Skylark does evaluation of the WORKSPACE file in a single pass evaluator, where the WORKSPACE file AST is treated like it's the body of a function def. Therefore it would stand to reason that the things which can be done in the WORKSPACE file can also be done when the evaluator recurses into the function defs it loads. But it sounds like there might be a separate evaluator that happens in multiple passes.

@hsyed
Copy link

hsyed commented Sep 5, 2017

This feature would really aid our build.

The pubref maven_rules do a very good job at managing dependencies for codebases containing multiple micro services. The pubref rules generate a target containing all the deps of the repository. So in the example below I can depend on @k8sclient//:compile in a binary.

maven_repository(
    name = 'k8sclient',
    deps = [
        'io.fabric8:kubernetes-client:2.6.2',
    ],
    force = [
        'com.fasterxml.jackson.core:jackson-databind:2.7.7',
        'com.fasterxml.jackson.core:jackson-core:2.7.7',
        'org.slf4j:slf4j-api:1.7.25',
    ],
    transitive_deps = [
        '19f42c154ffc689f40a77613bc32caeb17d744e3:com.fasterxml.jackson.core:jackson-annotations:2.7.0',
        '1b45281e277d07ef8c02e9726daef1175442759a:com.fasterxml.jackson.core:jackson-core:2.7.7',
        'f595bc5c04adf4db9d49909216128904a315f198:com.fasterxml.jackson.core:jackson-databind:2.7.7',
        '82cad686d626685c052481655a04216682752fda:com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.7.7',
        '76970966f107ce710f7dcf36be510b4b9607d634:com.fasterxml.jackson.module:jackson-module-jaxb-annotations:2.7.5',
        'd8b3573e6d59f0a61dfc37f85dd568ad3bf49e83:com.github.mifmif:generex:1.0.1',
        'feab46062803513d6a8307c74b0084265855de1a:com.squareup.okhttp3:logging-interceptor:3.8.1',
        '4d060ca3190df0eda4dc13415532a12e15ca5f11:com.squareup.okhttp3:okhttp:3.8.1',
        'a9283170b7305c8d92d25aff02a6ab7e45d06cbe:com.squareup.okio:okio:1.13.0',
        '6ebfa65eb431ff4b715a23be7a750cbc4cc96d0f:dk.brics.automaton:automaton:1.11-8',
        '502c8290f991487dfa15ec1c08842613ffea1cc5:io.fabric8:kubernetes-client:2.6.2',
        '257e1337d65e2bcf01d968574033092d657f904b:io.fabric8:kubernetes-model:1.1.4',
        'd3ebf0f291297649b4c8dc3ecc81d2eddedc100d:io.fabric8:zjsonpatch:0.3.0',
        '8613ae82954779d518631e05daa73a6a954817d5:javax.validation:validation-api:1.1.0.Final',
        '43759e986de5fec7045e35e9533e5ad2f6cd1b05:org.slf4j:jul-to-slf4j:1.7.13',
        'da76ca59f6a57ee3102f8f9bd9cee742973efa8a:org.slf4j:slf4j-api:1.7.25',
        '3b132bea69e8ee099f416044970997bde80f4ea6:org.yaml:snakeyaml:1.15',
    ],
)
load("@k8sclient//:rules.bzl", "k8sclient_compile")
k8sclient_compile()

However creating a transitive block for a single top level dependency requires a load and a function call. For this reason I have had to resort to a single uber dependency declaration for our codebase -- the transitive block for this contains 250 jars. The alternative is to have a workspace files with dozens of declarations as above which would need to be copy pasted if we modularise the build.

Now dependencies like Spring Boot (unfortunately we have a SB app in our stack), Hibernate, Jersey etc are a royal pain to curate manually.

Similar cases exist with building go codebases -- unless the vendored approach is used and external dependencies in this case aren't managed by bazel.

Perhaps this particular use-case will be solved by transitive workspaces. However, I think For huge codebases that aren't using code generation to manage parts of the build logic, load from macros is a very powerful abstraction. It ensures the workspace file can be properly modularised.

@pauldraper
Copy link
Contributor

pauldraper commented Apr 7, 2018

Unless we have a strong use case for it, it is not going to be implemented.

The original issue by @jart is well-stated. I thinks that as good as the use case gets:

The workaround is that all repositories providing a foo_repositories() function must be explicitly specified in the WORKSPACE file. This makes the external dependency convention that Closure Rules has adopted less useful.

#1943 (if anything) will likely be the solution.

mhala-tolar pushed a commit to Tolar-HashNET/grpc that referenced this issue Nov 15, 2019
This could be useful for projects that use gRPC as an external
repository. This could not be done within the same grpc_deps.bzl due to
the problem discussed in
- bazelbuild/bazel#1550
- bazelbuild/bazel#1943
jerrymarino added a commit to bazel-xcode/xchammer that referenced this issue Dec 2, 2019
It isn't possible to call load() within a macro
bazelbuild/bazel#1550
jerrymarino added a commit to bazel-xcode/xchammer that referenced this issue Dec 6, 2019
It isn't possible to call load() within a macro
bazelbuild/bazel#1550
@aiuto aiuto added the team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. label Jun 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.
Projects
None yet
Development

No branches or pull requests

6 participants