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

[RFC] vscode support #2845

Merged
merged 3 commits into from
May 25, 2024
Merged

[RFC] vscode support #2845

merged 3 commits into from
May 25, 2024

Conversation

joholl
Copy link
Collaborator

@joholl joholl commented May 17, 2024

This one might be opinionated.

So far there is no clean way to get first-class language support in vscode. Add make target for creating a compilation database using bear. This enables you to do the following

make -j compile_commands.json

This file is recognized by many tools including e.g. clang-tidy and the vscode clangd extension. This way, the vscode language server knows exactly where symbols come from, which #ifdefs are compiled and where to find the right headers.

I also added a launch.json which lets you debug single unit and integration tests easily. What do you think of this?

Things I thought of but did not want to do in this first step:

  • Automatic test discovery/running with something like Test Explorer
  • A devcontainer
  • IDE-integrated formatting (clang-format) and linting (e.g. clang-tidy, I will probably submit that as a future PR)

Why _vscode and not .vscode? Because this leaves users who want to use their own, different .vscode with a dirty working tree. Once-tracked files cannot be .gitignore-d and there is no clean way to resolve this.

@joholl
Copy link
Collaborator Author

joholl commented May 17, 2024

Although this is a draft, it is ready for comments!

@JuergenReppSIT
Copy link
Member

@joholl I tried it on ubuntu 22.04 (bear installed with apt) But compile_commands.json only contained [] ?

@joholl
Copy link
Collaborator Author

joholl commented May 19, 2024

@JuergenReppSIT Weird, that happens when no files are built by make. The command should rebuild the whole project (depending on how you ./configured it. The following is the command executed. Can you try calling make clean before?

bear -- make --always-make check-programs

@JuergenReppSIT
Copy link
Member

@joholl I tried the following:
make clean
make -j compile_commands.json
compile_commands.json was again produced with [] and tss was not compiled.
If I enter make -j afterwards tss was compiled.
The begining of the output of make -j compile_commands.json was as follows:

oot@0d5cd7b145f2:/workspace/tpm2-tss# make -j compile_commands.json
  GEN      compile_commands.json
E0519 12:17:35.683570767  533308 server_chttp2.cc:40]        {"created":"@1716121055.683527884","description":"Only 1 addresses added out of total 2 resolved","file":"src/core/ext/transport/chttp2/server/chttp2_server.cc","file_line":406,"referenced_errors":[{"created":"@1716121055.683522157","description":"Address family not supported by protocol","errno":97,"file":"src/core/lib/iomgr/socket_utils_common_posix.cc","file_line":403,"os_error":"Address family not supported by protocol","syscall":"socket","target_address":"[::1]:33281"}]}
make[1]: Entering directory '/workspace/tpm2-tss'
CDPATH="${ZSH_VERSION+.}:" && cd . && /bin/bash '/workspace/tpm2-tss/missing' aclocal-1.16 -I m4 --install
CDPATH="${ZSH_VERSION+.}:" && cd . && /bin/bash '/workspace/tpm2-tss/missing' autoconf
 cd . && /bin/bash /workspace/tpm2-tss/missing automake-1.16 --foreign
/bin/bash ./config.status --recheck
running CONFIG_SHELL=/bin/bash /bin/bash ./configure --enable-debug CC=clang --no-create --no-recursion
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a race-free mkdir -p... /usr/bin/mkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes

@joholl
Copy link
Collaborator Author

joholl commented May 21, 2024

@JuergenReppSIT Ah, that clears it up. I used the make target check-programs (but you configured with ./configure --enable-debug CC=clang), that's why nothing was compiled. I changed it to all check-programs, that should fix it.

For reference:

❯ docker run --rm -it -v $PWD:/workspace/tpm2-tss ghcr.io/tpm2-software/ubuntu-22.04 /bin/bash
root@7dae04857971:/# cd /workspace/tpm2-tss/
root@7dae04857971:/# apt install bear -y && bear --version
# ...
bear 3.0.18
root@7dae04857971:/workspace/tpm2-tss# git config --global --add safe.directory /workspace/tpm2-tss
root@7dae04857971:/workspace/tpm2-tss# ./bootstrap
# ...
root@7dae04857971:/workspace/tpm2-tss# ./configure --enable-unit --enable-integration
# ...
root@7dae04857971:/workspace/tpm2-tss# make -j compile_commands.json
# ...
root@7dae04857971:/workspace/tpm2-tss# head compile_commands.json 
[
  {
    "arguments": [
      "/usr/bin/gcc",
      "-DHAVE_CONFIG_H",
      "-I.",
      "-I./src",
      "-I./include/tss2",
      "-I./test/fuzz/tcti",
      "-std=c99",

Johannes Holland added 3 commits May 21, 2024 13:18
Many IDEs rely on a llvm compilation database to correctly resolve
include paths and other compiler flags. Add a make target for generating
a compilation database.

This file is automatically detected by many tools such as clang-tidy or
the vscode extension clangd.

Signed-off-by: Johannes Holland <[email protected]>
Add _vscode which can be renamed to .vscode. It contains a launch.json
for executing the unit and integration tests (when they are the
currently active editor tab). Additionally, launch.json contains the
relevant build tasks.

Signed-off-by: Johannes Holland <[email protected]>
@JuergenReppSIT
Copy link
Member

@joholl Thank you for the fix. Now it worked and I could compile files with clang-tidy

@AndreasFuchsTPM
Copy link
Member

I like it!

@JuergenReppSIT
Copy link
Member

@joholl I tried:
clang-tidy-14 src/tss2-policy/tss2_policy.c
but the error you found was not shown. How did you use clang-tidy to find Tss2_PolicyGetDescription() does not null-terminate?

@joholl joholl marked this pull request as ready for review May 25, 2024 13:27
@joholl joholl merged commit 6c46325 into tpm2-software:master May 25, 2024
25 checks passed
@joholl
Copy link
Collaborator Author

joholl commented May 25, 2024

@JuergenReppSIT You have to configure clang-tidy via cli options or a .clang-tidy file. See #2847.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants