Bazaar Subversion Plugin

Merge lp:~vila/bzr-svn/525571-minimal-atomic-write-bazaar-conf-files-2.1 into lp:bzr-svn/1.0

  1. 525571-minimal-atomic-write-bazaar-conf-files-2.1
  2. Merge into 1.0
Proposed by Vincent Ladeuil
Status: Merged
Merged at revision: 3326
Proposed branch: lp:~vila/bzr-svn/525571-minimal-atomic-write-bazaar-conf-files-2.1
Merge into: lp:bzr-svn/1.0
Diff against target: 42 lines (+10/-6)
2 files modified
NEWS (+3/-0)
config.py (+7/-6)
To merge this branch: bzr merge lp:~vila/bzr-svn/525571-minimal-atomic-write-bazaar-conf-files-2.1
Reviewer Review Type Date Requested Status
bzr-svn developers Pending
Review via email: mp+29439@code.launchpad.net

Commit message

Use atomic writes to protect config files against concurrent writers.

Description of the change

This patch should fix bug #525571 for lp.

https://code.edge.launchpad.net/~vila/bzr/525571-lock-bazaar-conf-files/+merge/28898
won't land soon but https://code.edge.launchpad.net/~vila/bzr/525571-minimal-atomic-write-bazaar-conf-files-2.1/+merge/29086 has already landed for bzr-2.1

The later is a minimal fix but needs to be done too in bzr-svn (which this patch does).

The former is a more correct/robust/whatever solution but lp:bzr-svn@3323 is not enough to use it.
The discussion I had with Jelmer that led to 3323 was based on a flawed design that didn't
require to use @needs_write_lock on set_user_option() methods (the correct design will break the backward compatibility for the 3323 fix :-/).

See the mp discussions mentioned above for the details, but in summary *this* patch:
- mimics the minimal fix landed in bzr-2.1,
- will fix the issue encountered on lp,
- doesn't require deploying a new bzr on lp,
- requires that a new bzr-svn is deployed on lp.

To post a comment you must log in.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2010-06-29 17:30:48 +0000
+++ NEWS 2010-07-08 07:21:42 +0000
@@ -55,6 +55,9 @@
55 * Cope with concurrent access to ~/.bazaar/subversion.conf. 55 * Cope with concurrent access to ~/.bazaar/subversion.conf.
56 (Vincent Ladeuil, Jelmer Vernooij, #525571)56 (Vincent Ladeuil, Jelmer Vernooij, #525571)
5757
58 * Concurrent processes don't mix up writes in config files (interim
59 fix for lp). (Vincent Ladeuil, #525571)
60
58 FEATURES61 FEATURES
5962
60 * Support fetching a limited number of revisions. (Michael Hudson)63 * Support fetching a limited number of revisions. (Michael Hudson)
6164
=== modified file 'config.py'
--- config.py 2010-06-29 17:30:48 +0000
+++ config.py 2010-07-08 07:21:42 +0000
@@ -1,4 +1,4 @@
1# Copyright (C) 2007-2009 Jelmer Vernooij <jelmer@samba.org>1# Copyright (C) 2007-2010 Jelmer Vernooij <jelmer@samba.org>
22
3# This program is free software; you can redistribute it and/or modify3# This program is free software; you can redistribute it and/or modify
4# it under the terms of the GNU General Public License as published by4# it under the terms of the GNU General Public License as published by
@@ -107,12 +107,13 @@
107 :param name: Name of the option.107 :param name: Name of the option.
108 :param value: Value of the option.108 :param value: Value of the option.
109 """109 """
110 conf_dir = os.path.dirname(self._get_filename())110 file_name = self._get_filename()
111 ensure_config_dir_exists(conf_dir)111 ensure_config_dir_exists(os.path.dirname(file_name))
112 atomic_file = atomicfile.AtomicFile(file_name)
112 self[name] = value113 self[name] = value
113 f = open(self._get_filename(), 'wb')114 self._get_parser().write(atomic_file)
114 self._get_parser().write(f)115 atomic_file.commit()
115 f.close()116 atomic_file.close()
116117
117118
118class SvnRepositoryConfig(Config):119class SvnRepositoryConfig(Config):

Subscribers

People subscribed via source and target branches