Skip to content

Commit

Permalink
Fix Bazel incompatible issues. (#648)
Browse files Browse the repository at this point in the history
* Fix Bazel incompatible issues.

Some refs:
- bazelbuild/bazel#5817
  • Loading branch information
xingao267 authored Jan 11, 2019
1 parent 7899c5b commit a98bc3b
Show file tree
Hide file tree
Showing 21 changed files with 95 additions and 85 deletions.
18 changes: 18 additions & 0 deletions .bazelci/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ platforms:
# tests/contrib/test_compare_ids_test_* expect 'bazel' on path
test_flags:
- "--action_env=PATH"
- "--all_incompatible_changes"
# TODO(xingao): Remove after https://github.com/bazelbuild/rules_scala/issues/644 is addressed.
- "--incompatible_disallow_legacy_javainfo=false"
ubuntu1604:
test_targets:
# We are running the skipped targets remotely only.
Expand All @@ -36,15 +39,24 @@ platforms:
# tests/contrib/test_compare_ids_test_* expect 'bazel' on path
test_flags:
- "--action_env=PATH"
- "--all_incompatible_changes"
# TODO(xingao): Remove after https://github.com/bazelbuild/rules_scala/issues/644 is addressed.
- "--incompatible_disallow_legacy_javainfo=false"
macos:
build_targets:
- "//tests/docker:test_digest_output1"
build_flags:
- "--action_env=PATH"
- "--all_incompatible_changes"
# TODO(xingao): Remove after https://github.com/bazelbuild/rules_scala/issues/644 is addressed.
- "--incompatible_disallow_legacy_javainfo=false"
test_targets:
- "//tests/docker:test_digest_output1"
test_flags:
- "--action_env=PATH"
- "--all_incompatible_changes"
# TODO(xingao): Remove after https://github.com/bazelbuild/rules_scala/issues/644 is addressed.
- "--incompatible_disallow_legacy_javainfo=false"
rbe_ubuntu1604:
build_targets:
- "--"
Expand All @@ -53,6 +65,9 @@ platforms:
- "--extra_execution_platforms=@bazel_toolchains//configs/ubuntu16_04_clang/1.1:nosla_xenial_docker"
- "--host_platform=@bazel_toolchains//configs/ubuntu16_04_clang/1.1:nosla_xenial_docker"
- "--platforms=@bazel_toolchains//configs/ubuntu16_04_clang/1.1:nosla_xenial_docker"
- "--all_incompatible_changes"
# TODO(xingao): Remove after https://github.com/bazelbuild/rules_scala/issues/644 is addressed.
- "--incompatible_disallow_legacy_javainfo=false"
test_targets:
- "--"
- "//tests/..."
Expand All @@ -66,3 +81,6 @@ platforms:
- "--extra_execution_platforms=@bazel_toolchains//configs/ubuntu16_04_clang/1.1:nosla_xenial_docker"
- "--host_platform=@bazel_toolchains//configs/ubuntu16_04_clang/1.1:nosla_xenial_docker"
- "--platforms=@bazel_toolchains//configs/ubuntu16_04_clang/1.1:nosla_xenial_docker"
- "--all_incompatible_changes"
# TODO(xingao): Remove after https://github.com/bazelbuild/rules_scala/issues/644 is addressed.
- "--incompatible_disallow_legacy_javainfo=false"
26 changes: 15 additions & 11 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -160,19 +160,23 @@ load(

_java_image_repos()

load("@bazel_tools//tools/build_defs/repo:jvm.bzl", "jvm_maven_import_external")

# For our java_image test.
maven_jar(
jvm_maven_import_external(
name = "com_google_guava_guava",
artifact = "com.google.guava:guava:18.0",
sha1 = "cce0823396aa693798f8882e64213b1772032b09",
artifact_sha256 = "d664fbfc03d2e5ce9cab2a44fb01f1d0bf9dfebeccc1a473b1f9ea31f79f6f99",
licenses = ["notice"], # Apache 2.0
server_urls = ["http://central.maven.org/maven2"],
)

# For our scala_image test.
http_archive(
name = "io_bazel_rules_scala",
sha256 = "83d40e0bc7377e77fa0d32af6c4b276374b4efbcb3120c437e425947cdb3ce38",
strip_prefix = "rules_scala-5130b97524684beceba729b9dab1528e2a90cdfb",
urls = ["https://github.com/bazelbuild/rules_scala/archive/5130b97524684beceba729b9dab1528e2a90cdfb.tar.gz"],
sha256 = "902e30b931ded41905641895b90c41727e01a732aba67dfda604b764c1e1e494",
strip_prefix = "rules_scala-1354d935a74395b3f0870dd90a04e0376fe22587",
urls = ["https://github.com/bazelbuild/rules_scala/archive/1354d935a74395b3f0870dd90a04e0376fe22587.tar.gz"],
)

load("@io_bazel_rules_scala//scala:scala.bzl", "scala_repositories")
Expand Down Expand Up @@ -219,9 +223,9 @@ _go_image_repos()
# For our rust_image test
http_archive(
name = "io_bazel_rules_rust",
sha256 = "500d06096a44ff6d77256635dbe6ab61b23c2be626e2acb08a4c060092e711d0",
strip_prefix = "rules_rust-db81b42d98e1232e001e26a50c37f2097d61a207",
urls = ["https://github.com/bazelbuild/rules_rust/archive/db81b42d98e1232e001e26a50c37f2097d61a207.tar.gz"],
sha256 = "ed0c81084bcc2bdcc98cfe56f384b20856840825f5e413e2b71809b61809fc87",
strip_prefix = "rules_rust-f32695dcd02d9a19e42b9eb7f29a24a8ceb2b858",
urls = ["https://github.com/bazelbuild/rules_rust/archive/f32695dcd02d9a19e42b9eb7f29a24a8ceb2b858.tar.gz"],
)

load("@io_bazel_rules_rust//rust:repositories.bzl", "rust_repositories")
Expand All @@ -237,9 +241,9 @@ bazel_version(name = "bazel_version")
# For our d_image test
http_archive(
name = "io_bazel_rules_d",
sha256 = "527908e02d7bccf5a4eb89b690b003247eb6c57d69cc3234977c034d27c59d6e",
strip_prefix = "rules_d-0400b9b054013274cee2ed15679da19e1fc94e07",
urls = ["https://github.com/bazelbuild/rules_d/archive/0400b9b054013274cee2ed15679da19e1fc94e07.tar.gz"],
sha256 = "873022774f2f31ab57e7ff36b3f39c60fd4209952bfcc6902924b7942fa2973d",
strip_prefix = "rules_d-2d38613073f3eb138aee0acbcb395ebada2f8ebf",
urls = ["https://github.com/bazelbuild/rules_d/archive/2d38613073f3eb138aee0acbcb395ebada2f8ebf.tar.gz"],
)

load("@io_bazel_rules_d//d:d.bzl", "d_repositories")
Expand Down
6 changes: 3 additions & 3 deletions container/container.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -193,9 +193,9 @@ py_library(
if "subpar" not in excludes:
http_archive(
name = "subpar",
sha256 = "ee65e35c1cd9a723fb4d501e8055e10b34a27a0a557d10312af7b83d8e0101f5",
strip_prefix = "subpar-7e12cc130eb8f09c8cb02c3585a91a4043753c56",
urls = ["https://github.com/google/subpar/archive/7e12cc130eb8f09c8cb02c3585a91a4043753c56.tar.gz"],
sha256 = "cf3762b10426a1887d37f127b4c1390785ecb969254096eb714cc1db371f78d6",
strip_prefix = "subpar-a4f9b23bf01bcc7a52d458910af65a90ee991aff",
urls = ["https://github.com/google/subpar/archive/a4f9b23bf01bcc7a52d458910af65a90ee991aff.tar.gz"],
)

if "structure_test_linux" not in excludes:
Expand Down
4 changes: 2 additions & 2 deletions container/image.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,8 @@ def _assemble_image_digest(ctx, name, image, image_tarball, output_digest):

ctx.actions.run(
outputs = [output_digest],
inputs = [image["config"]] + blobsums + blobs +
([image["legacy"]] if image.get("legacy") else []),
tools = [image["config"]] + blobsums + blobs +
([image["legacy"]] if image.get("legacy") else []),
executable = ctx.executable._digester,
arguments = arguments,
mnemonic = "ImageDigest",
Expand Down
3 changes: 1 addition & 2 deletions container/image_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -708,8 +708,7 @@ def test_java_image(self):
'./app/io_bazel_rules_docker/testdata',
'./app/io_bazel_rules_docker/testdata/libjava_image_library.jar',
'./app/com_google_guava_guava',
'./app/com_google_guava_guava/jar',
'./app/com_google_guava_guava/jar/guava-18.0.jar',
'./app/com_google_guava_guava/guava-18.0.jar',
])

def test_war_image(self):
Expand Down
2 changes: 1 addition & 1 deletion container/layer.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ def build_layer(
ctx.actions.run(
executable = build_layer_exec,
arguments = args,
inputs = files + file_map.values() + tars + debs + [manifest_file],
tools = files + file_map.values() + tars + debs + [manifest_file],
outputs = [layer],
use_default_shell_env = True,
mnemonic = "ImageLayer",
Expand Down
4 changes: 2 additions & 2 deletions container/layer_tools.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def _extract_layers(ctx, name, artifact):
"--manifestoutput",
manifest_file.path,
],
inputs = [artifact],
tools = [artifact],
outputs = [config_file, manifest_file],
mnemonic = "ExtractConfig",
)
Expand Down Expand Up @@ -102,7 +102,7 @@ def assemble(ctx, images, output, stamp = False):
ctx.actions.run(
executable = ctx.executable.join_layers,
arguments = args,
inputs = inputs,
tools = inputs,
outputs = [output],
mnemonic = "JoinLayers",
)
Expand Down
2 changes: 1 addition & 1 deletion contrib/compare_ids_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
def _compare_ids_test_impl(ctx):
tar_files = []
for image in ctx.attr.images:
tar_files += list(image.files)
tar_files += image.files.to_list()

if (len(tar_files) == 0):
fail("No images provided for test.")
Expand Down
8 changes: 3 additions & 5 deletions contrib/idd.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Used for finding differences between image targets.
Args:
image1: Image target or image tarball file (from docker save) - first image to compare
image2: Image target or image tarball file (from docker save) - second image to compare
args: (optional) list of strings - arguments to apply to idd.py call
args: (optional) list of strings - arguments to apply to idd.py call
refer to idd.py docs for more info
Ex.
Expand All @@ -56,13 +56,11 @@ idd = rule(
attrs = {
"image1": attr.label(
mandatory = True,
allow_files = container_filetype,
single_file = True,
allow_single_file = container_filetype,
),
"image2": attr.label(
mandatory = True,
allow_files = container_filetype,
single_file = True,
allow_single_file = container_filetype,
),
"_idd_script": attr.label(
default = ":idd",
Expand Down
8 changes: 3 additions & 5 deletions contrib/push-all.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ def _impl(ctx):

return struct(runfiles = ctx.runfiles(files = [
ctx.executable._pusher,
] + stamp_inputs + runfiles + list(ctx.attr._pusher.default_runfiles.files)))
] + stamp_inputs + runfiles + ctx.attr._pusher.default_runfiles.files.to_list()))

container_push = rule(
attrs = {
Expand All @@ -110,13 +110,11 @@ container_push = rule(
),
"_all_tpl": attr.label(
default = Label("//contrib:push-all.sh.tpl"),
single_file = True,
allow_files = True,
allow_single_file = True,
),
"_tag_tpl": attr.label(
default = Label("//container:push-tag.sh.tpl"),
single_file = True,
allow_files = True,
allow_single_file = True,
),
"_pusher": attr.label(
default = Label("@containerregistry//:pusher"),
Expand Down
6 changes: 2 additions & 4 deletions contrib/test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,7 @@ _container_test = rule(
),
"image_tar": attr.label(
doc = "When using the tar driver, label of the container image tarball",
allow_files = [".tar"],
single_file = True,
allow_single_file = [".tar"],
),
"loaded_name": attr.string(
doc = "When using the docker driver, the name:tag of the image when loaded into the docker daemon",
Expand Down Expand Up @@ -105,8 +104,7 @@ _container_test = rule(
),
"_structure_test_tpl": attr.label(
default = Label("//contrib:structure-test.sh.tpl"),
allow_files = True,
single_file = True,
allow_single_file = True,
),
},
executable = True,
Expand Down
40 changes: 18 additions & 22 deletions java/image.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ The signature of java_image is compatible with java_binary.
The signature of war_image is compatible with java_library.
"""

load("@bazel_tools//tools/build_defs/repo:jvm.bzl", "jvm_maven_import_external")
load(
"//container:container.bzl",
"container_pull",
Expand Down Expand Up @@ -75,9 +76,12 @@ def repositories():
digest = _JETTY_DIGESTS["debug"],
)
if "javax_servlet_api" not in excludes:
native.maven_jar(
jvm_maven_import_external(
name = "javax_servlet_api",
artifact = "javax.servlet:javax.servlet-api:3.0.1",
artifact_sha256 = "377d8bde87ac6bc7f83f27df8e02456d5870bb78c832dac656ceacc28b016e56",
server_urls = ["http://central.maven.org/maven2"],
licenses = ["notice"], # Apache 2.0
)

DEFAULT_JAVA_BASE = select({
Expand All @@ -98,15 +102,15 @@ def java_files(f):
files = []
if java_common.provider in f:
java_provider = f[java_common.provider]
files += list(java_provider.transitive_runtime_jars)
files += java_provider.transitive_runtime_jars.to_list()
if hasattr(f, "files"): # a jar file
files += list(f.files)
files += f.files.to_list()
return files

def java_files_with_data(f):
files = java_files(f)
if hasattr(f, "data_runfiles"):
files += list(f.data_runfiles.files)
files += f.data_runfiles.files.to_list()
return files

def _jar_dep_layer_impl(ctx):
Expand Down Expand Up @@ -150,28 +154,24 @@ jar_dep_layer = rule(
def _jar_app_layer_impl(ctx):
"""Appends the app layer with all remaining runfiles."""

available = depset()
for jar in ctx.attr.jar_layers:
available += java_files(jar) # layers don't include runfiles
# layers don't include runfiles
available = depset(transitive = [depset(java_files(jar)) for jar in ctx.attr.jar_layers])

# We compute the set of unavailable stuff by walking deps
# in the same way, adding in our binary and then subtracting
# out what it available.
unavailable = depset()
for jar in ctx.attr.deps + ctx.attr.runtime_deps:
unavailable += java_files_with_data(jar)

unavailable += java_files_with_data(ctx.attr.binary)
unavailable = [x for x in unavailable if x not in available]
unavailable = depset(transitive = [depset(java_files_with_data(jar)) for jar in ctx.attr.deps + ctx.attr.runtime_deps])
unavailable = depset(transitive = [unavailable, depset(java_files_with_data(ctx.attr.binary))])
unavailable = [x for x in unavailable.to_list() if x not in available.to_list()]

# Remove files that are provided by the JDK from the unavailable set,
# as these will be provided by the Java image.
jdk_files = depset(list(ctx.files._jdk))
jdk_files = depset(ctx.files._jdk)
unavailable = [x for x in unavailable if x not in ctx.files._jdk]

classpath = ":".join([
layer_file_path(ctx, x)
for x in available + unavailable
for x in depset(transitive = [available, depset(unavailable)]).to_list()
])

# Classpaths can grow long and there is a limit on the length of a
Expand Down Expand Up @@ -349,19 +349,15 @@ _war_dep_layer = rule(
def _war_app_layer_impl(ctx):
"""Appends the app layer with all remaining runfiles."""

available = depset()
for jar in ctx.attr.jar_layers:
available += java_files(jar)
available = depset(transitive = [depset(java_files(jar)) for jar in ctx.attr.jar_layers])

# This is based on rules_appengine's WAR rules.
transitive_deps = depset()
transitive_deps += java_files(ctx.attr.library)

transitive_deps = depset(java_files(ctx.attr.library))
# TODO(mattmoor): Handle data files.

# If we start putting libs in servlet-agnostic paths,
# then consider adding symlinks here.
files = [d for d in transitive_deps if d not in available]
files = [d for d in transitive_deps.to_list() if d not in available.to_list()]

return _container.image.implementation(ctx, files = files)

Expand Down
Loading

0 comments on commit a98bc3b

Please sign in to comment.