Patchwork init: changed creation order of 00changelog and requires files (issue3960)

login
register
mail settings
Submitter Lucas Moscovicz
Date March 14, 2014, 11:52 p.m.
Message ID <6e4f55fbc42b4d06d1ef.1394841170@dev1037.prn2.facebook.com>
Download mbox | patch
Permalink /patch/3946/
State Changes Requested
Headers show

Comments

Lucas Moscovicz - March 14, 2014, 11:52 p.m.
# HG changeset patch
# User Lucas Moscovicz <lmoscovicz@fb.com>
# Date 1389639192 28800
#      Mon Jan 13 10:53:12 2014 -0800
# Node ID 6e4f55fbc42b4d06d1efd610d21851935dc90f26
# Parent  e87bd3485a07f4b231febef26c6797855a64af3d
init: changed creation order of 00changelog and requires files (issue3960)

When initializing a repo with hg init if another client cloned the repo after
the 00changelog file was created but before the requires file was created they
would get an error about unknown revlog format.  To fix it, I changed the
creation order of those files since the changelog file can be created at any
time during the initialization.

Also, to solve the race problem that came up where an old client commited
something to the repo before both the requires and the 00changelog file were
created and then the 00changelog file would overwrite that commit, the initial
name of the .hg folder was changed to .hgtmp, getting renamed back to .hg once
the requires and 00changelog files are created leaving no point in time where
an old client can commit to the new styled repo.
Siddharth Agarwal - March 18, 2014, 2:26 a.m.
On 03/14/2014 04:52 PM, Lucas Moscovicz wrote:
> Also, to solve the race problem that came up where an old client commited
> something to the repo before both the requires and the 00changelog file were
> created and then the 00changelog file would overwrite that commit, the initial
> name of the .hg folder was changed to .hgtmp, getting renamed back to .hg once
> the requires and 00changelog files are created leaving no point in time where
> an old client can commit to the new styled repo.

This bit should be in a followup patch.
Pierre-Yves David - March 18, 2014, 2:28 a.m.
ccing Lucas with an email address that he can actually reach

On 03/17/2014 07:26 PM, Siddharth Agarwal wrote:
> On 03/14/2014 04:52 PM, Lucas Moscovicz wrote:
>> Also, to solve the race problem that came up where an old client commited
>> something to the repo before both the requires and the 00changelog
>> file were
>> created and then the 00changelog file would overwrite that commit, the
>> initial
>> name of the .hg folder was changed to .hgtmp, getting renamed back to
>> .hg once
>> the requires and 00changelog files are created leaving no point in
>> time where
>> an old client can commit to the new styled repo.
>
> This bit should be in a followup patch.
Matt Mackall - March 18, 2014, 6:52 p.m.
On Mon, 2014-03-17 at 19:28 -0700, Pierre-Yves David wrote:
> ccing Lucas with an email address that he can actually reach
>
> On 03/17/2014 07:26 PM, Siddharth Agarwal wrote:
> > On 03/14/2014 04:52 PM, Lucas Moscovicz wrote:
> >> Also, to solve the race problem that came up where an old client commited
> >> something to the repo before both the requires and the 00changelog
> >> file were
> >> created and then the 00changelog file would overwrite that commit, the
> >> initial
> >> name of the .hg folder was changed to .hgtmp, getting renamed back to
> >> .hg once
> >> the requires and 00changelog files are created leaving no point in
> >> time where
> >> an old client can commit to the new styled repo.
> >
> > This bit should be in a followup patch.

This is all just a little bit too hairy. I think there are some nice
opportunities to tidy things up here, rather than make them more
complicated.
Pierre-Yves David - April 14, 2014, 4:20 a.m.
Changes seems to have been requested to this patches long time ago,

Shall we expect a new version of this patches?

On 03/18/2014 02:52 PM, Matt Mackall wrote:
> On Mon, 2014-03-17 at 19:28 -0700, Pierre-Yves David wrote:
>> ccing Lucas with an email address that he can actually reach
>>
>> On 03/17/2014 07:26 PM, Siddharth Agarwal wrote:
>>> On 03/14/2014 04:52 PM, Lucas Moscovicz wrote:
>>>> Also, to solve the race problem that came up where an old client commited
>>>> something to the repo before both the requires and the 00changelog
>>>> file were
>>>> created and then the 00changelog file would overwrite that commit, the
>>>> initial
>>>> name of the .hg folder was changed to .hgtmp, getting renamed back to
>>>> .hg once
>>>> the requires and 00changelog files are created leaving no point in
>>>> time where
>>>> an old client can commit to the new styled repo.
>>>
>>> This bit should be in a followup patch.
>
> This is all just a little bit too hairy. I think there are some nice
> opportunities to tidy things up here, rather than make them more
> complicated.
>

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -195,26 +195,37 @@  class localrepository(object):
 
         if not self.vfs.isdir():
             if create:
-                if not self.wvfs.exists():
-                    self.wvfs.makedirs()
-                self.vfs.makedir(notindexed=True)
-                requirements = self._baserequirements(create)
-                if self.ui.configbool('format', 'usestore', True):
-                    self.vfs.mkdir("store")
-                    requirements.append("store")
-                    if self.ui.configbool('format', 'usefncache', True):
-                        requirements.append("fncache")
-                        if self.ui.configbool('format', 'dotencode', True):
-                            requirements.append('dotencode')
-                    # create an invalid changelog
-                    self.vfs.append(
-                        "00changelog.i",
-                        '\0\0\0\2' # represents revlogv2
-                        ' dummy changelog to prevent using the old repo layout'
-                    )
-                if self.ui.configbool('format', 'generaldelta', False):
-                    requirements.append("generaldelta")
-                requirements = set(requirements)
+                oldvfs = self.vfs
+                try:
+                    self.vfs = scmutil.vfs(self.wvfs.join(".hgtmp"))
+                    self.opener = self.vfs
+                    if not self.wvfs.exists():
+                        self.wvfs.makedirs()
+                    self.vfs.makedir(notindexed=True)
+                    requirements = self._baserequirements(create)
+                    if self.ui.configbool('format', 'usestore', True):
+                        self.vfs.mkdir("store")
+                        requirements.append("store")
+                        if self.ui.configbool('format', 'usefncache', True):
+                            requirements.append("fncache")
+                            if self.ui.configbool('format', 'dotencode', True):
+                                requirements.append('dotencode')
+                    if self.ui.configbool('format', 'generaldelta', False):
+                        requirements.append("generaldelta")
+                    requirements = set(requirements)
+                    self._writerequirements(requirements)
+                    if self.ui.configbool('format', 'usestore', True):
+                        # create an invalid changelog
+                        self.vfs.append(
+                            "00changelog.i",
+                            '\0\0\0\2' # represents revlogv2
+                            ' dummy changelog to prevent using the old repo \
+                            layout'
+                        )
+                    self.wvfs.rename(".hgtmp", ".hg")
+                finally:
+                    self.vfs = oldvfs
+                    self.opener = oldvfs
             else:
                 raise error.RepoError(_("repository %s not found") % path)
         elif create:
@@ -247,9 +258,6 @@  class localrepository(object):
         self.sjoin = self.store.join
         self.vfs.createmode = self.store.createmode
         self._applyrequirements(requirements)
-        if create:
-            self._writerequirements()
-
 
         self._branchcaches = {}
         self.filterpats = {}
@@ -285,9 +293,11 @@  class localrepository(object):
         if chunkcachesize is not None:
             self.sopener.options['chunkcachesize'] = chunkcachesize
 
-    def _writerequirements(self):
+    def _writerequirements(self, requirements=None):
         reqfile = self.opener("requires", "w")
-        for r in sorted(self.requirements):
+        if requirements is None:
+            requirements = self.requirements
+        for r in sorted(requirements):
             reqfile.write("%s\n" % r)
         reqfile.close()