Skip to content
Snippets Groups Projects
Commit ed66ec39 authored by Katsunori FUJIWARA's avatar Katsunori FUJIWARA
Browse files

vfs: create copy at renaming to avoid file stat ambiguity if needed

In order to fix issue5418, bff5ccbe5ead made vfs.rename(checkambig=True)
omit advancing mtime of renamed file, if renamed file is owned by
another (EPERM is raised in this case).

But this omission causes rewinding mtime at restoration in such
situation, and makes avoiding file stat ambiguity difficult, because
ExactCacheValidationPlan assumes that mtime should be advanced, if a
file is changed in same ctime.

    https://www.mercurial-scm.org/wiki/ExactCacheValidationPlan

Ambiguity of file stat also requires issue5584 to be fixed with other
than file stat, but "hash of file", "generation ID" and so on were
already rejected ideas (please see original RFC linked from "Outline
of issue" in ExactCacheValidationPlan page).

This omission occurs:

  - only for non append-only files (dirstate, bookmarks, and phaseroots), and
  - only if previous transaction is rollbacked by another user

The latter means "sharing a repository clone via group permission".
This is reasonable usecase, but not ordinary for many users, IMHO.
"hg rollback" itself has been deprecated since Mercurial 2.7, too.

Therefore, increasing the cost at rollbacking previous transaction
executed by another a little seems reasonable, for avoidance of file
stat ambiguity.

This patch does:

  - create copy of (already renamed) source file, if advancing mtime
    fails for EPERM
  - rename from copied file to destination file, and
  - advance mtime of renamed file, which is now owned by current user

This patch also factors "self.join(src)" out to reduce redundancy.
parent 1a79de47
No related branches found
No related tags found
No related merge requests found
......@@ -174,6 +174,7 @@
only if destination file is guarded by any lock
(e.g. repo.lock or repo.wlock).
"""
srcpath = self.join(src)
dstpath = self.join(dst)
oldstat = checkambig and util.filestat(dstpath)
if oldstat and oldstat.stat:
......@@ -184,5 +185,9 @@
# stat of renamed file is ambiguous to original one
return ret, newstat.avoidambig(dpath, oldstat)
return ret, True
ret, avoided = dorename(self.join(src), dstpath)
ret, avoided = dorename(srcpath, dstpath)
if not avoided:
# simply copy to change owner of srcpath (see issue5418)
util.copyfile(dstpath, srcpath)
ret, avoided = dorename(srcpath, dstpath)
return ret
......@@ -188,5 +193,5 @@
return ret
return util.rename(self.join(src), dstpath)
return util.rename(srcpath, dstpath)
def readlink(self, path):
return os.readlink(self.join(path))
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment