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

Loading macros from buckaroo dependencies #371

Open
roey-e opened this issue Apr 18, 2020 · 6 comments
Open

Loading macros from buckaroo dependencies #371

roey-e opened this issue Apr 18, 2020 · 6 comments

Comments

@roey-e
Copy link

roey-e commented Apr 18, 2020

Description

My goal is to use macros defined in a dependency's "bzl" file. This is even more useful when using Bazel, as Bazel provides custom rules in addition to macros.

From my understanding, labels of buckaroo cells should be use with the buckaroo macros and not written explicitly.

protobuf_cell = buckaroo_cell('github.com/roey-e/protobuf')  # buckaroo.github.roey-e.protobuf

In Bazel mode, some hash is added to the workspaces' names. This makes the use of the macros mandatory.

def buckaroo_setup():
  native.local_repository(
    name = "buckaroo_github_com_roey_e_protobuf_a367be9a",  # Can't guess the repo name
    path = "buckaroo/github/roey-e/protobuf"
  )

def buckaroo_workspace(package_name):
  if package_name == "github.com/roey-e/protobuf":
    return "@buckaroo_github_com_roey_e_protobuf_a367be9a"

  fail(package_name + " is not a recognized package")

The problem here is that the load statement in Buck and in Bazel accept string literal only.

Examples:
In Buck:

load('//:buckaroo_macros.bzl', 'buckaroo_cell')

# The cell name of the Protobuf package
protobuf_cell = buckaroo_cell('github.com/roey-e/protobuf')

load(protobuf_cell+'//:buck_protobuf.bzl', 'proto_gen')

In Bazel:

load("//buckaroo:defs.bzl", "buckaroo_workspace")

protobuf_workspace = buckaroo_workspace("github.com/roey-e/protobuf")

load(protobuf_workspace + '//:protobuf.bzl', 'proto_gen')

Expected Behavior

Is there a proper way for loading macros with Buckaroo? How can I make it work?

Actual Behavior

In Buck:

Starting new Buck daemon...
Buck daemon started.
Using additional configuration options from .buckconfig.d/.buckconfig.buckaroo
ERROR: /home/roey/git/nanopb/generator/BUCK:6:6: syntax error at 'protobuf_cell': expected string literal
Cannot parse /home/roey/git/nanopb/generator/BUCK.  It was referenced from //generator:BUCK

In Bazel:

ERROR: /home/roey/git/nanopb/examples/simple/BUILD:5:6: in load statement: empty package-relative label
ERROR: Skipping '//examples/simple:protobuf_workspace': error loading package 'examples/simple': malformed load statements
WARNING: Target pattern parsing failed.
ERROR: error loading package 'examples/simple': malformed load statements
INFO: Elapsed time: 0.222s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (1 packages loaded)

Context

Specifically, I'm trying to use protobuf's "proto_gen" rule/macro.

Your Environment

  • Version used: Buckaroo 3.0.1; Buck version dfd6587a6290c638bac2e5e8e1c0e1d25baf2632; Bazel version 3.0.0
  • Operating System and Architecture: WSL 1's Ubuntu 18.04 on top of Windows 10 1909, intel
@nikhedonia
Copy link
Collaborator

hmm i'm pretty sure that worked in the past.
I'll try to reproduce this on my machine later..
@njlr do you think there was a breaking API change in Buck or Buckaroo?

@njlr
Copy link
Collaborator

njlr commented Apr 21, 2020

According to the Bazel docs:

Arguments must be string literals (no variable) and load statements must appear at top-level, i.e. they cannot be in a function body.

So we cannot use function outputs or variables in a load.

The reason the actual workspace name has a weird hash appended is to ensure that even when there are two different versions of the same package somewhere in the resolution tree there is not a name collision.

I will have to think further about how this might work.

@errangutan
Copy link

I'm not getting something. The package which contains buckaroo_setup is generated? Who generates it?

@njlr
Copy link
Collaborator

njlr commented Apr 23, 2020

@errangutan

I'm not getting something. The package which contains buckaroo_setup is generated? Who generates it?

Buckaroo is a tool that fetches packages containing Buck or Bazel projects. It downloads them to a folder local to the project (a bit like node_modules) and also generates a set of macros for the user to use in their own BUILD files. Buckaroo solves the transitive workspace dependency problem for Bazel.

Buckaroo packages are usually identified by a name like github.com/someorg/someproject. However, it is possible to depend on two different versions of one package, so the workspace names, which must be globally unique to satisfy Bazel, have a hash appended like this:

package name                      ->    workspace name
github.com/someorg/someproject    ->    github_someorg_someproject_aabbccdd

The challenge now is that the user wants to load a macro from a package. We can get the workspace name using a function generated by Buckaroo:

load("//buckaroo:defs.bzl", "buckaroo_workspace")

someproject_workspace = buckaroo_workspace("github.com/someorg/someproject") 

someproject_workspace # github_someorg_someproject_aabbccdd

But we cannot use this variable in a load:

# Not allowed
load(someproject_workspace, "my_macro")

Perhaps you can give some insight into how we can achieve this in Bazel?

@errangutan
Copy link

I've given this some thought and I don't think this is possible. I am by no means an expert on Bazel so take this with a grain of salt. I would personally open a feature request and discuss this with the Bazel team.
In you current stage, do you simply have the required version hardcoded in the load statement?

@njlr
Copy link
Collaborator

njlr commented Aug 13, 2020

@nikhedonia Another option to consider is to add a "cell_name" option to dependencies in the manifest. Users can then be guaranteed that this name will be used for the cell containing the macros. Buckaroo would have to check that these are globally unique, of course. Advantage of this approach is that we don't need any changes from Buck / Bazel, and I think their reasons for not allowing variables in load statements are good.

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

4 participants