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

Fix compile_commands.json #3538

Merged
merged 1 commit into from
May 5, 2023
Merged

Fix compile_commands.json #3538

merged 1 commit into from
May 5, 2023

Conversation

lieser
Copy link
Collaborator

@lieser lieser commented May 2, 2023

Removes newlines in JSON strings and normalize path to avoid backslash in JSON strings.
Also remove the additional added newline at the end.

Note if testing with ccache or sccache in VS Code:
Make sure your use the most recent version 1.15.4 of the C++ extension as this fixed a compatibility issue with them microsoft/vscode-cpptools#7616.

@lieser lieser added the bug label May 2, 2023
@lieser lieser requested a review from reneme May 2, 2023 10:12
@lieser lieser self-assigned this May 2, 2023
@lieser lieser force-pushed the pl/fix-compile_commands branch from 4d9323d to 2088a0b Compare May 2, 2023 10:24
@coveralls
Copy link

coveralls commented May 2, 2023

Coverage Status

Coverage: 91.726%. Remained the same when pulling 59958ae on pl/fix-compile_commands into b37705f on master.

@randombit
Copy link
Owner

Can you give a little context as to what is being fixed here? From the patch I don't really understand the issue.

@lieser
Copy link
Collaborator Author

lieser commented May 2, 2023

If I run ./configure.py --cc msvc --compiler-cache=C:\\foo\\sccache.exe --debug-mode a single file entry looks like the following (without the fixes):

    { "directory": "C:\dev\github\randombit\botan",
      "command": "C:\foo\sccache.exe cl  /DBOTAN_DLL=__declspec(dllexport) /Fdbuild/botan-3.pdb

   /std:c++20 /EHs /GR /D_WIN32_WINNT=0x0600 /MDd /bigobj /Z7 -DBOTAN_IS_BEING_BUILT /W4 /wd4251 /wd4275 /wd5072  /I build/include /external:W0 /external:I build/include/external /nologo /c src/lib/asn1/alg_id.cpp /Fobuild/obj/lib/asn1_alg_id.obj",
      "file": "src/lib/asn1/alg_id.cpp"
    },
  • The command value is over multiple lines.
    • Note that the multi line issue does only occur with the --debug-mode flag.
    • A similar think can be observed in the Makefile for the LIB_FLAGS variable. But there the additional newlines after LIB_FLAGS are not causing any issues.
  • The windows file path backslash appear un-escaped in directory value and the compiler cache command part of the command value.

Both are I think invalid in JSON, and at least the VS Code C++ extension complains that it could not parse the compile_commands.json.

With the fixes in this PR it looks like the following, and gets parsed by the VS Code C++ extension without a problem:

    { "directory": "C:/dev/github/randombit/botan",
      "command": "C:/foo/sccache.exe cl  /DBOTAN_DLL=__declspec(dllexport) /Fdbuild/botan-3.pdb   /std:c++20 /EHs /GR /D_WIN32_WINNT=0x0600 /MDd /bigobj /Z7 -DBOTAN_IS_BEING_BUILT /W4 /wd4251 /wd4275 /wd5072  /I build/include /external:W0 /external:I build/include/external /nologo /c src/lib/asn1/alg_id.cpp /Fobuild/obj/lib/asn1_alg_id.obj",
      "file": "src/lib/asn1/alg_id.cpp"
    },

configure.py Outdated
@@ -1813,7 +1813,13 @@ def insert_join(match):
output += lines[idx] + "\n"
idx += 1

return self.join_pattern.sub(insert_join, self.value_pattern.sub(insert_value, output)) + '\n'
subsituded_template = self.join_pattern.sub(insert_join, self.value_pattern.sub(insert_value, output))
Copy link
Owner

Choose a reason for hiding this comment

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

substituted_

But I think just output is simpler and sufficient.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changet to output

configure.py Outdated
if len(lines) == 1 and not template.endswith('\n'):
return subsituded_template.rstrip('\n')

return subsituded_template + '\n'
Copy link
Owner

Choose a reason for hiding this comment

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

This seems to be adding another, additional newline, which doesn't seem necessary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agree this is not needed. Removed it.

Removes newlines in JSON strings and
normalize path to avoid backslash in JSON strings.
Also remove the additional added newline at the end.
@lieser lieser force-pushed the pl/fix-compile_commands branch from 2088a0b to 59958ae Compare May 4, 2023 15:15
@lieser lieser requested a review from randombit May 4, 2023 15:16
@lieser lieser merged commit 0d0a287 into master May 5, 2023
@lieser lieser deleted the pl/fix-compile_commands branch May 5, 2023 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants