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

outdated dependencies #54

Open
zolo2013 opened this issue Feb 1, 2023 · 5 comments
Open

outdated dependencies #54

zolo2013 opened this issue Feb 1, 2023 · 5 comments

Comments

@zolo2013
Copy link

zolo2013 commented Feb 1, 2023

The separated LLVM libraries for linking are outdated. I have to manually replace these separated libraries
clangAST
clangASTMatchers
clangFrontend
clangSema
clangSerialization
clangTooling
with clang-cpp in the file backend/CMakeLists.txt

However, the bazel build fails and it probably is related to the outdated versions.

@rwgk
Copy link
Contributor

rwgk commented Feb 1, 2023

The bazel build never worked AFAIK.
For background: I don't like to say it, but it's true: what you see under google/clif isn't staffed, we're keeping this up in our spare time, more or less.
If you send PRs with the fixes you need I'll try my best to integrate them quickly. (I'll have to do this manually, we only have automatic export from our internal repo to this github repo.)

@zolo2013
Copy link
Author

zolo2013 commented Feb 2, 2023

Hi @rwgk, Thanks for the response. From your comment RobotLocomotion/drake#7889 (comment), we understand that your plan for clif is to adopt pybind11 for the generated code. There is a directory pybind11 in the code tree but we didn't find any inclusion of them in the cmake or bazel build scripts. It will be great if the base code and set up (such as bazel) is workable so that the community can help.

@rwgk
Copy link
Contributor

rwgk commented Feb 2, 2023

Hi @rwgk, Thanks for the response. From your comment RobotLocomotion/drake#7889 (comment), we understand that your plan for clif is to adopt pybind11 for the generated code. There is a directory pybind11 in the code tree but we didn't find any inclusion of them in the cmake or bazel build scripts. It will be great if the base code and set up (such as bazel) is workable so that the community can help.

PyCLIF is the predominant tool used internally, but when it comes to releasing to OSS, I usually point people to using pybind11 directly, even for Google-internal use.

We have 99.97% of tests working internally with PyCLIF-pybind11, but getting that last 0.03% turns out to consume an extraordinary amount of energy. We will be able to turn some attention to the OSS side only after we're through that.

@zolo2013
Copy link
Author

zolo2013 commented Feb 2, 2023

Thanks @rwgk. Glad to hear the progress for PyCLIF-pybind11.

Regarding PyCLIF, I was comparing the support of the native messages of protobuffers between PyCLIF and pybind11's protobuf support using fast native cpp cast.

At

inline const proto2::Message* GetCProto(PyObject* py) {
:
inline const proto2::Message* GetCProto(PyObject* py) {
return nullptr;
}

It returns nullptr, which means the native protobuffer message is not used for the binding. Is there any plan to add the support?

@rwgk
Copy link
Contributor

rwgk commented Feb 2, 2023

It returns nullptr, which means the native protobuffer message is not used for the binding. Is there any plan to add the support?

I'm amazed you were able to pin-point that line of code!

The plan is that we get better protobuf performance as part of switching over to PyCLIF-generated pybind11 code (ephemeral code, to not create a wrong impression).

I sunk a lot of my time into helping getting pybind11_protobuf to where it is. Most of that is not externally visible, it was an XL internal migration project.

Coincidentally, a couple days ago I tried to fix PyCLIF-pybind11 test failures (in the those 0.03%) related to protobuf issues. I learned the hard way: that's an L sized internal cleanup project, too. For practical reasons I had to back-peddle, patching our dev code to do the equivalent of the line you found, for now, leaving the cleanup for later.

To give you a glimpse of what it's like to have to satisfy many million tests.

IOW sorry ... use pybind11 directly for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants