Opened 6 years ago

Closed 6 years ago

#8674 closed defect (fixed)

8.2-764, Software Update erased complete /home/olpc/Activities directory

Reported by: tvoverbeek Owned by: cscott
Priority: blocker Milestone: 8.2.0 (was Update.2)
Component: sugar Version: olpc-3
Keywords: blocks:8.2.0 r+ Cc: mstone, joe
Blocked By: Blocking:
Deployments affected: Action Needed: test in release
Verified: no

Description

Updated to 8.2-764 using olpc-update.
Started software-update which wanted to install some updates and now
also for the first time a lot of library bundles.
Started OK, but remained hanging on the Wikipedia library bundle.
Canceled update and control panel.
Home view still showed all activities, including new ones like Implode.
But no activity wanted to start.
Looking with a terminal (Ctrl-Alt-F2) showed that the complete
/home/olpc/Activities had disappeared !!!
Managed to recover shell.log, which is attached.

Attachments (1)

shell.log (482.9 KB) - added by tvoverbeek 6 years ago.
shell.log from Software Update

Download all attachments as: .zip

Change History (19)

Changed 6 years ago by tvoverbeek

shell.log from Software Update

comment:1 Changed 6 years ago by mstone

  • Action Needed changed from never set to reproduce
  • Cc cscott added
  • Milestone changed from Not Triaged to 8.2.0 (was Update.2)

comment:2 follow-up: Changed 6 years ago by cscott

Looks like the program occurred during the upgrade of Wikislice or Wikipedia. My wild guess is that bundle.uninstall() got convinced that the bundle root dir was '.' and ended up deleting ~/Activities/. -- the logic for finding the zip file root is:

        file_names = self._zip_file.namelist()
...
        self._zip_root_dir = file_names[0].split('/')[0]

This seems failure-prone to me.

comment:3 in reply to: ↑ 2 Changed 6 years ago by tomeu

This is the method that performs the bundle deletion:

 175     def _uninstall(self, install_path):
 176         if not os.path.isdir(install_path):
 177             raise InvalidPathException
 178         if self._unzipped_extension is not None:
 179             (name_, ext) = os.path.splitext(install_path)
 180             if ext != self._unzipped_extension:
 181                 raise InvalidPathException
 182 
 183         for root, dirs, files in os.walk(install_path, topdown=False):
 184             for name in files:
 185                 os.remove(os.path.join(root, name))
 186             for name in dirs:
 187                 os.rmdir(os.path.join(root, name))
 188         os.rmdir(install_path)

For activity bundles, _unzipped_extension is '.activity' so that if a '/home/olpc/Activities' had been passed as install_path, an exception would have been raised and the deletion would have been aborted.

Any other idea about how this could have happened? which other code do we have that deletes dirs recursively?

comment:4 Changed 6 years ago by korakurider

I did clean install 8.2-764, but that didn't make /home/olpc/Activities !

comment:5 follow-up: Changed 6 years ago by tomeu

Just managed to reproduce with the instructions in #8685, but the Activities dir wasn't removed :/

comment:6 Changed 6 years ago by marco

Tomeu, in which build? Scott patched sugar from pilgrim to *try* to fix this, but I'm not sure when that went in.

comment:7 in reply to: ↑ 5 Changed 6 years ago by tomeu

Replying to tomeu:

Just managed to reproduce with the instructions in #8685, but the Activities dir wasn't removed :/

Sorry, wanted to mean that I tried but failed to reproduce.

Tomeu, in which build? Scott patched sugar from pilgrim to *try* to fix this, but I'm not sure when that went in.

joyride-2482. I don't currently know how Scott's patch could fix this issue, though. Perhaps he could explain the rationale behind that patch?

comment:8 Changed 6 years ago by marco

  • Cc cscott removed
  • Owner changed from marco to cscott

comment:9 Changed 6 years ago by cscott

  • Action Needed changed from reproduce to code
  • Cc mstone added
  • Keywords blocks:8.2.0 added; blocks?:8.2.0 removed

The (unconvincing) pilgrim patch was http://dev.laptop.org/git?p=projects/pilgrim;a=commitdiff;h=9fd697783a8c6c3f31dea490b24845b6a07b5ccf

Tomeu, _unzipped_extension is not set for content bundles. So I think somethink like this pilgrim patch should go in, as well as the fix that marco points out above.

comment:10 Changed 6 years ago by cscott

  • Action Needed changed from code to review

comment:11 Changed 6 years ago by tomeu

  • Keywords r+ added

Wonder if before any massive deletion we should check the path for some properties so that we limit the chances of disaster. I think that even if we end up with redundant code, this may pay up in the end.

comment:12 Changed 6 years ago by cscott

I generally prefer to do an operation like this by unpacking into a newly-created tempdir, and transferring the contents to the destination directory only on success. That way you know it's always safe to blow away the tempdir. But I was trying to keep the patch small...

comment:13 Changed 6 years ago by cscott

|Test Case|
(a bit involved)
Create a corrupt bundle, which python's ZipFile will still open. Truncating the file is not good enough: that will remove the zip's index and ZipFile will bail. It has to be a problem which ZipFile won't catch, but causes 'unzip' to return a non-zero status code. The best way is to open the bundle in a hex editor, page down a bit, and increment a single random byte by one. Put this bundle in a place where the updater will find it.

I've done these steps to create a Pippy-99.xo. Add the activity group

http://dev.laptop.org/~cscott/bundles/pippy-test.html

to the software updater.

Attempt to install this bundle with the bundle. Previously, this would result in deletion of ~/Activities. Now, only the corrupted Pippy bundle will be cleaned up.

comment:14 Changed 6 years ago by cscott

  • Action Needed changed from review to package

Tested, and pushed to sucrose-0.82 and master branches of sugar-toolkit.

comment:15 Changed 6 years ago by cscott

  • Action Needed changed from package to approve for release

Packaged in sugar-toolkit 0.82.11-4.olpc3, please test in joyride-2492 or later. Applying for approval for next stable release.

comment:16 Changed 6 years ago by cscott

  • Action Needed changed from approve for release to add to release

In person meeting of release managers and muckity-mucks; this bug was approved for 766.

comment:17 Changed 6 years ago by cscott

  • Action Needed changed from add to release to test in release

Packages added to stable repository in commit d9c3fbc0 for build 766. Please confirm the package versions are correct and test in stable build 766 or later.

comment:18 Changed 6 years ago by joe

  • Cc joe added
  • Resolution set to fixed
  • Status changed from new to closed

Tested in 8.2-767, works just fine. Closing...

Note: See TracTickets for help on using tickets.