Skip to content

kvs: align the internal repr between C++ and Rust - #490

Merged
vinodreddy-g merged 1 commit into
eclipse-score:mainfrom
qorix-group:arkjedrz_output-json-align
Oct 29, 2026
Merged

kvs: align the internal repr between C++ and Rust#490
vinodreddy-g merged 1 commit into
eclipse-score:mainfrom
qorix-group:arkjedrz_output-json-align

Conversation

@arkjedrz

@arkjedrz arkjedrz commented Oct 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor
  • Disallow non-type-tagged representation.
  • Replace From with TryFrom for Rust type types.
    • Conversion is not infallible.
  • Add kvsvalue_to_any object specialization.
    • To avoid unnecessary copy.
  • Disallow duplicate keys in KvsValue::Object.
  • Update tests.

KvsValue::Object representation is no longer aligned due to incomplete types used by unordered_map being an UB.
To be revisited once custom map container is available.

@arkjedrz arkjedrz self-assigned this Oct 9, 2026
@arkjedrz
arkjedrz deployed to workflow-approval October 9, 2026 14:08 — with GitHub Actions Active
@arkjedrz
arkjedrz deployed to workflow-approval October 9, 2026 14:08 — with GitHub Actions Active
@github-actions

github-actions Bot commented Oct 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run --lockfile_mode=error //:license-check

Status: ⚠️ Needs Review

Click to expand output 
[License Check Output]
Starting local Bazel server (8.7.0) and connecting to it...
INFO: Invocation ID: 7af292c6-08a8-4e05-80a5-e734dce760fd
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 2 packages loaded
Loading: 2 packages loaded
    currently loading: 
Loading: 2 packages loaded
    currently loading: 
Loading: 2 packages loaded
    currently loading: 
Loading: 2 packages loaded
    currently loading: 
Loading: 2 packages loaded
    currently loading: 
Loading: 2 packages loaded
    currently loading: 
Analyzing: target //:license-check (3 packages loaded, 0 targets configured)
Analyzing: target //:license-check (3 packages loaded, 0 targets configured)

Analyzing: target //:license-check (71 packages loaded, 14 targets configured)

Analyzing: target //:license-check (99 packages loaded, 14 targets configured)

Analyzing: target //:license-check (100 packages loaded, 14 targets configured)

Analyzing: target //:license-check (100 packages loaded, 14 targets configured)

Analyzing: target //:license-check (152 packages loaded, 1678 targets configured)

Analyzing: target //:license-check (201 packages loaded, 5518 targets configured)

Analyzing: target //:license-check (205 packages loaded, 9887 targets configured)

Analyzing: target //:license-check (207 packages loaded, 11661 targets configured)

Analyzing: target //:license-check (207 packages loaded, 11661 targets configured)

Analyzing: target //:license-check (208 packages loaded, 11785 targets configured)

INFO: Analyzed target //:license-check (213 packages loaded, 17903 targets configured).
[6 / 15] [Prepa] Writing repo mapping manifest for @@score_tooling+//dash/tool/formatters:dash_format_converter [for tool]
INFO: From Generating Dash formatted dependency file ...:
INFO: Successfully converted 50 packages from Cargo.lock to bazel-out/k8-fastbuild/bin/tools/formatted.txt
[14 / 16] [Prepa] JavaToolchainCompileBootClasspath external/rules_java+/toolchains/platformclasspath.jar
[14 / 16] JavaToolchainCompileBootClasspath external/rules_java+/toolchains/platformclasspath.jar; 1s disk-cache, processwrapper-sandbox
INFO: Found 1 target...
Target //tools:license.check.license_check up-to-date:
  bazel-bin/tools/license.check.license_check
  bazel-bin/tools/license.check.license_check.jar
INFO: Elapsed time: 88.599s, Critical Path: 3.15s
INFO: 16 processes: 12 internal, 3 processwrapper-sandbox, 1 worker.
INFO: Build completed successfully, 16 total actions
INFO: Running command line: bazel-bin/tools/license.check.license_check tools/formatted.txt <args omitted>
[main] INFO Querying Eclipse Foundation for license data for 42 items.
[main] INFO Found 32 items.
[main] INFO Querying ClearlyDefined for license data for 10 items.

@arkjedrz
arkjedrz force-pushed the arkjedrz_output-json-align branch from 7641148 to dd82292 Compare October 12, 2026 08:05
@arkjedrz
arkjedrz deployed to workflow-approval October 12, 2026 08:05 — with GitHub Actions Active
@arkjedrz
arkjedrz deployed to workflow-approval October 12, 2026 08:05 — with GitHub Actions Active
@arkjedrz
arkjedrz marked this pull request as ready for review October 12, 2026 09:48
@arkjedrz
arkjedrz force-pushed the arkjedrz_output-json-align branch from dd82292 to 1283777 Compare October 12, 2026 10:13
@arkjedrz
arkjedrz deployed to workflow-approval October 12, 2026 10:13 — with GitHub Actions Active
@arkjedrz
arkjedrz deployed to workflow-approval October 12, 2026 10:13 — with GitHub Actions Active
@arkjedrz
arkjedrz force-pushed the arkjedrz_output-json-align branch from 1283777 to b00ca98 Compare October 12, 2026 14:29
@PandaeDo
PandaeDo deployed to workflow-approval October 12, 2026 14:45 — with GitHub Actions Active
@PandaeDo
PandaeDo deployed to workflow-approval October 12, 2026 14:45 — with GitHub Actions Active
@arkjedrz
arkjedrz force-pushed the arkjedrz_output-json-align branch from 34471fd to 266b0ee Compare October 13, 2026 06:30
@arkjedrz
arkjedrz deployed to workflow-approval October 13, 2026 06:31 — with GitHub Actions Active
@arkjedrz
arkjedrz deployed to workflow-approval October 13, 2026 06:31 — with GitHub Actions Active
@arkjedrz
arkjedrz force-pushed the arkjedrz_output-json-align branch from 266b0ee to 8899453 Compare October 13, 2026 07:07
@arkjedrz
arkjedrz deployed to workflow-approval October 13, 2026 07:07 — with GitHub Actions Active
@arkjedrz
arkjedrz deployed to workflow-approval October 13, 2026 07:07 — with GitHub Actions Active
@PandaeDo
PandaeDo deployed to workflow-approval October 13, 2026 11:08 — with GitHub Actions Active
@PandaeDo
PandaeDo deployed to workflow-approval October 13, 2026 11:08 — with GitHub Actions Active
@github-actions

github-actions Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Documentation preview for this pull request is available at:
pr-490: https://eclipse-score.github.io/persistency/pr-490/

PandaeDo
PandaeDo previously approved these changes Oct 13, 2026
View reviewed changes

@PandaeDo PandaeDo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine from my perspective (understanding)

@arkjedrz
arkjedrz deployed to workflow-approval October 19, 2026 09:23 — with GitHub Actions Active
@arkjedrz
arkjedrz force-pushed the arkjedrz_output-json-align branch from 4e669c8 to ca5ea41 Compare October 19, 2026 13:46
@arkjedrz
arkjedrz deployed to workflow-approval October 19, 2026 13:46 — with GitHub Actions Active
@arkjedrz
arkjedrz deployed to workflow-approval October 19, 2026 13:46 — with GitHub Actions Active
@arkjedrz
arkjedrz force-pushed the arkjedrz_output-json-align branch from ca5ea41 to 58d8f14 Compare October 20, 2026 12:29
@arkjedrz
arkjedrz deployed to workflow-approval October 20, 2026 12:29 — with GitHub Actions Active
@arkjedrz
arkjedrz deployed to workflow-approval October 20, 2026 12:29 — with GitHub Actions Active
@arkjedrz
arkjedrz force-pushed the arkjedrz_output-json-align branch from 58d8f14 to 86415ee Compare October 21, 2026 06:06
@arkjedrz
arkjedrz force-pushed the arkjedrz_output-json-align branch from 86415ee to a556ee8 Compare October 21, 2026 06:58
@arkjedrz
arkjedrz deployed to workflow-approval October 21, 2026 06:58 — with GitHub Actions Active
@arkjedrz
arkjedrz deployed to workflow-approval October 21, 2026 06:58 — with GitHub Actions Active
@arkjedrz
arkjedrz force-pushed the arkjedrz_output-json-align branch from a556ee8 to 4184525 Compare October 21, 2026 11:57
@arkjedrz
arkjedrz force-pushed the arkjedrz_output-json-align branch from 4184525 to 0a51d63 Compare October 21, 2026 15:46
@arkjedrz
arkjedrz deployed to workflow-approval October 21, 2026 15:47 — with GitHub Actions Active
@arkjedrz
arkjedrz deployed to workflow-approval October 21, 2026 15:47 — with GitHub Actions Active
@arkjedrz
arkjedrz force-pushed the arkjedrz_output-json-align branch from 0a51d63 to 566e20c Compare October 22, 2026 06:12
@arkjedrz
arkjedrz deployed to workflow-approval October 22, 2026 06:13 — with GitHub Actions Active
@arkjedrz
arkjedrz deployed to workflow-approval October 22, 2026 06:13 — with GitHub Actions Active
@arkjedrz
arkjedrz force-pushed the arkjedrz_output-json-align branch from 566e20c to 14a9cad Compare October 22, 2026 07:26
@arkjedrz
arkjedrz deployed to workflow-approval October 22, 2026 07:26 — with GitHub Actions Active
@arkjedrz
arkjedrz deployed to workflow-approval October 22, 2026 07:26 — with GitHub Actions Active
@arkjedrz
arkjedrz force-pushed the arkjedrz_output-json-align branch from 14a9cad to 5393a50 Compare October 22, 2026 10:13
@arkjedrz arkjedrz mentioned this pull request Oct 23, 2026
Open
@arkjedrz
kvs: align the internal repr between C++ and Rust
172b9e0 
- Disallow non-type-tagged representation.
- Replace `From` with `TryFrom` for Rust type types.
  - Conversion is not infallible.
- Add `KvsValueToAny` object overloads.
  - To avoid unnecessary copy.
- Disallow duplicate keys in `KvsValue::Object`.
- Replace `Kvs::KeyValueMap` with `KvsMap` in `kvs_value.h`.
- Update tests.
karolnowak
karolnowak approved these changes Oct 29, 2026
View reviewed changes

@karolnowak karolnowak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - cpp part

vinodreddy-g
vinodreddy-g approved these changes Oct 29, 2026
View reviewed changes

@vinodreddy-g vinodreddy-g left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

This branch was successfully deployed

1 active deployment
workflow-approval — 172b9e0b Deployed Oct 28, 2026 by arkjedrz via qnx-build (score-qnx-x86_64) / approval #1578
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vinodreddy-g vinodreddy-g vinodreddy-g approved these changes
@PandaeDo PandaeDo PandaeDo left review comments
@antonkri antonkri Awaiting requested review from antonkri antonkri is a code owner
@umaucher umaucher Awaiting requested review from umaucher umaucher is a code owner
@Ahmed-Elsaka-JC Ahmed-Elsaka-JC Awaiting requested review from Ahmed-Elsaka-JC
+1 more reviewer
@karolnowak karolnowak karolnowak approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@arkjedrz arkjedrz

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@arkjedrz @vinodreddy-g @PandaeDo @Ahmed-Elsaka-JC @karolnowak