2
\$\begingroup\$

I am automating the building and unit testing of a personal project using shell scripts , CMake and make on the latest version of Fedora Linux. I have also tested building on the latest version of Ubuntu. I had to decrease the minimum CMake version on Ubuntu to make it work. Parts of the unit testing have previously been reviewed on Code Review A, B, C, C2.

My original development environment was/is Visual Studio 2019 on Windows 10 Pro, however, to make it easier to get reviews and to create a portable system and application I have developed this build system as well.

It is possible that I could have used CMake for the entire build system, but one of the requirements for this system is that each unit test can build as a separate unit test as well as being combined into other unit tests for regression testing purposes. Each unit test needs to stand on its own because I am using the unit tests to debug the core code, as well as unit test it. Using only CMake created only one object and binary tree and that was not the intention.

The unit tests themselves are not automated yet, that is the next step in the project. There are currently 2 unit tests that have been completed, the lexical analyzer and the parser. All the other unit tests are an empty shell at this point.

Requirements:

  1. Build on any system that supports the original Borne Shell and CMake.
  2. Build the unit tests as individual unit tests and as a single unit test that runs all the previous unit tests.
  3. Use regression testing in each progressive unit test to make sure the new code doesn’t break the previous functionality.
  4. Build the primary application after all the unit tests have been built.

What I want out of this review:

  1. I have tested the build on Fedora and Ubuntu, I would appreciate if someone test the build on Mac OSX, my Mac died 3 years ago.
  2. It’s been a long time since I’ve written shell scripts (at least 6 years and really much longer than that for complex shell scripts).
    1. Do my shell scripts follow best practices?
    2. How can I improve them?
    3. Do you see any portability problems with them?
  3. I’ve never written CMake scripts before, all suggestions will be helpful.
  4. It may be that this last request is off-topic, but how could I build this on Windows 10 using the scripts and CMake? That would make the build system truly portable.

You can review only the shell scripts or only the CMake code if you prefer. The shell scripts are first follow by 3 CMakeLists.txt files.

Build Directory Structure and Build Files

VMWithEditor  
    buildAll.sh  
    buildClean.sh  

    VMWithEditor/VMWithEditor:
        buildDebug.sh
        buildRelease.sh
        CMakeLists.txt

        VMWithEditor/VMWithEditor/UnitTests:
            buildAllDebug.sh
            buildAllRelease.sh

            VMWithEditor/VMWithEditor/UnitTests/CommandLine_UnitTest/CommandLine_UnitTest:
                buildDebug.sh
                buildRelease.sh
                CMakeLists.txt

            VMWithEditor/VMWithEditor/UnitTests/Common_UnitTest_Code:
                CodeReview.md
                unit_test_logging.c
                UTL_unit_test_logging.h

            VMWithEditor/VMWithEditor/UnitTests/ControlConsole_UnitTest/ControlConsole_UnitTest:
                buildDebug.sh
                buildRelease.sh
                CMakeLists.txt

            VMWithEditor/VMWithEditor/UnitTests/Editor_UnitTest/Editor_UnitTest:
                buildDebug.sh
                buildRelease.sh
                CMakeLists.txt

            VMWithEditor/VMWithEditor/UnitTests/HRF_UnitTest/HRF_UnitTest:
                buildDebug.sh
                buildRelease.sh
                CMakeLists.txt

            VMWithEditor/VMWithEditor/UnitTests/Parser_Unit_Test/Parser_Unit_Test:
                buildDebug.sh
                buildRelease.sh
                CMakeLists.txt

            VMWithEditor/VMWithEditor/UnitTests/RunAllUnitTests/RunAllUnitTests:
                buildDebug.sh
                buildRelease.sh
                CMakeLists.txt

            VMWithEditor/VMWithEditor/UnitTests/State_Machine_Unit_Test/State_Machine_Unit_Test:
                buildDebug.sh
                buildRelease.sh
                CMakeLists.txt

            VMWithEditor/VMWithEditor/UnitTests/VirtualMachine_UnitTest/VirtualMachine_UnitTest:
                buildDebug.sh
                buildRelease.sh
                CMakeLists.txt

The Code

I am presenting the shell scripts first and then the CMakeLists.txt files.

Top Shell Script Level Code

VMWithEditor/buildAll.sh

#! /usr/bin/sh
#
# Build the input version of the Virtual MAchine and all the unit tests
# Stop on any build errors.
#
if [ -z "$1" ] ; then
    echo "Usage: build.sh BUILDTYPE where BUILDTYPE is Debug or Release."
    exit 1
elif [ "$1" != 'Debug' ] && [ "$1" != 'Release' ] ; then
    printf "\n unknow build type %s \n" "$1"
    exit 1
fi
#
# Build the necessary variables
#
BUILDTYPE="$1"
UNITTESTDIRECTORY="./VMWithEditor/UnitTests"
SHELLFILE="buildAll${BUILDTYPE}.sh";
VMSHELLFILE="build${BUILDTYPE}.sh";
FULLSPECSHELLFILE="${UNITTESTDIRECTORY}/${SHELLFILE}";
LOGFILE="build${BUILDTYPE}log.txt"
#
# Execute the build scripts
#
# Build The Unit Tests
#
if [ -d "${UNITTESTDIRECTORY}" ] ; then
    if [ -f "${FULLSPECSHELLFILE}" ] ; then
        echo "Building $UNITTESTDIRECTORY";
    cd "${UNITTESTDIRECTORY}" || exit
        ./"${SHELLFILE}" > "${LOGFILE}" 2>&1 
        retVal=$?
        if [ $retVal -ne 0 ]; then
            echo "Unit Test Build Failed!"
            exit $retVal
        fi
        cd ../ || exit
    fi
#
# Build the Virtual Machine with Editor
#
    if [ -f "./buildDebug.sh" ] ; then
        ./"${VMSHELLFILE}" > "${LOGFILE}" 2>&1
        retVal=$?
        if [ ${retVal} -ne 0 ]; then
            echo "Virtual Machine With Editor Build Failed!"
            echo "Check logs for details"
            exit ${retVal}
        else
            printf "%s Version Virtual Machine With Editor Build and Unit Test Build Completed!\n" "${BUILDTYPE}"
            exit 0
        fi
    fi
fi

VMWithEditor/buildClean.sh

#! /usr/bin/bash
#
# Build the release version of the Virtual Machine and all the unit tests
# Stop on any build errors.
#
UNITTESTDIRECTORY="./VMWithEditor/UnitTests"
if [ -d "$UNITTESTDIRECTORY" ] ; then
    cd "$UNITTESTDIRECTORY" || exit
    make clean
    retVal=$?
    if [ $retVal -ne 0 ]; then
        exit $retVal
    fi
    cd ../ || exit
    make clean
fi

Middle Layer Shell Scripts

The 2 following shell scripts are in the UnitTests directory:

buildAllDebug.sh

#! /usr/bin/bash

# Build the debug version of all the unit tests
# Stop on any build errors.

for i in * 
do
    if [ -d $i ] ; then
        TESTDIRECTORY="$i/$i"
        SHELLFILE="$TESTDIRECTORY/buildDebug.sh";
        if [ -f $SHELLFILE ] ; then
            echo "Building $TESTDIRECTORY";
        cd "$TESTDIRECTORY"
            ./buildDebug.sh >& buildDebuglog.txt
            retVal=$?
            if [ $retVal -ne 0 ]; then
                exit $retVal
            fi
            cd ../..
        fi
    fi
done;

buildAllRelease.sh

#! /usr/bin/bash

# Build the debug version of all the unit tests
# Stop on any build errors.

for i in * 
do
    if [ -d $i ] ; then
        TESTDIRECTORY="$i/$i"
        SHELLFILE="$TESTDIRECTORY/buildRelease.sh";
        if [ -f $SHELLFILE ] ; then
            echo "Building $TESTDIRECTORY";
        cd "$TESTDIRECTORY"
            ./buildRelease.sh >& buildReleaselog.txt
            retVal=$?
            if [ $retVal -ne 0 ]; then
                exit $retVal
            fi
            cd ../..
        fi
    fi
done;

Lowest Level Shell Scripts

The following 2 shell scripts are in all the unit test directories where cmake is executed, the first builds a debugable version the second builds an optimized release version.

buildDebug.sh

#! /bin/sh

# Creat a Debug build directory and then build the target within the Debug directory
# Stop on any build errors and stop the parent process.

mkdir Debug
cd Debug || exit
cmake -DCMAKE_BUILD_TYPE=Debug ..
retVal=$?
if [ $retVal -ne 0 ]; then
    printf "\n\ncmake failed %s!\n\n" "$retVal"
    exit $retVal
fi
make VERBOSE=1
retVal=$?
if [ $retVal -ne 0 ]; then
    printf "\n\nmake failed! %s\n\n" "$retVal"
    exit $retVal
fi

buildRelease.sh

#! /bin/sh

# Creat a Release build directory and then build the target within the Release directory
# Stop on any build errors and stop the parent process.

mkdir Release
cd Release || exit
cmake -DCMAKE_BUILD_TYPE=Release ..
retVal=$?
if [ $retVal -ne 0 ]; then
    printf "\n\ncmake failed %s!\n\n" "$retVal"
    exit $retVal
fi
make
retVal=$?
if [ $retVal -ne 0 ]; then
    printf "\n\nmake failed! %s\n\n" "$retVal"
    exit $retVal
fi

The CMake Files:

There are 2.3 unit tests that actually test the existing code and one unit test that includes all the other unit tests which is working to the extent that the two existing unit tests work (testing is successful for all three tests). The first 2 CMake files presented are the lexical analyzer unit test and the parser unit test. The lexical analyzer unit test is fully complete and was used to debug the lexical analyzer. The parser unit test is complete, it executes the lexical analyzer unit tests prior to executing the parser unit tests. The parser unit test was used to debug the parser code in the main project.

The Lexical Analyzer Unit Test CMakeLists.txt file:

cmake_minimum_required(VERSION 3.16.1)

set(EXECUTABLE_NAME "Lexical_Analyzer_Unit_Test.exe")

project(${EXECUTABLE_NAME} LANGUAGES C VERSION 1.0)

if("${CMAKE_BUILD_TYPE}" STREQUAL "Debug")
    set(GCC_WARN_COMPILE_FLAGS  " -Wall ")
    set(CMAKE_C_FLAGS  "${CMAKE_CXX_FLAGS} ${GCC_WARN_COMPILE_FLAGS}")
endif()

set(VM_SRC_DIR "../../..")
set(COMMON_TEST_DIR "../../Common_UnitTest_Code")

add_executable(${EXECUTABLE_NAME} internal_character_transition_unit_tests.c internal_sytax_state_tests.c lexical_analyzer_test_data.c lexical_analyzer_unit_test_main.c lexical_analyzer_unit_test_utilities.c ${VM_SRC_DIR}/error_reporting.c ${VM_SRC_DIR}/lexical_analyzer.c ${VM_SRC_DIR}/safe_string_functions.c ${COMMON_TEST_DIR}/unit_test_logging.c)

set(CMAKE_C_STANDARD 99)
set(CMAKE_C_STANDARD_REQUIRED True)

configure_file(VMWithEditorConfig.h.in VMWithEditorConfig.h)

target_compile_definitions(${EXECUTABLE_NAME} PUBLIC UNIT_TESTING)
target_compile_definitions(${EXECUTABLE_NAME} PUBLIC LEXICAL_UNIT_TEST_ONLY)
target_include_directories(${EXECUTABLE_NAME} PUBLIC "${PROJECT_BINARY_DIR}")
target_include_directories(${EXECUTABLE_NAME} PRIVATE "${VM_SRC_DIR}")
target_include_directories(${EXECUTABLE_NAME} PRIVATE "${COMMON_TEST_DIR}")

The Parser Unit Test CMakeLists.txt file:

cmake_minimum_required(VERSION 3.16.1)

set(EXECUTABLE_NAME "Parser_Unit_Test.exe")

project(${EXECUTABLE_NAME} LANGUAGES C VERSION 1.0)

if("${CMAKE_BUILD_TYPE}" STREQUAL "Debug")
    set(GCC_WARN_COMPILE_FLAGS  " -Wall ")
    set(CMAKE_C_FLAGS  "${CMAKE_CXX_FLAGS} ${GCC_WARN_COMPILE_FLAGS}")
endif()

set(VM_SRC_DIR "../../..")
set(LEXICAL_TEST_DIR "../../State_Machine_Unit_Test/State_Machine_Unit_Test")
set(COMMON_TEST_DIR "../../Common_UnitTest_Code")

add_executable(${EXECUTABLE_NAME} internal_parser_tests.c  parser_unit_test.c  parser_unit_test_main.c ${VM_SRC_DIR}/error_reporting.c ${VM_SRC_DIR}/human_readable_program_format.c ${VM_SRC_DIR}/lexical_analyzer.c ${VM_SRC_DIR}/opcode.c ${VM_SRC_DIR}/parser.c ${VM_SRC_DIR}/safe_string_functions.c  ${VM_SRC_DIR}/virtual_machine.c ${COMMON_TEST_DIR}/unit_test_logging.c ${LEXICAL_TEST_DIR}/internal_character_transition_unit_tests.c ${LEXICAL_TEST_DIR}/internal_sytax_state_tests.c ${LEXICAL_TEST_DIR}/lexical_analyzer_test_data.c ${LEXICAL_TEST_DIR}/lexical_analyzer_unit_test_main.c ${LEXICAL_TEST_DIR}/lexical_analyzer_unit_test_utilities.c)

set(CMAKE_C_STANDARD 99)
set(CMAKE_C_STANDARD_REQUIRED True)

configure_file(VMWithEditorConfig.h.in VMWithEditorConfig.h)

target_compile_definitions(${EXECUTABLE_NAME} PUBLIC UNIT_TESTING)
target_compile_definitions(${EXECUTABLE_NAME} PUBLIC PARSER_UNIT_TEST_ONLY)
target_include_directories(${EXECUTABLE_NAME} PUBLIC "${PROJECT_BINARY_DIR}")
target_include_directories(${EXECUTABLE_NAME} PRIVATE "${VM_SRC_DIR}")
target_include_directories(${EXECUTABLE_NAME} PRIVATE "${COMMON_TEST_DIR}")
target_include_directories(${EXECUTABLE_NAME} PRIVATE "${LEXICAL_TEST_DIR}")

The RunAllUnitTests CMakeLists.txt file:

This file is the most complex of all the CMakeLists.txt files. It includes code from 7 other unit tests.

cmake_minimum_required(VERSION 3.16.1)

set(EXECUTABLE_NAME "Run_All_Unit_Tests.exe")

project(${EXECUTABLE_NAME} LANGUAGES C VERSION 1.0)

if("${CMAKE_BUILD_TYPE}" STREQUAL "Debug")
    set(GCC_WARN_COMPILE_FLAGS  " -Wall ")
    set(CMAKE_C_FLAGS  "${CMAKE_CXX_FLAGS} ${GCC_WARN_COMPILE_FLAGS}")
endif()

set(VM_SRC_DIR "../../..")
set(COMMON_TEST_DIR "../../Common_UnitTest_Code")
set(LEXICAL_TEST_DIR "../../State_Machine_Unit_Test/State_Machine_Unit_Test")
set(PARSER_TEST_DIR "../../Parser_Unit_Test/Parser_Unit_Test")
set(CMD_LINE_TEST_DIR "../../CommandLine_UnitTest/CommandLine_UnitTest")
set(HRF_TEST_DIR "../../HRF_UnitTest/HRF_UnitTest")


add_executable(${EXECUTABLE_NAME}
run_all_unit_tests_main.c
${HRF_TEST_DIR}/hrf_unit_test_main.c
${HRF_TEST_DIR}/unit_test_human_readable_program_format.c
${LEXICAL_TEST_DIR}/lexical_analyzer_unit_test_main.c 
${LEXICAL_TEST_DIR}/internal_character_transition_unit_tests.c
${LEXICAL_TEST_DIR}/internal_sytax_state_tests.c
${LEXICAL_TEST_DIR}/lexical_analyzer_test_data.c
${LEXICAL_TEST_DIR}/lexical_analyzer_unit_test_utilities.c
${VM_SRC_DIR}/error_reporting.c  
${VM_SRC_DIR}/safe_string_functions.c
${VM_SRC_DIR}/arg_flags.c
${VM_SRC_DIR}/file_io_vm.c
${VM_SRC_DIR}/opcode.c
${VM_SRC_DIR}/parser.c 
${VM_SRC_DIR}/default_program.c
${VM_SRC_DIR}/human_readable_program_format.c
${VM_SRC_DIR}/lexical_analyzer.c 
${VM_SRC_DIR}/virtual_machine.c 
${PARSER_TEST_DIR}/parser_unit_test_main.c
${PARSER_TEST_DIR}/internal_parser_tests.c
${PARSER_TEST_DIR}/parser_unit_test.c
${CMD_LINE_TEST_DIR}/command_line_unit_test_main.c
${VM_SRC_DIR}/error_reporting.c
${VM_SRC_DIR}/arg_flags.c
${VM_SRC_DIR}/safe_string_functions.c
${COMMON_TEST_DIR}/unit_test_logging.c
)

set(CMAKE_C_STANDARD 99)
set(CMAKE_C_STANDARD_REQUIRED True)

configure_file(VMWithEditorConfig.h.in VMWithEditorConfig.h)

target_compile_definitions(${EXECUTABLE_NAME} PUBLIC UNIT_TESTING)
target_compile_definitions(${EXECUTABLE_NAME} PUBLIC ALL_UNIT_TESTING)
target_include_directories(${EXECUTABLE_NAME} PUBLIC "${PROJECT_BINARY_DIR}")
target_include_directories(${EXECUTABLE_NAME} PRIVATE "${VM_SRC_DIR}")
target_include_directories(${EXECUTABLE_NAME} PRIVATE "${COMMON_TEST_DIR}")
target_include_directories(${EXECUTABLE_NAME} PRIVATE "${LEXICAL_TEST_DIR}")
target_include_directories(${EXECUTABLE_NAME} PRIVATE "${CMD_LINE_TEST_DIR}")
target_include_directories(${EXECUTABLE_NAME} PRIVATE "${PARSER_TEST_DIR}")
target_include_directories(${EXECUTABLE_NAME} PRIVATE "${HRF_TEST_DIR}")
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Here are some ideas that may help you improve your code.

Understand the purpose of CMake

Many people misunderstand CMake as a "build system" but it is not. It is a tool for creating a build system. Generally speaking, this means that you describe the desired artifacts (executables to be compiled, tests to be compiled and run, documentation to be constructed, etc.) and then CMake translates that into a build system that will run on the target system. Like autotools before it, it attempts to take care of the annoying arbitrary differences among systems, compilers and build environment to do three things:

  1. reduce your burden in creating and maintaining the build system
  2. allow the same software to be built/tested/packaged on multiple platforms
  3. allow for simple expansion, reuse and modification

When you add dependencies such a bash shell scripts, you impair the ability of CMake to provide these benefits. Through this review, I'll show how we can both restore those benefits and simplify.

Avoid hardcoding platform assumptions

One of the current CMakeLists.txt files includes these lines:

set(EXECUTABLE_NAME "Run_All_Unit_Tests.exe")

project(${EXECUTABLE_NAME} LANGUAGES C VERSION 1.0)

if("${CMAKE_BUILD_TYPE}" STREQUAL "Debug")
    set(GCC_WARN_COMPILE_FLAGS  " -Wall ")
    set(CMAKE_C_FLAGS  "${CMAKE_CXX_FLAGS} ${GCC_WARN_COMPILE_FLAGS}")
endif()

set(VM_SRC_DIR "../../..")

Even though that's not very many lines, there are a number of problems with platform assumptions. First, there's the executable name -- most platforms do not use an .exe suffix. CMake is smart enough to know the required suffix for each platform so simply omit it.

Second, the project command is typically going to only appear exactly once in the top level directory. Your files seem to have them sprinkled in multiple places. This makes the project more fragile and harder to reuse. It also leads an experienced CMake reader to wonder if you intend for these to actually be separate projects, but it doesn't make much sense to have a Parser_Unit_Test without a parser, so they can't really be separate.

Third, the if clause will fail unless we happen to be using gcc (or something compatible like clang). Better is to either use add_compile_options or use target_compile_options for settings per target.

Fourth, setting the VM_SRC_DIR explicitly enforces a very specific directory structure which is not easily altered. In some cases this is unavoidable, but this is not one of those. Instead, because this is within a unit test, a better approach would be to simply pass from a higher level CMakeLists.txt file the path to the thing you're attempting to test.

Avoid global directives

It's good that you're using target_compile_definitions instead of setting things globally, but you should also reconsider the use of lines like these:

set(CMAKE_C_STANDARD 99)
set(CMAKE_C_STANDARD_REQUIRED True)

The problem is that not all compilers, such as Microsoft Visual C++ before VS 16.7 even have a notion of standard compliance, so these won't have any effect on such platforms. Better is to use cmake-compile-features to ask CMake for just the features you need. That way, you don't lock into only C99 but could allow compilers which also incorporate newer features. This often results in better target code and better diagnostics.

Create libraries as intermediate products

If there are separable pieces of a project like this which have a parser and and lexer and a vm portion, I would highly recommend compiling those pieces into libraries (either shared or static or both) and then linking with the libraries for both compiling the final project and also for testing. This has the advantage that instead of listing all of the constituent source code files in multiple CMakeLists.txt files up and down the hierarchy, you can generally list source code files only once where they are compiled and have all others refer to the generated library. You will still need the usual interface .h files, but only those because you can set internal-only header files as PRIVATE. See add_library for details.

Flatten your directory tree

The project you've presented has twelve separate directories, each with its own code in it. This is not a sustainable strategy! I typically find it is better to flatten the directory structure (I frequently use just src, test and doc as the only subdirectories) and sort out which files go into which artifact in the CMakeLists.txt files instead of maintaining a complex multi-layered directory tree. If you reduced your structure to just two directories, src and test under a main project directory, you'd reduce your CMakeLists.txt maintenance workload by 75% over the current approach.

Use CTest for testing

Testing can be much more highly automated and portble by using CTest which is part of the suite of CMake programs. In particular see enable_testing and add_test for how to incorporate these into your existing CMake files.

Drive everything from a single top-level CMakeLists.txt file

It has been mentioned earlier, but the best approach for projects like this which have subcomponents but not independent subprojects is to have everything driven from a top-level CMakeLists.txt file. (Actually, it's even true if you do have subprojects, but the details look a little different.) Doing so means that you could compile everything with a single top level command. You could also build and run all of your tests from there or select subsets of test and control the reporting. Even better, cross-compiling and packaging is trivial. For example, I typically compile, test and package for Windows 10 on a Linux machine. Not a single line of my project changes -- the only difference is the command line arguments used for CMake.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.