Support builds of ngraph_dist with both gcc and clang (#306)

* Initial pass as supporting ngraph_dist builds with both gcc and clang.  Includes deprecated targets to support Jenkins CI with old feature and development branches that may not have these changes.

* Added build_all, check_all, and install_all targets

* Broke build and check steps into separate make sections.  Each step already generated its own log.

* Updated README.md

* Fixed a bug in the docker build context, which was overly general and included the build directories.  This caused a loop where the docker images rebuilt into multi-gigabyte images after testing was run (yikes).  New context is the contrib/docker directory, which is tiny.  No more image rebuilds.

* Rename the docker image to be less generic, so that another tool doesn't accidentally use the same name

* Change compatibility target "make check_cpu" to run "make check_all", so that all builds are done.

* Add a conversion step to change the test-suite names for all unit tests, to include an indicator for what compiler was used for the builds (_gcc vs _clang).

* Unfortunately, modifying the test-suite name with compiler suffix did not show in Jenkins test results.  Now modifying classname instead.

* Switch builds to be RelWithDebInfo, which turns on optimization and includes debug symbols.  Remove verbose output when creating tarball, to reduce log fluff.

contrib/docker/Makefile

@@ -23,8 +23,14 @@ DOCKUSER_HOME=/home/dockuser
 # /root/ngraph-cpp-test is not used, because /root is not accessible to user
 VOLUME = -v "${DIR}:${DOCKUSER_HOME}/ngraph-cpp-test"
 GIT_COMMIT = $(shell git rev-parse HEAD)
-BUILD_VERSION = ${GIT_COMMIT}_${PYTHON_VERSION}
-BUILD_DIR = ${DIR}/contrib/docker/.build-${BUILD_VERSION}
+DBUILD_VERSION = ${GIT_COMMIT}_${PYTHON_VERSION}
+DBUILD_DIR = ${DIR}/contrib/docker/.build-${DBUILD_VERSION}
+
+# For gcc builds, we do NOT regard warnings as errors
+# For clang builds, we DO make warnings into errors
+CMAKE_OPTIONS_COMMON=-DNGRAPH_BUILD_DOXYGEN_DOCS=ON -DNGRAPH_BUILD_SPHINX_DOCS=ON -DCMAKE_BUILD_TYPE=RelWithDebInfo
+CMAKE_OPTIONS_GCC=$CMAKE_OPTIONS_COMMON -DNGRAPH_INSTALL_PREFIX=${DOCKUSER_HOME}/ngraph-cpp-test/BUILD-GCC/ngraph_dist
+CMAKE_OPTIONS_CLANG=$CMAKE_OPTIONS_COMMON -DNGRAPH_INSTALL_PREFIX=${DOCKUSER_HOME}/ngraph-cpp-test/BUILD-CLANG/ngraph_dist -DCMAKE_CXX_COMPILER=clang++-3.9 -DCMAKE_C_COMPILER=clang-3.9 -DNGRAPH_WARNINGS_AS_ERRORS=ON
 
 CALLER_UID := $(shell id -u)
 CALLER_GID := $(shell id -g)
@@ -33,7 +39,10 @@ CALLER_GID := $(shell id -g)
 # line
 PYTHON_VERSION = 2
 
-.PHONY: clean build_ngraph_cpp_cpu check_cpu install shell build_all
+# Some targets are DEPRECATED and will be removed at a later date: check_cpu build_ngraph_cpp_cpu
+# These DEPRECATED targets are currently included for Jenkins job compatibility with older dev branches
+# Please see comments for individual targets for more details
+.PHONY: clean build_docker_image build_gcc check_gcc build_clang check_clang install_gcc install_clang shell check_cpu build_all build_ngraph_cpp_cpu
 
 DOCKER_BUILD=docker build --rm=true
 
@@ -47,58 +56,134 @@ DOCKER_BUILD+=--build-arg https_proxy=$(https_proxy)
 DOCKER_RUN_ENV+=--env "https_proxy=$(https_proxy)"
 endif
 
+all: check_gcc check_clang
+
+# Docker actions
+
 expand_dockerfile_templates:
 	cd "${DIR}"/contrib/docker
-	mkdir "${BUILD_DIR}" || true
-	sed -e 's/\(FROM ngraph.*\)/\1:${BUILD_VERSION}/' Dockerfile.ngraph_cpp_cpu > "${BUILD_DIR}"/Dockerfile.ngraph_cpp_cpu
-
-clean:
-	rm -f "${DIR}"/contrib/docker/.build-*/Dockerfile.* || echo "keep going if files are not present"
-	rmdir "${DIR}"/contrib/docker/.build-* || echo "keep going if directory is not present"
-	rm -fr "${DIR}"/BUILD
+	mkdir "${DBUILD_DIR}" || true
+	sed -e 's/\(FROM ngraph.*\)/\1:${DBUILD_VERSION}/' Dockerfile.ngraph_cpp > "${DBUILD_DIR}"/Dockerfile.build_ngraph_cpp
 
-build_ngraph_cpp_cpu: expand_dockerfile_templates
-	$(DOCKER_BUILD) -f="${BUILD_DIR}"/Dockerfile.ngraph_cpp_cpu --build-arg python_version="${PYTHON_VERSION}" -t=ngraph_cpp_cpu:"${BUILD_VERSION}" "${DIR}"
+build_docker_image: expand_dockerfile_templates
+	$(DOCKER_BUILD) -f="${DBUILD_DIR}"/Dockerfile.build_ngraph_cpp --build-arg python_version="${PYTHON_VERSION}" -t=build_ngraph_cpp:"${DBUILD_VERSION}" .
 	# remove the tag for the previous latest image
-	docker rmi ngraph_cpp_cpu:latest || echo "keep going if docker rmi command fails"
-	docker tag `docker images -q "ngraph_cpp_cpu:${BUILD_VERSION}"` ngraph_cpp_cpu:latest
+	docker rmi build_ngraph_cpp:latest || echo "keep going if docker rmi command fails"
+	docker tag `docker images -q "build_ngraph_cpp:${DBUILD_VERSION}"` build_ngraph_cpp:latest
 
-build_all: build_ngraph_cpp_cpu
+build_docker: build_docker_image
 
-check_cpu: build_ngraph_cpp_cpu
+# Build
+
+build_all: build_gcc build_clang
+
+build_gcc: build_docker_image
 	# Remove old distribution directory if present
-	( test -d "${DIR}"/BUILD/ngraph_dist && rm -fr "${DIR}"/BUILD/ngraph_dist && echo "Removed old ${DIR}/BUILD/ngraph_dist directory" ) || echo "Previous ngraph_dist directory not found"
-	# Make BUILD directory as user
-	mkdir -p "${DIR}"/BUILD
-	chmod ug+rwx "${DIR}"/BUILD
+	( test -d "${DIR}"/BUILD-GCC/ngraph_dist && rm -fr "${DIR}"/BUILD-GCC/ngraph_dist && echo "Removed old ${DIR}/BUILD-GCC/ngraph_dist directory" ) || echo "Previous ngraph_dist directory not found"
+	# Make BUILD-GCC directory as user
+	mkdir -p "${DIR}"/BUILD-GCC
+	chmod ug+rwx "${DIR}"/BUILD-GCC
 	docker run --rm --tty \
             ${VOLUME} \
 	    ${DOCKER_RUN_ENV} \
-            --env GTEST_OUTPUT="xml:${DOCKUSER_HOME}/ngraph-cpp-test/BUILD/unit-test-results.xml" \
+            --env GTEST_OUTPUT="xml:${DOCKUSER_HOME}/ngraph-cpp-test/BUILD-GCC/unit-test-results.xml" \
             --env RUN_UID="$(shell id -u)" \
-            --env RUN_CMD="set -e ; set -o pipefail ; cd ${DOCKUSER_HOME}/ngraph-cpp-test/BUILD; cmake -DCMAKE_CXX_COMPILER=clang++-3.9 -DCMAKE_C_COMPILER=clang-3.9 -DNGRAPH_BUILD_DOXYGEN_DOCS=ON -DNGRAPH_BUILD_SPHINX_DOCS=ON -DNGRAPH_WARNINGS_AS_ERRORS=ON .. 2>&1 | tee cmake.log ; env VERBOSE=1 make ${PARALLEL} 2>&1 | tee make.log ; env VERBOSE=1 make check 2>&1 | tee make_check.log" \
-            "ngraph_cpp_cpu:${BUILD_VERSION}" \
+            --env RUN_CMD="set -e ; set -o pipefail ; cd ${DOCKUSER_HOME}/ngraph-cpp-test/BUILD-GCC; cmake ${CMAKE_OPTIONS_GCC} .. 2>&1 | tee cmake_gcc.log ; env VERBOSE=1 make ${PARALLEL} 2>&1 | tee make_gcc.log" \
+            "build_ngraph_cpp:${DBUILD_VERSION}" \
 	    sh -c "${DOCKUSER_HOME}/ngraph-cpp-test/contrib/docker/run_as_user.sh"
 
-shell: build_ngraph_cpp_cpu
-	# "make shell" runs an interactive shell in the docker image, for debugging
-	docker run --rm --tty --interactive \
+build_clang:
+	# Remove old distribution directory if present
+	( test -d "${DIR}"/BUILD-CLANG/ngraph_dist && rm -fr "${DIR}"/BUILD-CLANG/ngraph_dist && echo "Removed old ${DIR}/BUILD-CLANG/ngraph_dist directory" ) || echo "Previous ngraph_dist directory not found"
+	# Make BUILD-CLANG directory as user
+	mkdir -p "${DIR}"/BUILD-CLANG
+	chmod ug+rwx "${DIR}"/BUILD-CLANG
+	docker run --rm --tty \
             ${VOLUME} \
 	    ${DOCKER_RUN_ENV} \
+            --env GTEST_OUTPUT="xml:${DOCKUSER_HOME}/ngraph-cpp-test/BUILD-CLANG/unit-test-results.xml" \
             --env RUN_UID="$(shell id -u)" \
-            "ngraph_cpp_cpu:${BUILD_VERSION}" \
-            sh -c "cd ${DOCKUSER_HOME} ; ${DOCKUSER_HOME}/ngraph-cpp-test/contrib/docker/run_as_user.sh"
+            --env RUN_CMD="set -e ; set -o pipefail ; cd ${DOCKUSER_HOME}/ngraph-cpp-test/BUILD-CLANG; cmake ${CMAKE_OPTIONS_CLANG} .. 2>&1 | tee cmake_clang.log ; env VERBOSE=1 make ${PARALLEL} 2>&1 | tee make_clang.log" \
+            "build_ngraph_cpp:${DBUILD_VERSION}" \
+	    sh -c "${DOCKUSER_HOME}/ngraph-cpp-test/contrib/docker/run_as_user.sh"
+
+# Check (run unit-tests)
+
+check_all: check_gcc check_clang
+
+check_gcc: build_gcc
+	docker run --rm --tty \
+            ${VOLUME} \
+	    ${DOCKER_RUN_ENV} \
+            --env GTEST_OUTPUT="xml:${DOCKUSER_HOME}/ngraph-cpp-test/BUILD-GCC/unit-test-results.xml" \
+            --env RUN_UID="$(shell id -u)" \
+            --env RUN_CMD="set -e ; set -o pipefail ; cd ${DOCKUSER_HOME}/ngraph-cpp-test/BUILD-GCC; env VERBOSE=1 make check 2>&1 | tee make_check_gcc.log ; sed -E -e 's/classname\=\"[a-zA-Z0-9_]+/&1_gcc/' unit-test-results.xml > unit-test-results-gcc.xml" \
+            "build_ngraph_cpp:${DBUILD_VERSION}" \
+	    sh -c "${DOCKUSER_HOME}/ngraph-cpp-test/contrib/docker/run_as_user.sh"
 
-install:
-	# Puts ngraph_dist in BUILD directory.  This is used by Jenkins ngraph-tensorflow batch job.
-	# Note: We currently have a bug where cmake only installs in $HOME.  Jira NGTF-205 is opened
-	#       for this.  For now, here we install to $HOME, then move the directory.
+check_clang: build_clang
 	docker run --rm --tty \
             ${VOLUME} \
 	    ${DOCKER_RUN_ENV} \
+            --env GTEST_OUTPUT="xml:${DOCKUSER_HOME}/ngraph-cpp-test/BUILD-CLANG/unit-test-results.xml" \
             --env RUN_UID="$(shell id -u)" \
-            --env RUN_CMD="set -e ; set -o pipefail; cd ${DOCKUSER_HOME}/ngraph-cpp-test/BUILD ; test -d ngraph_dist && rm -fr ngraph_dist && echo 'Removed old ngraph_dist directory' ; make install 2>&1 | tee make_install.log ; mv -v ${DOCKUSER_HOME}/ngraph_dist ${DOCKUSER_HOME}/ngraph-cpp-test/BUILD" \
-            "ngraph_cpp_cpu:${BUILD_VERSION}" \
+            --env RUN_CMD="set -e ; set -o pipefail ; cd ${DOCKUSER_HOME}/ngraph-cpp-test/BUILD-CLANG; env VERBOSE=1 make check 2>&1 | tee make_check_clang.log ; sed -E -e 's/classname\=\"[a-zA-Z0-9_]+/&1_clang/' unit-test-results.xml > unit-test-results-clang.xml" \
+            "build_ngraph_cpp:${DBUILD_VERSION}" \
 	    sh -c "${DOCKUSER_HOME}/ngraph-cpp-test/contrib/docker/run_as_user.sh"
 
-all: build_ngraph_cpp_cpu
+# Install
+
+install_all: install_gcc install_clang
+
+install_gcc: check_gcc
+	# Puts ngraph_dist in BUILD-GCC directory.  This is used by Jenkins ngraph-tensorflow batch job.
+	docker run --rm --tty \
+            ${VOLUME} \
+	    ${DOCKER_RUN_ENV} \
+            --env RUN_UID="$(shell id -u)" \
+            --env RUN_CMD="set -e ; set -o pipefail; cd ${DOCKUSER_HOME}/ngraph-cpp-test/BUILD-GCC ; test -d ngraph_dist && rm -fr ngraph_dist && echo 'Removed old ngraph_dist directory' ; make install 2>&1 | tee make_install_gcc.log ; tar czf ngraph_dist_gcc.tgz ngraph_dist 2>&1 | tee make_tarball_gcc.log" \
+            "build_ngraph_cpp:${DBUILD_VERSION}" \
+	    sh -c "${DOCKUSER_HOME}/ngraph-cpp-test/contrib/docker/run_as_user.sh"
+
+install_clang: check_clang
+	# Puts ngraph_dist in BUILD-CLANG directory.  This is used by Jenkins ngraph-tensorflow batch job.
+	docker run --rm --tty \
+            ${VOLUME} \
+	    ${DOCKER_RUN_ENV} \
+            --env RUN_UID="$(shell id -u)" \
+            --env RUN_CMD="set -e ; set -o pipefail; cd ${DOCKUSER_HOME}/ngraph-cpp-test/BUILD-CLANG ; test -d ngraph_dist && rm -fr ngraph_dist && echo 'Removed old ngraph_dist directory' ; make install 2>&1 | tee make_install_clang.log ; tar czf ngraph_dist_clang.tgz ngraph_dist 2>&1 | tee make_tarball_clang.log" \
+            "build_ngraph_cpp:${DBUILD_VERSION}" \
+	    sh -c "${DOCKUSER_HOME}/ngraph-cpp-test/contrib/docker/run_as_user.sh"
+
+# Interactive shell
+
+shell: build_docker_image
+	# "make shell" runs an interactive shell in the docker image, for debugging
+	docker run --rm --tty --interactive \
+            ${VOLUME} \
+	    ${DOCKER_RUN_ENV} \
+            --env RUN_UID="$(shell id -u)" \
+            "build_ngraph_cpp:${DBUILD_VERSION}" \
+            sh -c "cd ${DOCKUSER_HOME} ; ${DOCKUSER_HOME}/ngraph-cpp-test/contrib/docker/run_as_user.sh"
+
+# Clean
+
+clean:
+	rm -f "${DIR}"/contrib/docker/.build-*/Dockerfile.* || echo "keep going if files are not present"
+	rmdir "${DIR}"/contrib/docker/.build-* || echo "keep going if directory is not present"
+	rm -fr "${DIR}"/BUILD-GCC
+	rm -fr "${DIR}"/BUILD-CLANG
+
+#
+# DEPRECATED TARGETS -- These WILL BE REMOVED in a future revision.
+#                       They exist here to maintain compatibility in Jenkins
+#                       jobs on older development branches.
+#
+
+check_cpu: check_all
+	echo 'WARNING: "make check_cpu" is DEPRECATED and will be removed in a future revision'
+	echo '         "make check_cpu" runs "make check_all" now, building with all compilers (gcc and clang)'
+
+build_ngraph_cpp_cpu: build_docker_image
+	echo 'WARNING: "make build_ngraph_cpp_cpu" is DEPRECATED and will be removed in a future revision'
+	echo '         "make build_ngraph_cpp_cpu" runs "make build_docker_image" now'

contrib/docker/README.md

@@ -19,39 +19,42 @@ In order to use the make targets, you will need to do the following:
 * Have docker installed on your computer with the docker daemon running.
 * These scripts assume that you are able to run the `docker` command without using `sudo`.  You will need to add your account to the `docker` group so this is possible.
 * If your computer (running docker) sits behind a firewall, you will need to have the docker daemon properly configured to use proxies to get through the firewall, so that public docker registries and git repositories can be accessed.
-* You should _not_ run `make check_cpu` from a directory in an NFS filesystem, if that NFS filesystem uses _root squash_ (see **Notes** section below).  Instead, run `make check_cpu` from a cloned repo in a local filesystem.
+* You should _not_ run `make check_*` targets from a directory in an NFS filesystem, if that NFS filesystem uses _root squash_ (see **Notes** section below).  Instead, run `make check_*` targets from a cloned repo in a local filesystem.
 
 ## Make Targets
 
 The _make targets_ are designed to provide easy commands to run actions using the docker image.  All _make targets_ should be issued on the host OS, and _not_ in a docker image.
 
-* In general, you simply need to run the command **`make check_cpu`**.  This first makes the `build_ngraph_cpp_cpu` target as a dependency.  Then it makes the `check_cpu` target, which will build NGraph-Cpp using _cmake_ and _make_ and then run unit testing.  Please keep in mind that `make check_cpu` does not work when your working directory is in an NFS filesystem that uses _root squash_ (see **Notes** section below).
+Most make targets are structured in the form `<action>_<compiler>`.  The `<action>` indicates what you want to do (e.g. build, check, install), while the `<compiler>` indicates what you want to build with (i.e. gcc or clang).
 
-* You can also run the command **`make shell`** to start an interactive bash shell inside the docker image.  While this is not required for normal builds and unit testing, it allows you to run interactively within the docker image with the cloned repo mounted.  Again, `build_ngraph_cpp_cpu` is made first as a dependency.  Please keep in mind that `make shell` does not work when your working directory is in an NFS filesystem that uses _root squash_ (see **Notes** section below).
+* In general, you simply need to run the command **`make check_all`**.  This first makes the `build_docker_ngraph_cpp` target as a dependency.  Then it makes the `build_*` and `check_*` targets, which will build NGraph-Cpp using _cmake_ and _make_ and then run unit testing.  Please keep in mind that `make check_*` targets do not work when your working directory is in an NFS filesystem that uses _root squash_ (see **Notes** section below).
 
-* Running the command **`make build_ngraph_cpp_cpu`** is also available, if you simply want to build the docker image.  This target does work properly when your working directory is in an NFS filesystem.
+* Two builds are supported: building with `gcc` and `clang`.  Targets are named `*_gcc` and `*_clang` when they refer to building with a specific compiler, and the `*_all` targets are available to build with both compilers.  Output directories are BUILD-GCC and BUILD-CLANG, at the top level.
 
-Note that all operations performed inside the the docker image are run as **root**.  This has unfortunate side effects if you run a make target that does operations inside the docker image, while your working directory is in an NFS filesystem that uses _root squash_ (see **Notes** below).
+* You can also run the command **`make shell`** to start an interactive bash shell inside the docker image.  While this is not required for normal builds and unit testing, it allows you to run interactively within the docker image with the cloned repo mounted.  Again, `build_docker_ngraph_cpp` is made first as a dependency.  Please keep in mind that `make shell` does not work when your working directory is in an NFS filesystem that uses _root squash_ (see **Notes** section below).
 
-## Helper Scripts
+* Running the command **`make build_docker_ngraph_cpp`** is also available, if you simply want to build the docker image.  This target does work properly when your working directory is in an NFS filesystem.
 
-These helper scripts are included for use in the `Makefile` and automated (Jenkins) jobs.  **These scripts should _not_ be called directly unless you understand what they do.**
+* Finally, **`make clean`** is available to clean up the BUILD-* and docker build directories.
 
-#### `chown_files.sh`
+Note that all operations performed inside the the docker image are run as a regular user, using the `run-as-user.sh` script.  This is done to avoid writing root-owned files in mounted filesystems.
+
+## Helper Scripts
 
-Used in the `Makefile` to change the ownership of files while running _inside the docker image_.  This is used to fix a problem with docker where files written in a mounted filesystem, from within the docker image, will end up being owned by root in the host OS.  This leads to problems with Jenkins not being able to clean up its own job directories if docker images are used.
+These helper scripts are included for use in the `Makefile` and automated (Jenkins) jobs.  **These scripts should _not_ be called directly unless you understand what they do.**
 
 #### `docker_cleanup.sh`
 
 A helper script for Jenkins jobs to clean up old exited docker containers and `ngraph_*` docker images.
 
+#### `run_as_user.sh`
+
+A helper script to run as a normal user within the docker container.  This is done to avoid writing root-owned files in mounted filesystems.
+
 ## Notes
 
 * The top-level `Makefile` in this cloned repo can be used to build and unit-test NGraph-Cpp _outside_ of docker.  This directory is only for building and running unit tests for NGraph-Cpp in the _reference-OS_ docker image.
 
-* The `_cpu` suffix in the targets refers to the docker image being built such that testing is in a _cpu-only_ environment.  At a later date, _gpu_ and other environments may be added.  This convention was chosen to stay consistent with the _NervanaSystems ngraph_ project on Github.
-
-* Due to limitations in how docker mounts work, `make check_cpu` and `make shell` will fail
-if you try to run them while in a working directory that is in an NFS-mount that has _root squash_ enabled.  The cause results from the process in the docker container running as root.  When a file or directory is created by root in the mounted directory tree, from within the docker image, the NFS-mount (in the host OS) does not allow a root-created file, leading to a permissions error.  This is dependent on whether the host OS performs "root squash" when mounting NFS filesystems.  The fix to this is easy: run `make check_cpu` and `make shell` from a local filesystem.
+* Due to limitations in how docker mounts work, `make check_cpu` and `make shell` will fail if you try to run them while in a working directory that is in an NFS-mount that has _root squash_ enabled.  The cause results from the process in the docker container running as root.  When a file or directory is created by root in the mounted directory tree, from within the docker image, the NFS-mount (in the host OS) does not allow a root-created file, leading to a permissions error.  This is dependent on whether the host OS performs "root squash" when mounting NFS filesystems.  The fix to this is easy: run `make check_cpu` and `make shell` from a local filesystem.
 
-* These make targets have been tested on Ubuntu 14.04 and Ubuntu 16.04 with docker installed and the docker daemon properly configured.  Some adjustments may be needed to run these on other OSes.
+* These make targets have been tested on Ubuntu 16.04 with docker installed and the docker daemon properly configured.  Some adjustments may be needed to run these on other OSes.