Ubuntu One Client

Merge lp:~verterok/ubuntuone-client/bad-bad-symlink into lp:ubuntuone-client

  1. bad-bad-symlink
  2. Merge into trunk
Proposed by Guillermo Gonzalez
Status: Merged
Approved by: Guillermo Gonzalez
Approved revision: 188
Merged at revision: not available
Proposed branch: lp:~verterok/ubuntuone-client/bad-bad-symlink
Merge into: lp:ubuntuone-client
Diff against target: None lines
To merge this branch: bzr merge lp:~verterok/ubuntuone-client/bad-bad-symlink
Reviewer Review Type Date Requested Status
John O'Brien (community) Approve
Facundo Batista (community) Approve
Review via email: mp+10798@code.launchpad.net

Commit message

Fix the broken Shared With Me symlink that points to itself.

To post a comment you must log in.
Revision history for this message
Guillermo Gonzalez (verterok) wrote :

Fix the broken Shared With Me symlink that points to itself.

lp:~verterok/ubuntuone-client/bad-bad-symlink updated
187. By Guillermo Gonzalez

log in the right place

Revision history for this message
Facundo Batista (facundo) wrote :

+1 if the bad indentation in a log line is fixed.

review: Approve
Revision history for this message
John O'Brien (jdobrien) wrote :

Looks good.. I'm glad you wrote those tests too!

review: Approve
Revision history for this message
John O'Brien (jdobrien) wrote :

john@Monolith:~/canonical/ubuntuone/ubuntuone-client/bad-bad-symlink$ PYTHONPATH=. bin/ubuntuone-syncdaemon --debug
2009-08-27 11:06:25,183 - ubuntuone.SyncDaemon.VM - DEBUG - upgrading from metadata 3 (new layout)
2009-08-27 11:06:25,187 - ubuntuone.SyncDaemon - ERROR - Unexpected error
Traceback (most recent call last):
  File "bin/ubuntuone-syncdaemon", line 192, in <module>
    main(sys.argv)
  File "bin/ubuntuone-syncdaemon", line 145, in main
    shares_symlink_name='Shared With Me')
  File "/home/john/canonical/ubuntuone/ubuntuone-client/bad-bad-symlink/ubuntuone/syncdaemon/main.py", line 96, in __init__
    self.vm = volume_manager.VolumeManager(self)
  File "/home/john/canonical/ubuntuone/ubuntuone-client/bad-bad-symlink/ubuntuone/syncdaemon/volume_manager.py", line 125, in __init__
    upgrade_method(md_version)
  File "/home/john/canonical/ubuntuone/ubuntuone-client/bad-bad-symlink/ubuntuone/syncdaemon/volume_manager.py", line 563, in _upgrade_metadata_3
    share.path = share.path.replace(old_root_dir, self.m.root_dir)
AttributeError: 'NoneType' object has no attribute 'replace'
Traceback (most recent call last):
  File "bin/ubuntuone-syncdaemon", line 192, in <module>
    main(sys.argv)
  File "bin/ubuntuone-syncdaemon", line 145, in main
    shares_symlink_name='Shared With Me')
  File "/home/john/canonical/ubuntuone/ubuntuone-client/bad-bad-symlink/ubuntuone/syncdaemon/main.py", line 96, in __init__
    self.vm = volume_manager.VolumeManager(self)
  File "/home/john/canonical/ubuntuone/ubuntuone-client/bad-bad-symlink/ubuntuone/syncdaemon/volume_manager.py", line 125, in __init__
    upgrade_method(md_version)
  File "/home/john/canonical/ubuntuone/ubuntuone-client/bad-bad-symlink/ubuntuone/syncdaemon/volume_manager.py", line 563, in _upgrade_metadata_3
    share.path = share.path.replace(old_root_dir, self.m.root_dir)
AttributeError: 'NoneType' object has no attribute 'replace'

review: Needs Fixing
lp:~verterok/ubuntuone-client/bad-bad-symlink updated
188. By Guillermo Gonzalez

check if shared.path is None in the shared migration from v3

Revision history for this message
John O'Brien (jdobrien) wrote :

w00t!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tests/syncdaemon/test_vm.py'
2--- tests/syncdaemon/test_vm.py 2009-08-25 18:26:17 +0000
3+++ tests/syncdaemon/test_vm.py 2009-08-27 14:24:49 +0000
4@@ -66,6 +66,7 @@
5 self.rmtree(self.shares_dir)
6 self.log.info("finished test %s.%s", self.__class__.__name__,
7 self._testMethodName)
8+ VolumeManager.METADATA_VERSION = CURRENT_METADATA_VERSION
9 return BaseTwistedTestCase.tearDown(self)
10
11
12@@ -447,18 +448,23 @@
13 def setUp(self):
14 """ setup the test """
15 BaseTwistedTestCase.setUp(self)
16- self.root_dir = self.mktemp('root_dir')
17+ self.root_dir = self.mktemp('Ubuntu One')
18 self.data_dir = self.mktemp('data_dir')
19- self.shares_dir = self.mktemp('shares_dir')
20+ self.shares_dir = self.mktemp(os.path.join('Ubuntu One',
21+ 'Shared with Me'))
22
23 def tearDown(self):
24 """ cleanup main and remove the temp dir """
25 main = getattr(self, 'main', None)
26 if main:
27 main.shutdown()
28- self.rmtree(self.root_dir)
29- self.rmtree(self.data_dir)
30- self.rmtree(self.shares_dir)
31+ if os.path.exists(self.root_dir):
32+ self.rmtree(self.root_dir)
33+ if os.path.exists(self.data_dir):
34+ self.rmtree(self.data_dir)
35+ if os.path.exists(self.shares_dir):
36+ self.rmtree(self.shares_dir)
37+ VolumeManager.METADATA_VERSION = CURRENT_METADATA_VERSION
38 return BaseTwistedTestCase.tearDown(self)
39
40 def check_version(self):
41@@ -632,8 +638,51 @@
42 self.assertTrue(os.path.exists(os.path.join(share_path, 'test_dir')))
43 self.assertTrue(os.path.exists(os.path.join(share_path, 'test_file')))
44
45+ def test_3_to_4_with_symlink_in_myfiles(self):
46+ """upgrade from version 3 to 4"""
47+ vm_data_dir = os.path.join(self.data_dir, 'vm')
48+ os.makedirs(vm_data_dir)
49+ with open(os.path.join(vm_data_dir, '.version'), 'w') as fd:
50+ fd.write('3')
51+ os.rmdir(self.shares_dir)
52+ # build the old layout
53+ old_root = os.path.join(self.root_dir, 'My Files')
54+ old_shares = os.path.join(self.root_dir, 'Shared With Me')
55+ os.makedirs(os.path.join(old_root, 'test_dir'))
56+ open(os.path.join(old_root, 'test_file'), 'w').close()
57+ # create a file in the root
58+ open(os.path.join(self.root_dir, 'test_file'), 'w').close()
59+ share_path = os.path.join(old_shares, 'Bla from Foo')
60+ os.makedirs(share_path)
61+ os.makedirs(os.path.join(share_path, 'test_dir'))
62+ open(os.path.join(share_path, 'test_file'), 'w').close()
63+ # create the Shared with Me symlink in My Files
64+ os.symlink(old_shares, os.path.join(old_root, 'Shared With Me'))
65+ # fix permissions
66+ os.chmod(self.root_dir, 0555)
67+ os.chmod(old_shares, 0555)
68+ # migrate the data
69+ self.shares_dir = os.path.join(self.tmpdir, 'shares')
70+ self.main = FakeMain(self.root_dir, self.shares_dir, self.data_dir)
71+ self.assertFalse(os.path.exists(old_root))
72+ self.assertTrue(os.path.exists(old_shares))
73+ self.assertTrue(os.path.islink(old_shares))
74+ self.assertEquals(old_shares, self.main.shares_dir_link)
75+ self.assertTrue(os.path.exists(os.path.join(self.root_dir,
76+ 'test_dir')))
77+ self.assertTrue(os.path.exists(os.path.join(self.root_dir,
78+ 'test_file')))
79+ self.assertTrue(os.path.exists(os.path.join(self.root_dir,
80+ 'test_file.u1conflict')))
81+ self.assertTrue(os.path.exists(share_path))
82+ self.assertTrue(os.path.exists(os.path.join(share_path, 'test_dir')))
83+ self.assertTrue(os.path.exists(os.path.join(share_path, 'test_file')))
84+ self.assertEquals(self.main.shares_dir,
85+ os.readlink(self.main.shares_dir_link))
86+
87 def test_None_to_4(self):
88 """upgrade from version None to 4 (possibly a clean start)"""
89+ VolumeManager.METADATA_VERSION = '4'
90 vm_data_dir = os.path.join(self.data_dir, 'vm')
91 version_file = os.path.join(vm_data_dir, '.version')
92 if os.path.exists(version_file):
93@@ -656,6 +705,7 @@
94
95 def test_None_to_4_phantom_share_path(self):
96 """upgrade from version None to 4 (possibly a clean start)"""
97+ VolumeManager.METADATA_VERSION = '4'
98 vm_data_dir = os.path.join(self.data_dir, 'vm')
99 version_file = os.path.join(vm_data_dir, '.version')
100 if os.path.exists(version_file):
101@@ -684,3 +734,16 @@
102 self.assertEquals('4', fd.read())
103 else:
104 self.fail('missing .version file')
105+
106+ def test_4_to_5(self):
107+ """test migration from 4 to 5 (broken symlink in the root)"""
108+ vm_data_dir = os.path.join(self.data_dir, 'vm')
109+ os.makedirs(vm_data_dir)
110+ with open(os.path.join(vm_data_dir, '.version'), 'w') as fd:
111+ fd.write('4')
112+ # build the new layout with a broken symlink
113+ shares_link = os.path.join(self.root_dir, 'Shared With Me')
114+ os.symlink(shares_link, shares_link)
115+ self.main = FakeMain(self.root_dir, self.shares_dir, self.data_dir)
116+ self.assertEquals(self.main.shares_dir,
117+ os.readlink(self.main.shares_dir_link))
118
119=== modified file 'ubuntuone/syncdaemon/volume_manager.py'
120--- ubuntuone/syncdaemon/volume_manager.py 2009-08-26 13:33:23 +0000
121+++ ubuntuone/syncdaemon/volume_manager.py 2009-08-27 14:24:49 +0000
122@@ -91,7 +91,7 @@
123 class VolumeManager(object):
124 """Manages shares and mount points."""
125
126- METADATA_VERSION = '4'
127+ METADATA_VERSION = '5'
128
129 def __init__(self, main):
130 """Create the instance and populate the shares/d attributes
131@@ -129,13 +129,16 @@
132 os.makedirs(self._shared_dir)
133
134 # build the dir layout
135- # TODO: migrate from old dir layout
136 if not os.path.exists(self.m.root_dir):
137+ self.log.debug('creating root dir: %r', self.m.root_dir)
138 os.makedirs(self.m.root_dir)
139 if not os.path.exists(self.m.shares_dir):
140+ self.log.debug('creating shares directory: %r', self.m.shares_dir)
141 os.makedirs(self.m.shares_dir)
142 # create the shares symlink
143 if not os.path.exists(self.m.shares_dir_link):
144+ self.log.debug('creating Shares symlink: %r -> %r',
145+ self.m.shares_dir_link, self.m.shares_dir)
146 os.symlink(self.m.shares_dir, self.m.shares_dir_link)
147 # make the shares_dir read only
148 os.chmod(self.m.shares_dir, 0555)
149@@ -542,6 +545,8 @@
150 os.chmod(old_share_dir, 0775)
151 if not os.path.exists(os.path.dirname(self.m.shares_dir)):
152 os.makedirs(os.path.dirname(self.m.shares_dir))
153+ self.log.debug('moving shares dir from: %r to %r',
154+ old_share_dir, self.m.shares_dir)
155 shutil.move(old_share_dir, self.m.shares_dir)
156 # update the shares metadata
157 shares = ShareFileShelf(self._shares_dir)
158@@ -560,19 +565,45 @@
159 # move the My Files contents, taking care of dir/files with the same
160 # in the new root
161 if os.path.exists(old_root_dir):
162+ self.log.debug('moving My Files contents to the root')
163 # make My Files rw
164 os.chmod(old_root_dir, 0775)
165 path_join = os.path.join
166 for relpath in os.listdir(old_root_dir):
167 old_path = path_join(old_root_dir, relpath)
168 new_path = path_join(self.m.root_dir, relpath)
169+
170 if os.path.exists(new_path):
171 shutil.move(new_path, new_path+'.u1conflict')
172- shutil.move(old_path, new_path)
173+ if relpath == 'Shared With Me':
174+ # remove the Shared with Me symlink inside My Files!
175+ self.log.debug('removing shares symlink from old root')
176+ os.remove(old_path)
177+ else:
178+ self.log.debug('moving %r to %r', old_path, new_path)
179+ shutil.move(old_path, new_path)
180+ self.log.debug('removing old root: %r', old_root_dir)
181 os.rmdir(old_root_dir)
182+
183+ self._upgrade_metadata_4(md_version)
184 # update the .version file
185 self._update_metadata_version()
186
187+ def _upgrade_metadata_4(self, md_version):
188+ """
189+ Upgrade to version 5 (fix the broken symlink!)
190+ """
191+ self.log.debug('upgrading from metadata 4 (broken symlink!)')
192+ if os.path.islink(self.m.shares_dir_link):
193+ target = os.readlink(self.m.shares_dir_link)
194+ if os.path.normpath(target) == self.m.shares_dir_link:
195+ # the symnlink points to itself
196+ self.log.debug('removing broken shares symlink: %r -> %r',
197+ self.m.shares_dir_link, target)
198+ os.remove(self.m.shares_dir_link)
199+
200+ self._update_metadata_version()
201+
202 def _update_metadata_version(self):
203 """write the version of the metadata"""
204 if not os.path.exists(os.path.dirname(self._version_file)):

Subscribers

People subscribed via source and target branches