-
-
Notifications
You must be signed in to change notification settings - Fork 287
Switch to JarsToLabels provider and rework/cleanup scala_import #477
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
Changes from 2 commits
6c5147c
2ff3b38
a458f91
7cd9df4
8e21db9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
JarsToLabels = provider( | ||
doc = 'provides a mapping from jar files to defining labels for improved end user experience', | ||
fields = { | ||
'lookup' : 'dictionary with jar files as keys and labels as values', | ||
}, | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,114 +1,81 @@ | ||
#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 | ||
load(":providers.bzl", "JarsToLabels") | ||
|
||
def _scala_import_impl(ctx): | ||
target_data = _code_jars_and_intellij_metadata_from(ctx.attr.jars) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @andyscott why did you remove all the private methods? current code feels a bit like a wall of text |
||
(current_target_compile_jars, intellij_metadata) = (target_data.code_jars, target_data.intellij_metadata) | ||
current_jars = depset(current_target_compile_jars) | ||
exports = _collect(ctx.attr.exports) | ||
transitive_runtime_jars = _collect_runtime(ctx.attr.runtime_deps) | ||
jars = _collect(ctx.attr.deps) | ||
jars2labels = {} | ||
_collect_labels(ctx.attr.deps, jars2labels) | ||
_collect_labels(ctx.attr.exports, jars2labels) #untested | ||
_add_labels_of_current_code_jars(depset(transitive=[current_jars, exports.compile_jars]), ctx.label, jars2labels) #last to override the label of the export compile jars to the current target | ||
return struct( | ||
scala = struct( | ||
outputs = struct ( | ||
jars = intellij_metadata | ||
), | ||
|
||
# Tread lightly when modifying this code! IntelliJ support needs | ||
# to be tested manually: manually [re-]import an intellij project | ||
# and ensure imports are resolved (not red) and clickable | ||
|
||
direct_binary_jars = [] | ||
for jar in ctx.attr.jars: | ||
for file in jar.files: | ||
if not file.basename.endswith("-sources.jar"): | ||
direct_binary_jars += [file] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why use += here but append above? Can we use append here too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's proof that I am not as consistent as I wish to be. (Good catch, I'll fix it!) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm pretty sure that I wrote the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we're touching this, wdyt about moving it to a private method as well? same level of abstraction for the top level |
||
|
||
|
||
s_deps = java_common.merge([ | ||
entry[JavaInfo] | ||
for entry in ctx.attr.deps | ||
if JavaInfo in entry]) | ||
|
||
s_exports = java_common.merge([ | ||
entry[JavaInfo] | ||
for entry in ctx.attr.exports | ||
if JavaInfo in entry]) | ||
|
||
s_runtime_deps = java_common.merge([ | ||
entry[JavaInfo] | ||
for entry in ctx.attr.runtime_deps | ||
if JavaInfo in entry]) | ||
|
||
|
||
compile_time_jars = depset( | ||
direct = direct_binary_jars, | ||
transitive = [ | ||
s_exports.transitive_compile_time_jars]) | ||
|
||
transitive_compile_time_jars = depset( | ||
transitive = [ | ||
compile_time_jars, | ||
s_deps.transitive_compile_time_jars, | ||
s_exports.transitive_compile_time_jars]) | ||
|
||
transitive_runtime_jars = depset( | ||
transitive = [ | ||
compile_time_jars, | ||
s_deps.transitive_runtime_jars, | ||
s_exports.transitive_runtime_jars, | ||
s_runtime_deps.transitive_runtime_jars]) | ||
|
||
|
||
lookup = {} | ||
for jar in direct_binary_jars: | ||
lookup[jar.path] = ctx.label | ||
|
||
for entry in ctx.attr.deps: | ||
if JavaInfo in entry: | ||
for jar in entry[JavaInfo].compile_jars: | ||
lookup[jar.path] = entry.label | ||
if JarsToLabels in entry: | ||
lookup.update(entry[JarsToLabels].lookup) | ||
|
||
for entry in ctx.attr.exports: | ||
if JavaInfo in entry: | ||
for jar in entry[JavaInfo].compile_jars: | ||
lookup[jar.path] = entry.label | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we won't change this to |
||
if JarsToLabels in entry: | ||
lookup.update(entry[JarsToLabels].lookup) | ||
|
||
return [ | ||
JarsToLabels(lookup = lookup), | ||
java_common.create_provider( | ||
ctx.actions, | ||
use_ijar = False, | ||
compile_time_jars = compile_time_jars, | ||
transitive_compile_time_jars = transitive_compile_time_jars, | ||
transitive_runtime_jars = transitive_runtime_jars | ||
), | ||
jars_to_labels = jars2labels, | ||
providers = [ | ||
_create_provider(current_jars, transitive_runtime_jars, jars, exports) | ||
], | ||
) | ||
def _create_provider(current_target_compile_jars, transitive_runtime_jars, jars, exports): | ||
test_provider = java_common.create_provider() | ||
if hasattr(test_provider, "full_compile_jars"): | ||
return java_common.create_provider( | ||
use_ijar = False, | ||
compile_time_jars = depset(transitive = [current_target_compile_jars, exports.compile_jars]), | ||
transitive_compile_time_jars = depset(transitive = [jars.transitive_compile_jars, current_target_compile_jars, exports.transitive_compile_jars]) , | ||
transitive_runtime_jars = depset(transitive = [transitive_runtime_jars, jars.transitive_runtime_jars, current_target_compile_jars, exports.transitive_runtime_jars]) , | ||
) | ||
else: | ||
return java_common.create_provider( | ||
compile_time_jars = current_target_compile_jars, | ||
runtime_jars = transitive_runtime_jars + jars.transitive_runtime_jars, | ||
transitive_compile_time_jars = jars.transitive_compile_jars + current_target_compile_jars, | ||
transitive_runtime_jars = transitive_runtime_jars + jars.transitive_runtime_jars + current_target_compile_jars, | ||
) | ||
|
||
def _add_labels_of_current_code_jars(code_jars, label, jars2labels): | ||
for jar in code_jars.to_list(): | ||
jars2labels[jar.path] = label | ||
|
||
def _code_jars_and_intellij_metadata_from(jars): | ||
code_jars = [] | ||
intellij_metadata = [] | ||
for jar in jars: | ||
current_jar_code_jars = _filter_out_non_code_jars(jar.files) | ||
code_jars += current_jar_code_jars | ||
for current_class_jar in current_jar_code_jars: #intellij, untested | ||
intellij_metadata.append(struct( | ||
ijar = None, | ||
class_jar = current_class_jar, | ||
source_jar = None, | ||
source_jars = [], | ||
) | ||
) | ||
return struct(code_jars = code_jars, intellij_metadata = intellij_metadata) | ||
|
||
def _filter_out_non_code_jars(files): | ||
return [file for file in files.to_list() if not _is_source_jar(file)] | ||
|
||
def _is_source_jar(file): | ||
return file.basename.endswith("-sources.jar") | ||
|
||
def _collect(deps): | ||
transitive_compile_jars = [] | ||
runtime_jars = [] | ||
compile_jars = [] | ||
|
||
for dep_target in deps: | ||
java_provider = dep_target[java_common.provider] | ||
compile_jars.append(java_provider.compile_jars) | ||
transitive_compile_jars.append(java_provider.transitive_compile_time_jars) | ||
runtime_jars.append(java_provider.transitive_runtime_jars) | ||
|
||
return struct(transitive_runtime_jars = depset(transitive = runtime_jars), | ||
transitive_compile_jars = depset(transitive = transitive_compile_jars), | ||
compile_jars = depset(transitive = compile_jars)) | ||
|
||
def _collect_labels(deps, jars2labels): | ||
for dep_target in deps: | ||
java_provider = dep_target[java_common.provider] | ||
_transitively_accumulate_labels(dep_target, java_provider,jars2labels) | ||
|
||
def _transitively_accumulate_labels(dep_target, java_provider, jars2labels): | ||
if hasattr(dep_target, "jars_to_labels"): | ||
jars2labels.update(dep_target.jars_to_labels) | ||
#scala_library doesn't add labels to the direct dependency itself | ||
for jar in java_provider.compile_jars.to_list(): | ||
jars2labels[jar.path] = dep_target.label | ||
|
||
def _collect_runtime(runtime_deps): | ||
jar_deps = [] | ||
for dep_target in runtime_deps: | ||
java_provider = dep_target[java_common.provider] | ||
jar_deps.append(java_provider.transitive_runtime_jars) | ||
|
||
return depset(transitive = jar_deps) | ||
|
||
def _collect_exports(exports): | ||
exported_jars = depset() | ||
|
||
for dep_target in exports: | ||
java_provider = dep_target[java_common.provider] | ||
exported_jars += java_provider.full_compile_jars | ||
|
||
return exported_jars | ||
] | ||
|
||
scala_import = rule( | ||
implementation=_scala_import_impl, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did we delete the comment here? Seems like it is still relevant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overlooked it while porting this code over. I'll add it back (and actually prove out that this works for IntelliJ).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment added back in the body of the method.