feat(package-manager): Add initial support for Bazel #8409

haikoschol · 2024-03-13T11:41:49Z

This PR introduces a very limited Analyzer plugin for the Bazel build system. It only covers projects that use bzlmod for managing dependencies. Therefore it expects Bazel 7.0 or later, where bzlmod is the default.

So far the implementation has only been tested with a small toy example project.

A few things that are not covered, which very likely need to be added in subsequent iterations are:

Patches that can be applied to the source artifact of dependencies during the build process (e.g. https://github.com/bazelbuild/bazel-central-registry/blob/main/modules/glog/0.5.0/source.json#L4)
Linking: Since this is C/C++, dynamic and static linking is possible. Right now all dependency relationships have static linkage because Bazel is generally biased towards static linking.
Build targets are not taken into account. I'm not sure how this would look like yet, maybe as different scopes.
There are no Bazel specific PURLs yet.

clients/bazel-module-registry/src/main/kotlin/BazelModuleRegistryService.kt

clients/bazel-module-registry/src/funTest/kotlin/BazelModuleRegistryServiceFunTest.kt

clients/bazel-module-registry/src/main/kotlin/BazelModuleRegistryService.kt

clients/bazel-module-registry/build.gradle.kts

clients/bazel-module-registry/src/main/kotlin/BazelModuleRegistryService.kt

plugins/package-managers/bazel/src/main/kotlin/StarlarkParser.kt

mnonnenmacher

I haven't looked at the code, but please also add an entry to the package manager list in website/docs/tools/advisor.md and mention the limitations / mark it as experimental.

...agers/bazel/src/funTest/assets/projects/synthetic/bzlmod-cc-has-lockfile-expected-output.yml

sschuberth · 2024-03-13T14:22:38Z

I haven't looked at the code, but please also add an entry to the package manager list in website/docs/tools/advisor.md and mention the limitations / mark it as experimental.

@haikoschol that should say website/docs/tools/analyzer.md, obviously.

plugins/package-managers/bazel/src/main/kotlin/BazelModel.kt

plugins/package-managers/bazel/src/main/kotlin/Bazel.kt

clients/bazel-module-registry/src/funTest/kotlin/BazelModuleRegistryClientFunTest.kt

...nagers/bazel/src/funTest/assets/projects/synthetic/bzlmod-cc-no-lockfile-expected-output.yml

plugins/package-managers/bazel/src/funTest/kotlin/BazelFunTest.kt

plugins/package-managers/bazel/src/main/kotlin/Bazel.kt

haikoschol · 2024-03-14T08:38:49Z

I realized that I added a quick & dirty way to install Bazel in the Dockerfile for testing purposes. I removed that from the latest commit on this branch and am currently working on adding it "properly" in a subsequent commit.

edit: Since the PR is not ready to merge from my side until Bazel is added in Docker, I converted it to a draft.

plugins/package-managers/bazel/src/main/kotlin/Bazel.kt

haikoschol · 2024-03-14T12:06:06Z

I have no idea why this markdown-links check thinks https://bazel.build/ is a dead link.

haikoschol · 2024-03-15T06:21:15Z

I made the egregious mistake of assuming the "update branch" button would do the right thing. Instead it introduced a merge commit, which AFAIK is extremely verboten in ORT. How do I fix this?

sschuberth · 2024-03-15T07:10:04Z

Instead it introduced a merge commit, which AFAIK is extremely verboten in ORT.

Exactly! 👮🏻‍♂️ 😆

How do I fix this?

Well, your local branch should still have your latest changes, so just force-push that to the remote branch for this PR. Afterwards, you can either click on "Update with rebase" from the drop-down, or just rebase locally and force-push once more.

plugins/package-managers/bazel/src/main/kotlin/Bazel.kt

clients/bazel-module-registry/src/main/kotlin/BazelModuleRegistryClient.kt

plugins/package-managers/bazel/src/main/kotlin/Bazel.kt

website/docs/tools/analyzer.md

fviernau · 2024-03-20T08:08:37Z

I could probably find a couple of more nits, but I propose to draw a line here and merge it, once my last remaining open comments are addressed. (Some nits can still be fixed in future iterations) Is this ok with you as well @sschuberth @mnonnenmacher ?

fviernau · 2024-03-20T08:13:48Z

Dockerfile

@@ -403,6 +403,25 @@ RUN mkdir -p $DOTNET_HOME/bin \
 FROM scratch AS dotnet
 COPY --from=dotnetbuild /opt/dotnet /opt/dotnet

+#------------------------------------------------------------------------


@sschuberth @mnonnenmacher do we still use the "legacy" Docker? (Should we update that one as well)

Yes, it's e.g. used as part of the Jenkinsfile.

So, Dockerfile-legacy should be updated in this PR / commit as well ?!

Ideally, yes. But going forward I'd like to avoid the need to maintain Dockerfile-legacy in addition to Dockerfile, and I hope @mnonnenmacher's Docker experiments to be the basis for such a change.

That said, I'm not sure whether it's worth to put effort into maintaining Dockerfile-legacy just now, as part of this initial implementation. I'd also be fine either adding Bazel support to Dockerfile-legacy later, or replacing the whole Docker stuff with a more streamlined solution, whatever comes first.

fviernau · 2024-03-20T08:15:56Z

Dockerfile

+# BAZEL
+FROM base as bazelbuild
+
+ARG BAZEL_VERSION=7.0.1


We have that .versions file. I believe that BAZEL_VERSION needs to go there as well, but not sure if this should be done in this commit, or in the following one.

@heliocastro would you review the docker related bits as well, I guess you know this .versions mechanism ;-)

sschuberth · 2024-03-20T11:03:01Z

I could probably find a couple of more nits, but I propose to draw a line here and merge it, once my last remaining open comments are addressed. (Some nits can still be fixed in future iterations) Is this ok with you as well @sschuberth @mnonnenmacher ?

I first wanted the discussion between you and @haikoschol to settle down, but now that it comes to an end, I'd like to do a brief review myself, too. But please approve nonetheless if you're happy. As auto-merge is not on, we can have more reviews after that.

fviernau · 2024-03-20T12:53:20Z

I first wanted the discussion between you and @haikoschol to settle down, but now that it comes to an end, I'd like to do a brief review myself, too. But please approve nonetheless if you're happy. As auto-merge is not on, we can have more reviews after that.

Kotlin wise, it LGTM I agree with merge. Not approving now as there are open comments regarding the Docker bits. You may however start yourself already reviewing the Kotlin bits (I won't request further change on Kotlin side here).

sschuberth · 2024-03-20T16:33:06Z

@haikoschol, import com.jakewharton.retrofit2.converter.kotlinx.serialization.asConverterFactory now needs to be changed to import retrofit2.converter.kotlinx.serialization.asConverterFactory, see https://github.com/oss-review-toolkit/ort/pull/8440/files.

sschuberth · 2024-03-20T16:35:50Z

Regarding the failing markdown-links check, please add an entry for https://bazel.build/ to https://github.com/oss-review-toolkit/ort/blob/main/mlc_config.json.

plugins/package-managers/bazel/src/main/kotlin/Bazel.kt

sschuberth · 2024-03-21T07:54:53Z

Apparently funTest-docker still fail as bazel cannot be found. I suspect this is due to the known problems with our current Docker / PR setup, which doe snot reliably use a Docker image build from changes within the same PR. Not sure if it's worth fixing that setup as part of this PR. But I'll at least ensure that the Bazel funTests will pass for me locally.

haikoschol · 2024-03-22T04:12:59Z

Apparently funTest-docker still fail as bazel cannot be found. I suspect this is due to the known problems with our current Docker / PR setup, which doe snot reliably use a Docker image build from changes within the same PR. Not sure if it's worth fixing that setup as part of this PR. But I'll at least ensure that the Bazel funTests will pass for me locally.

@fviernau suggested to move the Docker changes to a separate PR that gets merged before. Should I give that a go?

sschuberth · 2024-03-22T07:36:33Z

@fviernau suggested to move the Docker changes to a separate PR that gets merged before. Should I give that a go?

If I can verify that funTest pass locally for me, I'd be fine with keeping things as-is. But I did not yet find the time to test, nor for a code review. I'll try today.

heliocastro · 2024-03-22T07:39:06Z

The PR for fixing the race condition on funtest Docker is almost done, but the tests need be done in my private repo to avoid polute ort registry, since it involves upload docker images and version matching
Sorry for the delay, but i'm working on this in private time, with is diminished

A Bazel module registry, such as https://bcr.bazel.build, is used to store metadata of dependencies managed by bzlmod. Signed-off-by: Haiko Schol <[email protected]>

This starts the work on issue oss-review-toolkit#264. Signed-off-by: Haiko Schol <[email protected]>

Signed-off-by: Haiko Schol <[email protected]>

#8409 (comment)

haikoschol requested a review from a team as a code owner March 13, 2024 11:41

github-advanced-security bot found potential problems Mar 13, 2024

View reviewed changes

sschuberth self-assigned this Mar 13, 2024

fviernau requested changes Mar 13, 2024

View reviewed changes

haikoschol force-pushed the bzlmod-cc branch from 3581696 to 0559d5c Compare March 13, 2024 13:48

github-advanced-security bot found potential problems Mar 13, 2024

View reviewed changes

plugins/package-managers/bazel/src/main/kotlin/StarlarkParser.kt Fixed Show fixed Hide fixed

plugins/package-managers/bazel/src/main/kotlin/StarlarkParser.kt Fixed Show fixed Hide fixed

mnonnenmacher reviewed Mar 13, 2024

View reviewed changes

fviernau reviewed Mar 13, 2024

View reviewed changes

...agers/bazel/src/funTest/assets/projects/synthetic/bzlmod-cc-has-lockfile-expected-output.yml Outdated Show resolved Hide resolved

haikoschol force-pushed the bzlmod-cc branch from 0559d5c to 90ffff9 Compare March 14, 2024 03:32

haikoschol requested a review from a team as a code owner March 14, 2024 03:32

github-advanced-security bot found potential problems Mar 14, 2024

View reviewed changes

plugins/package-managers/bazel/src/main/kotlin/BazelModel.kt Fixed Show resolved Hide resolved

plugins/package-managers/bazel/src/main/kotlin/Bazel.kt Fixed Show fixed Hide fixed

plugins/package-managers/bazel/src/main/kotlin/Bazel.kt Fixed Show fixed Hide fixed

haikoschol force-pushed the bzlmod-cc branch 2 times, most recently from 626067b to cb5d40d Compare March 14, 2024 04:30

fviernau requested changes Mar 14, 2024

View reviewed changes

haikoschol marked this pull request as draft March 14, 2024 08:39

haikoschol force-pushed the bzlmod-cc branch 5 times, most recently from 46ee3fa to f81c719 Compare March 14, 2024 11:46

github-advanced-security bot found potential problems Mar 14, 2024

View reviewed changes

haikoschol force-pushed the bzlmod-cc branch from ead2546 to 585bff2 Compare March 14, 2024 12:01

haikoschol marked this pull request as ready for review March 14, 2024 12:06

haikoschol force-pushed the bzlmod-cc branch 2 times, most recently from 6c0275f to e6d6150 Compare March 15, 2024 05:16

haikoschol force-pushed the bzlmod-cc branch 2 times, most recently from 583359a to 4686f39 Compare March 19, 2024 11:00

fviernau requested changes Mar 19, 2024

View reviewed changes

haikoschol force-pushed the bzlmod-cc branch 3 times, most recently from e991237 to 09039c1 Compare March 19, 2024 13:48

fviernau requested changes Mar 20, 2024

View reviewed changes

clients/bazel-module-registry/src/main/kotlin/BazelModuleRegistryClient.kt Show resolved Hide resolved

plugins/package-managers/bazel/src/main/kotlin/Bazel.kt Outdated Show resolved Hide resolved

website/docs/tools/analyzer.md Outdated Show resolved Hide resolved

fviernau previously requested changes Mar 20, 2024

View reviewed changes

haikoschol force-pushed the bzlmod-cc branch 2 times, most recently from 909d3b9 to 85794f3 Compare March 20, 2024 11:00

fviernau reviewed Mar 20, 2024

View reviewed changes

plugins/package-managers/bazel/src/main/kotlin/Bazel.kt Outdated Show resolved Hide resolved

haikoschol force-pushed the bzlmod-cc branch from 85794f3 to 0f24a3c Compare March 21, 2024 03:58

haikoschol added 4 commits March 23, 2024 14:19

feat(clients): Add Bazel module registry client

e234269

A Bazel module registry, such as https://bcr.bazel.build, is used to store metadata of dependencies managed by bzlmod. Signed-off-by: Haiko Schol <[email protected]>

feat(package-manager): Add initial support for Bazel

3df4ff4

This starts the work on issue oss-review-toolkit#264. Signed-off-by: Haiko Schol <[email protected]>

feat(docker): Add Bazel to runtime image and env path

74ddae5

Signed-off-by: Haiko Schol <[email protected]>

build(GitHub): Include Bazel in docker-ort workflow

60e9307

Signed-off-by: Haiko Schol <[email protected]>

sschuberth force-pushed the bzlmod-cc branch from 0f24a3c to 60e9307 Compare March 23, 2024 13:54

sschuberth approved these changes Mar 23, 2024

View reviewed changes

sschuberth merged commit 3126b41 into oss-review-toolkit:main Mar 23, 2024
34 of 35 checks passed

haikoschol deleted the bzlmod-cc branch April 5, 2024 04:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(package-manager): Add initial support for Bazel #8409

feat(package-manager): Add initial support for Bazel #8409

haikoschol commented Mar 13, 2024

mnonnenmacher left a comment

sschuberth commented Mar 13, 2024

haikoschol commented Mar 14, 2024 •

edited

Loading

haikoschol commented Mar 14, 2024

haikoschol commented Mar 15, 2024

sschuberth commented Mar 15, 2024

fviernau commented Mar 20, 2024

fviernau Mar 20, 2024

sschuberth Mar 20, 2024

fviernau Mar 20, 2024

sschuberth Mar 20, 2024

fviernau Mar 20, 2024

fviernau Mar 20, 2024

sschuberth commented Mar 20, 2024

fviernau commented Mar 20, 2024

sschuberth commented Mar 20, 2024

sschuberth commented Mar 20, 2024

sschuberth commented Mar 21, 2024

haikoschol commented Mar 22, 2024

sschuberth commented Mar 22, 2024

heliocastro commented Mar 22, 2024

feat(package-manager): Add initial support for Bazel #8409

feat(package-manager): Add initial support for Bazel #8409

Conversation

haikoschol commented Mar 13, 2024

mnonnenmacher left a comment

Choose a reason for hiding this comment

sschuberth commented Mar 13, 2024

haikoschol commented Mar 14, 2024 • edited Loading

haikoschol commented Mar 14, 2024

haikoschol commented Mar 15, 2024

sschuberth commented Mar 15, 2024

fviernau commented Mar 20, 2024

fviernau Mar 20, 2024

Choose a reason for hiding this comment

sschuberth Mar 20, 2024

Choose a reason for hiding this comment

fviernau Mar 20, 2024

Choose a reason for hiding this comment

sschuberth Mar 20, 2024

Choose a reason for hiding this comment

fviernau Mar 20, 2024

Choose a reason for hiding this comment

fviernau Mar 20, 2024

Choose a reason for hiding this comment

sschuberth commented Mar 20, 2024

fviernau commented Mar 20, 2024

sschuberth commented Mar 20, 2024

sschuberth commented Mar 20, 2024

sschuberth commented Mar 21, 2024

haikoschol commented Mar 22, 2024

sschuberth commented Mar 22, 2024

heliocastro commented Mar 22, 2024

haikoschol commented Mar 14, 2024 •

edited

Loading