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

java_binary rules labeled as manual disables the Debug icon #4546

Closed
rogerhu opened this issue Mar 4, 2023 · 11 comments
Closed

java_binary rules labeled as manual disables the Debug icon #4546

rogerhu opened this issue Mar 4, 2023 · 11 comments
Labels
lang: java Java rules integration P3 We're not considering working on this, but happy to review a PR. (No assignee) product: IntelliJ IntelliJ plugin topic: sync Issues related to the sync operation type: feature request

Comments

@rogerhu
Copy link
Contributor

rogerhu commented Mar 4, 2023

Description of the bug:

Suppose I have this Java rule:

java_binary(
    name = "shaded_jar",
    main_class = "com.squareup.MyApp",
    tags = ["manual"],
    runtime_deps = ["//myapp/src/main/java:lib"],
)

Because it is added with a manual tag, the IDE information (e.g.intellij-info.txt) will not be generated for this rule (confirmed by looking inside the bazel-out directory). There is a specific tag that excludes it here, which prevents the kind from derived. We can see this phenomenon in the UI:

image

As a result, the Debug icon gets disabled. Without the metadata intellij-info.txt created, the Bazel kind cannot be determined. As a result, the run configuration is viewed as not debuggable here.

A few options I can see:

  1. Do nothing. Remove the manual tag and run resync and you can workaround this issue.
  2. Stop performing Debug icon check (e.g. disable returning false here)
  3. Run a query on the command bazel query --output=label_kind //myapp:shaded_jar to figure out if it can derive things regardless of the manual tag. Changes to the ProjectTargetFinder method is likely to invoke the Bazel command should this information not be retrieved from intellij-info.txt.
  4. When syncing scan all targets and do not exclude those tagged with manual here

#1 is the manual way to workaround the issue, #2 is the simplest, #3 is the most additive, and #4 may create other side effects.

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

  1. Add manual tag to the example: https://github.com/bazelbuild/bazel/blob/master/examples/java-native/src/main/java/com/example/myproject/BUILD#L7

  2. Sync the repo.

  3. Add a Run Configuration with this target. Watch the debug icon get disabled!

Which Intellij IDE are you using? Please provide the specific version.

IntellIJ 2022.1 - but usiing upstream Bazel IntelliJ to repro

What programming languages and tools are you using? Please provide specific versions.

No response

What Bazel plugin version are you using?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

Confirmed that kind is null when manual tag is set:

image

Other references:

@rogerhu
Copy link
Contributor Author

rogerhu commented Mar 4, 2023

Posting #4547 as a proposal for option #3... please provide any feedback about this change?

@sgowroji sgowroji added type: bug product: IntelliJ IntelliJ plugin lang: java Java rules integration topic: debugging Issues related to debugging awaiting-maintainer Awaiting review from Bazel team on issues labels Mar 5, 2023
@mai93 mai93 added type: feature request P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) topic: sync Issues related to the sync operation and removed awaiting-maintainer Awaiting review from Bazel team on issues type: bug topic: debugging Issues related to debugging labels Mar 8, 2023
@mai93
Copy link
Collaborator

mai93 commented Mar 8, 2023

Another workaround is to explicitly specify the manual targets in the project view (.bazelproject file) under the targets section.

@mai93 mai93 removed their assignment Mar 8, 2023
@rogerhu
Copy link
Contributor Author

rogerhu commented Mar 8, 2023

@mai93 can you take a look at #4547 and provide feedback about whether this is acceptable?

@mai93 mai93 added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) labels May 1, 2023
@sfc-gh-abalik
Copy link
Contributor

Thanks for adding this @rogerhu. I tried allow_manual_targets_sync: true in my project and noticed that manual java_test targets are still not getting synced. I don't see intellij-info.txt files being generated for them.

My .bazelproject looks like:

targets:
  //MyApp/...

Do you expect allow_manual_targets_sync: true to work in this case? I'm guessing some work might be needed on the aspect and/or how it's invoked to ensure it picks up manual targets.

@rogerhu
Copy link
Contributor Author

rogerhu commented Aug 16, 2023

Thanks for adding this @rogerhu. I tried allow_manual_targets_sync: true in my project and noticed that manual java_test targets are still not getting synced. I don't see intellij-info.txt files being generated for them.

My .bazelproject looks like:

targets:
  //MyApp/...

Do you expect allow_manual_targets_sync: true to work in this case? I'm guessing some work might be needed on the aspect and/or how it's invoked to ensure it picks up manual targets.

It's supposed to stop applying the manual to exclude targets. There may be a bug somewhere we should check.

@rogerhu
Copy link
Contributor Author

rogerhu commented Aug 16, 2023

Can you try to remove the target and use derive_targets_from_directories: true instead and see if it shows up?

@sfc-gh-abalik
Copy link
Contributor

sfc-gh-abalik commented Aug 16, 2023

Can you try to remove the target and use derive_targets_from_directories: true instead and see if it shows up?

Yes it picks up the manual targets if I switch to derive_targets_from_directories: true. I think I see why:

In my original project, the aspect invocation looks like:

bazel build --tool_tag=ijwb:IDEA:community --aspects=@@intellij_aspect//:intellij_info_bundled.bzl%intellij_info_aspect  -- //MyApp/...

So any test target marked as manual would be excluded there. When I switch to derive_targets_from_directories it builds every target found by the bazel query that you modified, which includes the manual ones.

@rogerhu
Copy link
Contributor Author

rogerhu commented Oct 1, 2023

Can you try to remove the target and use derive_targets_from_directories: true instead and see if it shows up?

Yes it picks up the manual targets if I switch to derive_targets_from_directories: true. I think I see why:

In my original project, the aspect invocation looks like:

bazel build --tool_tag=ijwb:IDEA:community --aspects=@@intellij_aspect//:intellij_info_bundled.bzl%intellij_info_aspect  -- //MyApp/...

So any test target marked as manual would be excluded there. When I switch to derive_targets_from_directories it builds every target found by the bazel query that you modified, which includes the manual ones.

Yep, I just reviewed this issue. The aspect approach relies on bazel build to generate the IDE intellij-info.txt. The problem is that manual tags are excluded for bazel build invocations (https://bazel.build/run/build). I really think the aspect needs to first query available targets (e.g. bazel query "//src/main/java/com/example/example:all") after it receives the manual targets for the IDE integration to work correctly.

@rogerhu
Copy link
Contributor Author

rogerhu commented Oct 1, 2023

Proof of concept here: 178be12

I think I'll need to hide this behind the extra option as well.

@rogerhu
Copy link
Contributor Author

rogerhu commented Oct 11, 2023

I've dropped #5458 as a proposal to make this work for your issue!

@mrohan-ca
Copy link

@sfc-gh-abalik It looks like this issue has been fixed and can be closed.

@mai93 mai93 closed this as completed Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang: java Java rules integration P3 We're not considering working on this, but happy to review a PR. (No assignee) product: IntelliJ IntelliJ plugin topic: sync Issues related to the sync operation type: feature request
Projects
Development

No branches or pull requests

5 participants