Remove the lua tracer feature flag check from the codeql-action. - #1244
criemen merged 2 commits into
Conversation
Sorry, something went wrong.
Always defer to the CLI on the Lua tracer state from now on.
| CODEQL_VERSION_LUA_TRACING_GO_WINDOWS_FIXED | ||
| ))) | ||
| ) { | ||
| extraArgs.push("--internal-use-lua-tracing"); |
There was a problem hiding this comment.
Since we are no longer explicitly setting --internal-use-lua-tracing, does it mean that the modified action would work only with CLI versions that defaults to Lua tracing?
Sorry, something went wrong.
There was a problem hiding this comment.
No, the action works with all CLI versions, but on some CLI versions the action will now use the legacy tracer instead.
As both tracers are 100% compatible, this is not an issue.
On later CLI versions, where the Lua tracer has gained additional capabilities, we also use it by default.
Sorry, something went wrong.
This test is broken, as it first sets environment variables, and then immediately unsets it again. This only worked by chance with the legacy tracer, and breaks the Lua tracer.
|
I suggest we defer merging this to after we've cut the release of the CodeQL Action that'll go into GHES 3.7. |
Sorry, something went wrong.
|
Now that you mention it, I think we need to do the opposite and ensure this PR makes it into GHES, otherwise C# tracing will be broken on GHES: Without the PR, our logic will detect that we're running on GHES, which disables all feature flags. (I had not thought that through before you mentioned GHES here 😬 ) |
Sorry, something went wrong.
|
Ah, I wasn't aware we'd deleted the CLR tracer in rc/3.7. In that case, it sounds like we need to get this in. AIUI 2.10.5 enables the Lua tracer by default, so the behaviour here of dropping the --internal-use-lua-tracing flag will be to use Lua tracing. |
Sorry, something went wrong.
Always defer to the CLI on the Lua tracer state from now on.
This will turn off Lua tracing when combined with a CLI that has Lua tracing disabled by default, but as Lua tracing and legacy tracing on these CLIs are 100% compatible, that does not pose a problem.
Furthermore, the reduction of complexity in our code is worth turning off Lua tracing for those few users.
Querying the feature flag is becoming soon pointless, as dotcom will always return true anyways.
Merge / deployment checklist