Skip to content

Bazel: add support for DirectoryFileObject #597

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

Merged
merged 1 commit into from
Jun 15, 2023
Merged

Conversation

olafurpg
Copy link
Member

Previously, the scip-java Bazel support only supported SimpleFileObject in the Java compiler API and it crashed when encountering other types of file objects like DirectoryFileObject, which are used in some Bazel builds in the wild. This commit adds support for DirectoryFileObject.

Additionally, this commit adds more detailed documentation on how to use scip-java with Bazel.

Test plan

Green CI.

Manually tested there was no regression in the steps below

❯ bazel build //... --@scip_java//semanticdb-javac:enabled=true
INFO: Build option --@scip_java//semanticdb-javac:enabled has changed, discarding analysis cache.
INFO: Analyzed target //src/main/java/example:example (0 packages loaded, 843 targets configured).
INFO: Found 1 target...
Target //src/main/java/example:example up-to-date:
  bazel-bin/src/main/java/example/libexample.jar
INFO: Elapsed time: 0.152s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action

❯ bazel run @scip_java//scip-semanticdb:bazel -- --sourceroot $PWD
INFO: Build option --@scip_java//semanticdb-javac:enabled has changed, discarding analysis cache.
INFO: Analyzed target @scip_java//scip-semanticdb:bazel (0 packages loaded, 863 targets configured).
INFO: Found 1 target...
Target @scip_java//scip-semanticdb:bazel up-to-date:
  bazel-bin/external/scip_java/scip-semanticdb/bazel.jar
  bazel-bin/external/scip_java/scip-semanticdb/bazel
INFO: Elapsed time: 0.184s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: bazel-bin/external/scip_java/scip-semanticdb/bazel --sourceroot /Users/olafurpg/dev/sourcegraph/scip-java/examples
INFO: Build completed successfully, 1 total action
running: bazel query kind('.*_import', @maven//...) --output=proto
done: /Users/olafurpg/dev/sourcegraph/scip-java/examples/bazel-example/index.scip

Verified

This commit was signed with the committer’s verified signature.
timfish Tim Fish
Previously, the scip-java Bazel support only supported SimpleFileObject
in the Java compiler API and it crashed when encountering other types of
file objects like DirectoryFileObject, which are used in some Bazel
builds in the wild. This commit adds support for DirectoryFileObject.

Additionally, this commit adds more detailed documentation on how to
use scip-java with Bazel.
@olafurpg olafurpg requested a review from keynmol June 15, 2023 07:03
@olafurpg olafurpg merged commit 145e4cd into main Jun 15, 2023
@olafurpg olafurpg deleted the olafurpg/source branch June 15, 2023 09:32
throw new IllegalArgumentException("unsupported source file: " + toString);
// The Bazel Java compiler constructs `SimpleFileObject/DirectoryFileObject` with a
// "user-friendly" name that points to the original source file and an underlying/actual
// file path in a temporary directory. We're constrained by having to use only public APIs of
Copy link
Contributor

Choose a reason for hiding this comment

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

We're constrained by having to use only public APIs of the Java compiler

Is this a bazel restriction? I dont think this is true for scip-java, we almost definitely use non public parts

Copy link
Member Author

Choose a reason for hiding this comment

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

Not a bazel restriction. We try to limit non-public API usage as much as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants