sourcegraph / scip-java

SCIP Code Intelligence Protocol generator for Java
https://sourcegraph.github.io/scip-java/
Apache License 2.0
73 stars 29 forks source link

Amend docs to support Bazel 6 #600

Open keynmol opened 1 year ago

keynmol commented 1 year ago

waves hands Most things work, but langtools was removed:

INFO: Build option --@scip_java//semanticdb-javac:enabled has changed, discarding analysis cache.
ERROR: /private/var/tmp/_bazel_antonsviridov/941a6fce641affbe0a463a62863a7862/external/scip_java/semanticdb-javac/BUILD:19:12: no such package '@bazel_tools//third_party/java/jdk/langtools': BUILD file not found in directory 'third_party/java/jdk/langtools' of external repository @bazel_tools. Add a BUILD file to a directory to mark it as a package. and referenced by '@scip_java//semanticdb-javac:javac-import'
ERROR: Analysis of target '//:ProjectRunner' failed; build aborted:
INFO: Elapsed time: 0.105s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (0 packages loaded, 557 targets configured)
ciarand commented 1 year ago

fwiw just commenting out the javac-import line got me most of the way there: https://github.com/ciarand/scip-java/commit/d48f52005c909285b7c70aa27a10bfbcb7f452a0

With that and the javac options (https://github.com/ciarand/scip-java/commit/ebfa1541745ee5521e9c150681a4620049e4220e) most of our Java libraries are building as hoped for (.semanticdb file in the jar).

Unfortunately I have not been able to get our libraries that use auto value to work in conjunction with this plugin. I get an error indicating a sourceroot mismatch, which sort of makes sense:

bazel-out/k8-fastbuild-ST-4a519fd6d3e4/bin/java/path/to/my/java/app/_javac/my_lib_name/libmy_lib_name_sources/path/to/my/java/app/AutoValue_MyAutoValueClassName.java:1:
error: semanticdb-javac: sourceroot '/absolute/path/to/repo does not contain path 'bazel-out/k8-fastbuild-ST-4a519fd6d3e4/bin/java/path/to/my/java/app/_javac/my_lib_name/libmy_lib_name_sources:path/to/my/java/app/AutoValue_MyAutoValueClassName.java'. To fix this problem, update the -sourceroot flag to be a parent directory of this source file.

Is this a bazel 6 thing or should I file a separate issue?

ciarand commented 1 year ago

I managed to fix this by patching the absolutePathFromUri function to naively swap : with / and to absolutize the returned path if it isn't already https://github.com/ciarand/scip-java/blob/68ead17732c2b919b94f6cdb8d8a578c2bb7ec1e/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTaskListener.java#L115-L132

With these changes the indexing appears to work across all our Java libraries being built via Bazel 6. Is it worth me cleaning these patches up and putting together an MR? I haven't verified any of the more widespread implications of these changes

ciarand commented 1 year ago

Ah, more trouble, though these can probably be moved to a separate issue. The indexing is working, but the BazelBuildTool part (bazel run @scip_java//scip-semanticdb:bazel -- --sourceroot $PWD) is failing when remote caching is enabled because afaict the *0.params files it uses to find the jars that make up the targetroots option aren't included in the cache.

Swapping out the logic in BazelOptions with a hacky version that just iterates over all the jars that don't have "runfiles" in the name made the scip generation succeed, but that path doesn't seem particularly elegant (and it took a few more naive filters to get it to complete within a reasonable time / not overload the scip convert command I'm using next).

keynmol commented 1 year ago

I managed to fix this by patching the absolutePathFromUri function to naively swap : with / and to absolutize the returned path if it isn't already https://github.com/ciarand/scip-java/blob/68ead17732c2b919b94f6cdb8d8a578c2bb7ec1e/semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbTaskListener.java#L115-L132

I ended up doing the same as part of https://github.com/sourcegraph/scip-java/pull/602 - but additionally we now resolve the location where javac generates sources - which seems to match what Bazel gives it - so we can separately handle the generated sources that are not part of the source tree.

At this point I lean towards ignoring them entirely, because symbols in them point to nowhere (because those sources are not present anywhere) - but this might change.

varungandhi-src commented 1 year ago

I agree with @keynmol's point that ignoring generated files (in that, not attempting to emit SCIP Documents) is the right thing to do for now. Since there's no notion of virtual files in SCIP, there is no reasonable relative path that can be used. So it makes sense to skip emitting occurrences for them.

In scip-clang (indexer for C and C++), we also have something similar: https://sourcegraph.com/github.com/sourcegraph/scip-clang/-/blob/indexer/FileMetadata.h?L19-43 https://sourcegraph.com/search?q=context:global+repo:%5Egithub%5C.com/sourcegraph/scip-clang%24+isInProject&patternType=standard&sm=1&groupBy=path

Adding support for generated files would require a significant amount of work (cutting across the Sourcegraph backend and frontend and different indexers).