From 4f5a8bc78ab25ebf04ab72b0c40b0f4c00f4433f Mon Sep 17 00:00:00 2001 From: Paul Cody Johnston Date: Thu, 26 Oct 2017 19:51:56 -0600 Subject: [PATCH] Exclude omitted deps in generated BUILD and rules.bzl (#13) * Fixes bug(s) related to the omit attribute. * Drops antiquated local_maven_rules_repository * Include 0.6.1 in travis matrix Fixes #12 --- .travis.yml | 25 ++++----- README.md | 2 +- example/grpc_java/WORKSPACE | 4 +- .../local_rules_maven_repository.bzl | 28 ---------- example/robolectric/WORKSPACE | 4 +- .../local_rules_maven_repository.bzl | 28 ---------- example/scala/BUILD | 13 +++++ example/scala/WORKSPACE | 40 ++++++++++++++ example/scala/test_omit.py | 22 ++++++++ maven/internal/maven_repository.bzl | 53 ++++++++++++++----- 10 files changed, 130 insertions(+), 89 deletions(-) delete mode 100644 example/grpc_java/local_rules_maven_repository.bzl delete mode 100644 example/robolectric/local_rules_maven_repository.bzl create mode 100644 example/scala/BUILD create mode 100644 example/scala/WORKSPACE create mode 100644 example/scala/test_omit.py diff --git a/.travis.yml b/.travis.yml index 767208d..ecad608 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,6 +9,7 @@ os: env: #- V=HEAD + - V=0.6.1 - V=0.5.4 - V=0.5.3 - V=0.5.2 @@ -36,31 +37,23 @@ script: - | (cd example/grpc_java && bazel \ --output_base=$HOME/.cache/bazel \ - --batch \ - --host_jvm_args=-Xmx500m \ - --host_jvm_args=-Xms500m \ build \ --verbose_failures \ - --test_output=errors \ - --test_strategy=standalone \ - --spawn_strategy=standalone \ - --genrule_strategy=standalone \ - --local_resources=400,2,1.0 \ //src/main/java:main) \ && \ (cd example/robolectric && bazel \ --output_base=$HOME/.cache/bazel \ - --batch \ - --host_jvm_args=-Xmx500m \ - --host_jvm_args=-Xms500m \ test \ --verbose_failures \ --test_output=errors \ - --test_strategy=standalone \ - --spawn_strategy=standalone \ - --genrule_strategy=standalone \ - --local_resources=400,2,1.0 \ - //:import_test) + //:import_test) \ + && \ + (cd example/scala && bazel \ + --output_base=$HOME/.cache/bazel \ + test \ + --verbose_failures \ + --test_output=errors \ + //:test_omit) notifications: email: false diff --git a/README.md b/README.md index db763e9..d2b42e3 100644 --- a/README.md +++ b/README.md @@ -178,7 +178,7 @@ guice_compile() | `transitive_deps` | `string_list` | `[]` | List of maven artifacts in the transitive set reachable from `deps`. The have the form `SHA1:NAME:GROUP:VERSION`, and are calculated by rules_maven via a generated `build.gradle` file. | | `exclude` | `string_list_dict` | `{}` | List of artifacts to exclude, in the form `{ 'NAME:GROUP': ['EXCLUDED_GROUP:EXCLUDED_NAME']` | | `force` | `string_list` | `[]` | List of artifacts to force, in the form `[ 'NAME:GROUP:VERSION']` | -| `omit` | `string_list` | `[]` | List of artifacts to skip, in the form `[ 'NAME:GROUP:VERSION']` | +| `omit` | `string_list` | `[]` | List of patterns to skip. The pattern must either be a substring of the coordinate `[ 'NAME:GROUP:VERSION']` or equal to the generated workspace name. | | `repositories` | `string_list_dict` | `{}` | A mapping of artifact-id pattern to url (see below) | | `configurations` | `string_list` | `["compile", "default", "runtime", "compileOnly", "compileClasspath"]` | List of configurations to generate a corresponding rule for. | | `experimental_disambiguate_maven_jar_workspace_names` | `bool` | `False` | See Note | diff --git a/example/grpc_java/WORKSPACE b/example/grpc_java/WORKSPACE index 95ab895..30d5a2d 100644 --- a/example/grpc_java/WORKSPACE +++ b/example/grpc_java/WORKSPACE @@ -1,9 +1,9 @@ # Note: this 'local_rules_maven_repository' is to facilitate testing # and continuous integration. You should load rules_maven using more # traditional means (as per the main README.md). -load("//:local_rules_maven_repository.bzl", "local_rules_maven_repository") -local_rules_maven_repository( +local_repository( name = "org_pubref_rules_maven", + path = "../../", ) load("@org_pubref_rules_maven//maven:rules.bzl", "maven_repositories", "maven_repository") diff --git a/example/grpc_java/local_rules_maven_repository.bzl b/example/grpc_java/local_rules_maven_repository.bzl deleted file mode 100644 index 25976fc..0000000 --- a/example/grpc_java/local_rules_maven_repository.bzl +++ /dev/null @@ -1,28 +0,0 @@ -# -# Repository rule implementation that avoids the need for a -# `local_repository` rule referencing the parent WORKSPACE. -# - -def _local_rules_maven_repository_impl(rtx): - # Get the absolute path if the 'source' label and convert to a string - path = "%s" % rtx.path(rtx.attr._source) - - # Convert /Users/pcj/github/rules_maven/example/robolectric/local_rules_maven_repository.bzl - # to /Users/pcj/github/rules_maven/maven - parts = path.split('/') - maven_dir = "/".join(parts[0:-3] + ["maven"]) - - # Symlink it to expose this directory here. - rtx.symlink('/%s' % maven_dir, 'maven') - - -local_rules_maven_repository = repository_rule( - implementation = _local_rules_maven_repository_impl, - attrs = { - # We need to discover the absolute path of any file in the - # current directory to symlink the parent workspace maven dir. - "_source": attr.label( - default = Label("//:local_rules_maven_repository.bzl") - ), - }, -) diff --git a/example/robolectric/WORKSPACE b/example/robolectric/WORKSPACE index bde7092..56af864 100644 --- a/example/robolectric/WORKSPACE +++ b/example/robolectric/WORKSPACE @@ -1,6 +1,6 @@ -load("//:local_rules_maven_repository.bzl", "local_rules_maven_repository") -local_rules_maven_repository( +local_repository( name = "org_pubref_rules_maven", + path = "../../", ) load("@org_pubref_rules_maven//maven:rules.bzl", "maven_repositories", "maven_repository") diff --git a/example/robolectric/local_rules_maven_repository.bzl b/example/robolectric/local_rules_maven_repository.bzl deleted file mode 100644 index 25976fc..0000000 --- a/example/robolectric/local_rules_maven_repository.bzl +++ /dev/null @@ -1,28 +0,0 @@ -# -# Repository rule implementation that avoids the need for a -# `local_repository` rule referencing the parent WORKSPACE. -# - -def _local_rules_maven_repository_impl(rtx): - # Get the absolute path if the 'source' label and convert to a string - path = "%s" % rtx.path(rtx.attr._source) - - # Convert /Users/pcj/github/rules_maven/example/robolectric/local_rules_maven_repository.bzl - # to /Users/pcj/github/rules_maven/maven - parts = path.split('/') - maven_dir = "/".join(parts[0:-3] + ["maven"]) - - # Symlink it to expose this directory here. - rtx.symlink('/%s' % maven_dir, 'maven') - - -local_rules_maven_repository = repository_rule( - implementation = _local_rules_maven_repository_impl, - attrs = { - # We need to discover the absolute path of any file in the - # current directory to symlink the parent workspace maven dir. - "_source": attr.label( - default = Label("//:local_rules_maven_repository.bzl") - ), - }, -) diff --git a/example/scala/BUILD b/example/scala/BUILD new file mode 100644 index 0000000..9a1b145 --- /dev/null +++ b/example/scala/BUILD @@ -0,0 +1,13 @@ + +genquery( + name = "genson_compile_deps", + expression = "deps(@genson//:compile)", + scope = ["@genson//:compile"], +) + +py_test( + name = "test_omit", + srcs = ["test_omit.py"], + size = "small", + data = [":genson_compile_deps"], +) diff --git a/example/scala/WORKSPACE b/example/scala/WORKSPACE new file mode 100644 index 0000000..2335fbe --- /dev/null +++ b/example/scala/WORKSPACE @@ -0,0 +1,40 @@ +# +# See https://github.com/pubref/rules_maven/issues/12 +# Scala example by @BlueDragonX +# + +local_repository( + name = "org_pubref_rules_maven", + path = "../../", +) + +load("@org_pubref_rules_maven//maven:rules.bzl", "maven_repositories", "maven_repository") +maven_repositories() + +scala_version = "2.11" + +maven_repository( + name = "genson", + # Omit can take two forms: either the specific name of the + # workspace, or some substring of the artifact maven coodinate. + # Either form should work to exclude the dependency. NOTE: the + # transitive dependency is still printed in the transitive deps + # but has the token 'omit' rather than the sha1 value. + omit = [ + # workspace name variety + 'org_scala_lang_scala_library', + # artifact group:name variety + 'org.scala-lang:scala-reflect', + ], + deps = [ + "com.owlike:genson-scala_%s:1.4" % scala_version, + ], + transitive_deps = [ + '8d6f7ee301653b6a221483663cf1c72a52967c8a:com.owlike:genson:1.4', + '6309a5ca43d54eb9cbfe1776909f40c1d4c23503:com.owlike:genson-scala_2.11:1.4', + 'omit:org.scala-lang:scala-library:2.11.0', + 'omit:org.scala-lang:scala-reflect:2.11.0', + ], +) +load("@genson//:rules.bzl", "genson_compile") +genson_compile() diff --git a/example/scala/test_omit.py b/example/scala/test_omit.py new file mode 100644 index 0000000..895a743 --- /dev/null +++ b/example/scala/test_omit.py @@ -0,0 +1,22 @@ +# test_omit.py: Loop through the text file generated by the genquery +# rule that lists the deps of @genson//:compile. Make sure all +# whitelisted deps are present and all blacklisted deps are absent. + +white = [ + "@com_owlike_genson//jar:jar", + "@com_owlike_genson_scala_2_11//jar:jar", +] + +black = [ + "@org_scala_lang_scala_library//jar:jar", + "@org_scala_lang_scala_reflect//jar:jar", +] + +with open('genson_compile_deps') as f: + lines = [line for line in f.read().splitlines()] + deps = set(lines) + if not set(white).issubset(deps): + raise ValueError("One or more deps were expected to be present but not found in deps list: %s" % white) + if not deps.isdisjoint(set(black)): + raise ValueError("One or more deps were expected to be absent, but *were* present in deps list: %s" % black) + diff --git a/maven/internal/maven_repository.bzl b/maven/internal/maven_repository.bzl index 3c93033..0fb845e 100644 --- a/maven/internal/maven_repository.bzl +++ b/maven/internal/maven_repository.bzl @@ -222,7 +222,7 @@ def _parse_gradle_dependencies(rtx, transitive_artifacts, configurations, out, d return configs -def _format_build_file(configs): +def _format_build_file(rtx, configs): """Generate the BUILD file content. Args: configs: !dict> @@ -231,11 +231,11 @@ def _format_build_file(configs): lines = [] lines.append("# AUTO_GENERATED, DO NOT EDIT") for name, artifacts in configs.items(): - lines += _format_java_library(name, artifacts) + lines += _format_java_library(rtx, name, artifacts) return "\n".join(lines) -def _format_java_library(name, artifacts): +def _format_java_library(rtx, name, artifacts): """Format a java_library_rule. Args: name: string - the workspace name. @@ -246,13 +246,37 @@ def _format_java_library(name, artifacts): lines.append("java_library(") lines.append(" name = '%s'," % name) lines.append(" exports = [") - for ws_name in artifacts.keys(): - lines.append(" '@%s//jar'," % ws_name) + for ws_name, artifact in artifacts.items(): + if not should_omit(rtx.attr.omit, artifact): + lines.append(" '@%s//jar'," % ws_name) lines.append(" ],") lines.append(" visibility = ['//visibility:public'],") lines.append(")") return lines + +def should_omit(names, artifact): + """Return true if the given artifact matches on of the patterns given in [names]. + + Foreach name in the given list, see if it is either (1) a + substring of the maven coordinate or (2) equal to the workspace + name. For example, 'org.scala-lang' will return true for all + artifacts in that group, and 'org_scala_lang_scala_reflect' will + true true for that specific workspace. + + Args: + name: string_list - a list of string patterns to test. + artifact: !Artifact - the artifact object to match + Returns: boolean + """ + for name in names: + if name in artifact["coordinate"]: + return True + if name == artifact["ws_name"]: + return True + return False + + def _get_repository_url(rules, artifact): for url in rules.keys(): rule_list = rules[url] @@ -315,11 +339,7 @@ def _format_config_def(rtx, name, artifacts): lines.append("def %s(deps = DEPS, excludes = [], overrides = {}):" % name) lines.append(" require([") for ws_name, artifact in artifacts.items(): - should_include = True - for target in rtx.attr.omit: - if target == ws_name: - should_include = False - if should_include: + if not should_omit(rtx.attr.omit, artifact): lines.append(" '%s'," % ws_name) lines.append(" ], deps = deps, excludes = excludes, overrides = overrides)") return lines @@ -360,10 +380,19 @@ def _format_maven_repository(rtx, configs, all_artifacts): lines.append(" '%s'," % item) lines.append(" ],") + if rtx.attr.omit: + lines.append(" omit = [") + for item in rtx.attr.omit: + lines.append(" '%s'," % item) + lines.append(" ],") + lines.append(" transitive_deps = [") for ws_name in ws_names: artifact = all_artifacts[ws_name] - lines.append(" '%s:%s'," % (artifact["sha1"], artifact["coordinate"])) + if should_omit(rtx.attr.omit, artifact): + lines.append(" 'omit:%s'," % (artifact["coordinate"])) + else: + lines.append(" '%s:%s'," % (artifact["sha1"], artifact["coordinate"])) lines.append(" ],") if rtx.attr.experimental_disambiguate_maven_jar_workspace_names: @@ -527,7 +556,7 @@ def _maven_repository_impl(rtx): lines = _format_maven_repository(rtx, configs, transitive_artifacts) print("\n# CLOSED-FORM RULE:\n# You can copy this to your WORKSPACE To suppress this message. \n%s\n" % "\n".join(lines)) - rtx.file("BUILD", _format_build_file(configs)); + rtx.file("BUILD", _format_build_file(rtx, configs)); rtx.file("rules.bzl", _format_rules_file(rtx, rtx.name, configs, transitive_artifacts));