Support for incremental update #8
Conversation
The existing compile commands database is no longer completely overwritten but rather updated. Entries for removed sources are removed and entries for recompiled sources are updated.
Avoid having multiple entries for the same source file when multiple the commands refer to it non-canonically. E.g.: "cc foo.c" in "bar/" directory vs "cc ../foo.c" in "bar/baz/" directory "cc foo.c" vs "cc ./foo.c" "cc foo.c" in "bar" directory vs "cc ../bar/foo.cc" in "bar/" directory "cc foo.c" vs "cc <symlink to foo.c>" ...
| @@ -61,6 +61,7 @@ Extra options can precede `--` if needed. | |||
| [clang]: https://clang.llvm.org | |||
| [compile-db-gen]: https://github.com/sunlin7/compile-db-gen | |||
| [compiledb]: https://github.com/nickdiego/compiledb | |||
| [nlohmann/json]: https://github.com/nlohmann/json | |||
There was a problem hiding this comment.
I guess from basset user's perspective the most useful link will be this one:
| [nlohmann/json]: https://github.com/nlohmann/json | |
| [nlohmann/json]: https://json.nlohmann.me/integration/package_managers/ |
| bool CompilationDatabase::is_source_file(const string &argument) { | ||
| bool is_source_file(const string &argument) { |
There was a problem hiding this comment.
Why do you think that it is a bad idea to encapsulate is_source_file() into CompilationDtabase?
| if (!ifs) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
This check seems insufficient. Input file may not exist, in which case it is OK to return true; early. But it could as well be permission issue that prevented opening the file, in which case it makes more sense to return false;.
src/basset.cpp
Outdated
|
|
||
| if (exists(path(directory) / file)) { | ||
| index[make_index_key(directory, file)] = | ||
| make_pair(directory, (*it)["arguments"]); |
There was a problem hiding this comment.
This will definitely work with compilation database that was generated by basset. However, according to format description there may be "command" instead of "arguments". Would be nice to support both formats. But it means extra complexity to parse shell command into separate arguments. What do you think, is it doable?
src/basset.cpp
Outdated
| auto json = nlohmann::json::array(); | ||
| for (const auto &kv : index) { | ||
| json += nlohmann::json::object({{"directory", kv.second.first}, | ||
| {"file", kv.first}, |
There was a problem hiding this comment.
As far as I understand, using key as "file" will have an effect that it will always be an absolute path, right?
| if (!ofs) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
I am afraid it may be too late for this check. save() is called in the very end, after (potentially very time-consuming) build process is over. It would be very disappointing for the user to find out at this point that output file is not writable and all the collected data has nowhere to be dumped. I think this check needs to be done earlier. Maybe when constructing CompilationDatabase?
|
|
||
| if (!cdb) { | ||
| cerr << "cannot open '" << output << "'\n"; | ||
| CompilationDatabase cdb(output, is_source_file); |
There was a problem hiding this comment.
Hm, maybe we should introduce separate arguments for input and output?
Adds support for incremental update of the compile database if it already exists. Basically, if a new compilation entry is detected, it overrides the entry referring to the same source file if it already exists, otherwise it's simply added. The source file is uniquely identified by "canonized" path constructed from
directoryandfilevalues.An external library - nlohmann/json (https://github.com/nlohmann/json) - is used as JSON handling is significantly more complex now.