microsoft / vscode-cpptools

Official repository for the Microsoft C/C++ extension for VS Code.
Other
5.45k stars 1.53k forks source link

Enable clang-tidy/format to use terminal.integrated.automationProfile #12629

Open dfarley1 opened 2 weeks ago

dfarley1 commented 2 weeks ago

Environment

Bug Summary and Steps to Reproduce

Bug Summary:

I'm attempting to enable clang-tidy for my project but every time it gets invoked (by saving a file, etc), it reports Child exec failed 2147943568 and won't show any warnings in the editor (red squiggles, etc) or Problems tab.

Workspace config:

        "C_Cpp.default.compileCommands": "/depot/out/cur/compile_commands.json",
        "C_Cpp.codeAnalysis.clangTidy.enabled": true,
        "C_Cpp.codeAnalysis.clangTidy.path": "clang-tidy", // use our container's clang-tidy
        "C_Cpp.codeAnalysis.clangTidy.useBuildPath": true,
        "C_Cpp.codeAnalysis.clangTidy.codeAction.formatFixes": false,
        "C_Cpp.codeAnalysis.clangTidy.args": [
            // clang-tidy doesn't like some gcc only warnings
            "-extra-arg=-Wno-unknown-warning-option",
        ],

And I have a /depot/.clang-tidy with some config in it, including WarningsAsErrors: "*"

Extension logs when saving the file:

LSP: (invoked) textDocument/didSave: file:///depot/lib/js/src/js.cc
Intellisense update pending for: file:///depot/lib/js/src/js.cc
tag parsing file: /depot/lib/js/src/js.cc
clang-tidy
-extra-arg=-Wno-unknown-warning-option
--export-fixes=/tmp/{4033757821245334524}/fixes140298805001008.yaml
--quiet
--use-color=false
/depot/lib/js/src/js.cc
-p=/depot/out/cur/
Child exec failed 2147943568

Running in a terminal gives me

/depot/ $ clang-tidy -extra-arg=-Wno-unknown-warning-option --export-fixes=/tmp/{4033757821245334524}/fixes140298805001008.yaml --quiet --use-color=false /depot/lib/js/src/js.cc -p=/depot/out/cur/
6988 warnings generated.

... <snip all the actual warnings>

Error opening output file: No such file or directory
/depot/ $ echo $?
1

Presuming that last line is because /tmp/{4033757821245334524}/fixes140298805001008.yaml is created by the LSP, if I manually create it then everything seems happy:

/depot/ $ mkdir /tmp/{4033757821245334524}/
/depot/ $ touch /tmp/{4033757821245334524}/fixes140298805001008.yaml
/depot/ $ clang-tidy -extra-arg=-Wno-unknown-warning-option --export-fixes=/tmp/{4033757821245334524}/fixes140298805001008.yaml --quiet --use-color=false /depot/lib/js/src/js.cc -p=/depot/out/cur/
6988 warnings generated.

... <snip>

/depot/ $ echo $?
130

And the temp file has a big ol' blob of yaml that looks OK to me, except at the very end there's

/depot/ $ head /tmp/\{4033757821245334524\}/fixes140298805001008.yaml 
---
MainSourceFile:  '/depot/lib/js/src/js.cc'
Diagnostics:
  - DiagnosticName:  readability-inconsistent-declaration-parameter-name
    DiagnosticMessage:
      Message:         '<snip>'
      FilePath:        '../../lib/js/js.h'
      FileOffset:      7016
      Replacements:    []
    Notes:

/depot/ $ tail /tmp/\{4033757821245334524\}/fixes140298805001008.yaml 
      FilePath:        '../../lib/js/src/js.cc'
      FileOffset:      79790
      Replacements:
        - FilePath:        '../../lib/js/src/js.cc'
          Offset:          299
          Length:          0
          ReplacementText: "#include <cstring>\n"
    Level:           Error
    BuildDirectory:  '/depot/out/Default'
...

The trailing ... is actually in the file, is VSCode choking on that? Do I have way too many errors? (Yes I do, but that's what I want to fix!) I don't get warnings on any file though, even those with significantly fewer errors.

sean-mcmanus commented 2 weeks ago

@dfarley1 Can you try removing the "C_Cpp.codeAnalysis.clangTidy.path": "clang-tidy"` setting? If that doesn't work then we're not finding it in the environment path, so you should set it to the full path.

dfarley1 commented 2 weeks ago

Ah that does appear to be our issue. clang-tidy is only added to the path in /etc/profile, and thus only login shells will see it.

We've had similar issues in the past, and thus we created a wrapper and terminal profile:

        "terminal.integrated.automationProfile.linux": {
            "path": "/depot/.vscode/vsbash",
        },
/depot/ $ cat /depot/.vscode/vsbash 
#!/bin/bash

# Fix in progress at https://github.com/microsoft/vscode-remote-release/issues/3418
# This shim is necessary to propegate VSCode's bin path to login stage shells
# since login shells reset $PATH. This is necessary to run commands like
# `code .../foo.c` which opens the file in the current VSCode window.
# $VSBIN ends up being `/home/user/.vscode-server/bin/<some-hash>/bin`
# depending on who attached to the container

IFS=':' read -ra VSPATH <<< "$PATH"
for i in "${VSPATH[@]}"; do
    if [[ "$i" == *"vscode"* ]]; then
        VSBIN=$i
    fi
done

# The login shell appends its first argument to $PATH, shifts it out, and `exec` the rest
# "$1" inside the login shell becomes the value of $VSBIN and "$@" becomes `bash "${@}"`
/bin/bash --login -c 'export PATH="$PATH:$1"; shift; exec "${@}"' bash "$(echo $VSBIN)" bash "${@}"

Which is supposed to make sure all the VSCode tooling also uses a login shell. I would have expected cpptools to pick up that setting as well. I could set clangTidy.path to /usr/lib/llvm/17/bin/clang-tidy, but then we'd have to manually update it every time we update clang which isn't ideal.

Should this issue be recycled into something like "cpptools respects automationProfile"? Is there another mechanism to get cpptools to use a login shell and/or source /some/file to get a new $PATH?

sean-mcmanus commented 2 weeks ago

@dfarley1 I've changed this to a feature request (enhancement). I'm not familiar with another mechanism to get us to execute child process with the login shell.