Skip to content

Rust: tweak qltest logs#18918

Merged
redsun82 merged 2 commits intomainfrom
redsun82/rust-tweak-qltest-logs
Mar 10, 2025
Merged

Rust: tweak qltest logs#18918
redsun82 merged 2 commits intomainfrom
redsun82/rust-tweak-qltest-logs

Conversation

@redsun82
Copy link
Copy Markdown
Contributor

@redsun82 redsun82 commented Mar 4, 2025

  • verbosity is raised to DEBUG to have more information in the logs
  • color codes are now skipped in the qltest.log file
  • they are still printed out on the console when running with --show-extractor-output.

* verbosity is raised to DEBUG to have more information in the logs
* color codes are now skipped in the `qltest.log` file
* they are still printed out on the console when running with
  `--show-extractor-output`.
Copilot AI review requested due to automatic review settings March 4, 2025 11:35
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more

@redsun82 redsun82 requested a review from aibaars March 4, 2025 11:35
@github-actions github-actions Bot added the Rust Pull requests that update Rust code label Mar 4, 2025
Comment thread rust/tools/qltest.sh Outdated
QLTEST_LOG="$CODEQL_EXTRACTOR_RUST_LOG_DIR"/qltest.log
if ! "$CODEQL_EXTRACTOR_RUST_ROOT/tools/$CODEQL_PLATFORM/extractor" --qltest >> "$QLTEST_LOG" 2>&1; then
cat "$QLTEST_LOG"
TMP_OUT="$(mktemp)"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps use CODEQL_EXTRACTOR_RUST_SCRATCH_DIR ? That's easier to port to qltest.cmd too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ah, right, I forgot about qltest.cmd...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure this color removal is required on Windows, so I'd leave that alone until I try that. But I did apply the progress+ thing there as well now.

@redsun82 redsun82 requested a review from aibaars March 4, 2025 12:07
Comment thread rust/tools/qltest.sh
if ! "$CODEQL_EXTRACTOR_RUST_ROOT/tools/$CODEQL_PLATFORM/extractor" --qltest >> "$QLTEST_LOG" 2>&1; then
cat "$QLTEST_LOG"
mkdir -p "$CODEQL_EXTRACTOR_RUST_SCRATCH_DIR"
TMP_OUT="$(mktemp --tmpdir="$CODEQL_EXTRACTOR_RUST_SCRATCH_DIR" qltest-XXXXXX.log))"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wouldn't bother with the mktemp and trap commands. The scratch dir is cleaned up already when a database is finalized.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

right!

@redsun82 redsun82 merged commit d187625 into main Mar 10, 2025
@redsun82 redsun82 deleted the redsun82/rust-tweak-qltest-logs branch March 10, 2025 16:47
redsun82 pushed a commit that referenced this pull request Mar 13, 2025
Turns out some version of macOS do not support the way `mktemp` was
being used. In any case it wasn't really necessary, see
#18918 (comment)
(which I forgot to follow up on at the time after approval).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants