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

2010-12-16 Thread Aleric Inglewood

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

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

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

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

2010-12-16 Thread Brad Kittenbrink

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

Ship it!


Looks like a safe and sensible change to me.

- Brad


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
 


___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges