Patchwork [3,of,4] test-dirstate-race: ensure that a isn't in the lookup set at the end

login
register
mail settings
Submitter Katsunori FUJIWARA
Date June 12, 2017, 11:12 a.m.
Message ID <uziddxxw8.wl%foozy@lares.dti.ne.jp>
Download mbox | patch
Permalink /patch/21338/
State Not Applicable
Headers show

Comments

Katsunori FUJIWARA - June 12, 2017, 11:12 a.m.
At Mon, 12 Jun 2017 19:31:55 +0900,
FUJIWARA Katsunori wrote:
> 
> At Mon, 12 Jun 2017 19:04:28 +0900,
> FUJIWARA Katsunori wrote:
> > 
> > At Sun, 11 Jun 2017 22:24:20 -0700,
> > Siddharth Agarwal wrote:
> > > 
> > > On 6/11/17 10:21 PM, Martin von Zweigbergk via Mercurial-devel wrote:
> > > > Doesn't look like it's guaranteed to be unset. Running "run-tests.py
> > > > -j50 --runs-per-test=50 test-dirstate-race.t" seems to confirm that (2
> > > > of 5 failed for me). How do you rely on it in an upcoming patch?
> > > 
> > > 
> > > Interesting, I'm not seeing that on my Linux box. Could you drop this 
> > > for now? I think I can work around this problem.
> > 
> > Unfortunately, this can occur, if mtime of "a" is N, and _getfsnow()
> > returns N+1 or more (= "a" is written out at almost N+1 but N in
> > "int").
> > 
> > IMHO, "hg update" with --config debug.dirstate.delaywrite=2 should
> > avoid this problem.
> 
> Oops, sorry, this is incorrect, because this is the way to ENSURE
> mtime in dirstate :-<
> 
> IMHO, additional "touch -t YYYYmmddHHMM 'a'" and "hg status" with
> tests/fakedirstatewritetime.py in previous $TESTTMP/dirstaterace.sh
> can ensure "unset"-ness of mtime in dirstate in this
> test-dirstate-race.t case.
> 

FYI, changes below can always reproduce Martin's situation, in which
files are changed at N, but dirstate is written out N+1 or later in
sec.

And uncommenting two lines after "enabling below ..." line should fix
this issue.

After confirming that fakedirstatewritetime.py works as expected,
"--config debug.dirstate.delaywrite=2" can be omitted for efficiency :-)

========================================


> > 
> > > _______________________________________________
> > > Mercurial-devel mailing list
> > > Mercurial-devel@mercurial-scm.org
> > > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> > 
> > -- 
> > ----------------------------------------------------------------------
> > [FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> 
> -- 
> ----------------------------------------------------------------------
> [FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
>
Katsunori FUJIWARA - June 12, 2017, 11:51 a.m.
At Mon, 12 Jun 2017 20:12:39 +0900,
FUJIWARA Katsunori wrote:
> 
> At Mon, 12 Jun 2017 19:31:55 +0900,
> FUJIWARA Katsunori wrote:
> > 
> > At Mon, 12 Jun 2017 19:04:28 +0900,
> > FUJIWARA Katsunori wrote:
> > > 
> > > At Sun, 11 Jun 2017 22:24:20 -0700,
> > > Siddharth Agarwal wrote:
> > > > 
> > > > On 6/11/17 10:21 PM, Martin von Zweigbergk via Mercurial-devel wrote:
> > > > > Doesn't look like it's guaranteed to be unset. Running "run-tests.py
> > > > > -j50 --runs-per-test=50 test-dirstate-race.t" seems to confirm that (2
> > > > > of 5 failed for me). How do you rely on it in an upcoming patch?
> > > > 
> > > > 
> > > > Interesting, I'm not seeing that on my Linux box. Could you drop this 
> > > > for now? I think I can work around this problem.
> > > 
> > > Unfortunately, this can occur, if mtime of "a" is N, and _getfsnow()
> > > returns N+1 or more (= "a" is written out at almost N+1 but N in
> > > "int").
> > > 
> > > IMHO, "hg update" with --config debug.dirstate.delaywrite=2 should
> > > avoid this problem.
> > 
> > Oops, sorry, this is incorrect, because this is the way to ENSURE
> > mtime in dirstate :-<
> > 
> > IMHO, additional "touch -t YYYYmmddHHMM 'a'" and "hg status" with
> > tests/fakedirstatewritetime.py in previous $TESTTMP/dirstaterace.sh
> > can ensure "unset"-ness of mtime in dirstate in this
> > test-dirstate-race.t case.
> > 
> 
> FYI, changes below can always reproduce Martin's situation, in which
> files are changed at N, but dirstate is written out N+1 or later in
> sec.
> 
> And uncommenting two lines after "enabling below ..." line should fix
> this issue.
> 
> After confirming that fakedirstatewritetime.py works as expected,
> "--config debug.dirstate.delaywrite=2" can be omitted for efficiency :-)
> 
> ========================================
> diff --git a/tests/test-dirstate-race.t b/tests/test-dirstate-race.t
> --- a/tests/test-dirstate-race.t
> +++ b/tests/test-dirstate-race.t
> @@ -134,8 +134,21 @@ condition (see issue5584 for detail).
>    > # To reproduce similar situation easily and certainly, #2 and #3
>    > # are swapped.  "hg cat" below ensures #2 on "hg status" side.
>    > 
> -  > hg update -q -C 0
> +  > # "--config debug.dirstate.delaywrite=2" emulates that files are
> +  > # changed at N, but dirstate is written out N+1 or later in sec
> +  > hg update -q -C 0 --config debug.dirstate.delaywrite=2
>    > hg cat -r 1 b > b
> +  > 
> +  > #### enabling below can ensure that mtime of 'a' in dirstate is unset
> +  > # touch -t 200001010000 a
> +  > # hg status --config extensions.fakedirstatewritetime=$TESTDIR/fakedirstatewritetime.py
> +  > EOF
> +
> +  $ cat >> .hg/hgrc <<EOF
> +  > [fakedirstatewritetime]
> +  > # emulate invoking dirstate.write() via repo.status()
> +  > # at 2000-01-01 00:00
> +  > fakenow = 200001010000
>    > EOF
>  
>  "hg status" below should excludes "e", of which exec flag is set, for
> ========================================

Sorry for consecutive posting, but one more :-)

"hg status" with fakedirstatewritetime above causes "? b" output
before "skip updating dirstate: identity mismatch" message of outer
"hg status".

To avoid this meaningless output:

  - add "-m" option or so to "hg status" with fakedirstatewritetime, or
  - move "hg cat -r 1 b > b" after "hg status" with fakedirstatewritetime

(the former seems reasonable, in order to put commands together by
each purpose)

> 
> > > 
> > > > _______________________________________________
> > > > Mercurial-devel mailing list
> > > > Mercurial-devel@mercurial-scm.org
> > > > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> > > 
> > > -- 
> > > ----------------------------------------------------------------------
> > > [FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
> > > _______________________________________________
> > > Mercurial-devel mailing list
> > > Mercurial-devel@mercurial-scm.org
> > > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> > 
> > -- 
> > ----------------------------------------------------------------------
> > [FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
> > 
> 
> -- 
> ----------------------------------------------------------------------
> [FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
>
via Mercurial-devel - June 12, 2017, 1:34 p.m.
If Sid did want an unset timestamp in the dirstate (it sounded like he
didn't really need it), he could also use debugrebuilddirstate, right? I
think that always leaves the timestamp unset.

On Jun 12, 2017 4:52 AM, "FUJIWARA Katsunori" <foozy@lares.dti.ne.jp> wrote:

At Mon, 12 Jun 2017 20:12:39 +0900,
FUJIWARA Katsunori wrote:
>
> At Mon, 12 Jun 2017 19:31:55 +0900,
> FUJIWARA Katsunori wrote:
> >
> > At Mon, 12 Jun 2017 19:04:28 +0900,
> > FUJIWARA Katsunori wrote:
> > >
> > > At Sun, 11 Jun 2017 22:24:20 -0700,
> > > Siddharth Agarwal wrote:
> > > >
> > > > On 6/11/17 10:21 PM, Martin von Zweigbergk via Mercurial-devel
wrote:
> > > > > Doesn't look like it's guaranteed to be unset. Running
"run-tests.py
> > > > > -j50 --runs-per-test=50 test-dirstate-race.t" seems to confirm
that (2
> > > > > of 5 failed for me). How do you rely on it in an upcoming patch?
> > > >
> > > >
> > > > Interesting, I'm not seeing that on my Linux box. Could you drop
this
> > > > for now? I think I can work around this problem.
> > >
> > > Unfortunately, this can occur, if mtime of "a" is N, and _getfsnow()
> > > returns N+1 or more (= "a" is written out at almost N+1 but N in
> > > "int").
> > >
> > > IMHO, "hg update" with --config debug.dirstate.delaywrite=2 should
> > > avoid this problem.
> >
> > Oops, sorry, this is incorrect, because this is the way to ENSURE
> > mtime in dirstate :-<
> >
> > IMHO, additional "touch -t YYYYmmddHHMM 'a'" and "hg status" with
> > tests/fakedirstatewritetime.py in previous $TESTTMP/dirstaterace.sh
> > can ensure "unset"-ness of mtime in dirstate in this
> > test-dirstate-race.t case.
> >
>
> FYI, changes below can always reproduce Martin's situation, in which
> files are changed at N, but dirstate is written out N+1 or later in
> sec.
>
> And uncommenting two lines after "enabling below ..." line should fix
> this issue.
>
> After confirming that fakedirstatewritetime.py works as expected,
> "--config debug.dirstate.delaywrite=2" can be omitted for efficiency :-)
>
> ========================================
> diff --git a/tests/test-dirstate-race.t b/tests/test-dirstate-race.t
> --- a/tests/test-dirstate-race.t
> +++ b/tests/test-dirstate-race.t
> @@ -134,8 +134,21 @@ condition (see issue5584 for detail).
>    > # To reproduce similar situation easily and certainly, #2 and #3
>    > # are swapped.  "hg cat" below ensures #2 on "hg status" side.
>    >
> -  > hg update -q -C 0
> +  > # "--config debug.dirstate.delaywrite=2" emulates that files are
> +  > # changed at N, but dirstate is written out N+1 or later in sec
> +  > hg update -q -C 0 --config debug.dirstate.delaywrite=2
>    > hg cat -r 1 b > b
> +  >
> +  > #### enabling below can ensure that mtime of 'a' in dirstate is unset
> +  > # touch -t 200001010000 a
> +  > # hg status --config extensions.fakedirstatewritetime=$
TESTDIR/fakedirstatewritetime.py
> +  > EOF
> +
> +  $ cat >> .hg/hgrc <<EOF
> +  > [fakedirstatewritetime]
> +  > # emulate invoking dirstate.write() via repo.status()
> +  > # at 2000-01-01 00:00
> +  > fakenow = 200001010000
>    > EOF
>
>  "hg status" below should excludes "e", of which exec flag is set, for
> ========================================

Sorry for consecutive posting, but one more :-)

"hg status" with fakedirstatewritetime above causes "? b" output
before "skip updating dirstate: identity mismatch" message of outer
"hg status".

To avoid this meaningless output:

  - add "-m" option or so to "hg status" with fakedirstatewritetime, or
  - move "hg cat -r 1 b > b" after "hg status" with fakedirstatewritetime

(the former seems reasonable, in order to put commands together by
each purpose)

>
> > >
> > > > _______________________________________________
> > > > Mercurial-devel mailing list
> > > > Mercurial-devel@mercurial-scm.org
> > > > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> > >
> > > --
> > > ----------------------------------------------------------------------
> > > [FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
> > > _______________________________________________
> > > Mercurial-devel mailing list
> > > Mercurial-devel@mercurial-scm.org
> > > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> >
> > --
> > ----------------------------------------------------------------------
> > [FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
> >
>
> --
> ----------------------------------------------------------------------
> [FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
>

--
----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp

Patch

========================================
diff --git a/tests/test-dirstate-race.t b/tests/test-dirstate-race.t
--- a/tests/test-dirstate-race.t
+++ b/tests/test-dirstate-race.t
@@ -134,8 +134,21 @@  condition (see issue5584 for detail).
   > # To reproduce similar situation easily and certainly, #2 and #3
   > # are swapped.  "hg cat" below ensures #2 on "hg status" side.
   > 
-  > hg update -q -C 0
+  > # "--config debug.dirstate.delaywrite=2" emulates that files are
+  > # changed at N, but dirstate is written out N+1 or later in sec
+  > hg update -q -C 0 --config debug.dirstate.delaywrite=2
   > hg cat -r 1 b > b
+  > 
+  > #### enabling below can ensure that mtime of 'a' in dirstate is unset
+  > # touch -t 200001010000 a
+  > # hg status --config extensions.fakedirstatewritetime=$TESTDIR/fakedirstatewritetime.py
+  > EOF
+
+  $ cat >> .hg/hgrc <<EOF
+  > [fakedirstatewritetime]
+  > # emulate invoking dirstate.write() via repo.status()
+  > # at 2000-01-01 00:00
+  > fakenow = 200001010000
   > EOF
 
 "hg status" below should excludes "e", of which exec flag is set, for