From ed553724197ed169b0030c8485354eeb3721c2d6 Mon Sep 17 00:00:00 2001 From: Bill Sacks Date: Thu, 29 Oct 2020 16:39:57 -0600 Subject: [PATCH 1/4] Add ability to use a Docker image to do the documentation build --- doc_builder/build_commands.py | 30 +++++++++++++++- doc_builder/build_docs.py | 54 +++++++++++++++++++++++++++-- test/test_unit_get_build_command.py | 36 ++++++++++++++++++- 3 files changed, 116 insertions(+), 4 deletions(-) diff --git a/doc_builder/build_commands.py b/doc_builder/build_commands.py index cf1375a..5fcddcc 100644 --- a/doc_builder/build_commands.py +++ b/doc_builder/build_commands.py @@ -5,6 +5,12 @@ import os from doc_builder import sys_utils +# The Docker image used to build documentation via Docker +_DOCKER_IMAGE = "escomp/base" + +# The top-level directory in the above docker image +_DOCKER_ROOT = "/home/user" + def get_build_dir(build_dir=None, repo_root=None, version=None): """Return a string giving the path to the build directory. @@ -57,14 +63,36 @@ def get_build_dir(build_dir=None, repo_root=None, version=None): return build_dir -def get_build_command(build_dir, build_target, num_make_jobs): +def get_build_command(build_dir, run_from_dir, build_target, num_make_jobs, docker_name=None): """Return a string giving the build command. Args: - build_dir: string giving path to directory in which we should build + If this is a relative path, it is assumed to be relative to run_from_dir + - run_from_dir: string giving absolute path from which the build_docs command was run + This is needed when using Docker - build_target: string: target for the make command (e.g., "html") - num_make_jobs: int: number of parallel jobs + - docker_name: string or None: if not None, uses a Docker container to do the build, + with the given name """ builddir_arg = "BUILDDIR={}".format(build_dir) build_command = ["make", builddir_arg, "-j", str(num_make_jobs), build_target] + + if docker_name is not None: + if os.path.isabs(build_dir): + build_dir_abs = build_dir + else: + build_dir_abs = os.path.normpath(os.path.join(run_from_dir, build_dir)) + # mount the Docker image in a directory that is a parent of both build_dir and run_from_dir + docker_mountpoint = os.path.commonpath([build_dir_abs, run_from_dir]) + docker_workdir = run_from_dir.replace(docker_mountpoint, _DOCKER_ROOT, 1) + docker_command = ["docker", "run", + "--name", docker_name, + "--volume", "{}:{}".format(docker_mountpoint, _DOCKER_ROOT), + "--workdir", docker_workdir, + "--rm", + _DOCKER_IMAGE] + build_command = docker_command + build_command + return build_command diff --git a/doc_builder/build_docs.py b/doc_builder/build_docs.py index 38a235b..3550b0f 100644 --- a/doc_builder/build_docs.py +++ b/doc_builder/build_docs.py @@ -4,6 +4,11 @@ import subprocess import argparse +import os +import random +import string +import sys +import signal from doc_builder.build_commands import get_build_dir, get_build_command def commandline_options(cmdline_args=None): @@ -68,6 +73,17 @@ def commandline_options(cmdline_args=None): parser.add_argument("-c", "--clean", action="store_true", help="Before building, run 'make clean'.") + parser.add_argument("-d", "--build-with-docker", action="store_true", + help="Use the escomp/base Docker container to build the documentation,\n" + "rather than relying on locally-installed versions of Sphinx, etc.\n" + "\n" + "IMPORTANT NOTE: The Docker image is mounted in a common parent directory\n" + "of the build directory and the current working directory. Problems can\n" + "arise if the Docker image is mounted in your home directory, so it is\n" + "best if you arrange your directories so that the documentation source\n" + "and documentation build directories are both contained within a\n" + "subdirectory of your home directory.") + parser.add_argument("--num-make-jobs", default=4, help="Number of parallel jobs to use for the make process.\n" "Default is 4.") @@ -81,6 +97,27 @@ def run_build_command(build_command): print(build_command_str) subprocess.check_call(build_command) +def setup_for_docker(): + """Do some setup for running with docker + + Returns a name that should be used in the docker run command + """ + + docker_name = 'build_docs_' + ''.join(random.choice(string.ascii_lowercase) for _ in range(8)) + + # It seems that, if we kill the build_docs process with Ctrl-C, the docker process + # continues. Handle that by implementing a signal handler. There may be a better / + # more pythonic way to handle this, but this should work. + def sigint_kill_docker(signum, frame): + """Signal handler: kill docker process before exiting""" + # pylint: disable=unused-argument + docker_kill_cmd = ["docker", "kill", docker_name] + subprocess.check_call(docker_kill_cmd) + sys.exit(1) + signal.signal(signal.SIGINT, sigint_kill_docker) + + return docker_name + def main(cmdline_args=None): """Top-level function implementing build_docs. @@ -89,6 +126,15 @@ def main(cmdline_args=None): """ opts = commandline_options(cmdline_args) + if opts.build_with_docker: + # We potentially reuse the same docker name for multiple docker processes: the + # clean and the actual build. However, since a given process should end before the + # next one begins, and because we use '--rm' in the docker run command, this + # should be okay. + docker_name = setup_for_docker() + else: + docker_name = None + # Note that we do a separate build for each version. This is # inefficient (assuming that the desired end result is for the # different versions to be identical), but was an easy-to-implement @@ -106,11 +152,15 @@ def main(cmdline_args=None): if opts.clean: clean_command = get_build_command(build_dir=build_dir, + run_from_dir=os.getcwd(), build_target="clean", - num_make_jobs=opts.num_make_jobs) + num_make_jobs=opts.num_make_jobs, + docker_name=docker_name) run_build_command(build_command=clean_command) build_command = get_build_command(build_dir=build_dir, + run_from_dir=os.getcwd(), build_target=opts.build_target, - num_make_jobs=opts.num_make_jobs) + num_make_jobs=opts.num_make_jobs, + docker_name=docker_name) run_build_command(build_command=build_command) diff --git a/test/test_unit_get_build_command.py b/test/test_unit_get_build_command.py index 8f0a104..92da96e 100644 --- a/test/test_unit_get_build_command.py +++ b/test/test_unit_get_build_command.py @@ -12,10 +12,44 @@ class TestGetBuildCommand(unittest.TestCase): def test_basic(self): """Tests basic usage""" build_command = get_build_command(build_dir="/path/to/foo", + run_from_dir="/irrelevant/path", build_target="html", - num_make_jobs=4) + num_make_jobs=4, + docker_name=None) expected = ["make", "BUILDDIR=/path/to/foo", "-j", "4", "html"] self.assertEqual(expected, build_command) + def test_docker(self): + """Tests usage with use_docker=True""" + build_command = get_build_command(build_dir="/path/to/foorepos/foodocs/versions/main", + run_from_dir="/path/to/foorepos/foocode/doc", + build_target="html", + num_make_jobs=4, + docker_name='foo') + expected = ["docker", "run", + "--name", "foo", + "--volume", "/path/to/foorepos:/home/user", + "--workdir", "/home/user/foocode/doc", + "--rm", + "escomp/base", + "make", "BUILDDIR=/path/to/foorepos/foodocs/versions/main", "-j", "4", "html"] + self.assertEqual(expected, build_command) + + def test_docker_relpath(self): + """Tests usage with use_docker=True, with a relative path to build_dir""" + build_command = get_build_command(build_dir="../../foodocs/versions/main", + run_from_dir="/path/to/foorepos/foocode/doc", + build_target="html", + num_make_jobs=4, + docker_name='foo') + expected = ["docker", "run", + "--name", "foo", + "--volume", "/path/to/foorepos:/home/user", + "--workdir", "/home/user/foocode/doc", + "--rm", + "escomp/base", + "make", "BUILDDIR=../../foodocs/versions/main", "-j", "4", "html"] + self.assertEqual(expected, build_command) + if __name__ == '__main__': unittest.main() From 794366f1d2edbdb25d5e2a1f82e27b6e130c4f3f Mon Sep 17 00:00:00 2001 From: Bill Sacks Date: Thu, 29 Oct 2020 16:49:37 -0600 Subject: [PATCH 2/4] With Docker: run command via /bin/bash -c This will be needed in an upcoming change, where I do some initial work before running the actual command. --- doc_builder/build_commands.py | 37 ++++++++++++++++------------- test/test_unit_get_build_command.py | 6 +++-- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/doc_builder/build_commands.py b/doc_builder/build_commands.py index 5fcddcc..5e3b7e6 100644 --- a/doc_builder/build_commands.py +++ b/doc_builder/build_commands.py @@ -79,20 +79,23 @@ def get_build_command(build_dir, run_from_dir, build_target, num_make_jobs, dock builddir_arg = "BUILDDIR={}".format(build_dir) build_command = ["make", builddir_arg, "-j", str(num_make_jobs), build_target] - if docker_name is not None: - if os.path.isabs(build_dir): - build_dir_abs = build_dir - else: - build_dir_abs = os.path.normpath(os.path.join(run_from_dir, build_dir)) - # mount the Docker image in a directory that is a parent of both build_dir and run_from_dir - docker_mountpoint = os.path.commonpath([build_dir_abs, run_from_dir]) - docker_workdir = run_from_dir.replace(docker_mountpoint, _DOCKER_ROOT, 1) - docker_command = ["docker", "run", - "--name", docker_name, - "--volume", "{}:{}".format(docker_mountpoint, _DOCKER_ROOT), - "--workdir", docker_workdir, - "--rm", - _DOCKER_IMAGE] - build_command = docker_command + build_command - - return build_command + if docker_name is None: + return build_command + + # But if we're using Docker, we have more work to do to create the command.... + + if os.path.isabs(build_dir): + build_dir_abs = build_dir + else: + build_dir_abs = os.path.normpath(os.path.join(run_from_dir, build_dir)) + # mount the Docker image in a directory that is a parent of both build_dir and run_from_dir + docker_mountpoint = os.path.commonpath([build_dir_abs, run_from_dir]) + docker_workdir = run_from_dir.replace(docker_mountpoint, _DOCKER_ROOT, 1) + docker_command = ["docker", "run", + "--name", docker_name, + "--volume", "{}:{}".format(docker_mountpoint, _DOCKER_ROOT), + "--workdir", docker_workdir, + "--rm", + _DOCKER_IMAGE, + "/bin/bash", "-c", " ".join(build_command)] + return docker_command diff --git a/test/test_unit_get_build_command.py b/test/test_unit_get_build_command.py index 92da96e..f978a33 100644 --- a/test/test_unit_get_build_command.py +++ b/test/test_unit_get_build_command.py @@ -32,7 +32,8 @@ def test_docker(self): "--workdir", "/home/user/foocode/doc", "--rm", "escomp/base", - "make", "BUILDDIR=/path/to/foorepos/foodocs/versions/main", "-j", "4", "html"] + "/bin/bash", "-c", + "make BUILDDIR=/path/to/foorepos/foodocs/versions/main -j 4 html"] self.assertEqual(expected, build_command) def test_docker_relpath(self): @@ -48,7 +49,8 @@ def test_docker_relpath(self): "--workdir", "/home/user/foocode/doc", "--rm", "escomp/base", - "make", "BUILDDIR=../../foodocs/versions/main", "-j", "4", "html"] + "/bin/bash", "-c", + "make BUILDDIR=../../foodocs/versions/main -j 4 html"] self.assertEqual(expected, build_command) if __name__ == '__main__': From 1b25c570acfbc6e47e6f36651626dfa0f6cbe00b Mon Sep 17 00:00:00 2001 From: Bill Sacks Date: Thu, 29 Oct 2020 17:19:40 -0600 Subject: [PATCH 3/4] With Docker: Make a sym link mapping the docker mountpoint to home The need for this is subtle: For CTSM, the documentation build invokes 'git lfs pull'. However, when doing the documentation build from a git worktree, the .git directory is replaced with a text file giving the absolute path to the parent git repository, e.g., 'gitdir: /Users/sacks/ctsm/ctsm0/.git/worktrees/ctsm5'. So when trying to execute a git command from within the Docker image, you get a message like, 'fatal: not a git repository: /Users/sacks/ctsm/ctsm0/.git/worktrees/ctsm5', because in Docker-land, this path doesn't exist. To work around this problem, we create a sym link in Docker's file system with the appropriate mapping. For example, if the local file system's mount-point is /path/to/foo, then we create a sym link at /path/to/foo in Docker's file system, pointing to the home directory in the Docker file system. --- doc_builder/build_commands.py | 28 +++++++++++++++++++++++----- test/test_unit_get_build_command.py | 4 ++++ 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/doc_builder/build_commands.py b/doc_builder/build_commands.py index 5e3b7e6..b0dfe6f 100644 --- a/doc_builder/build_commands.py +++ b/doc_builder/build_commands.py @@ -8,8 +8,8 @@ # The Docker image used to build documentation via Docker _DOCKER_IMAGE = "escomp/base" -# The top-level directory in the above docker image -_DOCKER_ROOT = "/home/user" +# The assumed location of the home directory in the above docker image +_DOCKER_HOME = "/home/user" def get_build_dir(build_dir=None, repo_root=None, version=None): """Return a string giving the path to the build directory. @@ -90,12 +90,30 @@ def get_build_command(build_dir, run_from_dir, build_target, num_make_jobs, dock build_dir_abs = os.path.normpath(os.path.join(run_from_dir, build_dir)) # mount the Docker image in a directory that is a parent of both build_dir and run_from_dir docker_mountpoint = os.path.commonpath([build_dir_abs, run_from_dir]) - docker_workdir = run_from_dir.replace(docker_mountpoint, _DOCKER_ROOT, 1) + docker_workdir = run_from_dir.replace(docker_mountpoint, _DOCKER_HOME, 1) + + # The need for the following is subtle: For CTSM, the documentation build invokes 'git + # lfs pull'. However, when doing the documentation build from a git worktree, the .git + # directory is replaced with a text file giving the absolute path to the parent git + # repository, e.g., 'gitdir: /Users/sacks/ctsm/ctsm0/.git/worktrees/ctsm5'. So when + # trying to execute a git command from within the Docker image, you get a message + # like, 'fatal: not a git repository: /Users/sacks/ctsm/ctsm0/.git/worktrees/ctsm5', + # because in Docker-land, this path doesn't exist. To work around this problem, we + # create a sym link in Docker's file system with the appropriate mapping. For example, + # if the local file system's mount-point is /path/to/foo, then we create a sym link at + # /path/to/foo in Docker's file system, pointing to the home directory in the Docker + # file system. + docker_symlink_command = "sudo mkdir -p {} && sudo ln -s {} {}".format( + os.path.dirname(docker_mountpoint), _DOCKER_HOME, docker_mountpoint) + + # This is the full command that we'll run via Docker + docker_run_command = docker_symlink_command + " && " + " ".join(build_command) + docker_command = ["docker", "run", "--name", docker_name, - "--volume", "{}:{}".format(docker_mountpoint, _DOCKER_ROOT), + "--volume", "{}:{}".format(docker_mountpoint, _DOCKER_HOME), "--workdir", docker_workdir, "--rm", _DOCKER_IMAGE, - "/bin/bash", "-c", " ".join(build_command)] + "/bin/bash", "-c", docker_run_command] return docker_command diff --git a/test/test_unit_get_build_command.py b/test/test_unit_get_build_command.py index f978a33..4bfb08a 100644 --- a/test/test_unit_get_build_command.py +++ b/test/test_unit_get_build_command.py @@ -33,6 +33,8 @@ def test_docker(self): "--rm", "escomp/base", "/bin/bash", "-c", + # Note that the following two lines are all one long string + "sudo mkdir -p /path/to && sudo ln -s /home/user /path/to/foorepos && " "make BUILDDIR=/path/to/foorepos/foodocs/versions/main -j 4 html"] self.assertEqual(expected, build_command) @@ -50,6 +52,8 @@ def test_docker_relpath(self): "--rm", "escomp/base", "/bin/bash", "-c", + # Note that the following two lines are all one long string + "sudo mkdir -p /path/to && sudo ln -s /home/user /path/to/foorepos && " "make BUILDDIR=../../foodocs/versions/main -j 4 html"] self.assertEqual(expected, build_command) From e4f2dee28401e8999a149c1695ae5de00a501678 Mon Sep 17 00:00:00 2001 From: Bill Sacks Date: Fri, 30 Oct 2020 06:52:18 -0600 Subject: [PATCH 4/4] Add -t option to docker run command This is needed for output to be colorful --- doc_builder/build_commands.py | 1 + test/test_unit_get_build_command.py | 2 ++ 2 files changed, 3 insertions(+) diff --git a/doc_builder/build_commands.py b/doc_builder/build_commands.py index b0dfe6f..a034c63 100644 --- a/doc_builder/build_commands.py +++ b/doc_builder/build_commands.py @@ -113,6 +113,7 @@ def get_build_command(build_dir, run_from_dir, build_target, num_make_jobs, dock "--name", docker_name, "--volume", "{}:{}".format(docker_mountpoint, _DOCKER_HOME), "--workdir", docker_workdir, + "-t", # "-t" is needed for colorful output "--rm", _DOCKER_IMAGE, "/bin/bash", "-c", docker_run_command] diff --git a/test/test_unit_get_build_command.py b/test/test_unit_get_build_command.py index 4bfb08a..a2f4d58 100644 --- a/test/test_unit_get_build_command.py +++ b/test/test_unit_get_build_command.py @@ -30,6 +30,7 @@ def test_docker(self): "--name", "foo", "--volume", "/path/to/foorepos:/home/user", "--workdir", "/home/user/foocode/doc", + "-t", "--rm", "escomp/base", "/bin/bash", "-c", @@ -49,6 +50,7 @@ def test_docker_relpath(self): "--name", "foo", "--volume", "/path/to/foorepos:/home/user", "--workdir", "/home/user/foocode/doc", + "-t", "--rm", "escomp/base", "/bin/bash", "-c",