Uninstall removes all from c:\ubuntu

[Bart Kroon]

In case the c:\ubuntu directory already exists:

- the installer will write new files to it without asking confirmation to the user (small BUG)
- the uninstaller will delete not only the Ubuntu installation but the entire directory c:\ubuntu directory (big BUG)

In my case c:\ubuntu contained the files of a virtual machine image but I can imagine other users having c:\ubuntu directories for different valid reasons. I tried Wubi but ran into https://bugs.launchpad.net/wubi/+bug/241847 , so I uninstalled and lost the virtual machine image.

In my opinion and given the quality of Ubuntu in general, the "officially supported" label on Wubi is not yet deserved. Will try CD install now :S

[Agostino Russo (ago)]

There is some code to avoid conflicts in select_target_dir, but that is only triggered in certain circumstances, and admittedly the logic should have been improved. A possible (untested) patch is attached:

=== modified file 'src/wubi/backends/win32/backend.py' (properties changed: -x to +x)
--- src/wubi/backends/win32/backend.py 2009-04-20 08:37:37 +0000
+++ src/wubi/backends/win32/backend.py 2009-04-20 17:41:03 +0000
@@ -79,17 +79,11 @@
         target_dir.replace(' ', '_')
         target_dir.replace('__', '_')
         gold_target_dir = target_dir
- if os.path.exists(target_dir) \
- and self.info.previous_target_dir \
- and os.path.isdir(self.info.previous_target_dir):
- if self.info.previous_target_dir == target_dir:
- rm_tree(self.info.previous_target_dir)
- self.info.previous_target_dir = None
- else:
- for i in range(2, 1000):
- target_dir = gold_target_dir + '.' + str(i)
- if not os.path.exists(target_dir):
- break
+ if os.path.exists(target_dir):
+ for i in range(2, 1000):
+ target_dir = gold_target_dir + '.' + str(i)
+ if not os.path.exists(target_dir):
+ break
         self.info.target_dir = target_dir
         log.info('Installing into %s' % target_dir)
         self.info.icon = join_path(self.info.target_dir, self.info.distro.name + '.ico')

[Agostino Russo (ago)]

http://paste.ubuntu.com/154841/

[Agostino Russo (ago)]

(00:27:36) slangasek: xivulon: how's the wubi fix going?
(00:27:50) xivulon: slangasek: actually the fix to 364166 is not that simple, the installation folder is hardcoded at compile time into the embedded menu.lst (I was planning to move to grub2 and did not change that)
(00:28:22) xivulon: so I have 3 options: A) ask the user (which will be untraslated) B) rename the existing dir without asking C) leave things as they are
(00:29:24) slangasek: xivulon: C) is understandably ugly, but it also lets us avoid churn on the desktop CDs; what do you think about putting this in the release notes?
(00:31:34) xivulon: as mentioned this was probably an issue for a long time, but I only received the first (rightful) complaint toady, I can live with that
(00:32:18) xivulon: A should be a fairly safe fix, I simply quit with an error message, I can commit that and we only add it to the CD if you need to respin for some other reason
(00:32:47) slangasek: xivulon: that seems reasonable
(00:33:40) xivulon: I will add the above to the bug for reference

[Agostino Russo (ago)]

For completeness, the fourth option is to only remove a given list of files/directory during uninstallation without using deltree, but it is too invasive so late in the cycle.

[Agostino Russo (ago)]

(00:53:00) xivulon: slangasek: http://paste.ubuntu.com/155035/
(00:53:02) ScottK: OK.
(00:53:25) xivulon: tested and works as expected, shows an ugly error message
(00:53:40) lool: cjwatson: Yup, I ^F /live, thanks
(00:53:41) xivulon: if that's ok I will commit
(00:53:43) slangasek: lool: armel desktop is posted now too, btw
(00:54:58) slangasek: xivulon: s/with such a name/with this name/; s/before continuing/before continuing./
(00:55:33) slangasek: xivulon: otherwise, it looks acceptable, but sufficiently low-benefit that I'll continue avoiding respinning any images if I can

[Agostino Russo (ago)]

Have committed a quick and dirty workaround in r129, it simply throws an error if a pre-existing file/directory is detected. It is less than ideal, but can't do much more at this stage.

[Dave Morley (davmor2)]

This is indeed fixed in r129 You now have the system bail out with the warning that the folder already exists.

[Steve Langasek (vorlon)]

Documented at <https://wiki.ubuntu.com/JauntyJackalope/ReleaseNotes#Wubi reuses a pre-existing "ubuntu" directory and removes it on uninstallation>:

 If there is a pre-existing directory called "ubuntu" in the target drive, it will be reused during the Windows installation and the full directory will be removed when uninstalling (364166). This will result in the loss of any files which were originally in that directory. It is recommended to rename any directory called "ubuntu" before proceeding with a Wubi installation.

[Agostino Russo (ago)]

It was not possible to include the patch in the CD (see release notes), but r129 will be released as stand-alone download