Ubuntu One Client

Merge lp:~verterok/ubuntuone-client/fix-597870-stable into lp:ubuntuone-client/stable-1-2

  1. fix-597870-stable
  2. Merge into stable-1-2
Proposed by Guillermo Gonzalez
Status: Merged
Approved by: dobey
Approved revision: 518
Merged at revision: 518
Proposed branch: lp:~verterok/ubuntuone-client/fix-597870-stable
Merge into: lp:ubuntuone-client/stable-1-2
Diff against target: 162 lines (+108/-5)
2 files modified
tests/syncdaemon/test_fsm.py (+90/-2)
ubuntuone/syncdaemon/filesystem_manager.py (+18/-3)
To merge this branch: bzr merge lp:~verterok/ubuntuone-client/fix-597870-stable
Reviewer Review Type Date Requested Status
Facundo Batista (community) Approve
John O'Brien (community) Approve
Review via email: mp+28434@code.launchpad.net

Commit message

Fix fileystem manager _delete_dir_tree method to support read-only shares.

Description of the change

Fix fileystem manager _delete_dir_tree method to support read-only shares.
This will avoid conflicts in read only shares, when a lot of changes are going on and e.g: we get a delete for a node and another node(s) are added to the parent.

To post a comment you must log in.
Revision history for this message
John O'Brien (jdobrien) :
review: Approve
Revision history for this message
Facundo Batista (facundo) wrote :

It's ok

review: Approve
Revision history for this message
dobey (dobey) wrote :
Download full text (254.9 KiB)

The attempt to merge lp:~verterok/ubuntuone-client/fix-597870-stable into lp:ubuntuone-client/stable-1-2 failed.Below is the output from the failed tests.

/usr/bin/gnome-autogen.sh
checking for autoconf >= 2.53...
  testing autoconf2.50... not found.
  testing autoconf... found 2.65
checking for automake >= 1.10...
  testing automake-1.11... found 1.11.1
checking for libtool >= 1.5...
  testing libtoolize... found 2.2.6b
checking for intltool >= 0.30...
  testing intltoolize... found 0.41.1
checking for pkg-config >= 0.14.0...
  testing pkg-config... found 0.25
Checking for required M4 macros...
Checking for forbidden M4 macros...
Processing ./configure.ac
Running libtoolize...
libtoolize: putting auxiliary files in `.'.
libtoolize: copying file `./ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIR, `m4'.
libtoolize: copying file `m4/libtool.m4'
libtoolize: copying file `m4/ltoptions.m4'
libtoolize: copying file `m4/ltsugar.m4'
libtoolize: copying file `m4/ltversion.m4'
libtoolize: copying file `m4/lt~obsolete.m4'
Running intltoolize...
Running aclocal-1.11...
Running autoconf...
Running autoheader...
Running automake-1.11...
Running ./configure --with-protocol=/var/cache/tarmac/ubuntuone-storage-protocol/stable-1-2 ...
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
checking for style of include used by make... GNU
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking dependency style of gcc... gcc3
checking for library containing strerror... none required
checking for gcc... (cached) gcc
checking whether we are using the GNU C compiler... (cached) yes
checking whether gcc accepts -g... (cached) yes
checking for gcc option to accept ISO C89... (cached) none needed
checking dependency style of gcc... (cached) gcc3
checking build system type... i686-pc-linux-gnu
checking host system type... i686-pc-linux-gnu
checking for a sed that does not truncate output... /bin/sed
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for fgrep... /bin/grep -F
checking for ld used by gcc... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... yes
checking for BSD- or MS-compatible name lister (nm)... /usr/bin/nm -B
checking the name lister (/usr/bin/nm -B) interface... BSD nm
checking whether ln -s works... yes
checking the maximum length of command line arguments... 1572864
checking whether the shell understands some XSI constructs... yes
checking whether the shell understands "+="... yes
checking for /usr/bin/ld option to reload object files... -r
checking for objdump... objdump
checking how to recognize dependent libraries... pass_all
checking for ar...

Revision history for this message
Guillermo Gonzalez (verterok) wrote :

I'm not able to reproduce the error.

Revision history for this message
dobey (dobey) wrote :
Download full text (236.0 KiB)

The attempt to merge lp:~verterok/ubuntuone-client/fix-597870-stable into lp:ubuntuone-client/stable-1-2 failed.Below is the output from the failed tests.

/usr/bin/gnome-autogen.sh
checking for autoconf >= 2.53...
  testing autoconf2.50... not found.
  testing autoconf... found 2.65
checking for automake >= 1.10...
  testing automake-1.11... found 1.11.1
checking for libtool >= 1.5...
  testing libtoolize... found 2.2.6b
checking for intltool >= 0.30...
  testing intltoolize... found 0.41.1
checking for pkg-config >= 0.14.0...
  testing pkg-config... found 0.25
Checking for required M4 macros...
Checking for forbidden M4 macros...
Processing ./configure.ac
Running libtoolize...
libtoolize: putting auxiliary files in `.'.
libtoolize: copying file `./ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIR, `m4'.
libtoolize: copying file `m4/libtool.m4'
libtoolize: copying file `m4/ltoptions.m4'
libtoolize: copying file `m4/ltsugar.m4'
libtoolize: copying file `m4/ltversion.m4'
libtoolize: copying file `m4/lt~obsolete.m4'
Running intltoolize...
Running aclocal-1.11...
Running autoconf...
Running autoheader...
Running automake-1.11...
Running ./configure --with-protocol=/var/cache/tarmac/ubuntuone-storage-protocol/stable-1-2 ...
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
checking for style of include used by make... GNU
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking dependency style of gcc... gcc3
checking for library containing strerror... none required
checking for gcc... (cached) gcc
checking whether we are using the GNU C compiler... (cached) yes
checking whether gcc accepts -g... (cached) yes
checking for gcc option to accept ISO C89... (cached) none needed
checking dependency style of gcc... (cached) gcc3
checking build system type... i686-pc-linux-gnu
checking host system type... i686-pc-linux-gnu
checking for a sed that does not truncate output... /bin/sed
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for fgrep... /bin/grep -F
checking for ld used by gcc... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... yes
checking for BSD- or MS-compatible name lister (nm)... /usr/bin/nm -B
checking the name lister (/usr/bin/nm -B) interface... BSD nm
checking whether ln -s works... yes
checking the maximum length of command line arguments... 1572864
checking whether the shell understands some XSI constructs... yes
checking whether the shell understands "+="... yes
checking for /usr/bin/ld option to reload object files... -r
checking for objdump... objdump
checking how to recognize dependent libraries... pass_all
checking for ar...

Revision history for this message
dobey (dobey) wrote :
Download full text (251.4 KiB)

The attempt to merge lp:~verterok/ubuntuone-client/fix-597870-stable into lp:ubuntuone-client/stable-1-2 failed.Below is the output from the failed tests.

/usr/bin/gnome-autogen.sh
checking for autoconf >= 2.53...
  testing autoconf2.50... not found.
  testing autoconf... found 2.65
checking for automake >= 1.10...
  testing automake-1.11... found 1.11.1
checking for libtool >= 1.5...
  testing libtoolize... found 2.2.6b
checking for intltool >= 0.30...
  testing intltoolize... found 0.41.1
checking for pkg-config >= 0.14.0...
  testing pkg-config... found 0.25
Checking for required M4 macros...
Checking for forbidden M4 macros...
Processing ./configure.ac
Running libtoolize...
libtoolize: putting auxiliary files in `.'.
libtoolize: copying file `./ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIR, `m4'.
libtoolize: copying file `m4/libtool.m4'
libtoolize: copying file `m4/ltoptions.m4'
libtoolize: copying file `m4/ltsugar.m4'
libtoolize: copying file `m4/ltversion.m4'
libtoolize: copying file `m4/lt~obsolete.m4'
Running intltoolize...
Running aclocal-1.11...
Running autoconf...
Running autoheader...
Running automake-1.11...
Running ./configure --with-protocol=/var/cache/tarmac/ubuntuone-storage-protocol/stable-1-2 ...
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
checking for style of include used by make... GNU
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking dependency style of gcc... gcc3
checking for library containing strerror... none required
checking for gcc... (cached) gcc
checking whether we are using the GNU C compiler... (cached) yes
checking whether gcc accepts -g... (cached) yes
checking for gcc option to accept ISO C89... (cached) none needed
checking dependency style of gcc... (cached) gcc3
checking build system type... i686-pc-linux-gnu
checking host system type... i686-pc-linux-gnu
checking for a sed that does not truncate output... /bin/sed
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for fgrep... /bin/grep -F
checking for ld used by gcc... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... yes
checking for BSD- or MS-compatible name lister (nm)... /usr/bin/nm -B
checking the name lister (/usr/bin/nm -B) interface... BSD nm
checking whether ln -s works... yes
checking the maximum length of command line arguments... 1572864
checking whether the shell understands some XSI constructs... yes
checking whether the shell understands "+="... yes
checking for /usr/bin/ld option to reload object files... -r
checking for objdump... objdump
checking how to recognize dependent libraries... pass_all
checking for ar...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tests/syncdaemon/test_fsm.py'
2--- tests/syncdaemon/test_fsm.py 2010-03-26 20:24:15 +0000
3+++ tests/syncdaemon/test_fsm.py 2010-06-24 16:37:25 +0000
4@@ -2330,7 +2330,7 @@
5
6
7 class SharesTests(FSMTestCase):
8- """ Tests fsm with ro and rw shares. """
9+ """Tests fsm with ro and rw shares."""
10
11 def tearDown(self):
12 """ cleanup the mess """
13@@ -2391,6 +2391,95 @@
14 self.fsm.delete_file(testdir)
15 self.assertFalse(os.path.exists(testdir))
16
17+ def test_delete_non_empty_dir_ro_share(self):
18+ """Test that fsm is able to delete a non-empty dir in a ro.share."""
19+ share = self.create_share('ro_share', 'ro_share_name', self.fsm,
20+ self.shares_dir, access_level='View')
21+ testdir = os.path.join(share.path, "path2")
22+ mdid = self.fsm.create(testdir, share.volume_id, is_dir=True)
23+ self.fsm.set_node_id(testdir, "uuid2")
24+ self.fsm.create_partial('uuid2', share.volume_id)
25+ fd = self.fsm.get_partial_for_writing('uuid2', share.volume_id)
26+ fd.flush()
27+ fd.close()
28+ self.fsm.remove_partial('uuid2', share.volume_id)
29+ self.fsm.upload_finished(mdid, self.fsm.get_by_mdid(mdid).local_hash)
30+ # crete a file inside the testdir
31+ testfile = os.path.join(testdir, "a_file")
32+ mdid = self.fsm.create(testfile, share.volume_id, is_dir=False)
33+ self.fsm.set_node_id(testfile, "uuid3")
34+ self.fsm.create_partial('uuid3', share.volume_id)
35+ fd = self.fsm.get_partial_for_writing('uuid3', share.volume_id)
36+ fd.flush()
37+ fd.close()
38+ self.fsm.commit_partial('uuid3', share.volume_id, None)
39+ self.fsm.upload_finished(mdid, self.fsm.get_by_mdid(mdid).local_hash)
40+ self.assertTrue(os.path.exists(testdir))
41+ self.assertTrue(os.path.exists(testfile))
42+ self.fsm.delete_file(testdir)
43+ self.assertFalse(os.path.exists(testdir))
44+ self.assertFalse(os.path.exists(testfile))
45+
46+ def test_delete_non_empty_dir_rw_share(self):
47+ """Test that fsm is able to delete a non-empty dir in a rw.share."""
48+ share = self.create_share('rw_share', 'rw_share_name', self.fsm,
49+ self.shares_dir, access_level='Modify')
50+ testdir = os.path.join(share.path, "path2")
51+ mdid = self.fsm.create(testdir, share.volume_id, is_dir=True)
52+ self.fsm.set_node_id(testdir, "uuid2")
53+ self.fsm.create_partial('uuid2', share.volume_id)
54+ fd = self.fsm.get_partial_for_writing('uuid2', share.volume_id)
55+ fd.flush()
56+ fd.close()
57+ self.fsm.remove_partial('uuid2', share.volume_id)
58+ self.fsm.upload_finished(mdid, self.fsm.get_by_mdid(mdid).local_hash)
59+ # crete a file inside the testdir
60+ testfile = os.path.join(testdir, "a_file")
61+ mdid = self.fsm.create(testfile, share.volume_id, is_dir=False)
62+ self.fsm.set_node_id(testfile, "uuid3")
63+ self.fsm.create_partial('uuid3', share.volume_id)
64+ fd = self.fsm.get_partial_for_writing('uuid3', share.volume_id)
65+ fd.flush()
66+ fd.close()
67+ self.fsm.commit_partial('uuid3', share.volume_id, None)
68+ self.fsm.upload_finished(mdid, self.fsm.get_by_mdid(mdid).local_hash)
69+ self.assertTrue(os.path.exists(testdir))
70+ self.assertTrue(os.path.exists(testfile))
71+ self.fsm.delete_file(testdir)
72+ self.assertFalse(os.path.exists(testdir))
73+ self.assertFalse(os.path.exists(testfile))
74+
75+ def test_delete_non_empty_dir_bad_perms_rw_share(self):
76+ """Test that fsm is able to delete a non-empty dir in a rw.share."""
77+ share = self.create_share('rw_share', 'rw_share_name', self.fsm,
78+ self.shares_dir, access_level='Modify')
79+ testdir = os.path.join(share.path, "path2")
80+ mdid = self.fsm.create(testdir, share.volume_id, is_dir=True)
81+ self.fsm.set_node_id(testdir, "uuid2")
82+ self.fsm.create_partial('uuid2', share.volume_id)
83+ fd = self.fsm.get_partial_for_writing('uuid2', share.volume_id)
84+ fd.flush()
85+ fd.close()
86+ self.fsm.remove_partial('uuid2', share.volume_id)
87+ self.fsm.upload_finished(mdid, self.fsm.get_by_mdid(mdid).local_hash)
88+ # crete a file inside the testdir
89+ testfile = os.path.join(testdir, "a_file")
90+ mdid = self.fsm.create(testfile, share.volume_id, is_dir=False)
91+ self.fsm.set_node_id(testfile, "uuid3")
92+ self.fsm.create_partial('uuid3', share.volume_id)
93+ fd = self.fsm.get_partial_for_writing('uuid3', share.volume_id)
94+ fd.flush()
95+ fd.close()
96+ self.fsm.commit_partial('uuid3', share.volume_id, None)
97+ self.fsm.upload_finished(mdid, self.fsm.get_by_mdid(mdid).local_hash)
98+ self.assertTrue(os.path.exists(testdir))
99+ self.assertTrue(os.path.exists(testfile))
100+ # make the dir read-only
101+ os.chmod(testdir, 0555)
102+ self.assertRaises(OSError, self.fsm.delete_file, testdir)
103+ self.assertTrue(os.path.exists(testdir))
104+ self.assertTrue(os.path.exists(testfile))
105+
106 def test_delete_file_ro_share(self):
107 """ Test that fsm is able to delete a file in a ro-share. """
108 self.share = self.create_share('ro_share', 'ro_share_name', self.fsm,
109@@ -2458,7 +2547,6 @@
110 self.fsm.commit_partial('uuid3', self.share.volume_id, None)
111 self.assertTrue(os.path.exists(testfile))
112
113-
114 def test_share_and_root(self):
115 """ Test the creation of a file with the same relative path in a share
116 and in the root.
117
118=== modified file 'ubuntuone/syncdaemon/filesystem_manager.py'
119--- ubuntuone/syncdaemon/filesystem_manager.py 2010-06-02 18:02:03 +0000
120+++ ubuntuone/syncdaemon/filesystem_manager.py 2010-06-24 16:37:25 +0000
121@@ -687,7 +687,7 @@
122 del self._idx_node_id[(mdobj["share_id"], mdobj["node_id"])]
123 del self.fs[mdid]
124
125- def _delete_dir_tree(self, path):
126+ def _delete_dir_tree(self, path, share_id):
127 """Helper function to recursively remove a non-empty directory.
128
129 Removes the dir if there aren't local changes, and returns True.
130@@ -716,7 +716,21 @@
131 filter_name = is_dir and "FS_DIR_DELETE" or "FS_FILE_DELETE"
132 self.eq.add_to_mute_filter(filter_name, p)
133 self.delete_metadata(p)
134- shutil.rmtree(path)
135+
136+ # get the share and if it's read-only
137+ share = self._get_share(share_id)
138+ is_ro_share = not share.can_write()
139+ def handle_ro_share(func, subpath, exc_info):
140+ """Handle rmtree EACCES error for ro-shares."""
141+ if exc_info[0] == OSError and exc_info[1].errno == errno.EACCES \
142+ and is_ro_share:
143+ # use EnableShareWrite directly to avoid a lot of share
144+ # lookups.
145+ with EnableShareWrite(share, subpath):
146+ func(subpath)
147+ else:
148+ raise
149+ shutil.rmtree(path, onerror=handle_ro_share)
150 return True
151
152 def delete_file(self, path):
153@@ -741,7 +755,8 @@
154 except OSError, e:
155 self.eq.rm_from_mute_filter(expected_event, path)
156 if e.errno == errno.ENOTEMPTY:
157- if not self._delete_dir_tree(path=path):
158+ if not self._delete_dir_tree(path=path,
159+ share_id=mdobj['share_id']):
160 raise
161 else:
162 m = "OSError %s when trying to remove file/dir %r"

Subscribers

People subscribed via source and target branches