Restructure storage kvstore directory #13307

rajkan01 · 2020-07-17T19:10:52Z

Summary of changes

Preceding PR: #13300

Restructured storage/kvstore as per the new directory structure proposal:

storage
├── blockdevice
├── filesystem
├── kvstore
│   ├── include
│   │   └── kvstore // All headers previously in filesystemstore, global_api, kv_map, tdbstore
│   │       ├── kvstore_global_api.h
│   │       ├── KVStore.h
│   │       └── [...]
│   ├── source // All sources previously in filesystemstore, global_api, kv_map, tdbstore
│   │   ├── kvstore_global_api.cpp
│   │   └── [...]
│   ├── tests
│   ├── kv_config   // Renamed from "conf". In future, consider removing; application should provide the storage configuration
│   │   ├── mbed_lib.json
│   │   ├── include
│   │   │   └── kv_config
│   │   │       └── kv_config.h
│   │   ├── source
│   │   │   └── kv_config.cpp
│   │   ├── filesystem
│   │   │   └── mbed_lib.json
│   │   ├── filesystem_no_rbp
│   │   │   └── mbed_lib.json
│   │   └── [...]
│   ├── direct_access_devicekey // Used by bootloader so kept in a separate folder
│   │   ├── mbed_lib.json
│   │   ├── include
│   │   │   └── direct_access_devicekey
│   │   └── source
│   └── securestore
│       ├── mbed_lib.json
│       ├── include
│       │   └── securestore
│       └── source

Impact of changes

None.

Migration actions required

None.

Documentation

To Be Updated.

Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[x] Tests / results supplied as part of this PR

Manual testing: (Build for K64F target with GCC_ARM toolchain)

mbed-os-example-blockdevice
mbed-os-example-filesystem
mbed-os-example-kvstore
Greentea tests: Full and Bare metal profiles
Unit tests build and run

Reviewers

@0xc0170 @ARMmbed/mbed-os-core

mergify · 2020-07-17T19:11:28Z

This PR cannot be merged due to conflicts. Please rebase to resolve them.

ciarmcom · 2020-07-17T21:00:32Z

@rajkan01, thank you for your changes.
@0xc0170 @ARMmbed/mbed-os-storage @ARMmbed/mbed-os-maintainers please review.

LDong-Arm

LGTM

...ct_access_devicekey/tests/TESTS/direct_access_devicekey/direct_access_devicekey_tdb/main.cpp

rajkan01 · 2020-07-20T15:55:23Z

@0xc0170 Could you please start CI...

0xc0170 · 2020-07-21T08:57:19Z

CI started

Pull request has been modified.

0xc0170 · 2020-07-22T08:43:38Z

CI restarted

mbed-ci · 2020-07-22T09:53:47Z

Test run: FAILED

Summary: 1 of 6 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_dynamic-memory-usage

0xc0170 · 2020-07-22T10:02:31Z

@rajkan01 Can you review failures in dynamic memory?


[2020-07-22T09:39:33.931Z] [Fatal Error] DeviceKey.cpp@22,10: kvstore/KVStore.h: No such file or directory

[2020-07-22T09:39:33.931Z] [ERROR] ./mbed-os/features/device_key/source/DeviceKey.cpp:22:10: fatal error: kvstore/KVStore.h: No such file or directory

[2020-07-22T09:39:33.931Z]    22 | #include "kvstore/KVStore.h"

They look related to the changes.

0xc0170 · 2020-07-22T10:14:26Z

@0xc0170 Could you please start CI...

@rajkan01 Once you rebase or update PR to address the reviewers comment, please state what has changed. It helps everyone to get the state without going through all reviewers comments and commits to find out what has changed and if all was addressed.

rajkan01 · 2020-07-22T10:56:08Z

@rajkan01 Can you review failures in dynamic memory?

[2020-07-22T09:39:33.931Z] [Fatal Error] DeviceKey.cpp@22,10: kvstore/KVStore.h: No such file or directory

[2020-07-22T09:39:33.931Z] [ERROR] ./mbed-os/features/device_key/source/DeviceKey.cpp:22:10: fatal error: kvstore/KVStore.h: No such file or directory

[2020-07-22T09:39:33.931Z] 22 | #include "kvstore/KVStore.h"

They look related to the changes.

@0xc0170 As part of new storage/kvstore directory restructure, we added mbed_lib.json at kvstore root directory with config "kvstore" to prevent build tools includes kvstore source by default.
This causes a failure in mbed-bootloader CI dynamic-memory-usage builds, so updated "kvstore" config in mbed_app.json to include kvstore library in mbed-bootloader.
PR: PelionIoT/mbed-bootloader#80

evedon · 2020-07-22T15:50:46Z

@rajkan01 Can you review failures in dynamic memory?
[2020-07-22T09:39:33.931Z] [Fatal Error] DeviceKey.cpp@22,10: kvstore/KVStore.h: No such file or directory
[2020-07-22T09:39:33.931Z] [ERROR] ./mbed-os/features/device_key/source/DeviceKey.cpp:22:10: fatal error: kvstore/KVStore.h: No such file or directory
[2020-07-22T09:39:33.931Z] 22 | #include "kvstore/KVStore.h"
They look related to the changes.

@0xc0170 As part of new storage/kvstore directory restructure, we added mbed_lib.json at kvstore root directory with config "kvstore" to prevent build tools includes kvstore source by default.
This causes a failure in mbed-bootloader CI dynamic-memory-usage builds, so updated "kvstore" config in mbed_app.json to include kvstore library in mbed-bootloader.
PR: ARMmbed/mbed-bootloader#80

To prevent breaking changes to applications, instead of introducing a new library name like kvstore, we could re-use one of the existing storage library that we removed, for example kv-global-api. Of course kvstore is a more appropriate name but we could change that at a later stage.

rajkan01 · 2020-07-22T17:15:29Z

@rajkan01 Can you review failures in dynamic memory?
[2020-07-22T09:39:33.931Z] [Fatal Error] DeviceKey.cpp@22,10: kvstore/KVStore.h: No such file or directory
[2020-07-22T09:39:33.931Z] [ERROR] ./mbed-os/features/device_key/source/DeviceKey.cpp:22:10: fatal error: kvstore/KVStore.h: No such file or directory
[2020-07-22T09:39:33.931Z] 22 | #include "kvstore/KVStore.h"
They look related to the changes.

@0xc0170 As part of new storage/kvstore directory restructure, we added mbed_lib.json at kvstore root directory with config "kvstore" to prevent build tools includes kvstore source by default.
This causes a failure in mbed-bootloader CI dynamic-memory-usage builds, so updated "kvstore" config in mbed_app.json to include kvstore library in mbed-bootloader.
PR: PelionIoT/mbed-bootloader#80

To prevent breaking changes to applications, instead of introducing a new library name like kvstore, we could re-use one of the existing storage library that we removed, for example kv-global-api. Of course kvstore is a more appropriate name but we could change that at a later stage.

@evedon Yes I will re-use kv-global-api to unblock this PR to get merged to master at first level.

rajkan01 · 2020-07-23T08:49:07Z

@0xc0170 mbed-bootloader issue resolved by re-using existing library name kv-global-api config. Could you restart the CI

0xc0170 · 2020-07-23T10:32:08Z

CI restarted

mbed-ci · 2020-07-23T11:41:42Z

Test run: SUCCESS

Summary: 6 of 6 test jobs passed
Build number : 3
Build artifacts

rajkan01 · 2020-07-23T13:16:45Z

@0xc0170 CI passed, Could you review and merge

mergify bot added the needs: work label Jul 17, 2020

rajkan01 force-pushed the storage_kv_dir_resturcture branch 3 times, most recently from 2b302ee to 676633d Compare July 17, 2020 19:58

rajkan01 changed the title ~~Storage kv dir resturcture~~ Restructure storage kvstore directory Jul 17, 2020

ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Jul 17, 2020

ciarmcom requested review from 0xc0170 and a team July 17, 2020 21:00

ciarmcom added the needs: review label Jul 17, 2020

rajkan01 force-pushed the storage_kv_dir_resturcture branch from 676633d to 6dfdd82 Compare July 18, 2020 00:43

rajkan01 mentioned this pull request Jul 20, 2020

Storage system_storage directory restructure #13314

Merged

LDong-Arm approved these changes Jul 20, 2020

View reviewed changes

evedon reviewed Jul 20, 2020

View reviewed changes

...ct_access_devicekey/tests/TESTS/direct_access_devicekey/direct_access_devicekey_tdb/main.cpp Show resolved Hide resolved

rajkan01 added 3 commits July 20, 2020 07:28

Refactore storage/kvstore directory

d05d347

Refactor sub-directories - securestore, direct_access_devicekey, conf

72b4a14

move greentea and UNITTESTS closure to library

bb14b9f

rajkan01 force-pushed the storage_kv_dir_resturcture branch from 6dfdd82 to 8fc628e Compare July 20, 2020 14:32

Update greentea config, header reference

d956116

rajkan01 force-pushed the storage_kv_dir_resturcture branch from 8fc628e to d956116 Compare July 20, 2020 14:34

rajkan01 requested a review from evedon July 20, 2020 14:42

evedon previously approved these changes Jul 20, 2020

View reviewed changes

0xc0170 added needs: CI and removed needs: review needs: work labels Jul 21, 2020

mergify bot added the needs: work label Jul 21, 2020

rajkan01 mentioned this pull request Jul 22, 2020

Update "kvstore" config in the mbed_app.json PelionIoT/mbed-bootloader#80

Closed

0xc0170 added the needs: preceding PR label Jul 22, 2020

Fix CI build issue with GCC_ARM and ARMC6 toolchain

ca84e11

rajkan01 force-pushed the storage_kv_dir_resturcture branch from 25cc6f8 to ca84e11 Compare July 22, 2020 17:23

0xc0170 added needs: CI and removed needs: preceding PR needs: work labels Jul 23, 2020

evedon approved these changes Jul 23, 2020

View reviewed changes

mergify bot added ready for merge and removed needs: CI labels Jul 23, 2020

0xc0170 approved these changes Jul 23, 2020

View reviewed changes

0xc0170 merged commit 1d9c13e into ARMmbed:master Jul 23, 2020

mergify bot removed the ready for merge label Jul 23, 2020

mbedmain added release-version: 6.2.1 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Aug 16, 2020

LDong-Arm mentioned this pull request Nov 13, 2020

Restructure KVStore to one library per store type #13908

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restructure storage kvstore directory #13307

Restructure storage kvstore directory #13307

rajkan01 commented Jul 17, 2020 •

edited

Loading

mergify bot commented Jul 17, 2020

ciarmcom commented Jul 17, 2020

LDong-Arm left a comment

rajkan01 commented Jul 20, 2020

0xc0170 commented Jul 21, 2020

0xc0170 commented Jul 22, 2020

mbed-ci commented Jul 22, 2020

0xc0170 commented Jul 22, 2020

0xc0170 commented Jul 22, 2020

rajkan01 commented Jul 22, 2020 •

edited

Loading

evedon commented Jul 22, 2020

rajkan01 commented Jul 22, 2020 •

edited

Loading

rajkan01 commented Jul 23, 2020 •

edited

Loading

0xc0170 commented Jul 23, 2020

mbed-ci commented Jul 23, 2020

rajkan01 commented Jul 23, 2020

Restructure storage kvstore directory #13307

Restructure storage kvstore directory #13307

Conversation

rajkan01 commented Jul 17, 2020 • edited Loading

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

mergify bot commented Jul 17, 2020

ciarmcom commented Jul 17, 2020

LDong-Arm left a comment

Choose a reason for hiding this comment

rajkan01 commented Jul 20, 2020

0xc0170 commented Jul 21, 2020

0xc0170 commented Jul 22, 2020

mbed-ci commented Jul 22, 2020

Test run: FAILED

0xc0170 commented Jul 22, 2020

0xc0170 commented Jul 22, 2020

rajkan01 commented Jul 22, 2020 • edited Loading

evedon commented Jul 22, 2020

rajkan01 commented Jul 22, 2020 • edited Loading

rajkan01 commented Jul 23, 2020 • edited Loading

0xc0170 commented Jul 23, 2020

mbed-ci commented Jul 23, 2020

Test run: SUCCESS

rajkan01 commented Jul 23, 2020

rajkan01 commented Jul 17, 2020 •

edited

Loading

rajkan01 commented Jul 22, 2020 •

edited

Loading

rajkan01 commented Jul 22, 2020 •

edited

Loading

rajkan01 commented Jul 23, 2020 •

edited

Loading