Merge lp://qastaging/~lool/ubuntuone-client/tritcask-use-platform-api into lp://qastaging/ubuntuone-client

Proposed by Loïc Minier
Status: Merged
Approved by: dobey
Approved revision: 1400
Merged at revision: 1387
Proposed branch: lp://qastaging/~lool/ubuntuone-client/tritcask-use-platform-api
Merge into: lp://qastaging/ubuntuone-client
Diff against target: 262 lines (+52/-26)
7 files modified
ubuntuone/platform/__init__.py (+1/-0)
ubuntuone/platform/os_helper/__init__.py (+1/-0)
ubuntuone/platform/os_helper/darwin.py (+1/-0)
ubuntuone/platform/os_helper/linux.py (+1/-0)
ubuntuone/platform/os_helper/unix.py (+5/-0)
ubuntuone/platform/os_helper/windows.py (+6/-0)
ubuntuone/syncdaemon/tritcask.py (+37/-26)
To merge this branch: bzr merge lp://qastaging/~lool/ubuntuone-client/tritcask-use-platform-api
Reviewer Review Type Date Requested Status
Mike McCracken (community) Approve
dobey (community) Approve
Review via email: mp+147581@code.qastaging.launchpad.net

Commit message

Use platform API for os.path.* functions; fixes LP #1101344.

Description of the change

Use platform API for os.path.* functions; fixes bug #1101344.

To post a comment you must log in.
Revision history for this message
Loïc Minier (lool) wrote :

Pushed some more revisions; sorry, there's some history garbage: r1387 to r1390 were attempts to fix the SetFileSecurity issue:
* it's a deprecated function, so I tried using the new SetNamedSecurityInfo function
* I also tried matching permissions with files on my system which have an ACL entry for sysops too
* I dropped some unused function (_get_group_sid)

but I didn't want these revs to end up in this mp, and I forgot to revert my tree to the right revno before committing the other fixes, so r1391 has these changed reverted.

Let me know if you'd like some of them in another mp, but they don't fix the orginal bug so I'm not adding them here.

Revision history for this message
Loïc Minier (lool) wrote :

I pushed a last rev 1397 which is untested; I was hoping using normpath would fix all issues without having to call into the os_helper API for everything, but it didn't help after all, so I'm reverting the normpath changes.

I didn't actually test tritcask.py after reverting though.

Revision history for this message
dobey (dobey) wrote :

44 +is_dir = unit.is_dir

Typo here.

Have you not done ./autogen.sh && make check in the tree?

Revision history for this message
Loïc Minier (lool) wrote :

Sorry I hadn't (I thought I had noted that, but apparently I forgot)

I've pushed r1398 which fixes this particular issue, and then make check exposed another issue which I've fixed in r1399; this should hopefully be good to merge now. NB: some tests were actually skipped, not sure why:
PASSED (skips=12, successes=2759)

Also didn't run some part of the tests it seems:
ubuntuone.devtools.errors.TestError: The Python package providing the requested reactor is not installed. You can find it here: https://github.com/ghtdak/qtreactor

[ Would you mind adding some advice in HACKING on the packages that one should install? I had to install:
  xutils-dev gobject-introspection python-mocker
to run ./autogen.sh && make check, and I can see that other bits like gnome-common are needed. ]

Revision history for this message
dobey (dobey) wrote :

You can use "apt-get build-dep ubuntuone-client" for the most part. python-qt4reactor is packaged, but not in main, so it's not in the build-depends of ubuntuone-client in the main archive. But adding the nightlies ppa (ppa:ubuntuone/nightlies) and then apt-get update && apt-get build-dep ubuntuone-client will pull everything needed.

Also, can you please do a "bzr commit --fixes=lp:1101344 --unchanged" to link the bug into the branch? Thanks.

Revision history for this message
Loïc Minier (lool) wrote :

Done in r1400.

Revision history for this message
dobey (dobey) :
review: Approve
Revision history for this message
Mike McCracken (mikemc) wrote :

looks good. tests pass on darwin.

Thanks Loïc!

review: Approve
Revision history for this message
Ubuntu One Auto Pilot (otto-pilot) wrote :

There was a problem validating some authors of the branch. Authors must be either one of the listed Launchpad users, or a member of one of the listed teams on Launchpad.

Persons or Teams:

    contributor-agreement-canonical
    ubuntuone-hackers

Unaccepted Authors:

    Loïc Minier <email address hidden>

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches