Re: [cmake-developers] Severe regression caused by #14972 fixes
This will not be done before the freeze for 3.1 on Thursday. Reverting 7243c951 will resolve the problem for ReactOS in out-of-source builds. So, we either revert that or hope Ninja can be fixed to deal with the large dependency lists w/out crashing. In the discussion with Adam, he mentioned (about input files) that CMake can not know that some file will always exist or not, and that it doesn't know files is generated unless someone tells that, *unfortunately some projects don't* Reverting this change makes more sense, because CMake has always been advertised as being designed for out-of-source builds, and we cannot penalize every single sane CMake user out there simply because some projects do not properly mark their generated input files (in custom commands) as GENERATED. I don't understand why is it okay for the CMake project (at least in ReactOS case) to introduce now a (very) huge list of our codebase *source files* (files that exist in our *repo* and not *generated* nor we can even *assume* they may not exist at some point, because they will *always* exist) simply to solve a problem that is, if some projects do *not* properly use CMake (GENERATED property, adding dependencies properly on commands/target that generate the said input files...etc) we need to do this to keep them working. The extra bloat of generating a very large list of phony rules on *source* files to solve a CMake misuse problem suggests that we're Doing It Wrong. If ninja didn't crash on us, we would probably never find out about this bloat, unless some of us randomly had the idea of diffing the old build log (from CMake 2.8.x) with this new one. -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Severe regression caused by #14972 fixes
On 10/08/2014 03:45 AM, Amine Khaldi wrote: Reverting this change makes more sense This is the simplest solution for the upcoming 3.1 release because it just restores behavior to what 3.0 does. I've done it here: Ninja: Limit custom command side-effects to build folder http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=de8e534b simply to solve a problem that is, if some projects do *not* properly use CMake (GENERATED property, adding dependencies properly on commands/target that generate the said input files...etc) we need to do this to keep them working. This is not about supporting projects that do it wrong. Ninja wants to know not just that a file is generated, but also *which* custom command generates it. We have no interface for specifying side-effect outputs whose timestamps do not need to be newer than the inputs. That is what this issue is about: Add explicit specification of custom command side effect outputs http://www.cmake.org/Bug/view.php?id=14963 The reason is that CMake was designed long before Ninja existed, and all the other build systems not only do not need this information, but have no place to put it even if we did have it. See also my reply to Adam's sibling message to yours. -Brad -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Severe regression caused by #14972 fixes
On 10/07/2014 04:53 PM, Adam Strzelecki wrote: In meantime I've pushed stage/cmp0055-disable-ninja-side-effects that make CMake warn about this compatibility layer when such phony rules are about to be emitted. What I've been trying to say is that this is NOT about compatibility with broken projects. It is a *fundamental limitation* of the current CMake custom command specification interface (add_custom_command). If you start requiring explicit specification of side-effects as custom command outputs, you will make it *impossible* for project code to specify their build rules for cases involving side-effect outputs whose timestamps do not need to be newer than their inputs. Non-Ninja build systems do *not* support this. This issue: Add explicit specification of custom command side effect outputs http://www.cmake.org/Bug/view.php?id=14963 documents a few such cases. It explains that we need a new interface, perhaps as an option to add_custom_command, to specify side-effect outputs of rules. Another possible middle-ground workaround is to generate phony rules only for custom command dependencies that are marked with the GENERATED property. That would avoid the phony rule bloat and simply require projects to explicitly mark their generated side effects. We still won't be able to tell Ninja which rule generates them, but at least we could reduce the number of phony rules. However, there is no way this will be done for 3.1 by tomorrow, so it would be best to think about a full solution to #14963 to start development now and target the 3.2 release. -Brad -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Severe regression caused by #14972 fixes
This is the simplest solution for the upcoming 3.1 release because it just restores behavior to what 3.0 does. I've done it here: Okay. But still I feel like this is just workaround rather than solution for the problem. This is not about supporting projects that do it wrong. Ninja wants to know not just that a file is generated, but also *which* custom command generates it. We have no interface for specifying side-effect outputs whose timestamps do not need to be newer than the inputs. Yes, it isn't about doing anything wrong, but to make projects building fine with Make build fine with Ninja too. And you are right here, we have no means for provide proper transition to remove this implicit compatibility layer. We lack OUTPUT for add_custom_target. I can work on OUTPUT support for add_custom_target if you want? Together with new POLICY it will make sense, unless you have other opinion on that. But I guess it won't make it to 3.1? That is what this issue is about: Add explicit specification of custom command side effect outputs http://www.cmake.org/Bug/view.php?id=14963 Shouldn't it be called Add explicit specification of custom TARGET side effect outputs? --Adam -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Severe regression caused by #14972 fixes
On 10/07/2014 03:22 AM, Amine Khaldi wrote: Please note that from the http://www.cmake.org/Bug/view.php?id=14972 fixes on, we can no longer compile ReactOS. Thanks for trying the development version and reporting this regression before it was released. We have many DEPENDS on files that exist in the source (they are simply input files, they're not generated or so) like spec files, inf files, idl files...etc and they end up in the Unknown Build Time Dependencies which should not happen, and did not happen with CMake 2.8.x Would you please construct a CMake code example demonstrating this in a small test case? We'd rather not have to try building all of ReactOS to reproduce this. Also, what build-time failure do you actually get? Thanks, -Brad -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Severe regression caused by #14972 fixes
On 10/7/2014 8:47 AM, Brad King wrote: Please note that from thehttp://www.cmake.org/Bug/view.php?id=14972 fixes on, we can no longer compile ReactOS. Also, if you really care about ReactOS, you could run a nightly cmake dashboard. Or it is almost certain that it will get broken again. If we are not testing it, it is broken... http://www.cmake.org/testing/ Thanks. -Bill -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Severe regression caused by #14972 fixes
On 10/07/2014 04:09 PM, Bill Hoffman wrote: On 10/7/2014 8:47 AM, Brad King wrote: Please note that from thehttp://www.cmake.org/Bug/view.php?id=14972 fixes on, we can no longer compile ReactOS. Also, if you really care about ReactOS, you could run a nightly cmake dashboard. Or it is almost certain that it will get broken again. If we are not testing it, it is broken... http://www.cmake.org/testing/ To clarify as far as I understand it this particular issue is with building ReactOS on regular Windows rather than using CMake under ReactOS. Which of course doesn't mean that there shouldn't be a dashboard for it nonetheless :) Nils -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Severe regression caused by #14972 fixes
To clarify as far as I understand it this particular issue is with building ReactOS on regular Windows rather than using CMake under ReactOS. Which of course doesn't mean that there shouldn't be a dashboard for it nonetheless :) Yup, this is what I understand. It isn't a problem with compiling CMake on ReactOS, but problem of compiling ReactOS with latest CMake master. I'll follow the discussion with Amine and try to sort it out. --Adam -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Severe regression caused by #14972 fixes
As a result of this, it's no longer possible to use the new CMake to compile ReactOS, which makes us see this as a severe regression, considering that CMake is the build system we use to compile the project. Adam, Brad and co, can you please help ? If I understand correctly the change introduced by a33cf6d08853ea4c79324bdd36c04f311a23f20a (Ninja: Consider only custom commands deps as side-effects (#14972)) caused that regression? Can you please point to me any test case I can run in order to see it failing on CMake 3.x and running fine on CMake 2.8. If this is a bug I am willing to fix it ASAP. However please note that if these files are produced by some custom commands that not specify them as an output and then these are inputs (dependencies) to some regular build command, then it is expected behavior. Indeed it working was working previously in 2.8 just by coincidence, because CMake was generating suboptimal build graph simply adding all custom commands regardless of anything as implicit dependencies, even it was not necessary or expressed anywhere, causing severe clutter in Ninja build graph. So, (1) Are these .spec, .inf or .idl generated by some custom commands? (2) If yes, are these files specified as an output of these subcommand commands? If no, why? --Adam -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Severe regression caused by #14972 fixes
On 10/07/2014 04:56 PM, Adam Strzelecki wrote: (1) Are these .spec, .inf or .idl generated by some custom commands? (2) If yes, are these files specified as an output of these subcommand commands? If no, why? From what I remember from the IRC discussion ... They are regular (not generated) source files that are listed as dependencies (but not outputs) of custom commands. Nils -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Severe regression caused by #14972 fixes
On 10/7/2014 10:42 AM, Adam Strzelecki wrote: Which of course doesn't mean that there shouldn't be a dashboard for it nonetheless:) Yup, this is what I understand. It isn't a problem with compiling CMake on ReactOS, but problem of compiling ReactOS with latest CMake master. I'll follow the discussion with Amine and try to sort it out. I see. In that case a contract test might be a good thing... https://github.com/Kitware/CMake/tree/master/Tests/Contracts The idea is to add a build of a project as a test to CMake. See CMAKE_CONTRACT_PROJECTS in Tests/CMakeLists.txt for an idea of how this works. -Bill -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Severe regression caused by #14972 fixes
From what I remember from the IRC discussion ... They are regular (not generated) source files that are listed as dependencies (but not outputs) of custom commands. Okay now I get it. So actually 7243c951 (Ninja: Don't limit custom cmd side-effects to build folder (#14972)) is to blame not a33cf6d0 (Ninja: Consider only custom commands deps as side-effects (#14972)). Previously in 2.8 only files in build folder might have been be side-effects, but as we have discussed with Brad this is pretty inaccurate assumption. So we have decided that ANY file that is dependency of custom command may be a side effect, therefore ReactOS files landing in Unknown Build Time Dependencies. that serve only one purpose, to not bail out Ninja on very beginning if file does not exist as file may appear later on during build. But it does NOT stop Ninja to properly relaunch custom command if the file gets modified. So I guess we may point to the wrong direction. Simple example: -- test.txt 1 2 3 test -- CMakeLists.txt cmake_minimum_required(VERSION 2.6) project(DependTest C) add_custom_command( OUTPUT test-out.txt COMMAND cp test.txt test-out.txt DEPENDS test.txt) add_custom_target(depend-test ALL cat test-out.txt DEPENDS test-out.txt) (1) first run $ ninja -v [1/2] cd /tmp/depend cp test.txt test-out.txt [2/2] cd /tmp/depend cat test-out.txt 1 2 3 test (2) 2nd run $ ninja -v [1/1] cd /tmp/depend cat test-out.txt 1 2 3 test (3) touching test.txt $ touch test.txt $ ninja -v [1/2] cd /tmp/depend cp test.txt test-out.txt [2/2] cd /tmp/depend cat test-out.txt 1 2 3 test So with proper test case and detailed reports, something more than vague latest CMake makes our project not to compile anymore I am unable to help, but I am willing to help. So please either provide test case, or some info how to run this problematic build (I have access to OSX, Linux or Windows system), or find me on #cmake IRC channel under OnO. --Adam -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Severe regression caused by #14972 fixes
On 10/07/2014 02:21 PM, Adam Strzelecki wrote: So please either provide test case, or some info [snip] On 10/07/2014 03:20 PM, Adam Strzelecki wrote: I had a talk with Amine, and the problem was that due 7243c951 their build.ninja was getting numerous extra phony entries that were leading Ninja to crash! So yes we could point our fingers on Ninja and close the case, but I propose we do something better: For reference, that commit was: Ninja: Don't limit custom cmd side-effects to build folder (#14972) http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=7243c951 So now a large number of phony rules for files in the source tree appear that did not before. The case in question uses custom command dependencies so is not helped by the parent change: Ninja: Consider only custom commands deps as side-effects (#14972) http://www.cmake.org/gitweb?p=cmake.git;a=commitdiff;h=a33cf6d0 I kindly ask to make possible side-effects phony rules as deprecated by some new policy. Basically long story short all these side-effects tricks were to make some old projects don't explicitly specifying custom command outputs work with Ninja that stat files once at start, so if there is a side-effect file that isn't explicitly specified as command output it will be NOT restat, that will lead to some other command having that file as dependency to fail. That situation is discussed thoroughly here: Add explicit specification of custom command side effect outputs http://www.cmake.org/Bug/view.php?id=14963 and here: https://github.com/martine/ninja/issues/760#issuecomment-46540858 Ideally both CMake and Ninja will fix their pieces of the problem. Fixing the Ninja side as I propose in the above-linked comment would solve the problem outright with no further changes. The fix on the CMake side will also require projects to be updated to use the new feature, but will help generate more efficient Ninja build rules. Since Make does restat dependencies on each rule this won't happen with Make. So we put side-effects in phony ensuring Ninja to pickup the rules even the file didn't exist when Ninja was started. No. For build systems besides Ninja the side effects cannot be listed as outputs of custom commands. Their timestamps are not always updated when the custom command runs, so the rules may be left in an always-rerun state because the side-effect outputs will never be newer than inputs. Since CMake was designed for such build systems before Ninja existed, there is no interface to specify the side-effects explicitly. My original intention was actually to limit these phony rules and it was done by: (1) a33cf6d0 (Ninja: Consider only custom commands deps as side-effects (#14972)) However somewhere during discussion with Brad I raised question why such side-effect are to be in build folder in fact they can be anywhere and it we want to ensure Ninja build won't fail when Make build goes well we need introduced: (2) 7243c951 (Ninja: Don't limit custom cmd side-effects to build folder (#14972)) This unfortunately lead Ninja to crash with many extra side-effect phony rules in ReactOS case. So again I propose we make side-effect as policy enabled only for projects 3.1. That is not possible because CMake currently does not have enough information to produce valid Ninja files without the phony rules. The proper fix to address issue 14963 and provide projects with an interface to specify custom command side-effects explicitly. Then dependencies can be hooked up correctly for Ninja. I invite you to propose an interface to achieve this. This will not be done before the freeze for 3.1 on Thursday. Reverting 7243c951 will resolve the problem for ReactOS in out-of-source builds. So, we either revert that or hope Ninja can be fixed to deal with the large dependency lists w/out crashing. -Brad -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] Severe regression caused by #14972 fixes
This will not be done before the freeze for 3.1 on Thursday. Reverting 7243c951 will resolve the problem for ReactOS in out-of-source builds. So, we either revert that or hope Ninja can be fixed to deal with the large dependency lists w/out crashing. In meantime I've pushed stage/cmp0055-disable-ninja-side-effects that make CMake warn about this compatibility layer when such phony rules are about to be emitted. Setting policy to NEW or making all deps to be explicit outputs disables warning, setting to OLD removed warning keeping phony rules. IHMO this is win-win for everybody, since it will emit warnings once someone installs 3.1 and possibly relies on side-effect. One thing I am not sure about is if the CMP0055.rst provided is clear enough for the reader. --Adam -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
