Add Bazel support, fixes #88 by olafurpg · Pull Request #439 · sourcegraph/scip-java

olafurpg · 2022-05-05T09:46:03Z

This PR adds a new Bazel build to this codebase so that Bazel users can
build the semanticdb-javac compiler plugin from source and use it in
their Bazel build. This PR does not yet include documentation on how to use this configuration,
but this PR adds an end-to-end example of how to import the lsif-java rules via include(...)

Test plan

See new "bazel" CI job.

This commit adds a new Bazel build to this codebase so that Bazel users can build the semanticdb-javac compiler plugin from source and use it in their Bazel build. This commit includes examples of how a Bazel user could potentially enable the compiler plugin dynamically based on a command-line flag, but we don't know if this is the best way to integrate lsif-java with a Bazel codebase. This commit is just an incremental step towards Bazel support, it's not a complete solution yet because the `lsif-java index-semanticdb` command still needs additional functionality to read jar files from the `bazel-bin` directory.

varungandhi-src · 2022-05-05T14:08:04Z

Manually test the Bazel build. We don't yet compile the Bazel build in CI.

How hard would it be to add this? Any specific reason to do it later vs now?

WORKSPACE

olafurpg · 2022-05-05T14:12:43Z

I'm working on expanding this PR to include full support. We can keep this PR open until all the pieces are ready.

maven_install.json

varungandhi-src · 2022-05-05T14:17:20Z

semanticdb-javac/BUILD

+config_setting(
+    name = "is_enabled",
+    flag_values = {":enabled": "true"},
+)
+
+string_flag(
+    name = "enabled",
+    values = ["true", "false"],
+    build_setting_default = "false",
+)


What's the reason for having both is_enabled and enabled? 😅

One of them defines the name when specifying the flag from the command-line bazel build //... //semanticdb-javac:enabled=true.

The is_enabled target is used as a filter for a select(...) clause.

See official docs https://bazel.build/docs/configurable-attributes

semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbJavacOptions.java

semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTaskListener.java

varungandhi-src · 2022-05-05T14:50:43Z

semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTaskListener.java

+    int relativePathDepth = 0;
+    int uriPathDepth = uriPath.getNameCount();
+    int absolutePathDepth = absolutePath.getNameCount();
+    while (relativePathDepth < uriPathDepth && relativePathDepth < absolutePathDepth) {
+      String uriName = uriPath.getName(uriPathDepth - relativePathDepth - 1).toString();
+      String pathName = absolutePath.getName(absolutePathDepth - relativePathDepth - 1).toString();
+      if (!uriName.equals(pathName)) {
+        break;
+      }
+      relativePathDepth++;
+    }


suggestion(non-blocking): Perhaps add some unit tests for this function or specifically this logic? It seems a bit tricky.

Good suggestion. I opened a reminder issue and a // FIXME https://github.com/sourcegraph/lsif-java/issues/444 comment. Don't want to delay this PR further.

- Add LSIF generation support. - Add e2e example on how to import lsif-java as an external repo. - Add cross-repo support for bazelbuild/rules_jvm_external and twitter/bazel-multiversion.

olafurpg

Thank you @varungandhi-src for the detailed review! I appreciate the thoughtful comments and the resulting code is much cleaner.

WORKSPACE

maven_install.json

olafurpg · 2022-05-09T14:03:47Z

semanticdb-javac/BUILD

+config_setting(
+    name = "is_enabled",
+    flag_values = {":enabled": "true"},
+)
+
+string_flag(
+    name = "enabled",
+    values = ["true", "false"],
+    build_setting_default = "false",
+)


One of them defines the name when specifying the flag from the command-line bazel build //... //semanticdb-javac:enabled=true.

The is_enabled target is used as a filter for a select(...) clause.

See official docs https://bazel.build/docs/configurable-attributes

semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbJavacOptions.java

semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTaskListener.java

olafurpg · 2022-05-09T14:12:25Z

semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTaskListener.java

+    int relativePathDepth = 0;
+    int uriPathDepth = uriPath.getNameCount();
+    int absolutePathDepth = absolutePath.getNameCount();
+    while (relativePathDepth < uriPathDepth && relativePathDepth < absolutePathDepth) {
+      String uriName = uriPath.getName(uriPathDepth - relativePathDepth - 1).toString();
+      String pathName = absolutePath.getName(absolutePathDepth - relativePathDepth - 1).toString();
+      if (!uriName.equals(pathName)) {
+        break;
+      }
+      relativePathDepth++;
+    }


Good suggestion. I opened a reminder issue and a // FIXME https://github.com/sourcegraph/lsif-java/issues/444 comment. Don't want to delay this PR further.

olafurpg · 2022-05-09T14:15:19Z

I updated a new "bazel" CI job to test the generation e2e for internal usage (within our repo) and external usage (how end-users will consume lsif-java). Once we have snapshot testing logic we can additionally snapshot the generated index files, right now we just test that dump.lsif-typed gets created.

olafurpg · 2022-05-09T14:25:59Z

The CI tests are s now green in CI! I didn't get dependency pinning working but I don't think it's a problem because we have so few dependencies and they have no conflicts so the resolution will always be the same.

varungandhi-src · 2022-05-09T14:28:05Z

.github/workflows/ci.yml

+      - run: bazel run lsif-semanticdb:bazel -- --sourceroot $PWD
+      - run: du -h dump.lsif-typed
+      - run: bazel build //... --@lsif_java//semanticdb-javac:enabled=true
+        working-directory: examples/bazel-example
+      - run: bazel run @lsif_java//lsif-semanticdb:bazel -- --sourceroot $PWD


Suggested change

- run: bazel run lsif-semanticdb:bazel -- --sourceroot $PWD

- run: du -h dump.lsif-typed

- run: bazel build //... --@lsif_java//semanticdb-javac:enabled=true

working-directory: examples/bazel-example

- run: bazel run @lsif_java//lsif-semanticdb:bazel -- --sourceroot $PWD

- run: bazel run lsif-semanticdb:bazel -- --sourceroot "$PWD"

- run: du -h dump.lsif-typed

- run: bazel build //... --@lsif_java//semanticdb-javac:enabled=true

working-directory: examples/bazel-example

- run: bazel run @lsif_java//lsif-semanticdb:bazel -- --sourceroot "$PWD"

In case someone copy-pastes this to run locally, it'll not work if PWD has spaces.

varungandhi-src

LGTM, except for a couple of pending minor comments.

olafurpg force-pushed the bazel-build branch 3 times, most recently from 3deacd9 to 4bb14ad Compare May 5, 2022 10:59

olafurpg requested a review from varungandhi-src May 5, 2022 10:59

olafurpg force-pushed the bazel-build branch from 4bb14ad to a555e8d Compare May 5, 2022 11:39

olafurpg force-pushed the bazel-build branch from a555e8d to beda410 Compare May 5, 2022 11:41