<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Wed, Jul 8, 2015 at 1:15 AM FUJIWARA Katsunori <<a href="mailto:foozy@lares.dti.ne.jp">foozy@lares.dti.ne.jp</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"># HG changeset patch<br>
# User FUJIWARA Katsunori <<a href="mailto:foozy@lares.dti.ne.jp" target="_blank">foozy@lares.dti.ne.jp</a>><br>
# Date 1436342865 -32400<br>
# Wed Jul 08 17:07:45 2015 +0900<br>
# Node ID 17ca03748ee914f9696dcec623bc2bc3977d8c38<br>
# Parent a3f47d16a56c9c64f45b928749de85386b8a5dc9<br>
cmdutil: apply dirstate.normallookup on (maybe partially) committed files<br>
<br>
To detect change of a file without redundant comparison of file<br>
content, dirstate recognizes a file as certainly clean, if:<br>
<br>
(1) it is already known as "normal",<br>
(2) dirstate entry for it has valid (= not "-1") timestamp, and<br>
(3) mode, size and timestamp of it on the filesystem are as same as<br>
ones expected in dirstate<br>
<br>
This works as expected in many cases, but doesn't in the corner case<br>
that changing a file keeps mode, size and timestamp of it on the<br>
filesystem.<br>
<br>
The timetable below shows steps in one of typical such situations:<br>
<br>
---- ----------------------------------- ----------------<br>
timestamp of "f"<br>
----------------<br>
dirstate file-<br>
time action mem file system<br>
---- ----------------------------------- ---- ----- -----<br>
N *** ***<br>
- change "f" N<br>
<br>
- execute `hg commit -i`<br>
- backup "f" with timestamp N<br>
- revert "f" by `merge.update()` N<br>
with `partially`<br>
- apply selected hunks N<br>
by `patch.patch()`<br>
<br>
- `repo.commit()`<br>
- `dirstate.normal("f")` N<br>
N+1<br>
- `dirstate.write()` N N<br>
<br>
- restore "f" N+1<br>
- restore timestamp of "f" N<br>
<br>
- `hg status` shows "f" as "clean" N N N<br>
---- ----------------------------------- ---- ----- -----<br>
<br>
The most important point is that `dirstate.write()` is executed at N+1<br>
or later. This causes writing dirstate timestamp N of "f" out<br>
successfully. If it is executed at N, `parsers.pack_dirstate()`<br>
replaces timestamp N with "-1" before actual writing dirstate out.<br>
<br>
This issue can occur when `hg commit -i` satisfies conditions below:<br>
<br>
- the file is committed partially, and<br>
- mode and size of the file aren't changed before and after committing<br>
<br>
The root cause of this issue is that (maybe partially changed) files<br>
are restored with original timestamp but dirstate isn't updated for<br>
them.<br>
<br>
To detect changes of files correctly, this patch applies<br>
`dirstate.normallookup()` on restored files. Status check is needed<br>
before `dirstate.normallookup()`, because status other than "n(ormal)"<br>
should be kept at failure of committing.<br>
<br>
This patch doesn't examine whether each files are committed fully or<br>
partially, because interactive hunk selection makes it difficult.<br>
<br>
After this change, timetable is changed as below:<br>
<br>
---- ----------------------------------- ----------------<br>
timestamp of "f"<br>
----------------<br>
dirstate file-<br>
time action mem file system<br>
---- ----------------------------------- ---- ----- -----<br>
N *** ***<br>
- change "f" N<br>
<br>
- execute `hg commit -i`<br>
- backup "f" with timestamp N<br>
- revert "f" by `merge.update()` N<br>
with `partially`<br>
- apply selected hunks N<br>
by `patch.internalpatch()`<br>
<br>
- `repo.commit()`<br>
- `dirstate.normal("f")` N<br>
N+1<br>
- `dirstate.write()` N N<br>
<br>
- restore "f" N+1<br>
- restore timestamp of "f" N<br>
----------------------------------- ---- ----- -----<br>
- normallookup("f") -1<br>
- release wlock<br>
- `dirstate.write()` -1 -1 N<br>
----------------------------------- ---- ----- -----<br>
<br>
- `hg status` shows "f" as "clean" -1 -1 N<br>
---- ----------------------------------- ---- ----- -----<br>
<br>
To reproduce this issue in tests certainly, this patch emulates some<br>
timing critical actions as below:<br>
<br>
- change "f" at N<br>
<br>
`touch -t 200001010000` before command invocation changes mtime of<br>
"f" to "2000-01-01 00:00" (= N).<br>
<br>
- apply selected hunks at N<br>
<br>
`patch.internalpatch()` with `fakepatchtime.py` explicitly changes<br>
mtime of patched files to "2000-01-01 00:00" (= N).<br>
<br>
- `dirstate.write()` at N+1 (or "not at N")<br>
<br>
`pack_dirstate()` uses actual timestamp at runtime as "now", and<br>
it should be different from the "2000-01-01 00:00" of "f".<br>
<br>
BTW, in `test-commit-interactive.t`, files are sometimes treated as<br>
modified , even though they are just committed fully via `hg commit<br>
-i` and `hg diff` shows nothing for them.<br>
<br>
Enabling win32text causes EOL style mismatching below:<br>
<br>
- files are changed in LF style EOL<br>
<br>
=> files restored after committing uses LF style EOL (1)<br>
<br>
- `merge.update()` reverts files in CRLF style EOL<br>
- `patch.internalpatch()` changes files in CRLF style EOL<br>
<br>
=> `dirstate.normal()` via `repo.commit()` uses the size of files<br>
in CRLF style EOL (2)<br>
<br>
Therefore, fully committed files are treated as "modified", because<br>
`lstat()` returns size of (1) restored files in LF style EOL, but<br>
dirstate expects size of (2) committed files in CRLF style EOL.<br>
<br>
After this patch, `dirstate.normallookup()` on committed files forces<br>
subsequent `hg status` to examine changes exactly, and fully committed<br>
files are treated as clean as expected.<br>
<br>
This is reason why this patch also does:<br>
<br>
- add some `hg status` checking status of fully committed files<br>
- clear win32text configuration before size/timestamp sensitive examination<br>
<br>
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py<br>
--- a/mercurial/cmdutil.py<br>
+++ b/mercurial/cmdutil.py<br>
@@ -203,8 +203,16 @@<br>
finally:<br>
# 5. finally restore backed-up files<br>
try:<br>
+ dirstate = repo.dirstate<br>
for realname, tmpname in backups.iteritems():<br>
ui.debug('restoring %r to %r\n' % (tmpname, realname))<br>
+<br>
+ if dirstate[realname] == 'n':<br>
+ # without normallookup, restoring timestamp<br>
+ # may cause that partially committed files<br>
+ # aren't treated as modified<br>
+ dirstate.normallookup(realname)<br>
+<br></blockquote><div><br></div><div>Should this be done after writing the file as we do elsewhere? Maybe it doesn't matter since the file will get an old timestamp anyway (because of shutil.copystat)?</div><div><br></div><div>Besides that question, this series looks good to me. I haven't reviewed all of it, but I like the new direction of it.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
util.copyfile(tmpname, repo.wjoin(realname))<br>
# Our calls to copystat() here and above are a<br>
# hack to trick any editors that have f open that<br>
diff --git a/tests/test-commit-interactive.t b/tests/test-commit-interactive.t<br>
--- a/tests/test-commit-interactive.t<br>
+++ b/tests/test-commit-interactive.t<br>
@@ -1381,6 +1381,8 @@<br>
record this change to 'subdir/f1'? [Ynesfdaq?] y<br>
<br>
<br>
+ $ hg status -A subdir/f1<br>
+ C subdir/f1<br>
$ hg tip -p<br>
changeset: 28:* (glob)<br>
tag: tip<br>
@@ -1417,6 +1419,8 @@<br>
+e<br>
record this change to 'subdir/f1'? [Ynesfdaq?] y<br>
<br>
+ $ hg status -A subdir/f1<br>
+ C subdir/f1<br>
$ hg log --template '{author}\n' -l 1<br>
xyz<br>
$ HGUSER="test"<br>
@@ -1426,7 +1430,7 @@<br>
Moving files<br>
<br>
$ hg update -C .<br>
- 1 files updated, 0 files merged, 0 files removed, 0 files unresolved<br>
+ 0 files updated, 0 files merged, 0 files removed, 0 files unresolved<br>
$ hg mv plain plain3<br>
$ echo somechange >> plain3<br>
$ hg commit -i -d '23 0' -mmoving_files << EOF<br>
@@ -1447,6 +1451,8 @@<br>
record this change to 'plain3'? [Ynesfdaq?] y<br>
<br>
The #if execbit block above changes the hash here on some systems<br>
+ $ hg status -A plain3<br>
+ C plain3<br>
$ hg tip<br>
changeset: 30:* (glob)<br>
tag: tip<br>
@@ -1526,3 +1532,98 @@<br>
+foo<br>
<br>
$ cd ..<br>
+<br>
+ $ hg status -A folder/bar<br>
+ C folder/bar<br>
+<br>
+Clear win32text configuration before size/timestamp sensitive test<br>
+<br>
+ $ cat >> .hg/hgrc <<EOF<br>
+ > [extensions]<br>
+ > win32text = !<br>
+ > [decode]<br>
+ > ** = !<br>
+ > [encode]<br>
+ > ** = !<br>
+ > [patch]<br>
+ > eol = strict<br>
+ > EOF<br>
+ $ hg update -q -C null<br>
+ $ hg update -q -C tip<br>
+<br>
+Test that partially committed file is still treated as "modified",<br>
+even if none of mode, size and timestamp is changed on the filesystem<br>
+(see also issue4583).<br>
+<br>
+ $ cat > subdir/f1 <<EOF<br>
+ > A<br>
+ > a<br>
+ > a<br>
+ > b<br>
+ > c<br>
+ > d<br>
+ > E<br>
+ > EOF<br>
+ $ hg diff --git subdir/f1<br>
+ diff --git a/subdir/f1 b/subdir/f1<br>
+ --- a/subdir/f1<br>
+ +++ b/subdir/f1<br>
+ @@ -1,7 +1,7 @@<br>
+ -a<br>
+ +A<br>
+ a<br>
+ a<br>
+ b<br>
+ c<br>
+ d<br>
+ -e<br>
+ +E<br>
+<br>
+ $ touch -t 200001010000 subdir/f1<br>
+<br>
+ $ cat >> .hg/hgrc <<EOF<br>
+ > # emulate invoking patch.internalpatch() at 2000-01-01 00:00<br>
+ > [fakepatchtime]<br>
+ > fakenow = 200001010000<br>
+ ><br>
+ > [extensions]<br>
+ > fakepatchtime = $TESTDIR/fakepatchtime.py<br>
+ > EOF<br>
+ $ hg commit -i -m 'commit subdir/f1 partially' <<EOF<br>
+ > y<br>
+ > y<br>
+ > n<br>
+ > EOF<br>
+ diff --git a/subdir/f1 b/subdir/f1<br>
+ 2 hunks, 2 lines changed<br>
+ examine changes to 'subdir/f1'? [Ynesfdaq?] y<br>
+<br>
+ @@ -1,6 +1,6 @@<br>
+ -a<br>
+ +A<br>
+ a<br>
+ a<br>
+ b<br>
+ c<br>
+ d<br>
+ record change 1/2 to 'subdir/f1'? [Ynesfdaq?] y<br>
+<br>
+ @@ -2,6 +2,6 @@<br>
+ a<br>
+ a<br>
+ b<br>
+ c<br>
+ d<br>
+ -e<br>
+ +E<br>
+ record change 2/2 to 'subdir/f1'? [Ynesfdaq?] n<br>
+<br>
+ $ cat >> .hg/hgrc <<EOF<br>
+ > [extensions]<br>
+ > fakepatchtime = !<br>
+ > EOF<br>
+<br>
+ $ hg debugstate | grep ' subdir/f1$'<br>
+ n 0 -1 unset subdir/f1<br>
+ $ hg status -A subdir/f1<br>
+ M subdir/f1<br>
_______________________________________________<br>
Mercurial-devel mailing list<br>
<a href="mailto:Mercurial-devel@selenic.com" target="_blank">Mercurial-devel@selenic.com</a><br>
<a href="https://selenic.com/mailman/listinfo/mercurial-devel" rel="noreferrer" target="_blank">https://selenic.com/mailman/listinfo/mercurial-devel</a><br>
</blockquote></div></div>