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

Stamp scala_import jars #1160

Merged
merged 11 commits into from
Jan 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 51 additions & 4 deletions scala/scala_import.bzl
Original file line number Diff line number Diff line change
@@ -1,5 +1,40 @@
load("@io_bazel_rules_scala//scala:jars_to_labels.bzl", "JarsToLabelsInfo")

def _stamp_symlinked_jar(ctx, jar):
symlink_file = ctx.actions.declare_file(jar.basename)
ctx.actions.symlink(output = symlink_file, target_file = jar)

stamped_jar_filename = jar.basename.rstrip(".jar") + "-stamped.jar"

# Preferred way, but currently broken:
# java toolchain's ijar incorrectly handles MANIFEST sections
# https://github.com/bazelbuild/bazel/issues/12730
# return java_common.stamp_jar(
# actions = ctx.actions,
# jar = symlink_file,
# target_label = ctx.label,
# java_toolchain = ctx.attr._java_toolchain[java_common.JavaToolchainInfo],
# )

# Stamp with custom built (https://github.com/bazelbuild/bazel/pull/12771)
stamped_file = ctx.actions.declare_file(stamped_jar_filename)

ctx.actions.run(
executable = ctx.executable._ijar,
inputs = [symlink_file],
outputs = [stamped_file],
arguments = [
"--nostrip_jar",
"--target_label",
ctx.label.name,
symlink_file.path,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why symlink is needed? Can't you just pass jar.path?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because bazel does not allow modifications outside package

stamped_file.path,
],
mnemonic = "StampWithIjar",
)

return stamped_file

#intellij part is tested manually, tread lightly when changing there
#if you change make sure to manually re-import an intellij project and see imports
#are resolved (not red) and clickable
Expand All @@ -12,11 +47,17 @@ def _scala_import_impl(ctx):
current_target_compile_jars,
intellij_metadata,
) = (target_data.code_jars, target_data.intellij_metadata)
current_jars = depset(current_target_compile_jars)

current_stamped_jars = [
_stamp_symlinked_jar(ctx, jar)
for jar in current_target_compile_jars
]

current_jars = depset(current_stamped_jars)

exports = java_common.merge([export[JavaInfo] for export in ctx.attr.exports])
transitive_runtime_jars = \
java_common.merge([dep[JavaInfo] for dep in ctx.attr.runtime_deps]) \
.transitive_runtime_jars
java_common.merge([dep[JavaInfo] for dep in ctx.attr.runtime_deps]).transitive_runtime_jars
jars2labels = {}
_collect_labels(ctx.attr.deps, jars2labels)
_collect_labels(ctx.attr.exports, jars2labels) #untested
Expand All @@ -27,7 +68,7 @@ def _scala_import_impl(ctx):
) #last to override the label of the export compile jars to the current target

if current_target_compile_jars:
current_target_providers = [_new_java_info(ctx, jar) for jar in current_target_compile_jars]
current_target_providers = [_new_java_info(ctx, jar) for jar in current_stamped_jars]
else:
# TODO(#8867): Migrate away from the placeholder jar hack when #8867 is fixed.
current_target_providers = [_new_java_info(ctx, ctx.file._placeholder_jar)]
Expand Down Expand Up @@ -114,5 +155,11 @@ scala_import = rule(
allow_single_file = True,
default = Label("@io_bazel_rules_scala//scala:libPlaceHolderClassToCreateEmptyJarForScalaImport.jar"),
),
"_ijar": attr.label(
default = Label("@io_bazel_rules_scala//third_party/java_tools/ijar:ijar"),
executable = True,
cfg = "exec",
allow_files = True,
),
},
)
Original file line number Diff line number Diff line change
@@ -1,17 +1,49 @@
package scalarules.test.scala_import

import java.util.jar
import java.util.jar.JarFile

import com.google.common.cache.Cache
import org.apache.commons.lang3.ArrayUtils
import org.specs2.matcher.Matcher
import org.specs2.mutable.SpecificationWithJUnit

import scala.reflect.{ClassTag, _}

class ScalaImportExposesJarsTest extends SpecificationWithJUnit {
val targetLabel = "//test/src/main/scala/scalarules/test/scala_import:guava_and_commons_lang"

"scala_import" should {
"enable using the jars it exposes" in {
println(classOf[Cache[String, String]])
println(classOf[ArrayUtils])
success
}

"stamp jars with a target label" in {
findManifest[Cache[String, String]] must haveTargetLabel
findManifest[ArrayUtils] must haveTargetLabel
}

"preserve existing Manifest attributes" in {
findManifest[ArrayUtils] must haveMainAttribute("Bundle-Name")
}
}

def findManifest[T: ClassTag]: jar.Manifest = {
val file = classTag[T].runtimeClass.getProtectionDomain.getCodeSource.getLocation.getFile
val jar = new JarFile(file)
val manifest = jar.getManifest
jar.close()
manifest
}

def haveTargetLabel: Matcher[jar.Manifest] = haveMainAttribute("Target-Label")

def haveMainAttribute(attribute: String): Matcher[jar.Manifest] = {
not(beNull[String]) ^^ { (m: jar.Manifest) =>
m.getMainAttributes.getValue(attribute) aka s"an attribute $attribute"
}
}

}
8 changes: 4 additions & 4 deletions third_party/dependency_analyzer/src/test/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ licenses(["notice"]) # 3-clause BSD
load("//scala:scala.bzl", "scala_junit_test", "scala_test")

common_jvm_flags = [
"-Dplugin.jar.location=$(location //third_party/dependency_analyzer/src/main:dependency_analyzer)",
"-Dscala.library.location=$(location //external:io_bazel_rules_scala/dependency/scala/scala_library)",
"-Dscala.reflect.location=$(location //external:io_bazel_rules_scala/dependency/scala/scala_reflect)",
"-Dplugin.jar.location=$(execpath //third_party/dependency_analyzer/src/main:dependency_analyzer)",
"-Dscala.library.location=$(rootpath //external:io_bazel_rules_scala/dependency/scala/scala_library)",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't remember the difference between location and rootpath. Do you remember?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think they are good for runtime information, but as we use them there's some info on them:

https://docs.bazel.build/versions/master/be/make-variables.html#predefined_label_variables

and

bazelbuild/bazel#2475 (comment)

"-Dscala.reflect.location=$(rootpath //external:io_bazel_rules_scala/dependency/scala/scala_reflect)",
]

scala_test(
Expand Down Expand Up @@ -64,7 +64,7 @@ scala_test(
"io/bazel/rulesscala/dependencyanalyzer/StrictDepsTest.scala",
],
jvm_flags = common_jvm_flags + [
"-Dguava.jar.location=$(location @com_google_guava_guava_21_0_with_file//jar)",
"-Dguava.jar.location=$(rootpath @com_google_guava_guava_21_0_with_file//jar)",
"-Dapache.commons.jar.location=$(location @org_apache_commons_commons_lang_3_5_without_file//:linkable_org_apache_commons_commons_lang_3_5_without_file)",
],
unused_dependency_checker_mode = "off",
Expand Down
132 changes: 132 additions & 0 deletions third_party/java_tools/ijar/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
package(
default_visibility = [
"//visibility:public",
],
)

licenses(["notice"]) # Apache 2.0

cc_library(
name = "zip",
srcs = [
"mapped_file_unix.cc",
"zip.cc",
],
hdrs = [
"common.h",
"mapped_file.h",
"zip.h",
],
deps = [
":platform_utils",
":zlib_client",
],
)

cc_library(
name = "zlib_client",
srcs = ["zlib_client.cc"],
hdrs = [
"common.h",
"zlib_client.h",
],
deps = ["//third_party/java_tools/zlib"],
)

cc_library(
name = "platform_utils",
srcs = ["platform_utils.cc"],
hdrs = [
"common.h",
"platform_utils.h",
],
deps = [
"//third_party/java_tools/src/main/cpp/util:errors",
"//third_party/java_tools/src/main/cpp/util:filesystem",
"//third_party/java_tools/src/main/cpp/util:logging",
],
)
#
#cc_binary(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe remove this?

# name = "zipper",
# srcs = ["zip_main.cc"],
# visibility = ["//visibility:public"],
# deps = [":zip"],
#)

cc_binary(
name = "ijar",
srcs = [
"classfile.cc",
"ijar.cc",
],
deps = [":zip"],
)

filegroup(
name = "srcs",
srcs = glob(["**"]),
)

#filegroup(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say remove commented coded

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left it commented out to have it easier to update if needed. Otherwise it's a bit harder to pick what's needed to be changed.

# name = "embedded_zipper_sources",
# srcs = [
# "zip.cc",
# "zip.h",
# "zip_main.cc",
# "common.h",
# "mapped_file.h",
# "platform_utils.cc",
# "platform_utils.h",
# "zlib_client.cc",
# "zlib_client.h",
# "BUILD",
# ] + select({
# "//src:windows": [
# "mapped_file_windows.cc",
# ],
# "//conditions:default": [
# "mapped_file_unix.cc",
# ],
# }),
# visibility = ["//visibility:public"],
#)

#filegroup(
# name = "transitive_sources",
# srcs = [":srcs"] + ["//src/main/cpp/util:embedded_java_tools"],
#)

#genrule(
# name = "ijar_transitive_srcs_zip",
# srcs = [
# ":ijar_srcs_zip",
# "//src:zlib_zip",
# "//src/main/cpp/util:cpp_util_with_deps_zip",
# ],
# outs = ["ijar_srcs_with_deps.zip"],
# cmd = "$(location //src:merge_zip_files) java_tools $@ $(SRCS)",
# tools = ["//src:merge_zip_files"],
#)

#genrule(
# name = "ijar_deploy_zip",
# srcs = [
# ":ijar",
# ":zipper",
# ],
# outs = ["ijar_deploy.zip"],
# cmd = "$(location //src:zip_files) java_tools/ijar $@ $(SRCS)",
# tools = ["//src:zip_files"],
#)

#genrule(
# name = "ijar_srcs_zip",
# srcs = glob(
# ["**"],
# exclude = ["BUILD"],
# ),
# outs = ["ijar_srcs.zip"],
# cmd = "$(location //src:zip_files) ijar $@ $(SRCS)",
# tools = ["//src:zip_files"],
#)
Loading