View Issue Details [ Jump to Notes ] | [ Print ] | ||||||||
ID | Project | Category | View Status | Date Submitted | Last Update | ||||
0013105 | CMake | CMake | public | 2012-04-05 05:49 | 2013-01-09 10:55 | ||||
Reporter | Andreas Mohr | ||||||||
Assigned To | Peter Collingbourne | ||||||||
Priority | high | Severity | major | Reproducibility | always | ||||
Status | closed | Resolution | fixed | ||||||
Platform | x86 | OS | Linux | OS Version | RHEL5 | ||||
Product Version | CMake 2.8.7 | ||||||||
Target Version | CMake 2.8.9 | Fixed in Version | CMake 2.8.9 | ||||||
Summary | 0013105: Ninja generator renders a previously working build environment destroyed upon re-configure failure | ||||||||
Description | This destruction of the build environment does NOT happen with Makefile generator, thus it is a bug which needs fixing... This is a grave problem since I'm relying on a multi-staged build (even incorporating several vcproj2cmake elements, necessitating integration of even another conversion run prior to the CMake --> build conversion run), thus it's important to always preserve a working build environment to keep e.g. reconversion targets (vcproj2cmake rebuilder targets) available even in case of re-configure (i.e., re-generation) runs failing (e.g. due to CMake syntax issues). Tested with very recent git head. I recently submitted a very similar issue at http://public.kitware.com/Bug/view.php?id=13040 [^] , thus tried to analyze the (instantly provided, thanks!) patch and check what to do with similar use of cmGeneratedFileStream in cmGlobalNinjaGenerator.cxx, but so far I haven't found where exactly handling is broken. Possibly from cmGeneratedFileStream point of view everything is "fine" after Ninja generation, thus it does atomically rename the (newly broken, due to CMake configure run failure!!) into the (prior, working!) build.ninja file. I'll try to spend some more time tracking down this issue, but if anyone knows where to look, that would be awesome :) Thanks! | ||||||||
Steps To Reproduce | cmake_minimum_required(VERSION 2.8) project(ninja_reconfigure_destroy_test CXX) if(NOT CMAKE_GENERATOR MATCHES "Ninja") message(FATAL_ERROR "CMAKE_GENERATOR is not set to Ninja!") endif(NOT CMAKE_GENERATOR MATCHES "Ninja") file(WRITE "test.cpp" "int main() { return 0; }") set(file_list test.cpp # test_not_existing_file.cpp # <====== UNCOMMENT THIS LINE! ) add_library(ninja_reconfigure_destroy_test SHARED ${file_list}) | ||||||||
Additional Information | Output of actual test run, providing evidence of a destroyed build configuration (thus any and all targets gone fishin', potentially even some which need to remain executable to sucessfully get the CMake-side syntax back into working order!!): build]$ cmake -DCMAKE_GENERATOR=Ninja .. -- The CXX compiler identification is GNU 4.1.2 -- Check for working CXX compiler using: Ninja -- Check for working CXX compiler using: Ninja -- works -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done -- Configuring done -- Generating done -- Build files have been written to: /home/amoh/Projects/cmake_tests/ninja_reconfigure_destroy_test/build [amoh@lxamoh build]$ ninja [1/2] Building CXX object CMakeFiles/ninja_reconfigure_destroy_test.dir/test.cpp.o ../test.cpp:4:2: warning: no newline at end of file [2/2] Linking CXX shared library libninja_reconfigure_destroy_test.so [amoh@lxamoh build]$ ninja ninja: no work to do. [amoh@lxamoh build]$ vi ../CMakeLists.txt [amoh@lxamoh build]$ ninja [1/1] Re-running CMake... FAILED: /usr/local/bin/cmake -H/home/amoh/Projects/cmake_tests/ninja_reconfigure_destroy_test -B/home/amoh/Projects/cmake_tests/ninja_reconfigure_destroy_test/build -- Configuring done CMake Error at CMakeLists.txt:19 (add_library): Cannot find source file: test_not_existing.cpp Tried extensions .c .C .c++ .cc .cpp .cxx .m .M .mm .h .hh .h++ .hm .hpp .hxx .in .txx -- Build files have been written to: /home/amoh/Projects/cmake_tests/ninja_reconfigure_destroy_test/build ninja: error: rebuilding 'build.ninja': subcommand failed [amoh@lxamoh build]$ ninja ninja: error: loading 'build.ninja': line 18, col 20: unknown build rule 'RERUN_CMAKE' | ||||||||
Tags | No tags attached. | ||||||||
Attached Files | |||||||||
Relationships | |
Relationships |
Notes | |
(0029327) Andreas Mohr (reporter) 2012-04-24 17:36 |
Got some time now, thus been investigating this issue with current CMake master and a current Ninja master. Source/cmGeneratedFileStream.cxx: cmGeneratedFileStream::cmGeneratedFileStream(): std::cout << this << " name " << name << " TempName " << TempName << std::endl; cmGeneratedFileStream::~cmGeneratedFileStream(): std::cout << this << " Okay: " << this->Okay << std::endl; === Ninja sample ===: andi@andi:~/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja$ /usr/src/system/GIT/CMake/build/bin/cmake -DCMAKE_GENERATOR=Ninja .. -- Configuring done 0xbffd2bf4 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja/CMakeCache.txt TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja/CMakeCache.txt.tmp 0xbffd2bf4 Okay: 1 0x869d798 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja/build.ninja TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja/build.ninja.tmp 0x86a79f0 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja/rules.ninja TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja/rules.ninja.tmp 0xbffd2cfc name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja/cmake_install.cmake TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja/cmake_install.cmake.tmp 0xbffd2cfc Okay: 1 0xbffd2d4c name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja/CMakeFiles/TargetDirectories.txt TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja/CMakeFiles/TargetDirectories.txt.tmp 0xbffd2d4c Okay: 1 -- Generating done 0x86a79f0 Okay: 1 0x869d798 Okay: 1 0xbffd2c74 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja/CMakeCache.txt TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja/CMakeCache.txt.tmp 0xbffd2c74 Okay: 1 -- Build files have been written to: /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja andi@andi:~/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja$ vi ../CMakeLists.txt andi@andi:~/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja$ ninja [1/1] Re-running CMake... FAILED: /usr/src/system/GIT/CMake/build/bin/cmake -H/home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test -B/home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja -- Configuring done 0xbfba8714 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja/CMakeCache.txt TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja/CMakeCache.txt.tmp 0xbfba8714 Okay: 1 0x869d690 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja/build.ninja TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja/build.ninja.tmp 0x86a4b08 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja/rules.ninja TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja/rules.ninja.tmp CMake Error at CMakeLists.txt:15 (add_library): Cannot find source file: test_not_existing_file.cpp Tried extensions .c .C .c++ .cc .cpp .cxx .m .M .mm .h .hh .h++ .hm .hpp .hxx .in .txx 0x86a4b08 Okay: 1 0x869d690 Okay: 1 -- Build files have been written to: /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja ninja: ERROR: rebuilding 'build.ninja': subcommand failed andi@andi:~/prg/cmake_tests/ninja_reconfigure_destroy_test/build_ninja$ === Makefile sample ===: andi@andi:~/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make$ /usr/src/system/GIT/CMake/build/bin/cmake .. WARNING: CMAKE_GENERATOR is not set to Ninja! -- Configuring done 0xbf8ac884 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeCache.txt TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeCache.txt.tmp 0xbf8ac884 Okay: 1 0x86aa348 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/ninja_reconfigure_destroy_test.dir/build.make TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/ninja_reconfigure_destroy_test.dir/build.make.tmp 0xbf8ac83c name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/ninja_reconfigure_destroy_test.dir/depend.make TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/ninja_reconfigure_destroy_test.dir/depend.make.tmp 0xbf8ac83c Okay: 1 0x86a16c0 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/ninja_reconfigure_destroy_test.dir/flags.make TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/ninja_reconfigure_destroy_test.dir/flags.make.tmp 0xbf8ac338 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/ninja_reconfigure_destroy_test.dir/link.txt TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/ninja_reconfigure_destroy_test.dir/link.txt.tmp 0xbf8ac338 Okay: 1 0x86a0960 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/ninja_reconfigure_destroy_test.dir/DependInfo.cmake TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/ninja_reconfigure_destroy_test.dir/DependInfo.cmake.tmp 0x86aa348 Okay: 1 0x86a0960 Okay: 1 0x86a16c0 Okay: 1 0xbf8ac80c name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/Makefile TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/Makefile.tmp 0xbf8ac80c Okay: 1 0xbf8ac868 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/CMakeDirectoryInformation.cmake TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/CMakeDirectoryInformation.cmake.tmp 0xbf8ac868 Okay: 1 0xbf8ac83c name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/cmake_install.cmake TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/cmake_install.cmake.tmp 0xbf8ac83c Okay: 1 0xbf8ac88c name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/TargetDirectories.txt TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/TargetDirectories.txt.tmp 0xbf8ac88c Okay: 1 -- Generating done 0xbf8ac904 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/ninja_reconfigure_destroy_test.dir/progress.make TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/ninja_reconfigure_destroy_test.dir/progress.make.tmp 0xbf8ac904 Okay: 1 0xbf8aca50 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make//CMakeFiles/progress.marks TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make//CMakeFiles/progress.marks.tmp 0xbf8aca50 Okay: 1 0xbf8ac8e0 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/Makefile2 TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/Makefile2.tmp 0xbf8ac8e0 Okay: 1 0xbf8ac880 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/Makefile.cmake TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/Makefile.cmake.tmp 0xbf8ac880 Okay: 1 0xbf8ac904 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeCache.txt TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeCache.txt.tmp 0xbf8ac904 Okay: 1 -- Build files have been written to: /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make andi@andi:~/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make$ make Scanning dependencies of target ninja_reconfigure_destroy_test 0xbf9d694c name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/ninja_reconfigure_destroy_test.dir/depend.make TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/ninja_reconfigure_destroy_test.dir/depend.make.tmp 0xbf9d6a6c name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/ninja_reconfigure_destroy_test.dir/depend.internal TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/ninja_reconfigure_destroy_test.dir/depend.internal.tmp 0xbf9d6a6c Okay: 1 0xbf9d694c Okay: 1 [100%] Building CXX object CMakeFiles/ninja_reconfigure_destroy_test.dir/test.cpp.o Linking CXX shared library libninja_reconfigure_destroy_test.so [100%] Built target ninja_reconfigure_destroy_test andi@andi:~/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make$ vi ../CMakeLists.txt andi@andi:~/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make$ make WARNING: CMAKE_GENERATOR is not set to Ninja! -- Configuring done 0xbf983254 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeCache.txt TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeCache.txt.tmp 0xbf983254 Okay: 1 CMake Error at CMakeLists.txt:15 (add_library): Cannot find source file: test_not_existing_file.cpp Tried extensions .c .C .c++ .cc .cpp .cxx .m .M .mm .h .hh .h++ .hm .hpp .hxx .in .txx 0xbf983420 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make//CMakeFiles/progress.marks TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make//CMakeFiles/progress.marks.tmp 0xbf983420 Okay: 1 0xbf9832b0 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/Makefile2 TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/Makefile2.tmp 0xbf9832b0 Okay: 1 0xbf983250 name /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/Makefile.cmake TempName /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make/CMakeFiles/Makefile.cmake.tmp 0xbf983250 Okay: 1 -- Build files have been written to: /home/andi/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make make: *** [cmake_check_build_system] Fehler 1 andi@andi:~/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make$ ls CMakeCache.txt CMakeFiles/ cmake_install.cmake libninja_reconfigure_destroy_test.so* Makefile andi@andi:~/prg/cmake_tests/ninja_reconfigure_destroy_test/build_make$ From this test I think we can gather that both Makefile and Ninja generator "successfully" go through the entire operation of closing all build file streams (which I think they shouldn't!!), yet in the Makefile case the **main** Makefile (${CMAKE_BINARY_DIR}/Makefile) does _not_ get re-touched for the case of simply adding a (non-existing!) source file to the library to be built. Whereas in the Ninja case the only guilt by association that it is suffering of is the fact that it's a lightning-fast flat (i.e., _single-file_) build system, which doesn't leave much choice in the actual file to rebuild (read: destroy...) since there is only a single one... Or, to make a full story short, so far it looks like an unlucky deficiency in the cmGeneratedFileStream implementation in that it only flags Okay member according to (*this) (i.e., ios::operator!()), yet there seems to be no way for the _external_ user of this generated file stream class to (reliably) indicate that a grave error occurred (i.e. in our case, CMake re-configure run failed) and that thus the result file (i.e., the still-temporary file) should definitely be sent to bit heaven, irrespective of whether the stream itself is fine or not... Haven't spent much time on this analysis so far, so maybe I'm wrong about it not being possible to externally indicate fatal failure to cmGeneratedFileStream... |
(0029328) Andreas Mohr (reporter) 2012-04-24 17:54 |
Adding some missing parts: So since in the Makefile generator case the main Makefile somehow happens to not get touched and thus stay alive, we're lucky enough to have retained our capability to re-trigger a CMake configure run, which isn't possible in the Ninja case since the main and single build.ninja file is _gone_ for good. Should also mention that a crucial part of the (incomplete?) atomic file replacement handling happens in cmGeneratedFileStreamBase::Close(). So, to think about what should be done to rectify this: So far it looks like cmGeneratedFileStream only ever skips replacing the file in case the stream itself indicates that it's broken (e.g. conditions like out-of-disk-space etc.). Naively I would now think that this can easily be rectified by simply adding helper methods/members to additionally indicate that the file really shouldn't be replaced in the dtor, since the external user of this build stream indicated that a grave error occurred and the file stream's logical _content_ is toast and would thus disrupt the configuration environment. As food for thought, if one could actually easily enhance it like this, then what about coordinated efforts where _many_ temporary build streams get created/replaced yet only a _single_ one then ends up flagged to have been failed? In that case the configuration environment might still end up broken since new versions of _some_ streams have been written successfully yet another one has failed and thus not been replaced, thus there's a configuration-breaking incompatibility introduced due to _incompatible_ new <-> old files. Not sure whether we want to tread into this discussion, since at that time we might even need to think about atomic transaction/rollback series to always ensure consistent state of an entire build environment file bundling writeout... |
(0029489) Peter Collingbourne (developer) 2012-05-16 19:06 |
I've pushed a fix for this issue to next. |
(0029543) Andreas Mohr (reporter) 2012-05-22 10:19 |
Just tried current next branch, confirmed to be working fine (which is helping tremendously!), thanks a ton for adding it! :) (despite the underlying cause having turned out to not even be your "fault"...) Although I'm a bit unhappy about the diff, since I think it would be more useful to provide a helper method which abstracts away the particular way to indicate failure to the specific stream implementation (setting the failbit manually externally is a bit dirty since it conveys hardcoding of the stream's base class implementation). Suitable helper method names? Perhaps IndicateGenerationFailure(), GenerationFailedThusCancelActivation(), GenerationFailedNowCancelWriteout(). I cannot pretend that these names aren't awkward... any better candidates? |
(0029890) Peter Kuemmel (developer) 2012-07-03 05:59 |
Bug fixed. Andreas, you could post a code cleanup to the list. |
(0032016) Robert Maynard (manager) 2013-01-09 10:55 |
Closing resolved issues that have not been updated in more than 4 months. |
Notes |
Issue History | |||
Date Modified | Username | Field | Change |
2012-04-05 05:49 | Andreas Mohr | New Issue | |
2012-04-05 07:49 | Brad King | Assigned To | => Peter Collingbourne |
2012-04-05 07:49 | Brad King | Status | new => assigned |
2012-04-24 17:36 | Andreas Mohr | Note Added: 0029327 | |
2012-04-24 17:54 | Andreas Mohr | Note Added: 0029328 | |
2012-05-16 19:06 | Peter Collingbourne | Note Added: 0029489 | |
2012-05-22 10:19 | Andreas Mohr | Note Added: 0029543 | |
2012-07-03 05:59 | Peter Kuemmel | Note Added: 0029890 | |
2012-07-03 05:59 | Peter Kuemmel | Status | assigned => resolved |
2012-07-03 05:59 | Peter Kuemmel | Resolution | open => fixed |
2012-08-09 16:56 | David Cole | Fixed in Version | => CMake 2.8.9 |
2012-08-09 16:56 | David Cole | Target Version | => CMake 2.8.9 |
2013-01-09 10:55 | Robert Maynard | Note Added: 0032016 | |
2013-01-09 10:55 | Robert Maynard | Status | resolved => closed |
Issue History |
Copyright © 2000 - 2018 MantisBT Team |