Opened 12 years ago

Closed 10 years ago

Last modified 7 years ago

#897 closed defect (fixed)

sugar-launch script fails to find installed bundles

Reported by: garycmartin Owned by: alsroot
Priority: High Milestone:
Component: Sugar Version: Git as of bugdate
Severity: Trivial Keywords: r+
Cc: quozl, sayamindu Distribution/OS: Unspecified
Bug Status: New

Description

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.

Attachments (3)

0002-restore-sugar-launch-by-bundle-id-substring-fixes-89.patch (2.4 KB) - added by quozl 11 years ago.
proposed patch, change to API, adding a feature without impact to existing uses
0.84-0001-restore-sugar-launch-by-bundle-id-substring-fixes-89.patch (2.3 KB) - added by quozl 10 years ago.
0.88-0001-restore-sugar-launch-by-bundle-id-substring-fixes-89.patch (2.3 KB) - added by quozl 10 years ago.

Download all attachments as: .zip

Change History (25)

comment:1 Changed 12 years ago by garycmartin

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

comment:2 follow-up: Changed 11 years ago by tomeu

  • Bug Status changed from Unconfirmed to Needinfo
  • Milestone changed from Unspecified by Release Team to 0.86
  • Priority changed from Unspecified by Maintainer to High

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

comment:3 in reply to: ↑ 2 Changed 11 years ago by garycmartin

  • Bug Status changed from Needinfo to New
  • Severity changed from Unspecified to Trivial

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.

comment:4 Changed 11 years ago by quozl

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

Related ticket dev.laptop.org #9189.

Changed 11 years ago by quozl

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

comment:5 Changed 11 years ago by quozl

  • Keywords r? added

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

comment:6 Changed 11 years ago by quozl

  • Cc quozl added

comment:7 follow-up: Changed 11 years ago by 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

comment:8 in reply to: ↑ 7 Changed 11 years ago by 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

comment:9 follow-up: Changed 11 years ago by 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.

comment:10 in reply to: ↑ 9 ; follow-up: Changed 11 years ago by 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.

comment:11 in reply to: ↑ 10 Changed 11 years ago by 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.

comment:12 Changed 11 years ago by alsroot

  • Owner changed from tomeu to alsroot
  • Status changed from new to assigned

comment:13 follow-up: Changed 11 years ago by alsroot

  • Milestone changed from 0.86 to 0.90

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.

comment:14 in reply to: ↑ 13 Changed 11 years ago by 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.

comment:15 Changed 11 years ago by sayamindu

  • Cc sayamindu added

comment:16 follow-up: Changed 11 years ago by 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.

comment:17 in reply to: ↑ 16 Changed 11 years ago by 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?

comment:18 Changed 11 years ago by quozl

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

comment:19 follow-up: Changed 10 years ago by tomeu

  • Keywords r+ added; r? removed

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.

comment:20 in reply to: ↑ 19 Changed 10 years ago by 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.

comment:21 Changed 10 years ago by tomeu

  • Resolution set to fixed
  • Status changed from assigned to closed

Pushed to master.

comment:22 Changed 7 years ago by dnarvaez

  • Milestone 0.90 deleted

Milestone 0.90 deleted

Note: See TracTickets for help on using tickets.