Repository navigation
Depa ipc tracing feature flag usage - #1094
devendrapat wants to merge 3 commits into
Conversation
| bazel_dep(name = "score_baselibs", version = "0.2.12") | ||
| single_version_override( | ||
| module_name = "score_baselibs", | ||
| commit = "4abf22407106ec2461f5f80a619cd15b5d294238", |
There was a problem hiding this comment.
This would require all our customer to pin this exact version. Is a new release of baselibs planned? What is the ticket to track that? In general we only want to use release versions of baselibs.
There was a problem hiding this comment.
I created #1102 that bumps baselibs to the new released version. The new version contain the changes you need, you should just remove this check once the mentioned PR is merged.
There was a problem hiding this comment.
Currently we cannot bump because of eclipse-score/baselibs#527 (comment)
There was a problem hiding this comment.
Bump is done, you should be able to continue discarding the bump changes
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. If this PR is still relevant, please leave a comment or push new changes to keep it open. |
318eb84 to
658d017
Compare
db6d7c3 to
ca91d3d
Compare
limdor
added
the
test-qnx
GitHub Actions
Error
0b23e73 to
0d8eb93
Compare
GitHub Actions
Error
GitHub Actions
Error
fdce4f0 to
441f66e
Compare
GitHub Actions
Error
To decide, whether typed-mem should be used in case of IPC-tracing, now the corresponding feature flag is consulted.
441f66e to
320986a
Compare
GitHub Actions
Error
| skeleton_->PrepareOffer(events_, fields_, std::move(kEmptyRegisterShmObjectTraceCallback)).has_value()); | ||
| } | ||
|
|
||
| #ifdef REQUIRE_TYPED_MEMORY |
There was a problem hiding this comment.
Just had a look - interesstingly we never checked, that in case of "activated typed-mem" we are also calling SharedMemoryFactory::Create with prefer_typed_memory == true!
So in line 243 you see, that we have a call-expectation to the Create() func with prefer_typed_memory = false
... I guess we should add an explicit expectation here, that shared_memory_factory_mock_::Create is called with prefer_typed_memory = true for DATA and CTRL sections!
There was a problem hiding this comment.
| register_shm_object_trace_callback{}; | ||
|
|
||
| // Given a Skeleton constructed from a valid identifier referencing a QM deployment | ||
| InitialiseSkeleton(GetValidInstanceIdentifier()).WithNoConnectedProxy(); |
There was a problem hiding this comment.
See my comment above. Here you should then have a call expectation prefer_typed_memory = false for the Create call ...
There was a problem hiding this comment.
320986a to
5d749dd
Compare
GitHub Actions
Error
GitHub Actions
Waiting
| name = "skeleton_require_typed_memory", | ||
| testonly = True, | ||
| srcs = [ | ||
| "skeleton.cpp", |
There was a problem hiding this comment.
We should not duplicate a target and have one version with one value and another with another. This is just asking for trouble, most of the target is just repeated and copy pasted. You also have then the same translation unit in two libraries. The risk of ending up with ODR is way too high.
It should just be one target and through a feature flag the user can control it. We should not support having two version of the skeleton library in the same build tree.
This branch is waiting to be deployed
3 participants
Keeping the changes in this PR (https://github.com/eclipse-score/communication/pull/1022/changes) intact.
This PR also updates the version of score_baselibs for usage of required_typed_memory flag.