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

Add generate_compat_repositories attribute to maven_install for top level repository aliases and cross-workspace dependencies #160

Merged
merged 6 commits into from
Jun 7, 2019

Conversation

jin
Copy link
Member

@jin jin commented Jun 4, 2019

This PR adds a new attribute, generate_compat_repositories, to maven_install. The default is False. The motivation for this change is #85. (Fixes #85)

With this, we can refer to @maven//:junit_junit as @junit_junit//jar, where the latter is the most commonly used naming convention that is popularized by rules like maven_jar and java_import_external. This enables project maintainers to incrementall migrate from maven_jar to rules_jvm_external without a single change in their BUILD files. It also allows cross-external repository sharing of artifacts.

If enabled, maven_install generates an additional compat.bzl file in the @maven (or your custom) external repository. This file exposes one symbol, compat_repositories, to be loaded after maven_install, like this:

maven_install(
    # ...
    generate_compat_repositories = True,
)

load("@maven//:compat.bzl", "compat_repositories")
compat_repositories()

compat_repositories expands into a list of compat_repository declarations, one for each JAR artifact in the transitive closure of the resolved dependency tree. For example:

$ cat compat.bzl 
load("@maven//:compat_repository.bzl", "compat_repository")
def compat_repositories():
    compat_repository(name = "android_arch_core_common")
    compat_repository(name = "android_arch_lifecycle_common")
    compat_repository(name = "androidx_annotation_annotation")
    compat_repository(name = "com_android_support_support_annotations")
    compat_repository(name = "com_google_code_findbugs_jsr305")
    compat_repository(name = "com_squareup_javawriter")
    compat_repository(name = "javax_inject_javax_inject")
    compat_repository(name = "junit_junit")
    compat_repository(name = "net_sf_kxml_kxml2")
    compat_repository(name = "org_ccil_cowan_tagsoup_tagsoup")
    compat_repository(name = "org_hamcrest_hamcrest_core")
    compat_repository(name = "org_hamcrest_hamcrest_integration")
    compat_repository(name = "org_hamcrest_hamcrest_library")

Each compat_repository creates a new external repository in output_base/external. In the case of junit, the directory is output_base/external/junit_junit, and the contents are as follows:

jingwen@jingwen:~/.cache/bazel/_bazel_jingwen/f1b46d82f4f66241b4c332b551d2226c/external/junit_junit
$ tree
.
├── jar
│   └── BUILD
└── WORKSPACE

The WORKSPACE is an empty file. The BUILD file contains an alias to the @maven//:junit_junit target:

$ cat jar/BUILD 
alias(name = "jar", actual = "@maven//:junit_junit")

Using this, we can refer to the target as @junit_junit//jar:jar, or @junit_junit//jar for short.

TODO

  • Tests
  • Documentation

@jin
Copy link
Member Author

jin commented Jun 4, 2019

(fyi @dslomov, this can help with maven_jar deprecation)

@jin jin mentioned this pull request Jun 4, 2019
@Globegitter
Copy link

@jin that is a little bit off-topic but I wonder if it makes sense to write some "best practices" on how to expose external dependencies to not always having to think how to import an external dep when switching language. I feel nearly every rule does it differently, e.g. rules_nodejs and rules_pip also expose a single repository like the default here but they expose it without the :, so @npm//package or @pip//package respectively. Or rules_go also does it per-repository and exposes things as @com_github_spf13_cobra//:go_default_library. Though with gazelle one rarely has to write things per hand there. And I have seen other rules that expose a function, so one imports it in each BUILD file and then can write something like pip('package') or similar.

I personally like the function exposing way the least and I do not really have a preference on one repository or multiple repositories but I do prefer not having to write the :, it does save one character and somehow does feel more "natural".

I am also happy to move this conversation to an issue on bazel itself or on Slack to not sidetrack things any further here.

coursier.bzl Outdated Show resolved Hide resolved
@jin
Copy link
Member Author

jin commented Jun 5, 2019

@Globegitter thanks for raising this discussion. I personally haven't thought much about setting some kind of convention across these rules, and I think it's a good idea to do that in order to reduce cognitive load.

This PR set ups the framework for @foo_bar//jar, and it's trivial to extend that by also creating @maven//foo_bar:foo_bar/@maven//foo_bar, on top of @maven//:foo_bar. However, I'm on the fence about adding yet another alias that's very similar to @maven//:foo_bar. @foo_bar//jar has the additional functionality of better interop with the ecosystem.

Let's fork this discussion onto a new GitHub issue?

@jin jin requested a review from laurentlb as a code owner June 5, 2019 21:02
@jin jin merged commit 8e9fefb into bazelbuild:master Jun 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Top-level repository aliases and cross-workspace dependencies
4 participants