[PATCH STABLE v2] chmod: create a new file when flags are set on a hardlinked file

[Yuya Nishihara]

On Sat, 6 May 2017 12:54:41 -0400, Augie Fackler wrote:
> On Wed, May 03, 2017 at 11:58:17PM -0700, Gregory Szorc wrote:
> > On Wed, May 3, 2017 at 2:36 AM, Koen Van Hoof <koen.van_hoof at nokia.com>
> > wrote:
> >
> > > # HG changeset patch
> > > # User Koen Van Hoof <koen.van_hoof at nokia.com>
> > > # Date 1493215522 -7200
> > > #      Wed Apr 26 16:05:22 2017 +0200
> > > # Branch stable
> > > # Node ID 04b33b356260d9d4c27f8cd223c7e5446a51ac48
> > > # Parent  2de67783dd31c66be1677aa443b95eabb0129a75
> > > chmod: create a new file when flags are set on a hardlinked file
> > >
> > > For performance reasons we have several repositories where the files in
> > > the working
> > > directory of 1 repo are hardlinks to the files of the other repo
> > > When an update in one repo results in a chmod of a such a file, the
> > > hardlink
> > > has to be deleted and replaced by a regular file to make sure that the
> > > change
> > > does not happen in the other repo
> > >
> > > diff --git a/mercurial/posix.py b/mercurial/posix.py
> > > --- a/mercurial/posix.py
> > > +++ b/mercurial/posix.py
> > > @@ -98,7 +98,8 @@ def isexec(f):
> > >      return (os.lstat(f).st_mode & 0o100 != 0)
> > >
> > >  def setflags(f, l, x):
> > > -    s = os.lstat(f).st_mode
> > > +    st = os.lstat(f)
> > > +    s = st.st_mode
> > >      if l:
> > >          if not stat.S_ISLNK(s):
> > >              # switch file to link
> > > @@ -124,6 +125,14 @@ def setflags(f, l, x):
> > >          fp.close()
> > >          s = 0o666 & ~umask # avoid restatting for chmod
> > >
> > > +    if st.st_nlink > 1: # the file is a hardlink, break the hardlink

Perhaps we should break the link only if bool(x) != bool(sx).

> > > +        fpin = open(f,"r")
> > > +        unlink(f)
> > > +        fpout = open(f, "w")
> > > +        fpout.write(fpin.read())
> > > +        fpin.close()
> > > +        fpout.close()
> > > +
> > >
> >
> > I'll let someone else comment on the semantics of the change. But this
> > should use context managers so file descriptors aren't leaked [until next
> > garbage collection] in case of an exception.
> 
> I'm generally okay with the semantics of the patch - it was surprising
> to me as a long-standing source control hacker that this
> misbehaves. Sorry I didn't notice the context manager bit in the last
> cycle.
> 
> (If anyone has objections to these semantics, please speak up.)

I think both old/new semantics are okay. The core Mercurial shouldn't be
affected by this change. We don't do atomic write on working-copy files,
so hardlinks in wdir would be a problem anyway.