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

Amend docs to support Bazel 6 #600

Open
keynmol opened this issue Jun 27, 2023 · 5 comments
Open

Amend docs to support Bazel 6 #600

keynmol opened this issue Jun 27, 2023 · 5 comments
Labels
graph/scip-java team/graph type/documentation Improvements or additions to documentation

Comments

@keynmol
Copy link
Contributor

keynmol commented Jun 27, 2023

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)
@keynmol keynmol added the type/documentation Improvements or additions to documentation label Jun 27, 2023
@ciarand
Copy link

ciarand commented Jun 28, 2023

fwiw just commenting out the javac-import line got me most of the way there: ciarand@d48f520

With that and the javac options (ciarand@ebfa154) 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
Copy link

ciarand commented Jun 29, 2023

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
Copy link

ciarand commented Jun 29, 2023

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
Copy link
Contributor Author

keynmol commented Jun 30, 2023

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 #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
Copy link
Contributor

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
graph/scip-java team/graph type/documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants