-
Notifications
You must be signed in to change notification settings - Fork 73
Upgrade grpc library to v1.78.1 #499
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
Merged
Merged
Changes from 48 commits
Commits
Show all changes
53 commits
Select commit
Hold shift + click to select a range
7b94013
Initial changes
5bd3ba1
Removed all internal private api
6b72ee4
Removed forward decleration
f97b973
Using internal zigzag encode and decode
612b5e6
Upgraded grpc submodule to v1.78.0
2168edf
Updated template constructor
a84deab
Removed extra whitespace
yash-ni 8cc31e3
Removed hard coded utf8 validity check
3155c14
Added conditional_variable include
c5c2bbc
Removed periods from debug log
8e0ba50
Removed circular dependency
7f81183
Rename semaphore to lv_semaphore
65fcab3
Added unit tests
bf2caf1
Added cached datasize
4124261
Fixed parsing for various types
fed4c1a
Fixed byte message parsing
55d0d02
Added _cachedByteSize
e04a823
Fixed enum parsing
cf0ad3b
Fix nested message parsing
29f542c
Through rumtime error if string is invalid
a4677a9
Correctly parse oneof in codedstream
9d5dd7f
Minor cleanup
6dbd4d1
Fix parsing for singular types
7e1b27d
Invalidate bytesize cache
83f90ab
Added AreUtf8StringsEnabled feature toggle for byte parsing
575868c
Return error from seralizetostring
yash-ni dc3b40b
Remove commented headers
yash-ni cee22b7
Correct cache reset
140bb94
Merge branch 'serializationTraits' of https://github.com/yash-ni/grpc…
f3c60d5
Remove empty buffer check
yash-ni 4f29cef
Include break in wiretype group
3f8ec86
Upgrade grpc to v1.78.1
27ece25
Update src/lv_serialization_traits.h comments
yash-ni b724c3e
Return false if parsing fails
6fd988d
Removed buffer clear
14da4fa
Override recorderror and recordwarning
89c2d96
Added support for unpacked repeated field
de9faf8
Added tests for unpacked repeated fields
4934beb
Use wireformat from wireformatlite internal library
76717b8
Remove inline zigzag helpers
9d4f86f
Minor changes
671108a
Minor changes
b3d8e61
Changed parameter names to camelcase
b937508
Resolved build errors
1be6f27
Refactor protobuf numeric field parsing using traits-based template
340defe
Fix: merge multiple packed chunks for the same repeated field
78f8eea
Added commentes for bytesizelongcache
93a816a
Added tests for repeated string/bytes merging across multiple occurre…
41393df
Correctly handle empty buffers
5d987fa
Removed unnecessary copies and allocations in ParseFromByteBuffer
a584ff2
Minor changes and refactoring
aa84620
Minor changes and code cleanup
3ea9447
minor changes
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this have any backward compatibility impact for Linux RT? Have you done any testing with an RT target and LV 2023Q1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven’t tested this on Linux RT beyond the build pipeline. I tested it on LV 2019. Is there any specific reason you’re asking about 2023 Q1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last time we upgraded grpc libraries, it required removing support for some older versions of LVRT, and I don't think we really realized it until after the fact. See #439. I want to ensure the same thing isn't happening here.
I'm not familiar with the Linux RT toolchain and deployment which is why I asked the question. Presumably the update to C++ 20 requires a new version of the C++ standard runtime library to be deployed. How is that done for a Linux RT system? I don't believe the "install" pattern used for our binaries expects/includes any binaries other than what are built from this repo. So presumably this dependency would already need to be available on the system from a shared location? If so and the rest of the LV toolchain is using a different C++ runtime version then it seems like that could be a problem. Or are we statically linking in the dependency? It's not clear to me browsing through the cmake files. Anyway, this is what motivated the question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue before was when the RT toolchain that was used for build was changed (defined here:
https://github.com/yash-ni/grpc-labview/blob/de9faf8cc4504da024f140ae14971b7ef51c2527/.github/workflows/build_on_rt.yml#L30-L31)
The RT toolchain links to a version of libc and libstdc++, which is what breaks the shared library on previous versions of NILRT.
Since this PR does not change the toolchain - and the RT build workflow is passing - I would not expect that the compatible NILRT versions to be affected by this. (However I would still recommend testing just to be sure :))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition: libstdc++ uses symbol versioning to preserve compatibility with older versions: https://gcc.gnu.org/onlinedocs/libstdc++/manual/abi.html
If the code depends on symbols that are marked CXXABI_1.3.14 but the installed libstdc++ only supports up to CXXABI_1.3.12 or something, then you will get errors from dlopen or function calls.