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

# HG changeset patch
# User Lucas Moscovicz <lmoscovicz@fb.com>
# Date 1389639192 28800
#      Mon Jan 13 10:53:12 2014 -0800
# Node ID d648d25ecc9de3424cf843903a541e97800db37f
# Parent  f694cd81b600b65d23dcdc7a02cfd6a57dd1d018
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.

On Mon, 2014-01-13 at 11:44 -0800, Lucas Moscovicz wrote:
> # HG changeset patch
> # User Lucas Moscovicz <lmoscovicz@fb.com>
> # Date 1389639192 28800
> #      Mon Jan 13 10:53:12 2014 -0800
> # Node ID d648d25ecc9de3424cf843903a541e97800db37f
> # Parent  f694cd81b600b65d23dcdc7a02cfd6a57dd1d018
> 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.

This looks pretty good (and solves the precise issue of the bug report),
but one wonders if simply moving:

        if create:
            self._writerequirements()

up is a better strategy.

Think about it this way:

- you can't know how to read a repository before you've (attempted to)
read its requirements file
- a reader can come along at any point in the init process
- if you create ANY other file first, a reader can come along and read
that file before you've gotten to the requires file and potentially
misinterpret the repo contents

This is normally not a problem, but in the most pathological case, you
could have init race with commit.. and commit would create an
ancient-style check-in. Then the requires file would be written.. and
suddenly the ancient commit would be invisible because it's now a
new-style repo and commits belong somewhere else. In fact, our write of
the dummy 00changelog.i probably just overwrote the commit.

Thinking a bit further about this, it's actually a bit more complicated
because:

- an empty .hg/ is a valid old-style repository

So to be race-free on create, you need to create a temporary directory,
put the requires file in it, and then rename the directory to .hg/.




> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -206,12 +206,6 @@
>                          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)
> @@ -249,7 +243,13 @@
>          self._applyrequirements(requirements)
>          if create:
>              self._writerequirements()
> -
> +            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._branchcaches = {}
>          self.filterpats = {}
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel