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

Make use of multi-language and indirect tracing #744

Merged
merged 2 commits into from
Sep 23, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .github/workflows/pr-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,9 @@ jobs:

- name: Build code
shell: powershell
# Note we want to make sure that the .win32env file is read correctly, so we unset the CODEQL_EXTRACTOR_CSHARP_ROOT from the .sh file.
run: |
cat ./codeql-runner/codeql-env.sh | Invoke-Expression
$Env:CODEQL_EXTRACTOR_CSHARP_ROOT = ""
Comment on lines -210 to -213
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, the tracer is still supposed to be resistant towards build systems that clobber nonessential environment variables like this one. Even if the concrete regression this test was introduced to catch, wouldn't it be a good idea to keep it present, as defense in depth against tracer breakage?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good point - in fact, I'm not convinced this test was ever testing that the .win32env file was doing what it should as the tracer is resistant towards the unsetting the environment variable as you say. I've left the code as it is, and updated the comment to better describe what is going on here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The .win32env file is how the tracer resists unsetting ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right. So if I understand correctly, before this the test was checking that the .win32env file that the Action created for the new compound environment it synthesised worked correctly. Now, the test is just checking that the tracer is resistant to environment variables being unset by whatever mechanism it chooses which happens to also be creating a .win32env file.

$Env:CODEQL_EXTRACTOR_CSHARP_ROOT = "" # Unset an environment variable to make sure the tracer resists this
& $Env:CODEQL_RUNNER dotnet build /p:UseSharedCompilation=false

- name: Upload tracer logs
Expand Down
2 changes: 2 additions & 0 deletions lib/analyze-action.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/analyze-action.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 25 additions & 1 deletion lib/codeql.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/codeql.js.map

Large diffs are not rendered by default.

7 changes: 5 additions & 2 deletions lib/init-action.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading