Kotlin: build extractor with bazel #16117
Conversation
On Windows `git lfs smudge` was not working as expected.
| Building the extractor can be done via | ||
| ``` | ||
| bazel build //java/kotlin-extractor:codeql-extractor-kotlin-<variant>-<version> | ||
| ``` |
There was a problem hiding this comment.
It looks like this can't be done from <root>, but can be done from <root>/ql or <root>/ql/java/kotlin-extractor. Is that expected?
Also, I think that this should mention that you needs tools/bazel to be the bazel on your path.
There was a problem hiding this comment.
It looks like this can't be done from , but can be done from /ql or /ql/java/kotlin-extractor. Is that expected?
I can add a note about building from the internal repo (bazel build @codeql//java/kotlin-extractor:etc is what works from there)
Also, I think that this should mention that you needs tools/bazel to be the bazel on your path.
Actually, any installed bazel or bazelisk will do (bar http fetching problems 😅). On codeql, if it's bazelisk (which is now the recommended bazel executable to have and is what bazel is on github actions) it will pick up the correct bazel version. If it's a fixed bazel version, it will exit any way with a helpful error on a version mismatch.
Furthermore, on semmle-code, any installed bazel will actually auto-pick tools/bazel.
There was a problem hiding this comment.
The helpful error will have the user install bazel, though, and they will have to update their installation when we bump our bazel version. Wouldn't it be better to just tell them to use the tools/bazel they already have?
java/kotlin-extractor/BUILD.bazel
Outdated
| If `kotlinc` is updated, bazel won't be aware of it and will therefore keep the same default version. Possible workarounds for that: | ||
| * `bazel clean` | ||
| * `bazel fetch --force @codeql_kotlin_defaults\\:all` | ||
| * `CODEQL_KOTLIN_SINGLE_VERSION= bazel build //java/kotlin-extractor` |
There was a problem hiding this comment.
Can we add a note about this then, and/or drop this workaround?
Or maybe shell out to a source of randomness to set the variable content :D
java/kotlin-extractor/BUILD.bazel
Outdated
| "echo %s-%s > $(RULEDIR)/%s/com/github/codeql/extractor.name" % (_extractor_name_prefix, v, v), | ||
| ] + [ | ||
| "cp $(execpath %s) $(RULEDIR)/%s/%s" % (src, v, tgt) | ||
| for src, tgt in _resources |
There was a problem hiding this comment.
Nit doesn't seem to have been addressed, despite a thumbs-up emoji
| @@ -0,0 +1,31 @@ | |||
| module( | |||
There was a problem hiding this comment.
I'm not sure where these ended up.
java/kotlin-extractor/dev/kotlinc
Outdated
| @@ -0,0 +1,163 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
Shouldn't there be a java/kotlin-extractor/dev/kotlin as well?
There was a problem hiding this comment.
I guess? I did not have to call that, but my interaction with kotlin was not that deep. I've added both that and kapt. I'm guessing kolinc-* variants are not really needed directly? But those could be added too if needed.
There was a problem hiding this comment.
or would you just add kotlin?
There was a problem hiding this comment.
I think that kotlin is worthwhile, so people can easily run Kotlin programs while working on things. I don't think anything else is likely to be necessary, and we can always add more things later if it turns out to be useful.
java/ql/integration-tests/linux-only/kotlin/custom_plugin/test.py
Outdated
Show resolved
Hide resolved
misc/bazel/registry/modules/rules_kotlin/1.9.4-codeql.1/patches/module_dot_bazel_version.patch
Outdated
Show resolved
Hide resolved
My idea would be to remove all unneeded scripts after the internal repo change is merged. |
Usage overview
Building the extractor can be done via
where
<variant>is eitherstandaloneorembeddable, and<version>is one of the supported versions.For the moment both variants where tested by replacing them into
target/intree/codeql-javaand running one relevant integration test.will build a default variant:
CODEQL_KOTLIN_SINGLE_VERSION_EMBEDDABLEis set to true, in which case it will go for embeddablekotlincCODEQL_KOTLIN_SINGLE_VERSIONis set, that will be used insteadkotlincis not installed,1.9.20-Betawill be used (the current kotlin toolchain downloaded byrules_kotlinis 1.9.23).If using the provided
kotlincwrapper (see section below), bazel will be aware of versions selected by it and change the default//java/kotlinc-extractoraccordingly.kotlincwrapperA cross-platform
kotlincwrapper is provided injava/kotlinc-extractor/deps/dev. If this path is added to thePATHenvironment variable, one can callkotlincand have the wrapper take care of downloading the appropriate Kotlin compiler under the hood. The desired version can be selected withkotlinc --select x.y.z, or left to the current default of1.9.0.Selected and installed version data is stored in
.gitignored files in the same directory, and can be cleared withkotlinc --clear.These
--options cannot conflict with normalkotlincoptions, which uses single-options instead.If
ripunzipis installed, or in any case on windows from withinsemmle-codewhereripunzip.exeis provided, that will be used, which results in faster kotlin installations.Interactions with the existing build
For the moment nothing changes on the internal build, which will still call
build.py. After we integrate this into the internal build system we can come back here and clean up obsolete files.Implementation notes
Version variants
rules_kotlindoes not seem to really support building with different-language-versionsettings in the same workspace, so I ended up patching it to allow specifying that at akt_jvm_librarylevel viakt_kotlinc_options. This allows defining the different extractor versions in the same build file using a standard[...]comprehension.Standalone / embeddable
Embeddable requires not only changing one dependency, but also some imports in the source code itself. This is achieved by reexporting and patching the code in the
@codeql_kotlin_embeddableexternal repository. Thejava/kotlin-extractor/BUILD.bazelfile is shared with that repository, and behaves slightly differently depending on that (based onrepo_name()).External dependencies
The kotlin dependencies are given as LFS files in
java/kotlin-extractor/deps. However normalcodeqlusers won't have those files downloaded, whether they havegit lfsinstalled or not, as fetching those files are excluded by the repo's.lfsconfig. Additionally, those LFS files won't count towards users' quotas on forks, and users not having write permissions ongithub/codeqlwon't be able to push new LFS files there (so we can't be "LFS bombed").Building the extractor will require
git lfsto be installed on the system, but will then work and checkout those dependencies lazily on demand within the bazel cache. This means abazel cleanwill clear those files out. However if those files are present in the checkout and not just LFS pointers (as happens for example when updating/adding dependencies, or ifgit config lfs.fetchinclude java/kotlin-extractor/deps/*is specified followed by agit lfs checkout), then bazel will symlink them as they are, without incurring in any LFS download overhead, including afterbazel clean.Updating the dependencies doesn't require any special actions: when added and pushed the files will be uploaded as LFS files, and locally they will be picked up by bazel without any problem.
rules_kotlinpatchingWe do need to patch
rules_kotlinto provide different-language-versionvalues for different bazel targets. After some discussion we decided to to that with a local bazel registry rather than an override, because that will make sharing with the internalsemmle-coderepo easier.