sugar-launch script fails to find installed bundles

[garycmartin]

This script used to be quite useful in the testing/development cycle of Activities, but has been broken for a while now:

sugar-launch Moon
/use/lib/python2.6/site-packages/sugar/util.py:25: DeprecationWarning: the sha module is
deprecated; use the hashlib module instead
    import sha
Cannot find Moon bundle.

Pretty sure the deprecation warning has nothing to do with not finding the bundle, sugar-launch seems to not have worked for a while without the sha warning, at least as far as I remember.

[garycmartin]

FWIW: This script is failing under both sugar-jhbuild and SoaS environments.

[tomeu]

I need to pass it the full bundle id, passing part of it used to work?

[garycmartin]

Replying to tomeu:

I need to pass it the full bundle id, passing part of it used to work?

Yes, using the full bundle id works in 0.85.x, but it used to work with less – just tested back on 0.82 and using "Moon" was enough for it to find the activity. In 0.84 and 0.85.x you need the full id. Just a developer thing I guess, unless you think this might have other side effects.

[quozl]

The ability to launch using only part of an activity name was removed in commit b36db6599ed59412ce77c8ed4fb1dea782877a62.

Related ticket  dev.laptop.org #9189.

[quozl]

proposed patch, change to API, adding a feature without impact to existing uses

[quozl]

|testcase| is in patch.
patch is to HEAD and is tested on 0.84.

[alsroot]

since patch changes get_bundle() behaviour and get_bundle() is being used in many other places, I think just new function could be added and used in sugar-launch command

[alsroot]

Replying to alsroot:

since patch changes get_bundle() behaviour and get_bundle() is being used in many other places, I think just new function could be added and used in sugar-launch command

oops, missed that any BundleRegistry()'s user can iterate it, so patch's code could be just used directly in sugar-launch

[quozl]

Any instantiation of BundleRegistry() causes filesystem I/O and the setup of a change notifier for the Activities directory. The cost is 905ms tested on OLPC XO-1.5. Use of the D-Bus API ensures that no additional filesystem I/O is required, since the shell is queried directly. Those other uses of BundleRegistry() presumably suffer from the same delay. I question the value of just using the patch code directly in sugar-launch.

[alsroot]

Replying to quozl:

Any instantiation of BundleRegistry() causes filesystem I/O and the setup of a change notifier for the Activities directory. The cost is 905ms tested on OLPC XO-1.5.

yeah, makes sense
anyway changing get_bundle() behaviour would be wrong idea I think,
so it could be just new public method something like find_bundle()

Use of the D-Bus API ensures that no additional filesystem I/O is required, since the shell is queried directly. Those other uses of BundleRegistry() presumably suffer from the same delay. I question the value of just using the patch code directly in sugar-launch.

[quozl]

Replying to alsroot:

anyway changing get_bundle() behaviour would be wrong idea I think,
so it could be just new public method something like find_bundle()

Have you a patch for that? My change to get_bundle() is only where it would otherwise return None; it is an extension of the semantics. I've looked through uses of get_bundle() and none of them make a futile call in order to obtain a None response.

[alsroot]

Since we are in hard freeze for 0.88, any fix can be applied only to 0.90.

Clone for this ticket  http://git.sugarlabs.org/projects/sugar/repos/SL897 which could be used for 0.84 as well.

[alsroot]

Replying to alsroot:

Since we are in hard freeze for 0.88, any fix can be applied only to 0.90.

Clone for this ticket  http://git.sugarlabs.org/projects/sugar/repos/SL897 which could be used for 0.84 as well.

Code adds new shell FindBundlePath method which returns list of bundles by query dict (currently supported only 'name' and 'bundle_id' keys) and returns list of dicts per matched bundles with at lest 'name', 'bundle_id', 'path', 'activity_version' and 'installation_time'.

Script activity-launch preserves behaviour then first command argument is a substring to math (name or bundle_id) but adds --name and --bundle_id for particular search. Also, if there weren't exact matches, script outputs all possible variants to let user copy&paste right activity name/bundle_id.

[alsroot]

Comment(by quozl):

Has this been tested on 0.84? The patch I proposed was tested on > 0.84. I
need a patch for 0.84 at the moment, 0.88 and 0.90 are irrelevant > to me.

1st patch changes get_bundle() behaviour, fixing this particular bug it breaks other code that use get_bundle(). cloned repo just adds new code and doesn't affect existed one. It tested on 0.84 as well and could be applied as is.

[quozl]

Replying to alsroot:

1st patch changes get_bundle() behaviour, fixing this particular bug it breaks other code that use get_bundle().

Which other code is broken?

[quozl]

NAK, adds too much untested code, and no evidence of other code broken by former patch.

[tomeu]

I tend to agree with James here. The methods that change behavior are private to the shell and we can easily grep its sources to check for usage instances.

Aleksey, it's better to submit first a patch that addresses the bug, and in a separate patch do refactorings and other improvements. That way the fix can be backported and eventually reverted more easily.

James, please push.

[quozl]

Replying to tomeu:

James, please push.

EACCES. I do not have commit access. This patch still applies though line numbers have changed. To make it even easier for a maintainer to apply this patch, I've rebased the patch against sucrose-0.84 branch, and against HEAD. I've also retested the 0.84 patch on current build of XO-1.5 software.

[tomeu]

Pushed to master.