[opensource-dev] Review Request: scripts/install.py --uninstall does not always remove symbolic links.

[Boroondas Gupte]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/29/#review33
-----------------------------------------------------------

Ship it!


Your explanation is consistent with python's documentation and the change looks simple and sane. The change won't have any effect on platforms that don't have os.lstat(), but those seem to be the ones that don't have symlinks in the first place.

- Boroondas


On 2010-12-16 07:34:22, Aleric Inglewood wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/29/
> -----------------------------------------------------------
> 
> (Updated 2010-12-16 07:34:22)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> -------
> 
> Packages (tar balls) installed with scripts/install.py do contain symbolic links.
> Everything that they contain is written to the file installed.xml, and upon
> uninstall attempted to remove.
> 
> However, the python script first tests if a file exists before it removes it
> and uses os.path.exists for this, which only returns true when the target
> is a file, or a symbolic link *pointing* to an existing file.
> 
> Since the removal of the tar ball elements is arbitrary, it is possible (and
> often the case) that the file the symbolic link points to is removed before
> the symbolic link itself is removed, causing the test to fail and the symbolic
> link not to be removed.
> 
> This patch solves this bug by using os.path.lexists which returns true for
> normal files when they exist, and true for symbolic links if they exist,
> whether or not the file they point to exists, exactly what we want.
> 
> os.path.lexists was added to python 2.4, so that should not be problem.
> 
> 
> This addresses bug SNOW-744.
>     http://jira.secondlife.com/browse/SNOW-744
> 
> 
> Diffs
> -----
> 
>   scripts/install.py b0689af42a71 
> 
> Diff: http://codereview.secondlife.com/r/29/diff
> 
> 
> Testing
> -------
> 
> This patch was originally tested to work several months ago, and has been in
> use by the Imprudence TPV for a while now.
> 
> 
> Thanks,
> 
> Aleric
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20101216/06d53a0d/attachment.htm