Consider removing bind()

[jart]

What is the use case for bind()? I can't think of one. Even in situations where there are multiple ABI-compatible implementations of a library (e.g. OpenSSL, BoringSSL, etc.) this problem could still be solved by using vanilla externals.

Most Bazel projects don't seem to use bind(). The ones that do, it seems to have caused problems.

For example, the protobuf repository, rather than defining a protobuf_repositories() function, simply uses //external:foo for every single target upon which it depends, thereby punting the burden defining bind() rules not only for every single external, but every target within those externals.

As a result, the TensorFlow workspace.bzl file has developed a cargo cult pattern where superfluous bindings will be added, because I don't think people really understand what bind() does.

What is especially suboptimal is that the bind() namespace overlaps with the external repository namespace. We can't name externals like "six" to be "@six" because the protobuf BUILD file asked us for //external:six. So we don't have a choice. We have to name it @six_archive, which hurts readability. It would have been more optimal if the protobuf BUILD file should have just asked for @six//:six.

It would be nice if we could retire bind() and help projects like protobuf migrate to the foo_repositories() model that official Bazel projects use. We could recommend as a best practice the technique that is employed by the Closure Rules repositories.bzl file.

def closure_repositories(
    omit_foo=False,
    omit_bar=False):
  if not omit_foo:
    foo()
  if not omit_bar:
    bar()

def foo():
  native.maven_jar(name = "foo", ...)

def bar():
  native.maven_jar(name = "bar", ...)

This gives dependent Bazel projects the power to schlep in transitive Closure Rules dependencies using either a whitelist or blacklist model. For example, one project that uses Closure Rules has the following in its WORKSPACE file:

http_archive(
    name = "io_bazel_rules_closure",
    sha256 = "7d75688c63ac09a55ca092a76c12f8d1e9ee8e7a890f3be6594a4e7d714f0e8a",
    strip_prefix = "rules_closure-b8841276e73ca677c139802f1168aaad9791dec0",
    url = "http://bazel-mirror.storage.googleapis.com/github.com/bazelbuild/rules_closure/archive/b8841276e73ca677c139802f1168aaad9791dec0.tar.gz",  # 2016-10-02
)

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,
)

Because it directly depends on those transitive dependencies and wants to specify them on its own.

I think this is a much more desirable and flexible pattern than bind().

[philwo]

I personally also find bind(...) confusing and am not sure how to correctly use it.

Pinging @damienmg and @lberki for some input here, I think they know more about how this is supposed to work.

[lberki]

bind was indeed invented for selecting between e.g. different implementations of SSL or e.g. for different versions of GSON/Guava/... in the Closure case. I'm not sure how much use it sees, so I'd not be that trigger-happy with removing it.

[damienmg]

I think all bind uses-cases can be replaced with alias but it does not strike me as high priority. bind() has that weird thing that it creates a //external package that does not really exists, so only for that I would say +1

[jart]

It might be worth putting a quick change in the documentation saying that bind() is or will likely be deprecated and that alias should be used instead, or simply vanilla external repository names. That should hopefully lessen any refactoring we'll have to do in the future, if it is removed.

[lberki]

Considering that I'm not really sure we want to do that, I'd rather not deprecate bind() just now.

[jart]

Then would Bazel at the very least be open to a documentation change
dissuading people from using it?

On Oct 18, 2016 1:45 AM, "lberki" notifications@github.com wrote:

Considering that I'm not really sure we want to do that, I'd rather not
deprecate bind() just now.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#1952 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADAbqUFcPK40C1qQCoJ2B3pyKrI3LEIks5q1IcOgaJpZM4KYF52
.

[robertcrowe-zz]

It looks like this could be related to a problem I'm trying to solve. When I try to build my Tensorflow Serving client I'm getting this:

bazel build //FFv1:client
ERROR: /home/rcrowe/.cache/bazel/_bazel_rcrowe/b1867dd6bfd6249a885c91482eccde46/external/org_tensorflow/tensorflow/workspace.bzl:80:3: no such package '@six_archive//': In new_http_archive rule //external:six_archive the 'build_file' attribute does not specify an existing file (/home/rcrowe/serving/six.BUILD does not exist) and referenced by '//external:six'.
ERROR: Analysis of target '//FFv1:client' failed; build aborted.
INFO: Elapsed time: 1.286s

Any idea how I can fix this? I'm on Ubuntu 14.04

[jart]

@robertcrowe Did you modify the workspace.bzl file? Can you start a separate issue for this and CC me?

[robertcrowe-zz]

@jart - Thanks, I created #1963

[kchodorow]

+1 to removing bind, updating the docs in the meantime seems like a good idea.

[jart]

I reviewed the code to rules_web yesterday, which was recently introduced and makes extensive use of bind(). It uses bind() to allow the user to override executable attributes on its Skylark rules without having to repeat them every time the rule is used. I had a discussion with @DrMarcII about this. We both came to the conclusion that it would be better if those attributes were public and the user defined macro wrappers that customize the attribute.

I'm glad we're building consensus around removing bind(). Right now it's the first rule listed in the documentation, but it's actually the last rule we'd want to use. The same is true for git_repository(), which is also listed first, but has the biggest negative impact on performance for the project in question, and all dependent projects. Many users make use of these rules without considering the alternatives, like grabbing the snapshot tarball from GitHub. Whatever we can do to help the user make the correct choices, especially if we make the wrong choices impossible, is going to go a long way to fostering a healthy and lightning fast build ecosystem spanning many GitHub projects that all reference each other.

[damienmg]

The order of the rules are a bit random, I don't think they matter.

Anyway, IMO:

1. we should advertise bind as deprecated and point user to alias or use
   the repository name instead,
2. we should starts advertising maven_jar and git_repository as
   deprecated and point the user to http_* rules (for source distribution) and
   the skylark implementations (for those who git / maven support is better).
3. We should publish a best practice document about the good use of
   external repositories. A blog post maybe?

On Wed, Oct 19, 2016 at 8:36 PM Justine Tunney notifications@github.com
wrote:

I reviewed the code to rules_web https://github.com/bazelbuild/rules_web
yesterday, which was recently introduced and makes extensive use of bind().
It uses bind() to allow the user to override executable attributes on its
Skylark rules
https://github.com/bazelbuild/rules_web/blob/master/web/internal/web_test_config.bzl#L63
without having to repeat them every time the rule is used. I had a
discussion with @DrMarcII https://github.com/DrMarcII about this. We
both came to the conclusion that it would be better if those attributes
were public and the user defined macro wrappers that customize the
attribute.

I'm glad we're building consensus around removing bind(). Right now it's
the first rule listed in the documentation, but it's actually the last rule
we'd want to use. The same is true for git_repository(), which is also
listed first, but has the biggest negative impact on performance for the
project in question, and all dependent projects. Many users make use of
these rules without considering the alternatives, like grabbing the
snapshot tarball from GitHub. Whatever we can do to help the user make the
correct choices, especially if we make the wrong choices impossible, is
going to go a long way to fostering a healthy and lightning fast build
ecosystem spanning many GitHub projects that all reference each other.

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#1952 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADjHfzgrJ6tRZ8DwEjXfpJr2Yu-g31dhks5q1mMugaJpZM4KYF52
.