View Issue Details Jump to Notes ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0013105CMakeCMakepublic2012-04-05 05:492013-01-09 10:55
ReporterAndreas Mohr 
Assigned ToPeter Collingbourne 
PriorityhighSeveritymajorReproducibilityalways
StatusclosedResolutionfixed 
Platformx86OSLinuxOS VersionRHEL5
Product VersionCMake 2.8.7 
Target VersionCMake 2.8.9Fixed in VersionCMake 2.8.9 
Summary0013105: Ninja generator renders a previously working build environment destroyed upon re-configure failure
DescriptionThis 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 Reproducecmake_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 InformationOutput 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'
TagsNo tags attached.
Attached Files

 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.

 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


Copyright © 2000 - 2018 MantisBT Team