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

Remove dependency on cpp-jsonnet submodule from C bindings #482

Merged
merged 1 commit into from
Mar 31, 2025

Conversation

eagletmt
Copy link
Contributor

This change allows us to build libgojsonnet from tarball downloaded from
GitHub releases.

@google-cla google-cla bot added the cla: yes label Nov 24, 2020
@coveralls
Copy link

Coverage Status

Coverage remained the same at 81.499% when pulling 1b4d7f5 on eagletmt:no-libjsonnet-h into 57e6137 on google:master.

@coveralls
Copy link

coveralls commented Nov 24, 2020

Coverage Status

coverage: 55.582%. remained the same
when pulling 2f73f61 on eagletmt:no-libjsonnet-h
into 1c79f57 on google:master.

@sbarzowski
Copy link
Collaborator

We can probably go forward with this, but I don't like that we can now accidentally diverge from the original headers without noticing it.

Maybe we could keep a copy of the header in the repo and use it for compiling libgojsonnet. We could then check if it's the same as in cpp-jsonnet in tests.sh (which depends on cpp-jsonnet anyway). Let me think about this.

This change allows us to build libgojsonnet from tarball downloaded from
GitHub releases.
@johnbartholomew
Copy link
Collaborator

Four and a half years later, this pull request still seems useful to me.

Looking at the history of libjsonnet.h in the C++ repository, the declarations being copied here last changed in 2014: google/jsonnet@2c5fd87#diff-ee89e9022b7fdd09da47ce82a4b9b1fd6901967ffbbd8d1a30249a18414b715f

It would be nice to have an automated check in the test suite that the copied declarations don't diverge... but given the rate of change here it doesn't seem like a high urgency problem.

I've rebased the change onto master.

@johnbartholomew johnbartholomew merged commit 2f73f61 into google:master Mar 31, 2025
13 checks passed
@johnbartholomew
Copy link
Collaborator

Merged. Thanks for the contribution!

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