-
Notifications
You must be signed in to change notification settings - Fork 51
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
Dev/update grp cdevice 2 #1087
Draft
rajendra-desai-ni
wants to merge
16
commits into
ni:main
Choose a base branch
from
rajendra-desai-ni:dev/updateGRPCdevice_2
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Dev/update grp cdevice 2 #1087
rajendra-desai-ni
wants to merge
16
commits into
ni:main
from
rajendra-desai-ni:dev/updateGRPCdevice_2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
NILRT 11 will ship with grpc >1.60 and protobuf >v25.2. grpc-device doesn't compile with these toolchain versions, throwing errors about undefined symbols. Downgrading grpc back to 1.51 (with python3-grpcio and protobuf recipes) successfully works around these issues. But for security and general currency reasons, we cannot afford to ship NILRT 11 with these downgrades. Current CMakeLists.txt is limited to some of the bitbake functionalities which makes it difficult to build with new changes. Changes in this commit will make sure NILRT 11 compiles grpc-device with the latest/upgraded grpc version without affecting the existing build process. Changes: 1. refactor toolchain link logic - Deprecate the CMAKE_CROSSCOMPILING variable, in favor of USE_SUBMODULE_LIBS cmake option. Refactor the linking logic to be a consolidation of all the linking actions from across the file, and to better support builds in generic linux environments. 2. fixup utf8cpp library link - The utf8cpp cmake library namespace is incorrectly identified as 'utf8cpp', instead of the proper 'utf8cpp:utf8cpp'. As a result, cmake does not link the utf8.h header and compilation fails. 3. parameterize python3 venv - Create a USE_PYTHON_VIRTUALENV cmake option. When asserted, it will add the bespoke venv to the toolchain. Otherwise, the cmake config will use the system python environment. 4. link the device server to grpc_gpr - ni_grpc_device_server target depends on symbols from grpc gpr.so, namely gpr_log. Add grpc_gpr to link libraries for ni_grpc_device_server. 5. add abseil_sync dep to server target - ni_grpc_device_server uses symbology from libabsl_synchronization library. Add a library dependency to reflect that relationship. 6. add utf8cpp dep to IntegrationTestsRunner - The IntegrationTestsRunner depends on utf8.h header indirectly, via its access to the device server source. 7. fill out target lib deps - Shove ni_grpc_device_server library dependencies into a variable, so that it can be easily passed along to the test targets. 8. suppress protobuf installation in SM - Set protobuf_INSTALL=OFF, which suppresses the protobuf installation codepaths - that we don't want to use anyway and which cause the failure. 9. add necessary gRPC dep to ni_grpc_device_server - ni_grpc_device_server must be linked against libgrpc, as well as the grpccpp libs. 10.fixup venv codegen deps - Give the codegen targets a dependency on the python virtualenv via the all_codegen_dependencies variable. Signed-off-by: Rajendra Desai <[email protected]>
…d fixed a small typo Signed-off-by: Rajendra Desai <[email protected]>
Signed-off-by: Rajendra Desai <[email protected]>
MSVC_RUNTIME_LIBRARY property) which is supported on cmake version >3.15 in case of MSVC compiler Signed-off-by: Rajendra Desai <[email protected]>
This reverts commit 2ea12e9.
This reverts commit 36890c9.
This reverts commit 26a3c63.
This reverts commit 2ed6d06.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this Pull Request accomplish?
TODO: Include high-level description of the changes in this pull request.
Why should this Pull Request be merged?
TODO: Justify why this contribution should be part of the project.
What testing has been done?
TODO: Detail what testing has been done to ensure this submission meets requirements.