[cmake-developers] [CMake 0012589]: add_test() BROKEN: docs say target allowed, yet fails to evaluate properties (e.g. custom CMAKE_RUNTIME_OUTPUT_DIRECTORY)
The following issue has been SUBMITTED.
==
http://www.itk.org/Bug/view.php?id=12589
==
Reported By:Andreas Mohr
Assigned To:
==
Project:CMake
Issue ID: 12589
Category: CMake
Reproducibility:always
Severity: major
Priority: normal
Status: new
==
Date Submitted: 2011-11-23 05:15 EST
Last Modified: 2011-11-23 05:15 EST
==
Summary:add_test() BROKEN: docs say target allowed, yet
fails to evaluate properties (e.g. custom CMAKE_RUNTIME_OUTPUT_DIRECTORY)
Description:
[includes working/failing test case below]
At least my local CMake 2.8.5 SVN has a weird (IOW, broken) add_test()
which will result in CTest not being able to locate the test's binary,
despite add_test() docs (even in 2.8.2, but not yet in 2.6.4) stating that it
_is_ ok to simply pass the target as the executable argument to add_test().
And since aggregated, complete CMake configuration environment knowledge is put
to use to create a CTest configuration,
generically spoken a generator _SHOULD_ always use _ALL_ its implicit
knowledge about its objects (targets) to fill in _ALL_ attributes
required within the dumb generated file (CTest script).
If this crucial target-based mechanism does not work, then we could just as well
give up all the built-in automatic configuration elegance that CMake is supposed
to offer and code everything by hand instead ;-P
I'm sufficiently puzzled that this is even failing at all, since from an
object-oriented POV, it should be obvious that when querying a target object
about its prospective location, this location should _always_ be correctly
returned (and this does include properly obeying CMAKE_RUNTIME_OUTPUT_DIRECTORY
within the target object implementation), completely irrespective of whether
this target location request is being made by add_test() implementation or
somewhere else.
I suspect that CMake upstream might even unit test this feature,
but since CTest automatically searches various output paths
(Debug, Release, etc.) in case the binary cannot be found, a test would have
false-negatively worked in all cases where there has been no custom output path
specified (we're using a custom bin/!).
Thanks a ton!
Steps to Reproduce:
cmake_minimum_required(VERSION 2.8)
project(bug_add_test)
file(WRITE main.cpp
int main()
{
return 0;
}
)
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
add_executable(bug_add_test main.cpp)
function(hookup_test _target)
set(testname_ ${_target})
set(enable_workaround false) # TOGGLE THIS LINE!
if(enable_workaround)
# Work around incapable add_test(), by using a property which
# is said to be deprecated... (*** SIGH ***).
get_property(test_location_ TARGET ${_target} PROPERTY LOCATION)
set(testexe_ ${test_location_})
else(enable_workaround)
set(testexe_ ${_target})
endif(enable_workaround)
add_test(${testname_} ${testexe_})
endfunction(hookup_test _target)
# enable_testing() cannot be called within function scope,
# _needs_ to be done in root (~ dir).
enable_testing()
hookup_test(bug_add_test)
==
Issue History
Date ModifiedUsername FieldChange
==
2011-11-23 05:15 Andreas Mohr New Issue
==
--
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
Re: [cmake-developers] [PATCH 0/3] The CMake Ninja generator.
2011/11/22 Peter Collingbourne pe...@pcc.me.uk On Fri, Nov 18, 2011 at 10:02:54AM +0100, Nicolas Desprès wrote: On Wed, Nov 16, 2011 at 3:05 PM, OKUMURA Yuki m...@cltn.org wrote: (Sorry Bill, i repost here..) 2011/11/16 Bill Hoffman bill.hoff...@kitware.com: - snip - What type of evolution will Ninja need? I suppose it could use cmake scripts and exectue_process which can do type things pretty easy. Why not insert cmd /c everywhere? :) Anyway, i had tried this in early this month, https://github.com/okuoku/CMake/commit/db19df03606684f313c9e86b313218feee9cce3f but it didn't work well. How so? Was there an escaping issue? The cmd.exe documentation seems to indicate that this will work. http://www.microsoft.com/resources/documentation/windows/xp/all/proddocs/en-us/ntcmds_shelloverview.mspx#EBG I think almost all use of is just chdir /d $workdir compile/link-command. So, adding $workdir specifier to target syntax of Ninja might work for many cases. That's right. I just had a look at the build.ninja file generated by the Ninja Generator and it actually uses only for changing the working directory. I will suggest a patch to Ninja to support working directory for each build statement. That's not the only thing that the Ninja generator uses for. It also uses to prepend/append PRE_BUILD, PRE_LINK and POST_BUILD commands to the link command, and to concatenate multiple custom command lines. True. I realized that. Actually, it seems that cmd.exe have some sort of support for and || so maybe it would work. I have to try. 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
Re: [cmake-developers] slow regex implementation in RegularExpression
On 11/22/2011 4:39 PM, Brad King wrote: It is tempting to always require explicit requests for new TRE behavior, such as using TRE instead of REGEX in keyword locations, but one advantage of using a policy is that over time the old behavior will disappear completely from usage. I am pretty sure the last time we talked about adding a new regex we talked about requiring explicit requests. I think this would be a much safer approach. I am really scared that this regex will not be compatible with the old one, and it will break lots of stuff in very subtle ways that are hard for people to detect. It is not that much code to have both. Where performance is an issue, we can swap it out, and when people need better regex they can use TRE as well. I don't think the pain will be worth getting rid of the old usage. -Bill -- 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
Re: [cmake-developers] slow regex implementation in RegularExpression
On Wed, Nov 23, 2011 at 8:36 AM, Bill Hoffman bill.hoff...@kitware.comwrote: On 11/22/2011 4:39 PM, Brad King wrote: It is tempting to always require explicit requests for new TRE behavior, such as using TRE instead of REGEX in keyword locations, but one advantage of using a policy is that over time the old behavior will disappear completely from usage. I am pretty sure the last time we talked about adding a new regex we talked about requiring explicit requests. I think this would be a much safer approach. I am really scared that this regex will not be compatible with the old one, and it will break lots of stuff in very subtle ways that are hard for people to detect. It is not that much code to have both. Where performance is an issue, we can swap it out, and when people need better regex they can use TRE as well. I don't think the pain will be worth getting rid of the old usage. -Bill -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/** opensource/opensource.htmlhttp://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/**CMake_FAQhttp://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-**bin/mailman/listinfo/cmake-**developershttp://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers Why can't this be solved with a policy? One problem of using an explicit TRE command is that if you want to write code that *could* be used in an older version of CMake you won't be able to use it. I agree that making the usage explicit would allow for complete backward compatibility, but it clutters the language. Do you want to have two versions of every regular expression syntax? GLOB vs GLOB_TRE? MATCHES vs MATCHES_TRE? Another argument against using TRE explicitly is these words tell me nothing about what that function does unless I'm extremely familiar with the intricacies of CMake script. I think we want to make CMake easier to use, not harder. James -- 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
Re: [cmake-developers] slow regex implementation in RegularExpression
On 11/23/2011 11:43 AM, James Bigler wrote: Why can't this be solved with a policy? One problem of using an explicit TRE command is that if you want to write code that *could* be used in an older version of CMake you won't be able to use it. It could be, but that will not come without pain. I can see a future were this policy kept off in many projects. I agree that making the usage explicit would allow for complete backward compatibility, but it clutters the language. Do you want to have two versions of evegry regular expression syntax? GLOB vs GLOB_TRE? MATCHES vs MATCHES_TRE? Another argument against using TRE explicitly is these words tell me nothing about what that function does unless I'm extremely familiar with the intricacies of CMake script. I think we want to make CMake easier to use, not harder. For many folks the current regex is just fine, so they will not have to do anything. Only the people that have hit some sort of wall with the regex will need to change. If we have a policy, I can see projects changing the minimum required version and having a subtle bug show up because the regex changed. I don' think we even know what all the differences are at this point... -Bill -- 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
Re: [cmake-developers] slow regex implementation in RegularExpression
On 11/23/2011 12:06 PM, Bill Hoffman wrote: On 11/23/2011 11:43 AM, James Bigler wrote: Why can't this be solved with a policy? One problem of using an explicit TRE command is that if you want to write code that *could* be used in an older version of CMake you won't be able to use it. It could be, but that will not come without pain. I can see a future were this policy kept off in many projects. We can wrap both the TRE and KWSys regex implementations in a cmRegularExpression that provides the same interface for both. Then we can implement the policy at that level. If the policy is set to OLD, use the KWSys implementation. If the policy is set to NEW, use the TRE implementation. If the policy is not set, then do *both* implementations and compare the results of matching. If they are different warn about the policy not being set and favor the KWSys result for compatibility. I don't think we even know what all the differences are at this point... The above approach would reveal differences without breaking projects. Alex C. should produce some unit tests for the existing behavior which we should have had all along. Even though we probably cannot think of everything every project is doing at least we will know up front if something major is different. Policies are meant for use when the new behavior is clearly better than the old behavior in every way and should have been the way it was always done had it been available from the beginning. This is such a case. We are not switching between two desirable behaviors. The only argument in favor of the old behavior is that is how it works now. -Brad -- 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
Re: [cmake-developers] slow regex implementation in RegularExpression
Hi Bill,
On 2011-11-23, at 10:36 AM, Bill Hoffman wrote:
I am pretty sure the last time we talked about adding a new regex we talked
about requiring explicit requests. I think this would be a much safer
approach. I am really scared that this regex will not be compatible with the
old one, and it will break lots of stuff in very subtle ways that are hard
for people to detect. It is not that much code to have both. Where
performance is an issue, we can swap it out, and when people need better
regex they can use TRE as well. I don't think the pain will be worth getting
rid of the old usage.
I agree with you on multiple points here.
[1]
Your intuition is right. Just minutes before reading your email I tried to
compile ITK using CMake+TRE. And there was at least one regex that TRE refused
to compile.
So yes, we cannot use TRE for *all* regex needs in CMake.
[2]
It is not that much code to have both.
I agree, and I think Brad King does too.
The patch I submitted does exactly that:
- old regex code stays unchanged
--- Source/kwsys/RegularExpression.{cxx,hxx.in}
- new TRE regex code added
--- Utilities/cmtre
So, yes, we intend to keep both.
[3]
I think there's a simple solution to our problem. I think we can:
- solve the performance problem
- keep the old behaviour, i.e. not break any projects out there
The solution is to use the old regex code everywhere, except for the very
specific place where it causes problems.
By looking at the bug report for 12381
(http://public.kitware.com/Bug/view.php?id=12381), you can see that the only
place we have to use TRE is:
Source/CTest/cmCTestBuildHandler.cxx
What cmCTestBuildHandler does is very simple: for every single line of output
from the build process it tries to match about 100 regular expressions, in
order to find error or warning messages. These 100 regular expressions are
defined in the same file in four static arrays, that look something like this:
static const char* cmCTestErrorMatches[] = {
^[Bb]us [Ee]rror,
^[Ss]egmentation [Vv]iolation,
^[Ss]egmentation [Ff]ault,
:.*[Pp]ermission [Dd]enied,
([^ :]+):([0-9]+): ([^ \\t]),
([^:]+): error[ \\t]*[0-9]+[ \\t]*:,
// AND SO ON …
};
// + other 3 arrays
In this case, it is safe to use TRE, because we (the CMake developers) write
these regular expressions, and we can make sure they work with TRE.
All other regular expressions, including those written by users in their
CMakeList.txt files will run on the old regex code, and thus behave normally.
CONCLUSION:
- we can keep old behaviour and solve the performance problem
- solution in part [3]
If this solution is acceptable, I'll have to recreate the patch.
sincerely,
Alex Ciobanu--
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
Re: [cmake-developers] slow regex implementation in RegularExpression
On 11/23/2011 12:20 PM, Alexandru Ciobanu wrote: to compile ITK using CMake+TRE. And there was at least one regex that TRE refused to compile. What was it, and where in the ITK code is it? Thanks, -Brad -- 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
Re: [cmake-developers] slow regex implementation in RegularExpression
On 2011-11-23, at 12:24 PM, Brad King wrote: On 11/23/2011 12:20 PM, Alexandru Ciobanu wrote: to compile ITK using CMake+TRE. And there was at least one regex that TRE refused to compile. What was it, and where in the ITK code is it? The regex in question is: ^[^][:/*?]+\$ And it appears at this location in the ITK source tree: CMake/ExternalData.cmake:347 And the expression is correct, because you're allowed to have the ] metacharacter inside a [^xyz] class if it comes immediately after ^. TRE does not do it the same way, see (http://laurikari.net/tre/documentation/regex-syntax/ the Bracket expressions section): To include a literal ] in the list, make it either the first item, the second endpoint of a range, or enclose it in [. and.]. sincerely, Alex-- 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
Re: [cmake-developers] slow regex implementation in RegularExpression
On 11/23/2011 12:34 PM, Alexandru Ciobanu wrote: The regex in question is: ^[^][:/*?]+\$ And it appears at this location in the ITK source tree: CMake/ExternalData.cmake:347 And the expression is correct, because you're allowed to have the ] metacharacter inside a [^xyz] class if it comes immediately after ^. Ironically I was the one that wrote that regex ;) TRE does not do it the same way, see (http://laurikari.net/tre/documentation/regex-syntax/ the Bracket expressions section): Interesting, thanks. -Brad -- 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
Re: [cmake-developers] slow regex implementation in RegularExpression
On 11/23/2011 12:43 PM, Brad King wrote: On 11/23/2011 12:34 PM, Alexandru Ciobanu wrote: The regex in question is: ^[^][:/*?]+\$ And it appears at this location in the ITK source tree: CMake/ExternalData.cmake:347 And the expression is correct, because you're allowed to have the ] metacharacter inside a [^xyz] class if it comes immediately after ^. Ironically I was the one that wrote that regex ;) TRE does not do it the same way, see (http://laurikari.net/tre/documentation/regex-syntax/ the Bracket expressions section): Wait, that documentation does say the same thing: bracket-expression ::= [ item+ ] | [^ item+ ] To include a literal ] in the list, make it either the first item That's exactly what this regex does. It uses the second production rule in the above grammar fragment and puts the ']' first after '^'. -Brad -- 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
Re: [cmake-developers] slow regex implementation in RegularExpression
On 11/23/2011 12:48 PM, Brad King wrote: On 11/23/2011 12:43 PM, Brad King wrote: On 11/23/2011 12:34 PM, Alexandru Ciobanu wrote: The regex in question is: ^[^][:/*?]+\$ To include a literal ] in the list, make it either the first item It must be the [: in this regex that TRE sees as special since it allows expressions like [:digit:] inside a bracket expression. Still, this is a case that my proposed policy would pick up. -Brad -- 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
Re: [cmake-developers] slow regex implementation in RegularExpression
On 11/23/2011 12:51 PM, Brad King wrote: On 11/23/2011 12:48 PM, Brad King wrote: On 11/23/2011 12:43 PM, Brad King wrote: On 11/23/2011 12:34 PM, Alexandru Ciobanu wrote: The regex in question is: ^[^][:/*?]+\$ To include a literal ] in the list, make it either the first item It must be the [: in this regex that TRE sees as special since it allows expressions like [:digit:] inside a bracket expression. Still, this is a case that my proposed policy would pick up. -Brad I am still very wary about this policy. For 99% of folks the current regex is just fine. Making them eventually change to get the new regex is making them do work that they don't need or want. I would rather have two API's. I just don't see the big upside of TRE, and I see this causing pain for lots and lots of folks if we push them to make the change. CMake has most likely 100,000 or more users at this point. A change like this could easily inflict a man years of effort onto the world, and should not be taken lightly. -Bill -- 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
Re: [cmake-developers] slow regex implementation in RegularExpression
On Wed, Nov 23, 2011 at 2:03 PM, Bill Hoffman bill.hoff...@kitware.com wrote: On 11/23/2011 12:51 PM, Brad King wrote: On 11/23/2011 12:48 PM, Brad King wrote: On 11/23/2011 12:43 PM, Brad King wrote: On 11/23/2011 12:34 PM, Alexandru Ciobanu wrote: The regex in question is: ^[^][:/*?]+\$ To include a literal ] in the list, make it either the first item It must be the [: in this regex that TRE sees as special since it allows expressions like [:digit:] inside a bracket expression. Still, this is a case that my proposed policy would pick up. -Brad I am still very wary about this policy. For 99% of folks the current regex is just fine. Making them eventually change to get the new regex is making them do work that they don't need or want. I would rather have two API's. I just don't see the big upside of TRE, and I see this causing pain for lots and lots of folks if we push them to make the change. CMake has most likely 100,000 or more users at this point. A change like this could easily inflict a man years of effort onto the world, and should not be taken lightly. -Bill -- 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 Big upside:(quoting from Alexandru Ciobanu's email of Nov. 17th earlier in this thread) The impact on the build time is pretty dramatic: CMake: 7h39m CMake + TRE: 1h06m -- 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
Re: [cmake-developers] slow regex implementation in RegularExpression
On Wed, Nov 23, 2011 at 2:03 PM, Bill Hoffman bill.hoff...@kitware.com wrote: On 11/23/2011 12:51 PM, Brad King wrote: On 11/23/2011 12:48 PM, Brad King wrote: On 11/23/2011 12:43 PM, Brad King wrote: On 11/23/2011 12:34 PM, Alexandru Ciobanu wrote: The regex in question is: ^[^][:/*?]+\$ To include a literal ] in the list, make it either the first item It must be the [: in this regex that TRE sees as special since it allows expressions like [:digit:] inside a bracket expression. Still, this is a case that my proposed policy would pick up. -Brad I am still very wary about this policy. For 99% of folks the current regex is just fine. Making them eventually change to get the new regex is making them do work that they don't need or want. I would rather have two API's. I just don't see the big upside of TRE, and I see this causing pain for lots and lots of folks if we push them to make the change. CMake has most likely 100,000 or more users at this point. A change like this could easily inflict a man years of effort onto the world, and should not be taken lightly. Couldn't they defer by setting the policy to OLD? If they bump the minimum version the user knows that backward incompatible changes may be introduced. If there was a way to verify that the output of the regex were the same with both implementations too, that should reduce the possibility of subtle bugs. Marcus -- 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
Re: [cmake-developers] slow regex implementation in RegularExpression
On Wed, Nov 23, 2011 at 2:09 PM, David Cole david.c...@kitware.com wrote: On Wed, Nov 23, 2011 at 2:03 PM, Bill Hoffman bill.hoff...@kitware.com wrote: On 11/23/2011 12:51 PM, Brad King wrote: On 11/23/2011 12:48 PM, Brad King wrote: On 11/23/2011 12:43 PM, Brad King wrote: On 11/23/2011 12:34 PM, Alexandru Ciobanu wrote: The regex in question is: ^[^][:/*?]+\$ To include a literal ] in the list, make it either the first item It must be the [: in this regex that TRE sees as special since it allows expressions like [:digit:] inside a bracket expression. Still, this is a case that my proposed policy would pick up. -Brad I am still very wary about this policy. For 99% of folks the current regex is just fine. Making them eventually change to get the new regex is making them do work that they don't need or want. I would rather have two API's. I just don't see the big upside of TRE, and I see this causing pain for lots and lots of folks if we push them to make the change. CMake has most likely 100,000 or more users at this point. A change like this could easily inflict a man years of effort onto the world, and should not be taken lightly. -Bill -- 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 Big upside: (quoting from Alexandru Ciobanu's email of Nov. 17th earlier in this thread) The impact on the build time is pretty dramatic: CMake: 7h39m CMake + TRE: 1h06m And although there is a big upside, we do still have to be careful. We have to remember that regexes are used in the context of ctest -D invocations, ctest -S script running and cmake -P running, too, where policies are not really a reliable mechanism. So in addition to having a careful policy, we also have to decide what to do in those cases. The case that is in question here for the big performance gain is ctest running and filtering build output based on regexes. No cmake policy mechanism in sight for that scenario. -- 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
Re: [cmake-developers] slow regex implementation in RegularExpression
On Wed, Nov 23, 2011 at 3:24 PM, Sean McBride s...@rogue-research.com wrote: On Wed, 23 Nov 2011 14:03:20 -0500, Bill Hoffman said: For 99% of folks the current regex is just fine. AFAICT, this performance bug affects 100% of Xcode generator users. Even looking at CMake's dashboard, you can see the difference, just search it for 'xcode'. ex: 100% of Xcode users that use ctest to build. I would still put that in the 1% of those that build with CMake. A build with CMake does not go via ctest, but is built on the command line or in the IDE Xcode which will not have the regex slow down. Changing just ctest somehow is a much smaller scope than changing every regex in all of CMake. I stand by my 99% are OK with the regex that they have. :) -Bill -- 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
Re: [cmake-developers] slow regex implementation in RegularExpression
On 11/23/2011 5:43 PM, Brad King wrote: On 11/23/2011 12:44 PM, Brad King wrote: However, the above does not need to stand in the way of solving the problem you're addressing. We can simply set that goal aside for now by not exposing TRE in the CMake language anywhere. Use it just for cmCTestBuildHandler. but people kept going on the above part of the debate ;) After some more thought, I've realized that no approach currently proposed is practical: - cmCTestBuildHandler can use a list of custom regular expressions so we cannot assume all of them will be compatible with TRE - As David Cole pointed out there are many places, like CTest's -R and -E options, that use regular expressions in contexts where we cannot possibly use a policy. Any attempt to do so in such places would just turn into a second API to set the policy in the local context of the regex. - If we add a second API like MATCHES = MATCHES_TRE then we would eventually need to do that in *every* place that offers regex matching. That would mean alternatives to the above -R and -E options and a lot more. - People could write code that passes a regex around in a variable. This would hide from the author of the regex the context in which it will be used, so it is unknown whether it is TRE or traditional. I propose we go back to an approach discussed the first time PCRE was proposed. The indication of the type of regex must be in the regex itself. IIRC the proposal was something like REGEX:...# old PCRE:... # PCRE Of course that is ambiguous too because the prefixes are valid expressions. Instead we can use a prefix that is not otherwise a valid expression. We can use an idea from Python: http://docs.python.org/library/re.html that defines expressions of the form (?...) which are not otherwise valid. In order to avoid conflict with future use of the constructs they define, we can use the comment form Python defines: (?#OLD)... # old (?#TRE)... # TRE This is quite easy to implement. Just take the currently proposed patch that replaces use of cmsys::RegularExpression with the new cmFastRegularExpression wrapper (perhaps renamed cmRegularExpression). Inside the wrapper look for a leading comment of the above form to decide which regex impl to use internally. Then strip off the prefix and pass the rest of the regex to the underlying implementation. Once this is done update all the default warning and error regular expressions that CTest uses. Add the (?#TRE) prefix to them. This approach will solve the speed problem, give people access to the TRE extended features when they want it anywhere CMake already uses a regex, has no compatibility problems, is a very narrow second interface, and is extensible for future optional regex behavior. -Brad -- 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
Re: [cmake-developers] slow regex implementation in RegularExpression
On 11/24/2011 12:34 AM, Brad King wrote: On 11/23/2011 5:43 PM, Brad King wrote: On 11/23/2011 12:44 PM, Brad King wrote: However, the above does not need to stand in the way of solving the problem you're addressing. We can simply set that goal aside for now by not exposing TRE in the CMake language anywhere. Use it just for cmCTestBuildHandler. but people kept going on the above part of the debate ;) After some more thought, I've realized that no approach currently proposed is practical: - cmCTestBuildHandler can use a list of custom regular expressions so we cannot assume all of them will be compatible with TRE - As David Cole pointed out there are many places, like CTest's -R and -E options, that use regular expressions in contexts where we cannot possibly use a policy. Any attempt to do so in such places would just turn into a second API to set the policy in the local context of the regex. - If we add a second API like MATCHES = MATCHES_TRE then we would eventually need to do that in *every* place that offers regex matching. That would mean alternatives to the above -R and -E options and a lot more. - People could write code that passes a regex around in a variable. This would hide from the author of the regex the context in which it will be used, so it is unknown whether it is TRE or traditional. I propose we go back to an approach discussed the first time PCRE was proposed. The indication of the type of regex must be in the regex itself. IIRC the proposal was something like REGEX:...# old PCRE:... # PCRE Of course that is ambiguous too because the prefixes are valid expressions. Instead we can use a prefix that is not otherwise a valid expression. We can use an idea from Python: http://docs.python.org/library/re.html that defines expressions of the form (?...) which are not otherwise valid. In order to avoid conflict with future use of the constructs they define, we can use the comment form Python defines: (?#OLD)... # old (?#TRE)... # TRE This is quite easy to implement. Just take the currently proposed patch that replaces use of cmsys::RegularExpression with the new cmFastRegularExpression wrapper (perhaps renamed cmRegularExpression). Inside the wrapper look for a leading comment of the above form to decide which regex impl to use internally. Then strip off the prefix and pass the rest of the regex to the underlying implementation. Once this is done update all the default warning and error regular expressions that CTest uses. Add the (?#TRE) prefix to them. This approach will solve the speed problem, give people access to the TRE extended features when they want it anywhere CMake already uses a regex, has no compatibility problems, is a very narrow second interface, and is extensible for future optional regex behavior. -Brad I like that proposal a lot, although I'm afraid it is a bit verbose. Some of my regexes are already pretty lengthy, pushing the 80-columns limit. Michael -- 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
