sc_version_string: Use temporary names resilient against parallel builds

In #359 I identify a race condition between multiple parallel invocations of cmake, which can arise naturally during ctests. Now that the file contents will not change without an intervening git commit, it is sufficient to ensure that the parallel invocations use distinct temporary file names with high probability.
sc_version_string: omit the date and time when testing
2017-08-21 08:31:05 -05:00 · 2017-08-21 08:31:00 -05:00 · 2017-08-19 17:12:28 -04:00 · 2017-08-19 15:41:33 -04:00
7 changed files with 63 additions and 159 deletions
--- a/.appveyor.yml
+++ b/.appveyor.yml
@ -2,6 +2,10 @@ version: '{build}'

 # for Appveyor CI (windows)

+branches:
+  only:
+    - master
+
 os: Windows Server 2012 R2

 clone_folder: c:\projects\STEPcode
@ -42,127 +46,19 @@ build_script:
    cd build
    cmake -version
    grep --version
-    cmake .. -DSC_ENABLE_TESTING=ON -G"$env:GENERATOR" -DSC_BUILD_SCHEMAS="ifc2x3"
-    cmake --build . --config Debug --target tst_inverse_attr3
+    cmake .. -DSC_ENABLE_TESTING=ON -G"$env:GENERATOR" -DSC_BUILD_SCHEMAS="ifc2x3;ap214e3;ap209"
+    echo "filtering build output with grep"
+    cmake --build . --config Debug | grep -ve "CMake does not need to re-run because" -e "ZERO_CHECK.ZERO_CHECK" -e "^  Creating directory"

 #msbuld seems to provide no benefits, and I can't filter its output...
 #msbuild SC.sln /logger:"C:\Program Files\AppVeyor\BuildAgent\Appveyor.MSBuildLogger.dll" /p:Configuration=Debug /p:Platform=x64
 # /toolsversion:14.0 /p:PlatformToolset=v140

 test_script:
-  - cmd: echo inverse_attr3 test 100x
+  - cmd: echo Running CTest...
  - cmd: cd c:\projects\STEPcode\build
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: echo 10
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: echo 20
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: echo 30
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: echo 40
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: echo 50
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: echo 60
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: echo 70
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: echo 80
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: echo 90
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: bin\tst_inverse_attr3 ..\test\p21\test_inverse_attr.p21
-  - cmd: echo 100
-  - cmd: echo done
+  - cmd: echo excluding test_inverse_attr3, which hangs
+  - cmd: ctest -j1 . -C Debug -E test_inverse_attr3 --output-on-failure

 # - cmd: grep -niB20 "Test Failed" Testing/Temporary/LastTest.log

--- a/.travis.yml
+++ b/.travis.yml
@ -3,6 +3,17 @@ language: cpp
 compiler:
  - clang
 script: mkdir build && cd build && cmake .. -DSC_ENABLE_TESTING=ON && make -j3 && ctest -j2 --output-on-failure
+branches:
+  only:
+    - master
+notifications:
+  irc: "chat.freenode.net#stepcode"
+  email: scl-dev@groups.google.com
+  on_success: change
+  on_failure: always
 os:
  - linux
  - osx
+matrix:
+  allow_failures:
+    - os: osx
--- a/cmake/sc_version_string.cmake
+++ b/cmake/sc_version_string.cmake
@ -8,6 +8,7 @@
 # http://www.cmake.org/pipermail/cmake/2009-February/027014.html

 set(SC_IS_SUBBUILD "@SC_IS_SUBBUILD@")
+set(SC_ENABLE_TESTING "@SC_ENABLE_TESTING@")

 set(SC_VERSION_HEADER "${BINARY_DIR}/include/sc_version_string.h")

@ -47,15 +48,31 @@ string(REPLACE "\n" "" GIT_COMMIT_ID ${GIT_COMMIT_ID})
 #once cmake_minimum_required is >= 2.8.11, we can use TIMESTAMP:
 #string(TIMESTAMP date_time_string)

+if(SC_ENABLE_TESTING)
+  set (date_time_string "NA - disabled for testing")
+elseif(UNIX)
+  execute_process(COMMAND date "+%d %b %Y %H:%M" OUTPUT_VARIABLE date_time_string OUTPUT_STRIP_TRAILING_WHITESPACE)
+elseif(WIN32)
+  execute_process(COMMAND cmd /c date /t OUTPUT_VARIABLE currentDate OUTPUT_STRIP_TRAILING_WHITESPACE)
+  execute_process(COMMAND cmd /c time /t OUTPUT_VARIABLE currentTime OUTPUT_STRIP_TRAILING_WHITESPACE)
+  set (date_time_string "${currentDate} ${currentTime}")
+else()
+  set(date_time_string "\" __DATE__ \" \" __TIME__ \" ")
+  if(NOT SC_IS_SUBBUILD)
+    message(STATUS "Unknown platform - using date from preprocessor")
+  endif(NOT SC_IS_SUBBUILD)
+endif()
+
 set(header_string "/* sc_version_string.h - written by cmake. Changes will be lost! */\n"
  "#ifndef SC_VERSION_STRING\n"
  "#define SC_VERSION_STRING\n\n"
  "/*\n** The git commit id looks like \"test-1-g5e1fb47\", where test is the\n"
  "** name of the last tagged git revision, 1 is the number of commits since that tag,\n"
  "** 'g' is unknown, and 5e1fb47 is the first 7 chars of the git sha1 commit id.\n"
-  "*/\n\n"
+  "** timestamp is created from date/time commands on known platforms, and uses\n"
+  "** preprocessor macros elsewhere.\n*/\n\n"
  "static char sc_version[512] = {\n"
-  "    \"git commit id: ${GIT_COMMIT_ID}\"\n"
+  "    \"git commit id: ${GIT_COMMIT_ID}, build timestamp ${date_time_string}\"\n"
  "}\;\n\n"
  "#endif\n"
 )
--- a/src/cllazyfile/lazyFileReader.cc
+++ b/src/cllazyfile/lazyFileReader.cc
@ -50,7 +50,7 @@ instancesLoaded_t * lazyFileReader::getHeaderInstances() {
 }

 lazyFileReader::lazyFileReader( std::string fname, lazyInstMgr * i, fileID fid ): _fileName( fname ), _parent( i ), _fileID( fid ) {
-    _file.open( _fileName.c_str(), std::ios::binary );
+    _file.open( _fileName.c_str() );
    _file.imbue( std::locale::classic() );
    _file.unsetf( std::ios_base::skipws );
    assert( _file.is_open() && _file.good() );
--- a/src/cllazyfile/sectionReader.cc
+++ b/src/cllazyfile/sectionReader.cc
@ -47,7 +47,7 @@ std::streampos sectionReader::findNormalString( const std::string & str, bool se
        }
        if( c == '\'' ) {
            //push past string
-            _file.seekg( _file.tellg() - std::streampos(1) );
+            _file.unget();
            GetLiteralStr( _file, _lazyFile->getInstMgr()->getErrorDesc() );
        }
        if( ( c == '/' ) && ( _file.peek() == '*' ) ) {
@ -129,7 +129,7 @@ std::streampos sectionReader::seekInstanceEnd( instanceRefs ** refs ) {
                }
                break;
            case '\'':
-                _file.seekg( _file.tellg() - std::streampos(1) );
+                _file.unget();
                GetLiteralStr( _file, _lazyFile->getInstMgr()->getErrorDesc() );
                break;
            case '=':
@ -155,7 +155,7 @@ std::streampos sectionReader::seekInstanceEnd( instanceRefs ** refs ) {
                    if( _file.get() == ';' ) {
                        return _file.tellg();
                    } else {
-                        _file.seekg( _file.tellg() - std::streampos(1) );
+                        _file.unget();
                    }
                }
            default:
@ -186,7 +186,7 @@ instanceID sectionReader::readInstanceNumber() {
    if( ( c == '/' ) && ( _file.peek() == '*' ) ) {
        findNormalString( "*/" );
    } else {
-        _file.seekg( _file.tellg() - std::streampos(1) );
+        _file.unget();
    }
    skipWS();
    c = _file.get();
@ -210,7 +210,7 @@ instanceID sectionReader::readInstanceNumber() {
            digits++;

        } else {
-            _file.seekg( _file.tellg() - std::streampos(1) );
+            _file.unget();
            break;
        }

@ -221,7 +221,7 @@ instanceID sectionReader::readInstanceNumber() {
            _error->UserMsg( "A very large instance ID encountered" );
            _error->DetailMsg( errorMsg.str() );

-            delete [] buffer;
+            delete buffer;
            return 0;    
        }

@ -309,7 +309,7 @@ SDAI_Application_instance * sectionReader::getRealInstance( const Registry * reg
        assert( inst->getEDesc() );
        _file.seekg( begin );
        findNormalString( "(" );
-        _file.seekg( _file.tellg() - std::streampos(1) );
+        _file.unget();
        sev = inst->STEPread( instance, 0, _lazyFile->getInstMgr()->getAdapter(), _file, sName, true, false );
        //TODO do something with 'sev'
        inst->InitIAttrs();
--- a/src/clutils/errordesc.cc
+++ b/src/clutils/errordesc.cc
@ -131,7 +131,7 @@ void ErrorDescriptor::PrependToUserMsg( const char * msg ) {
 }

 void ErrorDescriptor::AppendToUserMsg( const char c ) {
-    _userMsg.push_back( c );
+    _userMsg.append( &c );
 }

 void ErrorDescriptor::AppendToUserMsg( const char * msg ) {
@ -147,7 +147,7 @@ void ErrorDescriptor::PrependToDetailMsg( const char * msg ) {
 }

 void ErrorDescriptor::AppendToDetailMsg( const char c ) {
-    _detailMsg.push_back( c );
+    _detailMsg.append( &c );
 }

 void ErrorDescriptor::AppendToDetailMsg( const char * msg ) {
--- a/src/express/error.c
+++ b/src/express/error.c
@ -67,9 +67,6 @@
 #include "express/info.h"
 #include "express/linklist.h"

-#if defined( _WIN32 ) || defined ( __WIN32__ )
-#  define snprintf _snprintf
-#endif

 bool __ERROR_buffer_errors = false;
 const char * current_filename = "stdin";
@ -115,7 +112,6 @@ static struct heap_element {
 static int ERROR_with_lines = 0;    /**< number of warnings & errors that have occurred with a line number */
 static char * ERROR_string;
 static char * ERROR_string_base;
-static char * ERROR_string_end;

 static bool ERROR_unsafe = false;
 static jmp_buf ERROR_safe_env;
@ -123,28 +119,6 @@ static jmp_buf ERROR_safe_env;

 #define error_file stderr /**< message buffer file */

-static int ERROR_vprintf( const char *format, va_list ap ) {
-    int result = snprintf( ERROR_string, ERROR_string_end - ERROR_string, format, ap );
-    if(result < 0) ERROR_string = ERROR_string_end;
-    else if(result > (ERROR_string_end - ERROR_string)) ERROR_string = ERROR_string_end;
-    else ERROR_string = ERROR_string + result;
-    return result;
-}
-
-static int ERROR_printf( const char *format, ... ) {
-    int result;
-    va_list ap;
-    va_start( ap, format );
-    result = ERROR_vprintf( format, ap );
-    va_end( ap );
-    return result;
-}
-
-static void ERROR_nexterror() {
-    if( ERROR_string == ERROR_string_end ) return;
-    ERROR_string++;
-}
-
 /** Initialize the Error module */
 void ERRORinitialize( void ) {
    ERROR_subordinate_failed =
@ -153,7 +127,6 @@ void ERRORinitialize( void ) {
        ERRORcreate( "%s, expecting %s in %s %s", SEVERITY_EXIT );

    ERROR_string_base = ( char * )sc_malloc( ERROR_MAX_SPACE );
-    ERROR_string_end = ERROR_string_base + ERROR_MAX_SPACE;
    ERROR_start_message_buffer();


@ -404,14 +377,20 @@ va_dcl {
            heap[child].msg = ERROR_string;

            if( what->severity >= SEVERITY_ERROR ) {
-                ERROR_printf( "%s:%d: --ERROR PE%03d: ", sym->filename, sym->line, what->serial );
-                ERROR_vprintf( what->message, args );
-                ERROR_nexterror();
+                sprintf( ERROR_string, "%s:%d: --ERROR PE%03d: ", sym->filename, sym->line, what->serial );
+                ERROR_string += strlen( ERROR_string );
+                vsprintf( ERROR_string, what->message, args );
+                ERROR_string += strlen( ERROR_string );
+                *ERROR_string++ = '\n';
+                *ERROR_string++ = '\0';
                ERRORoccurred = true;
            } else {
-                ERROR_printf( "%s:%d: WARNING PW%03d: ", sym->filename, sym->line, what->serial );
-                ERROR_vprintf( what->message, args );
-                ERROR_nexterror();
+                sprintf( ERROR_string, "%s:%d: WARNING PW%03d: ", sym->filename, sym->line, what->serial );
+                ERROR_string += strlen( ERROR_string );
+                vsprintf( ERROR_string, what->message, args );
+                ERROR_string += strlen( ERROR_string );
+                *ERROR_string++ = '\n';
+                *ERROR_string++ = '\0';
            }
            if( what->severity >= SEVERITY_EXIT ||
                    ERROR_string + ERROR_MAX_STRLEN > ERROR_string_base + ERROR_MAX_SPACE ||
@ -431,6 +410,7 @@ va_dcl {
                ERRORoccurred = true;
            } else {
                fprintf( error_file, "%s:%d: WARNING PW%03d: ", sym->filename, sym->line, what->serial );
+                ERROR_string += strlen( ERROR_string ) + 1;
                vfprintf( error_file, what->message, args );
                fprintf( error_file, "\n" );
            }
Author	SHA1	Message	Date
Jeff Epler	d39fbaf4d7	sc_version_string: Use temporary names resilient against parallel builds In #359 I identify a race condition between multiple parallel invocations of cmake, which can arise naturally during ctests. Now that the file contents will not change without an intervening git commit, it is sufficient to ensure that the parallel invocations use distinct temporary file names with high probability.	2017-08-21 08:31:05 -05:00
Jeff Epler	e422262877	sc_version_string: omit the date and time when testing As analyzed in #359, if the header contains the current time, it will be updated while running the testsuite; this, in turn, causes multiple cmake processes to attempt to update targets like lib/libexpress.so.2.0.0 at the same time, causing test failures.	2017-08-21 08:31:00 -05:00
Mark	71fe947ff5	Merge pull request #361 from jepler/appveyor-single-thread-test appveyor build: don't use ctest parallelism	2017-08-19 17:12:28 -04:00
Mark	beb2a595f1	Merge pull request #357 from jepler/nullptr-bool Fix build error with g++ 6.3 (Debian Stretch)	2017-08-19 15:41:33 -04:00