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

codeql-action/init prevents bazel from reading version information #17704

Open
werkt opened this issue Oct 8, 2024 · 6 comments
Open

codeql-action/init prevents bazel from reading version information #17704

werkt opened this issue Oct 8, 2024 · 6 comments
Labels
bug Something isn't working

Comments

@werkt
Copy link

werkt commented Oct 8, 2024

Before calling github/codeql-action/init@v3 in my job that uses bazel to build, bazel version reports:

Build label: 7.3.1
Build target: @@//src/main/java/com/google/devtools/build/lib/bazel:BazelServer
Build time: Mon Aug 19 16:12:50 2024 (1724083970)
Build timestamp: 1724083970
Build timestamp as int: 1724083970

After calling the init step, with a fresh bazel daemon start after shutdown, it reports:

Build target: @@//extractor-java-agent:bin
Build time: Thu Jan 01 00:00:00 1970 (0)
Build timestamp: Thu Jan 01 00:00:00 1970 (0)
Build timestamp as int: 0

I would like to continue using codeql, but this will be impossible if it breaks this feature, as it is used by external rules definitions (protobuf) to determine how to function based on version at runtime in their starlark definitions, causing issues that are unique to the codeql environment.

An example of this is available at https:/buildfarm/buildfarm/actions/runs/11245576544/job/31265839492?pr=1887

@werkt werkt added the question Further information is requested label Oct 8, 2024
@redsun82
Copy link
Contributor

redsun82 commented Oct 9, 2024

👋 @werkt thanks for notifying us of this. Indeed, I can reproduce this behaviour with the following commands on an empty directory:

codeql database init --language=java db
codeql database trace-command db bazel version

Interestingly, this is not happening for other traced languages, e.g. --language=cpp, which points at something in the java-specific tracer configuration causing this.
EDIT: this actually happens for all traced languages including cpp (cpp, java, csharp, go, swift), must have been some typo in the commands I tried this with. So this seems like a tracer issue.

Oddly enough, this seems to only affect the version command, with other bazel commands working as expected. We're having a look at this.

@criemen
Copy link
Collaborator

criemen commented Oct 9, 2024

Hi @werkt , could you elaborate on

it is used by external rules definitions (protobuf) to determine how to function based on version at runtime in their starlark definitions

Do you have a link to the rule source code? Is this calling a starlark API, or do the protobuf rules shell out to bazel version?

@redsun82
Copy link
Contributor

redsun82 commented Oct 9, 2024

@werkt I found the root cause of this, where a build-data.properties file in the java agent we route the build through is confusing the bazel version command. A fix should get in codeql 2.19.3 (which is not the very next release though).

In the meantime, if you can't wait, you could work around this by removing that file from that internal jar. To do so you could add the following workflow step right after github/codeql-action/init:

      - name: Remove file to work around bazel version not working under CodeQL
         run: |
             zip -d build-data.properties "$CODEQL_EXTRACTOR_JAVA_ROOT/tools/codeql-java-agent.jar"

If you do so, be advised that this will start to fail when the proper fix lands in the CodeQL release

@redsun82 redsun82 added bug Something isn't working and removed question Further information is requested labels Oct 9, 2024
@werkt
Copy link
Author

werkt commented Oct 9, 2024

@criemen the full route of this failure is convoluted. tl;dr: No, it doesn't shell out to bazel version, but it uses the same data presentation that bazel version draws from inside starlark, in repository contexts (and unavailable in BUILD file contexts).

Bazel reads the properties and populates backing referenced to create a WorkspaceFactory which binds into native

bazel_features through a MODULES binding that I'll spare you, populates bazel_features.globals via a map with maximum versions to compare against where it of course stamps a starlark file to declare this into bazel_features.globals.ProtoInfo. This version comparison function though, interprets the empty string as the highest possible version ever, making most of the well formed version checks None.

The protobuf rule implementation here, through here reads the bazel_features.globals.ProtoInfo value here

This only matters when mixing @protobuf//bazel:proto_library.bzl's proto_library and @rules_proto's variant, which is deprecated, but still in use by things like https:/googleapis/googleapis and https:/bazelbuild/bazel, both pulled in as external repositories for these definitions explicitly by https:/buildfarm/buildfarm, where the types don't align and bazel indicates that a mandatory provider for ProtoInfo is not present in one version of proto_library target versus another.

werkt added a commit to werkt/bazel-buildfarm that referenced this issue Oct 9, 2024
Prevents github/codeql#17704
Should be removed when CodeQL is >= 2.19.3
werkt added a commit to werkt/bazel-buildfarm that referenced this issue Oct 9, 2024
Prevents github/codeql#17704
Should be removed when CodeQL is >= 2.19.3
@werkt
Copy link
Author

werkt commented Oct 9, 2024

@redsun82 that doesn't seem to do the trick per https:/buildfarm/buildfarm/actions/runs/11255637377/job/31295652785?pr=1889 (your step with an || message, and a bazel version as the first action in the next step)

I'll poke around a bit more now that I see what you're doing here.

EDIT: zip does things backwards-ish. Flipped the args around to make it work. Workaround will be landed soon, looking forward to the release.

werkt added a commit to werkt/bazel-buildfarm that referenced this issue Oct 9, 2024
Prevents github/codeql#17704
Should be removed when CodeQL is >= 2.19.3
werkt added a commit to werkt/bazel-buildfarm that referenced this issue Oct 9, 2024
Prevents github/codeql#17704
Should be removed when CodeQL is >= 2.19.3
werkt added a commit to buildfarm/buildfarm that referenced this issue Oct 9, 2024
Prevents github/codeql#17704
Should be removed when CodeQL is >= 2.19.3
@redsun82
Copy link
Contributor

redsun82 commented Oct 9, 2024

EDIT: zip does things backwards-ish. Flipped the args around to make it work. Workaround will be landed soon, looking forward to the release.

ah, my bad, indeed the command needed to be zip -d "$CODEQL_EXTRACTOR_JAVA_ROOT/tools/codeql-java-agent.jar" build-data.properties, glad you figured that out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants