| Message ID | CABKoBm2Fe=2+-hF2ODO_octQ6o1xWKeCa7jvocaoZMFSVkDK1Q@mail.gmail.com |
|---|---|
| State | Not Applicable |
| Headers | show |
On Tue, Oct 11, 2016 at 04:58:26PM -0700, Andy Zhou wrote: > On Wed, Oct 5, 2016 at 6:28 PM, Ben Pfaff <blp@ovn.org> wrote: > > > When I ran "make check-valgrind -j10" and the testsuite needed to be > > rebuilt, two copies of it were rebuilt in parallel and sometimes they > > raced with each other. I don't have the full story on exactly why this > > happened, but this commit, which eliminates redundant dependencies from > > check-* targets, fixes the problem for me. The dependencies are redundant > > because these targets depend on "all", which also depends on them. > > > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > > > I was not clear to me either why the race happened, but the change looks > good to me. > > Acked-by: Andy Zhou <azhou@ovn.org> > > There are two additional targets that have the similar pattern, wonder if > we should change them as well... You're right. I made those changes too. My confidence is somewhat shaky on this, but it's easy enough to revert if we missed something, so I applied this to master.
On Wed, Oct 12, 2016 at 10:42:36AM -0700, Ben Pfaff wrote: > On Tue, Oct 11, 2016 at 04:58:26PM -0700, Andy Zhou wrote: > > On Wed, Oct 5, 2016 at 6:28 PM, Ben Pfaff <blp@ovn.org> wrote: > > > > > When I ran "make check-valgrind -j10" and the testsuite needed to be > > > rebuilt, two copies of it were rebuilt in parallel and sometimes they > > > raced with each other. I don't have the full story on exactly why this > > > happened, but this commit, which eliminates redundant dependencies from > > > check-* targets, fixes the problem for me. The dependencies are redundant > > > because these targets depend on "all", which also depends on them. > > > > > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > > > > > > I was not clear to me either why the race happened, but the change looks > > good to me. > > > > Acked-by: Andy Zhou <azhou@ovn.org> > > > > There are two additional targets that have the similar pattern, wonder if > > we should change them as well... > > You're right. I made those changes too. > > My confidence is somewhat shaky on this, but it's easy enough to revert > if we missed something, so I applied this to master. Well, at least travis is happy: https://travis-ci.org/openvswitch/ovs/builds/167124458 So any bugs are more subtle.
diff --git a/tests/automake.mk b/tests/automake.mk index f022feb..c170ae7 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -223,8 +223,7 @@ check-valgrind: all $(valgrind_wrappers) $(check_DATA) @echo '----------------------------------------------------------------------' @echo 'Valgrind output can be found in tests/testsuite.dir/*/valgrind.*' @echo '----------------------------------------------------------------------' -check-helgrind: all tests/atconfig tests/atlocal $(TESTSUITE) \ - $(valgrind_wrappers) $(check_DATA) +check-helgrind: all $(valgrind_wrappers) $(check_DATA)