Merge lp://staging/~michihenning/unity-scopes-api/lxc-registry into lp://staging/unity-scopes-api/devel

Proposed by Michi Henning
Status: Needs review
Proposed branch: lp://staging/~michihenning/unity-scopes-api/lxc-registry
Merge into: lp://staging/unity-scopes-api/devel
Diff against target: 2680 lines (+1005/-118)
83 files modified
CONFIGFILES (+7/-0)
STRUCTS (+2/-0)
debian/VERSION (+1/-1)
demo/click/scope-click/scope-click.ini.in (+1/-0)
demo/scopes/scope-A/scope-A.ini.in (+1/-0)
demo/scopes/scope-B/scope-B.ini.in (+1/-0)
demo/scopes/scope-C/scope-C.ini.in (+1/-0)
demo/scopes/scope-D/scope-D.ini.in (+1/-0)
demo/scopes/scope-N/scope-N.ini.in (+1/-0)
demo/scopes/scope-S/scope-S.ini.in (+1/-0)
doc/tutorial.dox (+7/-0)
include/unity/scopes/ScopeMetadata.h (+12/-0)
include/unity/scopes/internal/DfltConfig.h.in (+3/-0)
include/unity/scopes/internal/MWStateReceiver.h (+1/-1)
include/unity/scopes/internal/RegistryConfig.h (+2/-0)
include/unity/scopes/internal/RegistryObject.h (+18/-5)
include/unity/scopes/internal/ScopeConfig.h (+7/-1)
include/unity/scopes/internal/ScopeMetadataImpl.h (+6/-0)
include/unity/scopes/internal/StateReceiverObject.h (+3/-3)
include/unity/scopes/internal/zmq_middleware/ZmqStateReceiver.h (+1/-1)
include/unity/scopes/testing/ScopeMetadataBuilder.h (+2/-0)
scoperegistry/scoperegistry.cpp (+17/-17)
src/scopes/ScopeMetadata.cpp (+10/-0)
src/scopes/internal/ConfigBase.cpp (+1/-1)
src/scopes/internal/RegistryConfig.cpp (+8/-0)
src/scopes/internal/RegistryObject.cpp (+116/-27)
src/scopes/internal/RuntimeImpl.cpp (+2/-2)
src/scopes/internal/ScopeConfig.cpp (+62/-0)
src/scopes/internal/ScopeMetadataImpl.cpp (+56/-0)
src/scopes/internal/StateReceiverObject.cpp (+3/-3)
src/scopes/internal/zmq_middleware/StateReceiverI.cpp (+2/-1)
src/scopes/internal/zmq_middleware/ZmqStateReceiver.cpp (+2/-1)
src/scopes/internal/zmq_middleware/capnproto/StateReceiver.capnp (+1/-0)
src/scopes/testing/ScopeMetadataBuilder.cpp (+18/-0)
test/gtest/scopes/Activation/Activation_test.cpp (+4/-1)
test/gtest/scopes/Aggregation/AggTestScope.ini.in (+3/-0)
test/gtest/scopes/Aggregation/ChildScopes_test.cpp (+1/-1)
test/gtest/scopes/Filters/Filters_test.cpp (+4/-1)
test/gtest/scopes/IdleShutdown/IdleShutdown_test.cpp (+4/-1)
test/gtest/scopes/Invocation/Invocation_test.cpp (+4/-1)
test/gtest/scopes/Registry/RegistryTest.ini.in (+1/-0)
test/gtest/scopes/Registry/Registry_test.cpp (+85/-20)
test/gtest/scopes/Registry/Runtime.ini.in (+2/-0)
test/gtest/scopes/Registry/lxc-attach (+22/-0)
test/gtest/scopes/Registry/other_scopes/testscopeC/testscopeC.ini.in (+1/-0)
test/gtest/scopes/Registry/other_scopes/testscopeD/testscopeD.ini.in (+1/-0)
test/gtest/scopes/Registry/scopes/CMakeLists.txt (+1/-0)
test/gtest/scopes/Registry/scopes/lxc_return_config/CMakeLists.txt (+3/-0)
test/gtest/scopes/Registry/scopes/lxc_return_config/returnConfigScope-settings.ini.in (+4/-0)
test/gtest/scopes/Registry/scopes/lxc_return_config/returnConfigScope.cpp (+134/-0)
test/gtest/scopes/Registry/scopes/lxc_return_config/returnConfigScope.ini.in (+25/-0)
test/gtest/scopes/Registry/scopes/testscopeA/testscopeA.ini.in (+1/-0)
test/gtest/scopes/Registry/scopes/testscopeB/testscopeB.ini.in (+1/-0)
test/gtest/scopes/ReplyReaper/ReplyReaper_test.cpp (+4/-1)
test/gtest/scopes/ResultCache/CacheScope.ini.in (+1/-0)
test/gtest/scopes/Runtime/Runtime_test.cpp (+4/-1)
test/gtest/scopes/StripLocation/scopes/Aggregator/Aggregator.ini.in (+1/-0)
test/gtest/scopes/StripLocation/scopes/Leaf/Leaf.ini.in (+1/-0)
test/gtest/scopes/ThrowingClient/EchoScope.ini.in (+1/-0)
test/gtest/scopes/ThrowingScope/ThrowingScope.ini.in (+1/-0)
test/gtest/scopes/internal/ConfigBase/ConfigBase_test.cpp (+29/-0)
test/gtest/scopes/internal/ConfigBase/Test.ini.in (+3/-0)
test/gtest/scopes/internal/RegistryConfig/BadTimeout.ini.in (+5/-0)
test/gtest/scopes/internal/RegistryConfig/CMakeLists.txt (+1/-0)
test/gtest/scopes/internal/RegistryConfig/Registry.ini.in (+2/-1)
test/gtest/scopes/internal/RegistryConfig/RegistryConfig_test.cpp (+21/-0)
test/gtest/scopes/internal/RegistryObject/RegistryObject_test.cpp (+6/-2)
test/gtest/scopes/internal/ScopeConfig/CMakeLists.txt (+10/-0)
test/gtest/scopes/internal/ScopeConfig/ScopeConfig_test.cpp (+78/-0)
test/gtest/scopes/internal/ScopeConfig/complete_config.ini.in (+1/-0)
test/gtest/scopes/internal/ScopeConfig/framework_bad_major_version.ini.in (+6/-0)
test/gtest/scopes/internal/ScopeConfig/framework_bad_minor_version.ini.in (+6/-0)
test/gtest/scopes/internal/ScopeConfig/framework_no_parse.ini.in (+6/-0)
test/gtest/scopes/internal/ScopeConfig/framework_not_a_number.ini.in (+6/-0)
test/gtest/scopes/internal/ScopeConfig/framework_trailing_chars.ini.in (+6/-0)
test/gtest/scopes/internal/ScopeMetadataImpl/ScopeMetadataImpl_test.cpp (+103/-17)
test/gtest/scopes/internal/zmq_middleware/RegistryI/RegistryI_test.cpp (+14/-6)
test/gtest/scopes/stress/CMakeLists.txt (+1/-0)
test/gtest/scopes/stress/scopes-stress.cpp (+2/-0)
test/gtest/scopes/stress/scopes/scope1/scope1.ini.in (+1/-0)
test/gtest/scopes/stress/scopes/scope2/scope2.ini.in (+1/-0)
test/gtest/scopes/stress/scopes/scope3/scope3.ini.in (+1/-0)
test/gtest/scopes/testing/IsolatedScope/IsolatedScope_test.cpp (+5/-1)
To merge this branch: bzr merge lp://staging/~michihenning/unity-scopes-api/lxc-registry
Reviewer Review Type Date Requested Status
unity-api-1-bot continuous-integration Needs Fixing
Marcus Tomlinson (community) Needs Fixing
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+277410@code.staging.launchpad.net

Commit message

Modified registry to support running scopes compiled with the 4.9 ABI in an lxc container. New scopes using the 5.2 ABI must add a Framework key to their ini file, such as "Framework = 15.04". The code uses the major number to infer the ABI: anything >= 15.10 implies the new ABI.

Description of the change

Modified registry to support running scopes compiled with the 4.9 ABI in an lxc container. New scopes using the 5.2 ABI must add a Framework key to their ini file, such as "Framework = 15.04". The code uses the major number to infer the ABI: anything >= 15.10 implies the new ABI.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

Still looking at this, but 2 small observations so far:

* Could you add a "Framework" key to our scope.ini description under the "Deployment" section of tutorial.dox.

* Perhaps it'd be worth throwing in one or two "meta.framework_major()" checks into Registry_test.cpp as well.

review: Needs Fixing
Revision history for this message
Michi Henning (michihenning) wrote :

My apologies, I had updating the doc on my todo list, but forgot.

Also, I think the code needs to change to treat anything with 15.x as an old ABI scope. (The telegram scope uses the 15.04 framework at the moment, for example.)

Will add an extra test into the registry test, yes!

Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

2 more similar observations:

* Could you add "framework_major" to STRUCTS under the "ScopeMetadata" section,

* and, seeing that we've kept ScopeMetadataBuilder up-to-date thus far, could you add "framework_major" there too.

review: Needs Fixing
Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

> My apologies, I had updating the doc on my todo list, but forgot.
>
> Also, I think the code needs to change to treat anything with 15.x as an old
> ABI scope. (The telegram scope uses the 15.04 framework at the moment, for
> example.)
>
> Will add an extra test into the registry test, yes!

Hmmm, I suspect using only the major version of the framework is not going to cut it. The break occurred between 15.04 (vivid) and 15.10 (wily) no?

Revision history for this message
Michi Henning (michihenning) wrote :

> Hmmm, I suspect using only the major version of the framework is not going to
> cut it. The break occurred between 15.04 (vivid) and 15.10 (wily) no?

Yes, will fix.

Revision history for this message
Michi Henning (michihenning) wrote :

Updated code to provide access to both major and minor framework version. Anything < 15.10 means "old ABI"; anything >= 15.10 means "new ABI". Updated tutorial and STRUCTS doc.

One concern at the moment is that the variable names and config keys imply that an LXC container will be used when, in fact, we might end up with a chroot. I'll go through and fix the variable names once we have settled the issue. I don't think it should hold us up now though.

657. By Michi Henning

Updated code to provide access to both major and minor framework version. Anything < 15.10 means "old ABI"; anything >= 15.10 means "new ABI". Updated tutorial and STRUCTS doc.

658. By Michi Henning

Fixed three more config files.

659. By Michi Henning

Fixed loose checks when retrieving env vars and set desktop file dir for stress test.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

> Updated code to provide access to both major and minor framework version.
> Anything < 15.10 means "old ABI"; anything >= 15.10 means "new ABI". Updated
> tutorial and STRUCTS doc.
>
> One concern at the moment is that the variable names and config keys imply
> that an LXC container will be used when, in fact, we might end up with a
> chroot. I'll go through and fix the variable names once we have settled the
> issue. I don't think it should hold us up now though.

Considering that a refactoring of that nature (replacing "lxc" everywhere) breaks our public interface and documentation, I think it is worth holding up the branch until that change is made.

review: Needs Fixing
Revision history for this message
Michi Henning (michihenning) wrote :

I think the only place the lxc leaks out is in the LXC.ExecCommand key, which is not documented. But, yes, inappropriately named variables are no good regardless.

I agree that we shouldn't merge this mainly because we need a management decision to go ahead with the container approach. Otherwise, we might just end up with a bunch of useless code in the tree.

Once we have a decision, I can do the renaming as appropriate. For now, I don't want to sink any more time into it. I think the branch will survive for a few weeks. I'll re-merge devel every now and then to keep it alive until we know where we are headed.

Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

> I think the only place the lxc leaks out is in the LXC.ExecCommand key, which
> is not documented. But, yes, inappropriately named variables are no good
> regardless.
>
> I agree that we shouldn't merge this mainly because we need a management
> decision to go ahead with the container approach. Otherwise, we might just end
> up with a bunch of useless code in the tree.
>
> Once we have a decision, I can do the renaming as appropriate. For now, I
> don't want to sink any more time into it. I think the branch will survive for
> a few weeks. I'll re-merge devel every now and then to keep it alive until we
> know where we are headed.

Sorry, you're absolutely right, it doesn't leak as much as I thought. Yeah I saw your mail, lets wait a bit longer for a final decision to be made.

One small fix to keep in mind is to update the commit message and description of this MP. It still states that we're using major version such that FW>14 is new ABI etc.

Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :
review: Needs Fixing (continuous-integration)

Unmerged revisions

659. By Michi Henning

Fixed loose checks when retrieving env vars and set desktop file dir for stress test.

658. By Michi Henning

Fixed three more config files.

657. By Michi Henning

Updated code to provide access to both major and minor framework version. Anything < 15.10 means "old ABI"; anything >= 15.10 means "new ABI". Updated tutorial and STRUCTS doc.

656. By Michi Henning

Fixed race on shutdown.

655. By Michi Henning

Fixed bug in test.

654. By Michi Henning

Added lxc exec test.

653. By Michi Henning

Refactored, better diagnostics.

652. By Michi Henning

Fixed stale doc.

651. By Michi Henning

Added exec/kill functionality for lxc.

650. By Michi Henning

Made lxc_exec_command available to RegistryObject.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: