[PATCH] resolve issue 3375: Operation not permitted: journal.phaseroots

Matt Mackall mpm at selenic.com
Tue Apr 17 10:50:36 CDT 2012


On Tue, 2012-04-17 at 10:08 -0400, Michael Bacarella wrote:
> # HG changeset patch
> # User Michael Bacarella <mbacarella at janestreet.com>
> # Date 1334671188 14400
> # Node ID a7b53829e08f290f5e9c02bfff19c6713fb33efb
> # Parent  3387c7dc83b1c94b7ac16ed7fa152d9b19e74efe
> localrepo: fix unpushable repos when transaction aborted (issue3375)
> 
> This fix is similar to the fix for issue3317.
> 
> This patch makes journal file copying more consistent.

On closer inspection, everything here is wrong and has always been
wrong.

There are two patterns here:
        
        try:
          data = opener.read()
        except IOError:
          data = ""
        opener.write(data)
        
and:

        if os.path.exists():
          util.copyfile()
        else:
          write("")
        
..and they're both wrong. The first will silently lose data when we get
an IOError other than ENOENT, for instance EPERM. If a transaction's
aborted or rolled back, the file will be lost, because write() will
actually -delete- the target file first to ensure success.

The second opens existing files and tries to write to them and/or change
their modes, which can fail if they have unfortunate modes and/or
ownership.

The first pattern was introduced by me about 7 years ago:

 http://www.selenic.com/hg/diff/e8eb427c6d71/mercurial/hg.py

The second pattern was introduced here:

  http://www.selenic.com/hg/rev/89e7d35e0ef0

Note that the code it deletes in the first chunk is following the first
pattern. Lesson: don't be clever when moving code around.

So what's the fix? The right fix.. is to use the first pattern but to
explicitly check for ENOENT. In fact, I suspect just about every
instance of "except IOError:" without inspecting the exception in the
code is a bug.

I'll spin up a proper patch for this, thanks.

-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list