Ticket #7713 (new defect)

Opened 6 years ago

Last modified 6 years ago

Inconsitent behavior for activity installation

Reported by: erikos Owned by: tomeu
Priority: normal Milestone: 9.1.0-cancelled
Component: sugar Version: Development source as of this date
Keywords: 8.2.0:- relnote Cc: Eben, tomeu, gregorio, erikos, homunq, cscott, mstone
Action Needed: design Verified: no
Deployments affected: Blocked By:
Blocking:

Description

Steps to reproduce:

Download an activity that is already installed (e.g. Log9 is installed and you download Log10).

a) When the download has finished go to the activity list and check if the bundle is upgraded (it will still be the old version)

b) go to the journal and resume the bundle entry that has been created in the journal, when you check now in the activity list the bundle will be upgraded

a) we only install a bundle that is not installed already: http://dev.laptop.org/git?p=journal-activity;a=blob;f=journalactivity.py;h=f531e0fe860a46a4483d41bbb26a586794f888ad;hb=HEAD#l264

b) we upgrade a bundle when it is already installed: http://dev.laptop.org/git?p=sugar-toolkit;a=blob;f=src/sugar/datastore/datastore.py;h=b88a8775a962eb4b06fb740bc7616c2c711f9f24;hb=HEAD#l155

Since this seems inconsistent I would like to ask for clarification what path we should go.

Change History

in reply to: ↑ description   Changed 6 years ago by Eben

  • cc homunq added
  • keywords 8.2.0:? added
  • next_action changed from diagnose to communicate

Replying to erikos:

Steps to reproduce: Download an activity that is already installed (e.g. Log9 is installed and you download Log10). a) When the download has finished go to the activity list and check if the bundle is upgraded (it will still be the old version)

Bad!

a) we only install a bundle that is not installed already: http://dev.laptop.org/git?p=journal-activity;a=blob;f=journalactivity.py;h=f531e0fe860a46a4483d41bbb26a586794f888ad;hb=HEAD#l264 b) we upgrade a bundle when it is already installed: http://dev.laptop.org/git?p=sugar-toolkit;a=blob;f=src/sugar/datastore/datastore.py;h=b88a8775a962eb4b06fb740bc7616c2c711f9f24;hb=HEAD#l155

I'm not sure I agree with the implied definitions of "upgrade" and "install" in these cases (though I agree you use them appropriately with regard to the current, inconsistent behavior). Clearly case (a) should also be considered an "upgrade" of an existing activity, and we should simply install any bundle of any version number (including the same version). I see no reason to attempt to optimize this, even when the version numbers match.

I think this actually relates to a recent patch from homunq. Was that the same issue?

It seems that, especially in the face of potential incompatibilities between old activities and the new OS, we should make sure that these upgrade paths all work smoothly. Furthermore, it seems that simply removing the condition which checks to see if the activity is already installed should be an easy fix.

  Changed 6 years ago by mstone

  • next_action changed from communicate to design

What plan is proposed to fix this?

  Changed 6 years ago by marco

Without deep knowledge of that part of the code, it seem to me that the journal should be upgrading.

follow-up: ↓ 5   Changed 6 years ago by cscott

  • cc cscott, mstone added

Having looked at the code, it doesn't seem like we actually support having multiple versions of the same activity installed yet. We would need to tweak the directory into which the .xo unpacks as well as add 'version' parameters to a number of API methods which currently only take bundle_ids.

So it seems like 'upgrade' is the right solution for now. Hopefully in 9.1 we can fix the API issues and actually support multiple versions concurrently.

But I'm not entirely clear what question erikos is asking. There are some security issues involving bundle installation that require an affirmative action by the user in the journal, but michael assures me that installing an activity does involve executing any of its code, and so it should be safe just to install/upgrade the activity as soon as it is placed in the Journal. (In the 9.1 time frame I hope that we will be able to execute activities directly from their representation in the datastore, avoiding this 'installation' step.)

BUT for library bundles, installing does involve executing code, so it's not necessarily safe to install them without explicit confirmation from the user. (This is a bug; installing libraries should not require executing code.)

in reply to: ↑ 4 ; follow-up: ↓ 7   Changed 6 years ago by Eben

Replying to cscott:

Having looked at the code, it doesn't seem like we actually support having multiple versions of the same activity installed yet. We would need to tweak the directory into which the .xo unpacks as well as add 'version' parameters to a number of API methods which currently only take bundle_ids.

That's right.

So it seems like 'upgrade' is the right solution for now. Hopefully in 9.1 we can fix the API issues and actually support multiple versions concurrently.

Definitely. We've recently been discussing that, upon activity upgrade, the bundle should also be starred according to the previously installed version.

But I'm not entirely clear what question erikos is asking. There are some security issues involving bundle installation that require an affirmative action by the user in the journal, but michael assures me that installing an activity does involve executing any of its code, and so it should be safe just to install/upgrade the activity as soon as it is placed in the Journal. (In the 9.1 time frame I hope that we will be able to execute activities directly from their representation in the datastore, avoiding this 'installation' step.)

This is not true now? The whole point of exposing the bundle directly in the Journal is to allow one to open a "clean instance" of the activity. What happens when you click on a bundle now, if it doesn't launch the activity?

BUT for library bundles, installing does involve executing code, so it's not necessarily safe to install them without explicit confirmation from the user. (This is a bug; installing libraries should not require executing code.)

Yeah, good point. I wasn't aware of this.

  Changed 6 years ago by marco

  • owner changed from marco to tomeu

in reply to: ↑ 5   Changed 6 years ago by cscott

Replying to Eben:

This is not true now? The whole point of exposing the bundle directly in the Journal is to allow one to open a "clean instance" of the activity. What happens when you click on a bundle now, if it doesn't launch the activity?

I was referring to the fact that currently the datastore maintains the .xo file (in packed form), while execution happens from ~/Activities/foo.activity (in unpacked form). We have to do an installation/upgrade step in the datastore before we can "resume" the .xo entry in the datastore.

I hope that we can run the activity directly from the .xo bundle (or, conversely, maintain the activity in unpacked form in the datastore) in the 9.1 timeframe. This drastically reduces the space used when you try to install WikiBrowse, for example.

  Changed 6 years ago by kimquirk

  • keywords 8.2.0:- relnote added; 8.2.0:? removed
  • milestone changed from 8.2.0 (was Update.2) to 9.1.0

Not a blocker for 8.2. release note.

Note: See TracTickets for help on using tickets.