[PATCH] D34197: Added Dockerfiles to build clang from sources.

[Manuel Klimek via Phabricator via llvm-commits]

klimek added inline comments.


================
Comment at: docs/Docker.rst:8-9
+
+In `llvm/utils/docker` there are a number of sources to build docker images with
+precompiled clang. They can be used by anyone who wants to build
+the docker images for their own use, or as a starting point for someone who
----------------
Grammatically a bit weird. Perhaps "You can find a number of source to build docker images for clang in llvm/utils/docker".


================
Comment at: docs/Docker.rst:14
+We currently provide Dockerfiles with `debian8` and `nvidia-cuda` base images.
+We also provide an `example` image, which contains placeholder that one would need
+to fill out in order to produce Dockerfiles for a new docker image.
----------------
placeholders


================
Comment at: docs/Docker.rst:30
+`Docker <https://www.docker.com/>`_ is a popular solution for running programs in
+an isolated and reproducible environment.
+It uses linux kernel namespaces and cgroups to provide a lightweight isolation
----------------
Perhaps add ", especially to maintain releases for software deployed to large distributed fleets." or somesuch.


================
Comment at: utils/docker/build_docker_image.sh:40
+echo "Building $DOCKER_REPOSITORY-build$DOCKER_TAG"
+docker build -t "$DOCKER_REPOSITORY-build$DOCKER_TAG" \
+  --build-arg "buildscript_args=$*" \
----------------
Perhaps check that docker is installed at the beginning. Also, did you by chance try all this on a mac?


================
Comment at: utils/docker/debian8/build/Dockerfile:12
+
+LABEL maintainer "Ilya Biryukov <ibiryukov at google.com>"
+
----------------
I'd have expected "LLVM Developers <...>"? Chandler, opinions?


================
Comment at: utils/docker/nvidia-cuda/build/build_install_clang.sh:1
+#!/usr/bin/env bash
+#===- llvm/utils/docker/nvidia-cuda/build/build_install_clang.sh ---------===//
----------------
Are these build_install_clang.sh actually different at all?


https://reviews.llvm.org/D34197