Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search for alternative copies after deleting a bundle, fixes #4836 #488

Closed
wants to merge 1 commit into from

Conversation

samdroid-apps
Copy link
Contributor

Replaces #487

If the user removes a $HOME activity, sugar should fall back to
displaying a /usr/share copy of the same activity. This patch
searches through the system paths after removing an activity in
order to add it back if needed.

Steps to reproduce can be found in the ticket.

Ticket URL: http://bugs.sugarlabs.org/ticket/4836

If the user removes a $HOME activity, sugar should fall back to
displaying a `/usr/share` copy of the same activity.  This patch
searches through the system paths after removing an activity in
order to add it back if needed.

Steps to reproduce can be found in the ticket.

Ticket URL:  <http://bugs.sugarlabs.org/ticket/4836>
@quozl
Copy link
Contributor

quozl commented Apr 20, 2015

Reviewed. You are also handling the possibility of multiple versions of the same bundle. Well done. Did not test.

@tchx84
Copy link
Member

tchx84 commented Apr 21, 2015

Hello @samdroid-apps,

This looks way better, specially since it can cover more cases than the previous approach. There seem to be a few problems though, because is not working:

Traceback (most recent call last):
  File "/home/tch/Devel/sugar-build/build/out/install/lib/python2.7/site-packages/jarabe/desktop/activitieslist.py", line 565, in __erase_confirmation_dialog_response_cb
    registry.uninstall(bundle, delete_profile=True)
  File "/home/tch/Devel/sugar-build/build/out/install/lib/python2.7/site-packages/jarabe/model/bundleregistry.py", line 551, in uninstall
    alt_bundles = self.get_system_bundles(act.get_bundle_id())
  File "/home/tch/Devel/sugar-build/build/out/install/lib/python2.7/site-packages/jarabe/model/bundleregistry.py", line 574, in get_system_bundles
    for activity_dir in os.listdir(root):
OSError: [Errno 2] No such file or directory: '/usr/share/sugar/activities'

@quozl
Copy link
Contributor

quozl commented Apr 21, 2015

No, don't check for existence first, that introduces a race; instead check for an exception and handle it.

@tchx84
Copy link
Member

tchx84 commented Apr 21, 2015

@quozl can you elaborate on how checking for a path existence before doing operations on it can creates a race condition?

@godiard
Copy link
Contributor

godiard commented Apr 21, 2015

@tchx84, if the path is removed just after you do the check :)

@quozl
Copy link
Contributor

quozl commented Apr 21, 2015

@tchx84, as @godiard says. in this scenario it increases the possible error paths. result is harmless. so it is a comment on code design that is not critical. it also increases the processing to handle a scenario that is rare. more efficient to do the operation and handle the failure than to do an extra operation to check for possible failure first.

@tchx84
Copy link
Member

tchx84 commented Apr 21, 2015

Alright, lets just make sure that whatever goes to the log when it fails is in DEBUG mode, because at least one of the directories will be missing, resulting in unnecessary logs entries. ie., just check the error I just showed while running code in sugar-build, it means that the paths returned by GLib.get_system_data_dirs() doesn't necessarily exist, thus os.listdir() will fail often.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants