On Tue, Aug 6, 2013 at 4:31 PM, Brad King <brad.k...@kitware.com> wrote:
> On 08/06/2013 09:40 AM, Nicolas Desprès wrote:
> > I have force-pushed all the fixes you've asked for:
>
> Great, thanks!
>
> > * I taught cmMakefile copy-ctor
>
> Good.
>
> > * I re-write the test so that it is part of CMakeOnly sub-test suite. I
> also make it more robust to machine load by increasing the number of
> targets. This way the performance gap between the logarithmic version and
> the linear version is bigger. On my machine the logarithmic version run in
> 60sec
> > whereas the linear version had not finished after 10min. The timeout is
> at 90sec now. When running the whole test suite in parallel with 8 jobs it
> passes.
>
> Okay. Timing-dependent tests have been frequently sporadic on
> our test infrastructure because the machines are often busy
> with other projects' tests and there is a wide variety of
> machine speeds available. We'll see how it goes.
>
Sure.
>
> > * I added a test covering the issue I got while testing the previous
> buggy version of the optimized algorithm. This is a very weird bug. Master
> does not suffer from it, neither this topic branch. Basically the previous
> version had problem with empty dependency and it was not covered by the test
> > suite. I think there is something else involved in this test case
> because the test file named "0" is important too. At least the test is here
> now and we could investigate that later if it fails again.
>
> It looks like
>
> + add_test_macro(EmptyDepAndZeroOutput
> + ${CMAKE_CMAKE_COMMAND} -P check.cmake)
>
> appears twice, once in each test addition commit.
>
Oups. Squashing error. Fixed now.
>
> While hunting down all the call sites for GetSourceFileWithOutput
> I found two that are not needed and removed them:
>
> http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=eccb39d7
>
> Please rebase your topic on that.
>
> I still can't convince myself that the comparison function is
> correct. Also as Steve suggested it will be faster to use a
> standard ordering function. That should work if we always
> use it with full paths. The expected use cases always use
> full paths and it is only for backward compatibility that we
> support the path suffix magic. Therefore I think the best
> approach is to optimize the common (full-path) case with a
> direct lookup, perhaps even with "cmsys/hash_map.hxx". Only
> when the input path is relative should we fall back to the
> linear suffix search for compatibility.
Good idea! I should have done that in the first place it would have save me
a lot of effort understanding the old behavior.
> The test suite passes on my computer. I have force-pushed again.
However I had one problem with BootstrapTest when trying to use
cmsys::hash_map. I pushed by work so far on this topic in another branch (
https://github.com/nicolasdespres/CMake/commits/topic/use-cmsys-hash_map):
https://github.com/nicolasdespres/CMake/commit/1437d51a87b5a5a131b3829210710734266d29cc
I did not succeed to teach the bootstrap script how to include cstddef.
Cheers,
--
Nicolas Desprès
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
