[PATCH] update: fix edge-case with update.atomic-file and read-only files
Martin von Zweigbergk
martinvonz at google.com
Fri Jan 11 12:24:41 EST 2019
On Fri, Jan 11, 2019 at 7:26 AM Boris Feld <boris.feld at octobus.net> wrote:
> # HG changeset patch
> # User Boris Feld <boris.feld at octobus.net>
> # Date 1547128621 -3600
> # Thu Jan 10 14:57:01 2019 +0100
> # Node ID 9d5ed4b088da0ece901b80c0fa40177cc6eaf676
> # Parent 963462786f6e028563bcedc9008622e0f3b59c86
> # EXP-Topic atomic-update-read-only
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> # hg pull https://bitbucket.org/octobus/mercurial-devel/ -r
> 9d5ed4b088da
> update: fix edge-case with update.atomic-file and read-only files
>
> We used to create the tempfile with the original file mode. That means
> creating a read-only tempfile when the original file is read-only, which
> crash
> if we need to write on the tempfile.
>
> The file in the working directory ends up being writable with and without
> the
> atomic update config, so the behavior is the same.
>
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -48,6 +48,7 @@ from . import (
> policy,
> pycompat,
> urllibcompat,
> + util
>
Delete.
> )
> from .utils import (
> procutil,
> @@ -2205,6 +2206,14 @@ class atomictempfile(object):
> self.__name = name # permanent name
> self._tempname = mktempcopy(name, emptyok=('w' in mode),
> createmode=createmode)
> +
> + # If we are gonna write on the tempfile, we need to make sure we
> have
> + # the permission to do so
> + if 'w' in mode and os.path.isfile(name):
> + oldstat = filestat.frompath(name)
> + newstat = oldstat.stat.st_mode | stat.S_IWUSR
> + os.chmod(self._tempname, newstat)
> +
> self._fp = posixfile(self._tempname, mode)
> self._checkambig = checkambig
>
> diff --git a/tests/test-update-atomic.t b/tests/test-update-atomic.t
> new file mode 100644
> --- /dev/null
> +++ b/tests/test-update-atomic.t
> @@ -0,0 +1,124 @@
> +Checking that experimental.atomic-file works.
> +
> + $ hg init .
> + $ cat > .hg/showwrites.py <<EOF
> + > def uisetup(ui):
> + > from mercurial import vfs
> + > class newvfs(vfs.vfs):
> + > def __call__(self, *args, **kwargs):
> + > print('vfs open', args, sorted(list(kwargs.items())))
> + > return super(newvfs, self).__call__(*args, **kwargs)
> + > vfs.vfs = newvfs
> + > EOF
> + $ for v in a1 a2 b1 b2 c ro; do echo $v > $v; done
> + $ chmod +x b*
>
I think you need "#require execbit" for this.
> + $ hg commit -Aqm _
> +
> +# We check that
> +# - the changes are actually atomic
> +# - that permissions are correct (all 4 cases of (executable before) *
> (executable after))
> +# - that renames work, though they should be atomic anyway
> +# - that it works when source files are read-only (but directories are
> read-write still)
> +
> + $ for v in a1 a2 b1 b2 ro; do echo changed-$v > $v; done
> + $ chmod -x *1; chmod +x *2
> + $ hg rename c d
> + $ hg commit -qm _
> +
> +Check behavior without update.atomic-file
> +
> + $ hg update -r 0 -q
> + $ hg update -r 1 --config extensions.showwrites=.hg/showwrites.py |&
> grep "a1'.*wb"
>
test-check-code.t fails:
@@ -15,6 +15,34 @@
Skipping i18n/polib.py it has no-che?k-code (glob)
Skipping mercurial/statprof.py it has no-che?k-code (glob)
Skipping tests/badserverext.py it has no-che?k-code (glob)
+ tests/test-update-atomic.t:31:
+ > $ hg update -r 1 --config extensions.showwrites=.hg/showwrites.py
|& grep "a1'.*wb"
+ don't use |&, use 2>&1
+ tests/test-update-atomic.t:34:
+ > $ find * -printf "%f:%p:%m\n"
+ don't use 'find -printf', it doesn't exist on BSD find(1)
+ tests/test-update-atomic.t:56:
+ > $ find * -printf "%f:%p:%m\n"
+ don't use 'find -printf', it doesn't exist on BSD find(1)
+ tests/test-update-atomic.t:68:
+ > $ find ro -printf "%f:%p:%m\n"
+ don't use 'find -printf', it doesn't exist on BSD find(1)
+ tests/test-update-atomic.t:76:
+ > $ find ro -printf "%f:%p:%m\n"
+ don't use 'find -printf', it doesn't exist on BSD find(1)
+ tests/test-update-atomic.t:90:
+ > $ hg update -r 1 --config extensions.showwrites=.hg/showwrites.py
|& grep "a1'.*wb"
+ don't use |&, use 2>&1
+ tests/test-update-atomic.t:101:
+ > $ find * -printf "%f:%p:%m\n"
+ don't use 'find -printf', it doesn't exist on BSD find(1)
+ tests/test-update-atomic.t:113:
+ > $ find ro -printf "%f:%p:%m\n"
+ don't use 'find -printf', it doesn't exist on BSD find(1)
+ tests/test-update-atomic.t:121:
+ > $ find ro -printf "%f:%p:%m\n"
+ don't use 'find -printf', it doesn't exist on BSD find(1)
+ [1]
> + ('vfs open', ('a1', 'wb'), [('atomictemp', False), ('backgroundclose',
> True)])
> +
> + $ find * -printf "%f:%p:%m\n"
> + a1:a1:644
> + a2:a2:755
> + b1:b1:644
> + b2:b2:755
> + d:d:644
> + ro:ro:644
> +
> +Add a second revision for the ro file so we can test update when the file
> is
> +present or not
> +
> + $ echo "ro" > ro
> +
> + $ hg commit -qm _
> +
> +Check behavior without update.atomic-file first
> +
> + $ hg update -C -r 0 -q
> +
> + $ hg update -r 1
> + 6 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +
> + $ find * -printf "%f:%p:%m\n"
> + a1:a1:644
> + a2:a2:755
> + b1:b1:644
> + b2:b2:755
> + d:d:644
> + ro:ro:644
> +
> +Manually reset the mode of the read-only file
>
Can we also have a test case where a read-only file is committed (so
updating to it should create it in read-only mode)?
> +
> + $ chmod a-w ro
> +
> + $ find ro -printf "%f:%p:%m\n"
> + ro:ro:444
> +
> +Now the file is present and read-only
>
Unlike most other comments, this one seems to be about the state *above*
the comment, not about what it's testing below. Could you make the comments
consistently say what the next few lines do instead of what the previous
few lines did?
+
> + $ hg up -r 2
> + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +
> + $ find ro -printf "%f:%p:%m\n"
> + ro:ro:644
> +
> +# The file which was read-only is now writable in the default behavior
> +
> +Check behavior with update.atomic-files
> +
> +
> + $ cat >> .hg/hgrc <<EOF
> + > [experimental]
> + > update.atomic-file = true
> + > EOF
> +
> + $ hg update -C -r 0 -q
> + $ hg update -r 1 --config extensions.showwrites=.hg/showwrites.py |&
> grep "a1'.*wb"
> + ('vfs open', ('a1', 'wb'), [('atomictemp', True), ('backgroundclose',
> True)])
> + $ hg st -A --rev 1
> + C a1
> + C a2
> + C b1
> + C b2
> + C d
> + C ro
> +
> +Check the file permission after update
> + $ find * -printf "%f:%p:%m\n"
> + a1:a1:644
> + a2:a2:755
> + b1:b1:644
> + b2:b2:755
> + d:d:644
> + ro:ro:644
> +
> +Manually reset the mode of the read-only file
> +
> + $ chmod a-w ro
> +
> + $ find ro -printf "%f:%p:%m\n"
> + ro:ro:444
> +
> +Now the file is present and read-only
> +
> + $ hg update -r 2
> + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +
> + $ find ro -printf "%f:%p:%m\n"
> + ro:ro:644
> +
> +# The behavior is the same as without atomic update
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20190111/5c72ef4c/attachment.html>
More information about the Mercurial-devel
mailing list