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 preliminary support for exe packaging type #908

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,10 @@ maven_install(
group = "com.google.apis",
version = "v1-rev235-1.25.0",
),
# https://github.com/bazelbuild/rules_jvm_external/issues/907
# Any two platforms to ensure that it doesn't work _only_ under the host operating system
Copy link
Collaborator

Choose a reason for hiding this comment

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

Excellent. Thank you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do thank you, @shs96c, knowing how time-consuming code reviews can be...

"com.google.protobuf:protoc:exe:linux-x86_64:3.21.12",
"com.google.protobuf:protoc:exe:osx-aarch_64:3.21.12",
],
generate_compat_repositories = True,
maven_install_json = "//tests/custom_maven_install:regression_testing_install.json",
Expand Down
7 changes: 5 additions & 2 deletions coursier.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -666,8 +666,11 @@ def make_coursier_dep_tree(
cmd.extend(artifact_coordinates)
if version_conflict_policy == "pinned":
for coord in artifact_coordinates:
# Undo any `,classifier=` suffix from `utils.artifact_coordinate`.
cmd.extend(["--force-version", coord.split(",classifier=")[0]])
# Undo any `,classifier=` and/or `,type=` suffix from `utils.artifact_coordinate`.
cmd.extend([
"--force-version",
",".join([c for c in coord.split(",") if not c.startswith("classifier=") and not c.startswith("type=")]),
])
cmd.extend(["--artifact-type", ",".join(SUPPORTED_PACKAGING_TYPES + ["src", "doc"])])
cmd.append("--verbose" if _is_verbose(repository_ctx) else "--quiet")
cmd.append("--no-default")
Expand Down
1 change: 1 addition & 0 deletions private/coursier_utilities.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ SUPPORTED_PACKAGING_TYPES = [
"aar",
"bundle",
"eclipse-plugin",
"exe",
"orbit",
"test-jar",
"hk2-jar",
Expand Down
10 changes: 8 additions & 2 deletions private/dependency_tree_parser.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,20 @@ def _genrule_copy_artifact_from_http_file(artifact, visibilities):
if not len(artifact["urls"]):
return ""
http_file_repository = escape(artifact["coordinates"])
return "\n".join([
genrule = [
"genrule(",
" name = \"%s_extension\"," % http_file_repository,
" srcs = [\"@%s//file\"]," % http_file_repository,
" outs = [\"%s\"]," % artifact["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 = {}
Expand Down Expand Up @@ -110,6 +115,7 @@ def _generate_imports(repository_ctx, dependencies, explicit_artifacts, neverlin
# Iterate through the list of artifacts, and generate the target declaration strings.
for artifact in dependencies:
artifact_path = artifact["file"]

# Skip the maven local dependencies if requested
if skip_maven_local_dependencies and is_maven_local_path(artifact_path):
continue
Expand All @@ -128,7 +134,7 @@ def _generate_imports(repository_ctx, dependencies, explicit_artifacts, neverlin
all_imports.append(
"filegroup(\n\tname = \"%s\",\n\tsrcs = [\"%s\"],\n\ttags = [\"javadoc\"],\n)" % (target_label, artifact_path),
)
elif get_packaging(artifact["coordinates"]) == "json":
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(
Expand Down
6 changes: 4 additions & 2 deletions specs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -251,13 +251,15 @@ json = struct(
#

#
# Couriser expects artifacts to be defined in the form `group:artifact:version`, but it also supports two attributes: classifier and url.
# Coursier expects artifacts to be defined in the form `group:artifact:version`, but it also supports some attributes:
# `url`, `ext`, `classifier`, `exclude` and `type`.
# In contrast with group, artifact and version, the attributes are a key=value comma-separated string appended at the end,
# For example: `coursier fetch group:artifact:version,classifier=xxx,url=yyy`
#
def _artifact_to_coord(artifact):
classifier = (",classifier=" + artifact["classifier"]) if artifact.get("classifier") != None else ""
return artifact["group"] + ":" + artifact["artifact"] + ":" + artifact["version"] + classifier
type = (",type=" + artifact["packaging"]) if artifact.get("packaging") != None else ""
return artifact["group"] + ":" + artifact["artifact"] + ":" + artifact["version"] + classifier + type

#
# Returns a string "{hostname} {user}:{password}" for a repository_spec
Expand Down
17 changes: 15 additions & 2 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": 187641354,
"__RESOLVED_ARTIFACTS_HASH": -835404132,
"__INPUT_ARTIFACTS_HASH": 1163673577,
"__RESOLVED_ARTIFACTS_HASH": 1891065348,
"artifacts": {
"android.arch.core:common": {
"shasums": {
Expand Down Expand Up @@ -519,6 +519,13 @@
},
"version": "3.7.0"
},
"com.google.protobuf:protoc:exe": {
"shasums": {
"linux-x86_64": "e6a75cf6660945fbfec0d321808442ce3649cd7820079eece1098c881b315f55",
"osx-aarch_64": "9f40368c858666e7036df0b9b58a4acd085fed58ab531294c3e5b5820f8af360"
},
"version": "3.21.12"
},
"com.googlecode.matrix-toolkits-java:mtj": {
"shasums": {
"jar": "27a53db335bc6af524b30f97ec3fb4b6df65e7648d70e752447c7dd9bc4697c8"
Expand Down Expand Up @@ -5601,6 +5608,8 @@
"com.google.j2objc:j2objc-annotations",
"com.google.oauth-client:google-oauth-client",
"com.google.protobuf:protobuf-java",
"com.google.protobuf:protoc:exe:linux-x86_64",
"com.google.protobuf:protoc:exe:osx-aarch_64",
"com.googlecode.matrix-toolkits-java:mtj",
"com.googlecode.netlib-java:netlib-java",
"com.intellij:annotations",
Expand Down Expand Up @@ -5852,6 +5861,8 @@
"com.google.j2objc:j2objc-annotations",
"com.google.oauth-client:google-oauth-client",
"com.google.protobuf:protobuf-java",
"com.google.protobuf:protoc:exe:linux-x86_64",
"com.google.protobuf:protoc:exe:osx-aarch_64",
"com.googlecode.matrix-toolkits-java:mtj",
"com.googlecode.netlib-java:netlib-java",
"com.intellij:annotations",
Expand Down Expand Up @@ -6103,6 +6114,8 @@
"com.google.j2objc:j2objc-annotations",
"com.google.oauth-client:google-oauth-client",
"com.google.protobuf:protobuf-java",
"com.google.protobuf:protoc:exe:linux-x86_64",
"com.google.protobuf:protoc:exe:osx-aarch_64",
"com.googlecode.matrix-toolkits-java:mtj",
"com.googlecode.netlib-java:netlib-java",
"com.intellij:annotations",
Expand Down
18 changes: 18 additions & 0 deletions tests/unit/build_tests/BUILD
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
load("@bazel_skylib//rules:build_test.bzl", "build_test")
load("@bazel_skylib//rules:native_binary.bzl", "native_binary")

# Regression tests for particular artifacts that revealed problems for this
# project.
Expand All @@ -19,6 +20,23 @@ build_test(
"@regression_testing//:com_fasterxml_jackson_jackson_bom",
"@regression_testing//:org_junit_junit_bom",
"@regression_testing//:io_netty_netty_tcnative_boringssl_static",
"@regression_testing//:com_google_protobuf_protoc_linux_x86_64",
"@regression_testing//:com_google_protobuf_protoc_osx_aarch_64",
],
)

# Note: skylib's `native_test` would fail on "Exec format error" under the other platform, hence the following 2 steps
[native_binary(
name = "src_that_refers_to_a_valid_executable_{}_target".format(platform),
src = "@regression_testing//:com_google_protobuf_protoc_{}".format(platform),
out = "protoc_{}.exe".format(platform),
) for platform in ("linux_x86_64", "osx_aarch_64")]

build_test(
name = "native_binaries_indeed_refer_to_valid_executable_targets",
targets = [
"src_that_refers_to_a_valid_executable_{}_target".format(platform)
for platform in ("linux_x86_64", "osx_aarch_64")
],
)

Expand Down