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

Stop copying jar files #1074

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
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
4 changes: 4 additions & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ maven_install(
],
)

load("@global_exclusion_testing//:defs.bzl", _global_exclusion_testing_pinned_maven_install = "pinned_maven_install")

_global_exclusion_testing_pinned_maven_install()

maven_install(
name = "manifest_stamp_testing",
artifacts = [
Expand Down
31 changes: 29 additions & 2 deletions coursier.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ load(
"SUPPORTED_PACKAGING_TYPES",
"contains_git_conflict_markers",
"escape",
"get_packaging",
"is_maven_local_path",
)
load("//private:dependency_tree_parser.bzl", "parser")
Expand Down Expand Up @@ -548,6 +549,14 @@ def _pinned_coursier_fetch_impl(repository_ctx):
netrc_entries = importer.get_netrc_entries(maven_install_json_content)

for artifact in importer.get_artifacts(maven_install_json_content):
if not artifact["sha256"]:
# some artifacts (most likely POMs) are defined like:
# {"coordinates": "org.apache.flink:flink-table:jar:sources:1.11.6", "sha256": None, "file": None, "deps": [], "urls": []}
# it makes no sense to generate an artifact for this at all, when this happens artifact will also be None
# which relates to https://github.com/bazelbuild/rules_jvm_external/issues/1028
continue
if not artifact["file"]:
fail("there's a sha256 for this artifact but no file defined: %s" % artifact)
http_file_repository_name = escape(artifact["coordinates"])
maven_artifacts.extend([artifact["coordinates"]])
http_files.extend([
Expand All @@ -559,6 +568,8 @@ def _pinned_coursier_fetch_impl(repository_ctx):
# File-path is relative defined from http_file traveling to repository_ctx.
" netrc = \"../%s/netrc\"," % (repository_ctx.name),
])
if get_packaging(artifact["coordinates"]) == "exe":
http_files.append(" executable = True,")
if len(artifact["urls"]) == 0 and importer.has_m2local(maven_install_json_content) and artifact.get("file") != None:
if _is_windows(repository_ctx):
user_home = repository_ctx.os.environ.get("USERPROFILE").replace("\\", "/")
Expand All @@ -575,7 +586,7 @@ def _pinned_coursier_fetch_impl(repository_ctx):

# https://github.com/bazelbuild/rules_jvm_external/issues/1028
# http_rule does not like directories named "build" so prepend v1 to the path.
download_path = "v1/%s" % artifact["file"] if artifact["file"] else artifact["file"]
download_path = "v1/%s" % artifact["file"]
http_files.append(" downloaded_file_path = \"%s\"," % download_path)
http_files.append(" )")

Expand Down Expand Up @@ -783,6 +794,16 @@ def make_coursier_dep_tree(
"--" +
":".join([e["group"], e["artifact"]]))

# extract java_version
java_path = _java_path(repository_ctx)
java_version = 0
if java_path:
exec_result = _execute(repository_ctx, [java_path, "-version"])
if (exec_result.return_code != 0):
fail("Error while running java -version: " + exec_result.stderr)

java_version = parse_java_version(exec_result.stdout + exec_result.stderr)

cmd = _generate_java_jar_command(repository_ctx, repository_ctx.path("coursier"))
cmd.extend(["fetch"])

Expand Down Expand Up @@ -850,7 +871,13 @@ def make_coursier_dep_tree(
exec_result = _execute(repository_ctx, [java_path, "-version"])
if (exec_result.return_code != 0):
fail("Error while running java -version: " + exec_result.stderr)
if parse_java_version(exec_result.stdout + exec_result.stderr) > 8:

java_version = parse_java_version(exec_result.stdout + exec_result.stderr)
if java_version > 13:
if "-noverify" in cmd:
cmd.remove("-noverify")

if java_version > 8:
java_cmd = cmd[0]
java_args = cmd[1:]

Expand Down
86 changes: 33 additions & 53 deletions private/dependency_tree_parser.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -29,26 +29,6 @@ load(
"strip_packaging_and_classifier_and_version",
)

def _genrule_copy_artifact_from_http_file(artifact, visibilities):
http_file_repository = escape(artifact["coordinates"])

file = artifact.get("out", artifact["file"])

genrule = [
"genrule(",
" name = \"%s_extension\"," % http_file_repository,
" srcs = [\"@%s//file\"]," % http_file_repository,
" outs = [\"%s\"]," % file,
" cmd = \"cp $< $@\",",
]
if get_packaging(artifact["coordinates"]) == "exe":
genrule.append(" executable = True,")
genrule.extend([
" visibility = [%s]" % (",".join(["\"%s\"" % v for v in visibilities])),
")",
])
return "\n".join(genrule)

def _deduplicate_list(items):
seen_items = {}
unique_items = []
Expand All @@ -66,6 +46,13 @@ def _find_repository_url(artifact_url, repositories):
longest_match = repository
return longest_match

_ARTIFACT_JAR = """
alias(
name = "{artifact_path}",
actual = "{source}",
visibility = ["//visibility:public"],
)"""

def _generate_target(
repository_ctx,
jar_versionless_target_labels,
Expand All @@ -87,7 +74,7 @@ def _generate_target(
# (jvm|aar)_import(
#
packaging = artifact_path.split(".").pop()
if packaging == "jar":
if packaging == "jar" or artifact_path.endswith("//file"):
# Regular `java_import` invokes ijar on all JARs, causing some Scala and
# Kotlin compile interface JARs to be incorrect. We replace java_import
# with a simple jvm_import Starlark rule that skips ijar.
Expand Down Expand Up @@ -120,7 +107,7 @@ def _generate_target(
# srcjar = "https/repo1.maven.org/maven2/org/hamcrest/hamcrest-library/1.3/hamcrest-library-1.3-sources.jar",
#
is_dylib = False
if packaging == "jar":
if packaging == "jar" or artifact_path.endswith("//file"):
target_import_string.append("\tjars = [\"%s\"]," % artifact_path)
if srcjar_paths != None and target_label in srcjar_paths:
target_import_string.append("\tsrcjar = \"%s\"," % srcjar_paths[target_label])
Expand All @@ -133,18 +120,18 @@ def _generate_target(
jar_versionless_target_labels.append(target_label)
dylib = simple_coord.split(":")[-1] + "." + packaging
to_return.append(
"""
"""
genrule(
name = "{dylib}_extension",
srcs = ["@{repository}//file"],
outs = ["{dylib}"],
cmd = "cp $< $@",
visibility = ["//visibility:public"],
)""".format(
dylib = dylib,
repository = escape(artifact["coordinates"])))


dylib = dylib,
repository = escape(artifact["coordinates"]),
),
)

# 4. Generate the deps attribute with references to other target labels.
#
Expand All @@ -161,7 +148,6 @@ genrule(
else:
target_import_string.append("\tdeps = [")


# Dedupe dependencies here. Sometimes coursier will return "x.y:z:aar:version" and "x.y:z:version" in the
# same list of dependencies.
target_import_labels = []
Expand Down Expand Up @@ -320,18 +306,6 @@ genrule(
to_return.append("alias(\n\tname = \"%s\",\n\tactual = \"%s\",\n%s)" %
(versioned_target_alias_label, target_label, alias_visibility))

# 11. If using maven_install.json, use a genrule to copy the file from the http_file
# repository into this repository.
#
# genrule(
# name = "org_hamcrest_hamcrest_library_1_3_extension",
# srcs = ["@org_hamcrest_hamcrest_library_1_3//file"],
# outs = ["@maven//:v1/https/repo1.maven.org/maven2/org/hamcrest/hamcrest-library/1.3/hamcrest-library-1.3.jar"],
# cmd = "cp $< $@",
# )
if repository_ctx.attr.maven_install_json:
to_return.append(_genrule_copy_artifact_from_http_file(artifact, default_visibilities))

return to_return

# Generate BUILD file with jvm_import and aar_import for each artifact in
Expand All @@ -351,6 +325,8 @@ def _generate_imports(repository_ctx, dependencies, explicit_artifacts, neverlin
# seen_imports :: string -> bool
seen_imports = {}

added_aliases = {}

# A list of versionless target labels for jar artifacts. This is used for
# generating a compatibility layer for repositories. For example, if we generate
# @maven//:junit_junit, we also generate @junit_junit//jar as an alias to it.
Expand Down Expand Up @@ -378,8 +354,6 @@ def _generate_imports(repository_ctx, dependencies, explicit_artifacts, neverlin
seen_imports[artifact_path] = True
target_label = escape(strip_packaging_and_classifier_and_version(artifact["coordinates"]))
srcjar_paths[target_label] = artifact_path
if repository_ctx.attr.maven_install_json:
all_imports.append(_genrule_copy_artifact_from_http_file(artifact, default_visibilities))

# Iterate through the list of artifacts, and generate the target declaration strings.
for artifact in dependencies:
Expand All @@ -401,31 +375,26 @@ def _generate_imports(repository_ctx, dependencies, explicit_artifacts, neverlin
elif repository_ctx.attr.fetch_javadoc and get_classifier(artifact["coordinates"]) == "javadoc":
seen_imports[target_label] = True
all_imports.append(
"filegroup(\n\tname = \"%s\",\n\tsrcs = [\"%s\"],\n\ttags = [\"javadoc\"],\n\tvisibility = [\"//visibility:public\"],\n)" % (target_label, artifact_path),
"filegroup(\n\tname = \"%s\",\n\tsrcs = [\"%s\"],\n\ttags = [\"javadoc\"],\n\tvisibility = [\"//visibility:public\"],\n)" % (target_label, escape(artifact["coordinates"])),
)
elif get_packaging(artifact["coordinates"]) in ("exe", "json"):
seen_imports[target_label] = True
versioned_target_alias_label = "%s_extension" % escape(artifact["coordinates"])
all_imports.append(
"alias(\n\tname = \"%s\",\n\tactual = \"%s\",\n\tvisibility = [\"//visibility:public\"],\n)" % (target_label, versioned_target_alias_label),
)
if repository_ctx.attr.maven_install_json:
all_imports.append(_genrule_copy_artifact_from_http_file(artifact, default_visibilities))
all_imports.append(
"alias(\n\tname = \"%s\",\n\tactual = \"@%s//file\",\n\tvisibility = [\"//visibility:public\"],\n)" % (target_label, escape(artifact["coordinates"])),
)
elif target_label in labels_to_override:
# Override target labels with the user provided mapping, instead of generating
# a jvm_import/aar_import based on information in dep_tree.
seen_imports[target_label] = True
all_imports.append(
"alias(\n\tname = \"%s\",\n\tactual = \"%s\",\n\tvisibility = [\"//visibility:public\"],)" % (target_label, labels_to_override.get(target_label)),
"alias(\n\tname = \"%s\",\n\tactual = \"%s\",\n\tvisibility = [\"//visibility:public\"],\n)" % (target_label, labels_to_override.get(target_label)),
)
if repository_ctx.attr.maven_install_json:
# Provide the downloaded artifact as a file target.
all_imports.append(_genrule_copy_artifact_from_http_file(artifact, default_visibilities))
raw_artifact = dict(artifact)
raw_artifact["coordinates"] = "original_" + artifact["coordinates"]
raw_artifact["maven_coordinates"] = artifact["coordinates"]
raw_artifact["out"] = "original_" + artifact["file"]

raw_artifact["file"] = "@%s//file" % escape(artifact["coordinates"])
all_imports.extend(_generate_target(
repository_ctx,
jar_versionless_target_labels,
Expand All @@ -440,6 +409,17 @@ def _generate_imports(repository_ctx, dependencies, explicit_artifacts, neverlin
))

elif artifact_path != None:
if artifact["file"] not in added_aliases:
added_aliases[artifact["file"]] = True
repo = escape(artifact["coordinates"])
if repository_ctx.attr.maven_install_json:
all_imports.append(
_ARTIFACT_JAR.format(
artifact_path = artifact["file"],
source = "@%s//file" % repo,
),
)

all_imports.extend(_generate_target(
repository_ctx,
jar_versionless_target_labels,
Expand Down
20 changes: 14 additions & 6 deletions private/extensions/download_pinned_deps.bzl
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_file")
load("//private:coursier_utilities.bzl", "get_packaging")
load("//private/rules:urls.bzl", "get_m2local_url")

def escape(string):
Expand Down Expand Up @@ -34,12 +35,19 @@ def download_pinned_deps(mctx, artifacts, http_files, has_m2local):
seen_repo_names.append(http_file_repository_name)
http_files.append(http_file_repository_name)

http_file(
name = http_file_repository_name,
sha256 = artifact["sha256"],
urls = urls,
attrs = {
"name": http_file_repository_name,
"sha256": artifact["sha256"],
"urls": urls,
}

if artifact["file"] and artifact["file"] != "None":
# https://github.com/bazelbuild/rules_jvm_external/issues/1028
downloaded_file_path = "v1/%s" % artifact["file"] if artifact["file"] else artifact["file"],
)
attrs["downloaded_file_path"] = "v1/%s" % artifact["file"]

if get_packaging(artifact["coordinates"]) == "exe":
attrs["executable"] = True

http_file(**attrs)

return seen_repo_names
4 changes: 2 additions & 2 deletions private/rules/jvm_import.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def _jvm_import_impl(ctx):

injar = ctx.files.jars[0]
if ctx.attr._stamp_manifest[StampManifestProvider].stamp_enabled:
outjar = ctx.actions.declare_file("processed_" + injar.basename, sibling = injar)
outjar = ctx.actions.declare_file("processed_" + injar.basename)
args = ctx.actions.args()
args.add_all(["--source", injar, "--output", outjar])
args.add_all(["--manifest-entry", "Target-Label:{target_label}".format(target_label = label)])
Expand All @@ -40,7 +40,7 @@ def _jvm_import_impl(ctx):
else:
outjar = injar

compilejar = ctx.actions.declare_file("header_" + injar.basename, sibling = injar)
compilejar = ctx.actions.declare_file("header_" + injar.basename)
args = ctx.actions.args()
args.add_all(["--source", outjar, "--output", compilejar])

Expand Down
14 changes: 7 additions & 7 deletions tests/custom_maven_install/regression_testing_install.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY": "THERE_IS_NO_DATA_ONLY_ZUUL",
"__INPUT_ARTIFACTS_HASH": 827556415,
"__RESOLVED_ARTIFACTS_HASH": -929249637,
"__RESOLVED_ARTIFACTS_HASH": 198634068,
"artifacts": {
"android.arch.core:common": {
"shasums": {
Expand Down Expand Up @@ -1477,7 +1477,7 @@
"org.openjfx:javafx-base": {
"shasums": {
"jar": "c5084a74417a89c69a0c122fae96a4b70bf619fc3d6218ea102a4047ec85ad04",
"mac": "2d8052a08fd2e5d98e1d5a16d724ea5dd02102879de20a193225f57199803983"
"linux": "2ebf6fa2cbbe1c8b4f7780e06e97beb038f644d5ecf9f15a41c5e88ee0ef9cf1"
},
"version": "11.0.1"
},
Expand Down Expand Up @@ -2429,7 +2429,7 @@
"org.objenesis:objenesis"
],
"org.openjfx:javafx-base": [
"org.openjfx:javafx-base:jar:mac"
"org.openjfx:javafx-base:jar:linux"
],
"org.ow2.asm:asm-analysis": [
"org.ow2.asm:asm-tree"
Expand Down Expand Up @@ -5623,7 +5623,7 @@
"org.objenesis.instantiator.util",
"org.objenesis.strategy"
],
"org.openjfx:javafx-base:jar:mac": [
"org.openjfx:javafx-base:jar:linux": [
"com.sun.javafx",
"com.sun.javafx.beans",
"com.sun.javafx.binding",
Expand Down Expand Up @@ -6065,7 +6065,7 @@
"org.mockito:mockito-core",
"org.objenesis:objenesis",
"org.openjfx:javafx-base",
"org.openjfx:javafx-base:jar:mac",
"org.openjfx:javafx-base:jar:linux",
"org.ow2.asm:asm",
"org.ow2.asm:asm-analysis",
"org.ow2.asm:asm-commons",
Expand Down Expand Up @@ -6334,7 +6334,7 @@
"org.mockito:mockito-core",
"org.objenesis:objenesis",
"org.openjfx:javafx-base",
"org.openjfx:javafx-base:jar:mac",
"org.openjfx:javafx-base:jar:linux",
"org.ow2.asm:asm",
"org.ow2.asm:asm-analysis",
"org.ow2.asm:asm-commons",
Expand Down Expand Up @@ -6603,7 +6603,7 @@
"org.mockito:mockito-core",
"org.objenesis:objenesis",
"org.openjfx:javafx-base",
"org.openjfx:javafx-base:jar:mac",
"org.openjfx:javafx-base:jar:linux",
"org.ow2.asm:asm",
"org.ow2.asm:asm-analysis",
"org.ow2.asm:asm-commons",
Expand Down