Add C++ API. #77
Add C++ API. #77
Conversation
| ALL = \ | ||
| jsonnet \ | ||
| libjsonnet.so \ | ||
| libjsonnet-cpp.so \ |
There was a problem hiding this comment.
For some reason, running make only builds jsonnet but not the .so libraries, even though the so's are under ALL.
There was a problem hiding this comment.
This is expected. Running make all builds the .so's.
davidzchen
force-pushed
the
cpp2
branch
3 times, most recently
from
c9e5f2b
to
425a5c6
Compare
This change also adds an external dependency for gmock to the Bazel build, which is used by the C++ library's unit test. Issue google#71
|
This PR is ready for review. I have just added some unit tests for the C++ API. Since the Currently, the C++ API is a simple wrapper around the C APIs. This is not ideal since Jsonnet itself is implemented in C++. It would be better to refactor common logic out of libjsonnet.c and expose both C and C++ APIs. This way, we would able to also expose classes such as FmtOpts in the C++ API, which would be more suitable than having separate methods for setting each formatter option. |
| @@ -8,7 +8,7 @@ filegroup( | |||
| genrule( | |||
| name = "gen-std-jsonnet-h", | |||
| srcs = ["stdlib/std.jsonnet"], | |||
| outs = ["core/std.jsonnet.h"], | |||
| outs = ["std.jsonnet.h"], | |||
There was a problem hiding this comment.
Seems unrelated to this PR :)
There was a problem hiding this comment.
I was getting an Bazel build error about being unable to find the generated header file:
❯❯❯ bazel build //:jsonnet-common
INFO: Found 1 target...
ERROR: /Volumes/Ocean/Projects/google/jsonnet/BUILD:17:1: undeclared inclusion(s) in rule '//:jsonnet-common':
this rule is missing dependency declarations for the following files included by 'core/desugarer.cpp':
'bazel-out/local_darwin-fastbuild/genfiles/std.jsonnet.h'.
Target //:jsonnet-common failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 1.457s, Critical Path: 1.29s
This might be a bug in the C/C++ rules related to using the includes attribute while looking for a header file in the genfiles tree. Changing the genrule to output the file in the current package rather than core/ worked.
There was a problem hiding this comment.
Ok as long as it works, no need for it to be the same location as the makefile for example.
|
lgtm |
Fixes #71
Background
I started working on this to get a better idea of what the Java bindings should look like. This implementation is a thin wrapper on top of the C API, providing some C++ niceties such as
std::string, STL containers, andstd::functionfor import callbacks.I am opening this pull request as an RFC for now to get feedback on the API's design. There are still some additional changes I would like to add before this is fully ready, including:
.sofiles by default for some reason, even though they are under$(ALL)