[PROPOSAL] make2compdb.c - #392
Conversation
|
Epic! I'll be test driving this myself. |
|
Thanks so much, @g-berthiaume! It's interesting to see these concepts through another person's lens. Make all functions except the entry point When I compile with GCC 16 in the latest w64dk I get a It's "peek" not "peak". I was genuinely confused by this because the latter would normally mean highest value (highest memory use, highest count, etc.). Also "threated" instead of "treated" and "stars" instead of "starts", and "emtpy" instead of "empty". The program always outputs UTF-16, and there seems to be a misunderstanding how output is supposed to work. I'm surprised the resulting If the output device is a console, use As a see it for yourself test of the above, these must both produce the same result: The parser should be more oriented around tokens than substring search. Don't substring search for Because it's substring matching, this doesn't work as intended: Produces an
Produces output with escaped single quotes, The Only captures the second command. Add a test for this when you fix it. Unless this is intentional? The Compiler
Trivial to adjust, but even 16MiB arena may be a little tight for larger builds. Maybe 64M? Build commands can be quite bloated. Nothing below is a problem, nor must change. Just pointing them out as commentary. If you're committed to GNU-style toolchains (e.g. GCC, Clang), as MSVC is excluded, then because you're linking Might be surprising (or might not given those void example(Str const s)
{
s.ptr[0] = 0;
}But |
Would calling setlocale before starting the process not help? Then it wouldn't matter what the user's locale is. |
|
@skeeto Thank you for your review.
I'm not a Linux expert, but even if we assume that the shell starts That said, even if we could, I would prefer not to touch the locale config. In cases of a crash, we could break users environment because EDIT (2026-05-29): |
|
Round 2! The focus for this new release has been to make Changes
As you can see, the output is now identical, regardless of the destination. λ printf 'cc π.c' | ./make2compdb | grep file | xxd
00000000: 2020 2020 2020 2020 2266 696c 6522 3a20 "file":
00000010: 22cf 802e 6322 2c0a "...c",.
λ printf 'cc π.c' | ./make2compdb >json && grep file json | xxd
00000000: 2020 2020 2020 2020 2266 696c 6522 3a20 "file":
00000010: 22cf 802e 6322 2c0a "...c",.
Now λ printf 'ccache clang -c x.c' | ./make2compdb
[
{
"directory": "C:\\dev",
"file": "x.c",
"arguments": [
"clang",
"-c",
"x.c"
]
}
]
λ printf "cc -I'a b' x.c" | ./make2compdb
[
{
"directory": "C:\\dev",
"file": "x.c",
"arguments": [
"cc",
"-Ia b",
"x.c"
]
}
]
λ printf 'cc a.c; cc b.c' | ./make2compdb
[
{
"directory": "C:\\dev",
"file": "a.c",
"arguments": [
"cc",
"a.c"
]
},
{
"directory": "C:\\dev",
"file": "b.c",
"arguments": [
"cc",
"b.c"
]
}
]
λ printf 'gcc main.c -o main &>>test.txt' | ./make2compdb
[
{
"directory": "C:\\dev",
"file": "main.c",
"output": "main",
"arguments": [
"gcc",
"-o",
"main",
"main.c"
]
}
]
λ printf 'gcc main.c --output main' | ./make2compdb
[
{
"directory": "C:\\dev",
"file": "main.c",
"output": "main",
"arguments": [
"gcc",
"--output",
"main",
"main.c"
]
}
]
λ printf "distcc ccache clang main.c" | ./make2compdb
[
{
"directory": "C:\\dev",
"file": "main.c",
"arguments": [
"clang",
"main.c"
]
}
]
Notice that λ printf "gcc -x c lib.h main.c -x none some_include.h" | ./make2compdb
[
{
"directory": "C:\\dev",
"file": "lib.h",
"arguments": [
"gcc",
"-x",
"c",
"lib.h",
"main.c",
"-x",
"none",
"some_include.h"
]
},
{
"directory": "C:\\dev",
"file": "main.c",
"arguments": [
"gcc",
"-x",
"c",
"lib.h",
"main.c",
"-x",
"none",
"some_include.h"
]
}
]
Here's a (nasty) example: λ printf 'gcc main.c -o 2&> lib.c main' | ./make2compdb
[
{
"directory": "C:\\dev",
"file": "main.c",
"output": "main",
"arguments": [
"gcc",
"-o",
"main",
"main.c"
]
}
]
DetailsAt its core, this release focuses on the shell tokenizer re-implementation. After thinking about it and re-reading Arenas and the almighty concatenation operator, I realized that I could simplify my memory allocation strategy by composing Performance-wise, the new parsing strategy is a bit slower, which is to be expected, as we now look at every token. Note for reviewerThanks again for your last feedback. |
|
@skeeto @Peter0x44 |
| return tokens; | ||
| } | ||
|
|
||
| // :: Compiler |
There was a problem hiding this comment.
What's the purpose of this CompilerKind? It doesn't seem to be doing anything needed?
There was a problem hiding this comment.
You're right, currently CompilerKind is only used in the CompilerInvocation struct, which is not really needed.
The idea is that I could extend make2compdb to support other compilers like Microsoft's CL.exe.
You can see the scaffolding for executing this idea in compiler_command_from_invocation():
There was a problem hiding this comment.
I don't believe this distinction between compilers will be ever needed.
The chance for some flag to clash when, e.g. one compiler expects it to consume the argument while the other does not, it's probably not something worth worrying about.
GCC, Clang and all it's forks (emcc, zig cc, icx, etc...) they all share flags which should be compatible. For CL the flags are usually prefixed with slash / rather than dash -, so you can cheat a bit here :)
As I mentioned in #251 (comment), hardcoding compilers/flags will never be reliable, so you may as well ignore small incompatibilities and keep the code simple, that is, just detect if the compiler is known, but don't try to do anything smart about it.
There was a problem hiding this comment.
I like this discussion a lot.
I'm hesitant to fully commit to this idea because the core of my parsing strategy relies on finding the "start" of the compilation command, and therefore identifying the compiler.
Without it, I'm not sure how I would be able to navigate the noise that is a makefile stdout.
| Arena scratch = *perm; | ||
|
|
||
| Str cwd = dirstack_pop(&dir_stack, &scratch); | ||
| (void)cwd; |
There was a problem hiding this comment.
The "gymnastic" around returning cwd is a bit odd. dirstack_pop is only used in this one place. Wouldn't it make more sense to just edit it like so [diff below] ?
tbh, the code related to dirstack_* and strlist_* seems like (potentially) unnecessary boiler plate, but I haven't looked through all of it though.
diff --git a/src/make2compdb.c b/src/make2compdb.c
index 39f9d27..46ce8a0 100644
--- a/src/make2compdb.c
+++ b/src/make2compdb.c
@@ -838,15 +838,12 @@ static Str dirstack_peek(DirectoryStack ds, Arena *perm)
return directory_copy;
}
-static Str dirstack_pop(DirectoryStack *ds, Arena *perm)
+static void dirstack_pop(DirectoryStack *ds)
{
Str directory = strlist_pop_back(&ds->stack);
if (directory.len > 0) {
- Str directory_copy = str_duplicate(perm, directory);
arena_reset_to(&ds->arena, (byte *)directory.ptr);
- return directory_copy;
}
- return directory;
}
// :: OS
@@ -2152,10 +2149,7 @@ static int make2compdb(Arena *perm, OsWriterInterface *w, StrList cli_args, Str
Str dir = directory_from_make_dir_line(&input);
if (dir.len > 0) {
- Arena scratch = *perm;
-
- Str cwd = dirstack_pop(&dir_stack, &scratch);
- (void)cwd;
+ dirstack_pop(&dir_stack);
}
break;
}There was a problem hiding this comment.
The "gymnastic" around returning cwd is a bit odd. dirstack_pop is only used in this one place.
I agree with you. I used to use this value to validate that we pop the expected directory, but for fuzzing, this made no sense.
Should be fixed in my next commit.
| "Options:\n" | ||
| " --version: Display the version number.\n" | ||
| " -h, --help: Display this help page.\n" | ||
| " -v, --verbose: Display debug information to help with problem diagnosis.\n" |
There was a problem hiding this comment.
Any chance for diagnostic to appear at stderr when using --verbose and similar, so it can be easily piped and split from json output?
I see there is some platform specific separation of stdin/out/err already, but the diagnostic still goes to stdout, so it's not really practical to deal with. (basically, everything that isn't json should go to stderr)
There was a problem hiding this comment.
Thanks for the suggestion.
I just tried it. To be honest, it feels to me like it would be a bit confusing for new users.
That said, I'm not opposed to this change.
I will commit it so you guys can try it, and I'll revert it if need be.
| SL("-bundle_loader"), | ||
| SL("--param"), | ||
| SL("-arch"), | ||
| SL("-wrapper"), SL("-wrapper"), |
There was a problem hiding this comment.
This is fixed in my next commit.
Thanks for your attention to detail.
|
Thanks @sleeptightAnsiC for your review. |
Round 3!The focus for this new release is addressing the issues identified by @Peter0x44 and @sleeptightAnsiC. Changelog
Note for reviewer As always, thanks again for your last feedback. Previous rounds
|
|
Hi @skeeto |
|
Sorry, I did not prioritize a followup review enough and so I kept putting it off. It takes awhile to get my head back into the problem after being away from it more than a couple of weeks.
Here the apostrophe is lost, and the
@@ -2558,2 +2558,5 @@ static void test_compiler_parse(Arena a)
+ run_test_compiler_parser(SL("clang"), SL("clang"), COMPILER_IS_GCC_COMPATIBLE);
+ run_test_compiler_parser(SL("clang++"), SL("clang++"), COMPILER_IS_GCC_COMPATIBLE);
+
// zig cc
@@ -2573,3 +2576,3 @@ static void test_compiler_parse(Arena a)
// NVIDIA HPC C++ compiler (nvc++ -std=c++17 -O2 -c main.cpp -o main.o)
- run_test_compiler_parser(SL("nvc"), SL("nvc"), COMPILER_IS_GCC_COMPATIBLE);
+ run_test_compiler_parser(SL("nvc++"), SL("nvc++"), COMPILER_IS_GCC_COMPATIBLE);
Should Some typos in the header doc: @@ -1,3 +1,3 @@
// make2compdb
-// Generates a Clang's JSON Compilation Database files (`compiler_commands.json`) from make build systems.
+// Generates a Clang's JSON Compilation Database files (`compile_commands.json`) from make build systems.
// This json file can be usefull for multiple tools including the [clangd](https://clangd.llvm.org/) LSP.
@@ -6,4 +6,4 @@
//
-// $ make -Bwn | make2compdb.exe > compiler_commands.json
-// $ cat compiler_commands.json
+// $ make -Bwn | make2compdb.exe > compile_commands.json
+// $ cat compile_commands.json
// [I think these are meant to go to standard error? --- a/src/make2compdb.c
+++ b/src/make2compdb.c
@@ -2101,5 +2101,5 @@ static int make2compdb(Arena *perm, OsWriterInterface *os_stdout, OsWriterInterf
else {
- print_str(os_stdout, SL("Unknown CLI arg: '"));
- print_str(os_stdout, arg->str);
- println_str(os_stdout, SL("'\n"));
+ print_str(os_stderr, SL("Unknown CLI arg: '"));
+ print_str(os_stderr, arg->str);
+ println_str(os_stderr, SL("'\n"));
return -1;Also please exit with 1, not -1. That's a bad habit from the unix world where conventionally it's clamped to 255, but Windows uses the whole 32-bit exit status, and so it clamps to The busybox-w32 shell reports such high-valued errors loudly because they're often significant (special Windows errors).
Bad text replacement here? goto reject; // unU8_paired low surrogateI think we're pretty close to merging. |
There was a problem hiding this comment.
Pull request overview
Introduces make2compdb, converting GNU Make output into Clang JSON compilation databases.
Changes:
- Adds shell parsing, compiler detection, JSON generation, tests, and fuzzing.
- Supports CRT-free Windows and POSIX execution.
- Integrates the executable into w64devkit packaging.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
src/make2compdb.c |
Implements the utility and platform support. |
Dockerfile |
Builds and installs make2compdb.exe. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| switch (c) { | ||
| case '\'': break; //< Unquote | ||
| case '\"': break; //< Unquote | ||
| case '\\': { |
| case '\v': print_str(w, SL("\\v")); break; | ||
| case '\b': print_str(w, SL("\\b")); break; | ||
| case '\f': print_str(w, SL("\\f")); break; | ||
| case '\a': print_str(w, SL("\\a")); break; | ||
| case '\\': print_str(w, SL("\\\\")); break; | ||
| default: print_u8(w, c); break; | ||
| } |
|
|
||
| (void)first; //< We cannot rely on "make" as this program name changes. | ||
|
|
||
| if (str_equal(third, SL("directory"))) { |
|
|
||
| switch (make_u16(mode, c)) { | ||
| case make_u16(IN_QUOTE, '\''): mode = NORMAL; break; | ||
| case make_u16(IN_DOUBLE_QUOTE, '\"'): mode = NORMAL; break; |
| if (str_equal(segment.head, SL("clang"))) { | ||
| compiler.kind = COMPILER_IS_GCC_COMPATIBLE; | ||
| compiler.string = input; | ||
| return compiler; |
|
|
||
| switch (state) { | ||
| case SEARCH_COMPILER: { | ||
| compiler = compiler_parse(token); |
| print_str(os_stdout, SL("Unknown CLI arg: '")); | ||
| print_str(os_stdout, arg->str); | ||
| println_str(os_stdout, SL("'\n")); |
| u8 *src = NULL; | ||
| src = realloc(src, len); | ||
| assert(src); | ||
| memcpy(src, buf, len); |
| bs->stream.err = !WriteConsoleW(bs->stream.handle, buf, buf_len, NULL, 0); | ||
| bs->buf.len = 0; |
| -o $PREFIX/bin/makensis.exe $PREFIX/src/alias.c -lkernel32 \ | ||
| && $ARCH-gcc \ | ||
| -Oz -fno-asynchronous-unwind-tables -fno-builtin -Wl,--gc-sections \ | ||
| -s -nostdlib -o $PREFIX/bin/make2compdb.exe $PREFIX/src/make2compdb.c \ |
No worries. Thank you for your review; I'll address those problems shortly. |
This PR introduces
make2compdb.A new CLI tool to generates Clang's JSON Compilation Database files (
compiler_commands.json) from make build systems.The API can be used in the following way:
Properties
Expand to see peports
$ peports make2compdb.exe KERNEL32.dll 0 ExitProcess 0 GetCommandLineW 0 GetConsoleMode 0 GetCurrentDirectoryW 0 GetStdHandle 0 ReadFile 0 VirtualAlloc 0 WriteConsoleW 0 WriteFile SHELL32.dll 0 CommandLineToArgvWExpand to see performance benchmarks
While I'm sure, we could do better, the CLI tool seems to be acceptably fast.
It seems to be a bit faster on Linux than on Windows, but I'm not sure if I'm not just measuring piping speeds.
With a small project (linux)
With a small project (windows)
With ffmpeg (windows)
With ffmpeg (linux)
By comparison, the https://github.com/nickdiego/compiledb takes 18 seconds to analyzer ffmpeg.
$ hyperfine "compiledb -p ffmpeg_output.txt" Benchmark 1: compiledb -p ffmpeg_output.txt Time (mean ± σ): 18.469 s ± 0.258 s [User: 18.128 s, System: 0.205 s] Range (min … max): 18.127 s … 19.003 s 10 runsSo it's a 331x speedup.
Features
\\), EOL (\r\n) and executable extension (.exe).gcc main.c lib.c -o main).Address this issue compiledb returns nothing when compiling headers with
-xswitch nickdiego/compiledb#140Address this issue [Feature Request] Support non-standard make command in directory parsing nickdiego/compiledb#146
Address this issue Different results betweent "compiledb make" and "make | compiledb" nickdiego/compiledb#102
Address this issue Support ccache (and other 'wrappers') nickdiego/compiledb#2
Two additional features
1. No Make mode
I'm a big fan of the "unity build" compilation technique (like the one used in u-config).
Therefore, I often don't have a use for a makefiles in my projects.
That said, I still need
compiler_commands.jsonfor my IDE to work.How great would it be to just pipe my build command into
make2compdb?Well you can!
2.
--verboseI believe that there's a lot of value in providing your technical users tools to help them self-diagnose issues.
In my experience, this usually leads to better bug reports and therefore help maintaining the project.
This is why I have added the
--verboseCLI flag.When it's passed to
make2compdb, the stdout contains debug information.Limitations
As a non-native English speaker, I can empathize with this being a pain point.
That said, I didn't find a robust way to parse the Makefile output in different languages.
Note for reviewer
I've learn a lot building this project: Arena, fuzzing, CRT-less windows programming.
When starting this project, I think I underestimated the number of corner cases to handle.
To be honest, the shell parsing humbled me a little bit. :^)
One of the reasons I appreciate w64devkit, is its high standard when it comes to programming.
I think I have a lot to learn from the maintainers of this project, so I welcome any of your suggestions on how to improve
make2compdb.Closes #251