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

"Don't build unnecessary jars" causes built-with-aspect failure #6614

Closed
tpasternak opened this issue Aug 6, 2024 · 6 comments · Fixed by #6623
Closed

"Don't build unnecessary jars" causes built-with-aspect failure #6614

tpasternak opened this issue Aug 6, 2024 · 6 comments · Fixed by #6623
Assignees
Labels
awaiting-maintainer Awaiting review from Bazel team on issues lang: java Java rules integration product: IntelliJ IntelliJ plugin type: bug

Comments

@tpasternak
Copy link
Collaborator

tpasternak commented Aug 6, 2024

Description of the bug:

for jar in java_outputs:
if jar.ijar:
jar_artifacts.append(jar.ijar)
elif jar.class_jar and not should_optimize_building_jars:
jar_artifacts.append(jar.class_jar)
if hasattr(jar, "source_jars") and jar.source_jars:
source_jar_artifacts.extend(_list_or_depset_to_list(jar.source_jars))
elif hasattr(jar, "source_jar") and jar.source_jar:
source_jar_artifacts.append(jar.source_jar)
if should_optimize_building_jars:
if len(source_jar_artifacts) == 0 or len(jar_artifacts) == 0:
jar_artifacts.extend([jar.class_jar for jar in java_outputs.jars if jar.class_jar])

In line 789 we assume that java_outputs is an iterable (it is a List), while in the other place, in line 801 we assume it has an attribute named jars. Hence, it is impossible to import even this repository (bazelbuild/intellij). It results in

Error: 'list' value has no field or method 'jars'

@tpasternak tpasternak added type: bug awaiting-maintainer Awaiting review from Bazel team on issues labels Aug 6, 2024
@tpasternak
Copy link
Collaborator Author

cc @nkoroste

@nkoroste
Copy link
Contributor

nkoroste commented Aug 6, 2024

Related to #5515
The fix should be simple

@iancha1992 iancha1992 added product: IntelliJ IntelliJ plugin lang: java Java rules integration labels Aug 6, 2024
@tpasternak
Copy link
Collaborator Author

Yeah, but how it was working for you? Will it work if we just change java_outputs.jars to java_outputs?

@nkoroste
Copy link
Contributor

nkoroste commented Aug 7, 2024

Honestly I don't remember anymore, this PR took so long to get it. All existing and new tests are passing in this public repo on this PR.

Did you inspect the output of java_outputs in the passing and failing case? Curious to see how it looks like and why you guys are only hitting it internally if I'm understanding it right.

@tpasternak
Copy link
Collaborator Author

so java_outputs is just a list. Is it possible that in your case the condition if len(source_jar_artifacts) == 0 or len(jar_artifacts) == 0: is never satisfied?

tpasternak added a commit to tpasternak/bazel-intellij that referenced this issue Aug 8, 2024
@mai93 mai93 closed this as completed in f7a7ae1 Aug 8, 2024
@nkoroste
Copy link
Contributor

nkoroste commented Aug 8, 2024

so java_outputs is just a list. Is it possible that in your case the condition if len(source_jar_artifacts) == 0 or len(jar_artifacts) == 0: is never satisfied?

Yep that could be it.

p.s. thanks for the fix!

mai93 pushed a commit that referenced this issue Aug 8, 2024
…ptimize_building_jars is enabled (#6623)

closes #6614

(cherry picked from commit f7a7ae1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-maintainer Awaiting review from Bazel team on issues lang: java Java rules integration product: IntelliJ IntelliJ plugin type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants