> On July 4, 2017, 12:02 p.m., Benjamin Bannier wrote: > > This is a great start, but I would prefer if we wouldn't directly mount an > > existing source directory into the container and bootstrap, configure and > > build in it (this is also broken for e.g., in-source builds). > > > > What about instead modify mounting the Mesos source tree into the > > container, then inside the container cloning from it, and building in the > > clean checkout? This woudl not only minimize the risk of destructive > > side-effects outside the container, but also e.g., make sure that we do not > > by accident pick up uncommitted changes. This is the approach we also > > followed in `support/mesos-tidy`. > > > > I left some first comments, but will look again once the handling of the > > source tree is under control.
Good point. This script was originally intended to be used by CI and not by human users. The CI job does a clean checkout of mesos source and site repos, so there shouldn't be any problem with picking up unexpected uncommitted changes from mesos source repo. If this script will be eventually used by humans as well, as suggested in the next review, then yes I agree we need to be more careful. Although for dev workflow case, it might actually be preferrable that the website dev build picks up uncommitted changes while testing; otherwise users need to commit their code to see how it looks on website while dev/testing, which is not a great experience IMO. So I would like to punt on this for now until we figure out merging the dev and publish workflows. > On July 4, 2017, 12:02 p.m., Benjamin Bannier wrote: > > support/jenkins/websitebot.sh > > Lines 31 (patched) > > <https://reviews.apache.org/r/60439/diff/1/?file=1763077#file1763077line31> > > > > If the source tree contains uncommitted changes, the output will not > > reproducibly correspond to this SHA1. > > > > I'd suggest to instead clone the source tree instead. see my comment above. > On July 4, 2017, 12:02 p.m., Benjamin Bannier wrote: > > support/mesos-website.sh > > Lines 33 (patched) > > <https://reviews.apache.org/r/60439/diff/1/?file=1763078#file1763078line33> > > > > Not crucial here right now, but let's put this string into single > > quotes, see https://github.com/koalaman/shellcheck/wiki/SC2064. > > > > Also please consider working on getting this image published in the > > Mesos dockerhub org so we can pull from there instead of having the rebuild > > the same image over and over again. See e.g., `support/mesos-tidy.sh` how > > we already do this for other images. Yea, I very much liked to use a image hosted on dockerhub; but one thing at a time :) > On July 4, 2017, 12:02 p.m., Benjamin Bannier wrote: > > support/mesos-website/Dockerfile > > Lines 1-11 (patched) > > <https://reviews.apache.org/r/60439/diff/1/?file=1763079#file1763079line1> > > > > We now have one Docker image to run tests in CI (via > > `docker_build.sh`), one to run clang-tidy checks (`mesos-tidy/Dockerfile`), > > and are adding another one here to build the website. > > > > We should try to reduce the number of these images or remove > > duplication, e.g., this image could be in principle implemented as a small > > addon on top of `mesos/mesos-tidy` > > > > FROM mesos/mesos-tidy:latest > > LABEL Description="This image is used for generating Mesos web > > site." > > > > # Install ruby and doxygen tools ... I agree. We'll fix it as part of https://issues.apache.org/jira/browse/MESOS-6984 - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60439/#review179310 ----------------------------------------------------------- On June 29, 2017, 9:59 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60439/ > ----------------------------------------------------------- > > (Updated June 29, 2017, 9:59 p.m.) > > > Review request for mesos, Benjamin Bannier, Benjamin Mahler, and haosdent > huang. > > > Bugs: MESOS-7625 > https://issues.apache.org/jira/browse/MESOS-7625 > > > Repository: mesos > > > Description > ------- > > These scripts are expected to be run by ASF CI on any commit to > the mesos repo. The directory layout is similar to tidybot CI job. > > > Diffs > ----- > > support/jenkins/websitebot.sh PRE-CREATION > support/mesos-website.sh PRE-CREATION > support/mesos-website/Dockerfile PRE-CREATION > support/mesos-website/build.sh PRE-CREATION > support/mesos-website/entrypoint.sh PRE-CREATION > > > Diff: https://reviews.apache.org/r/60439/diff/1/ > > > Testing > ------- > > Tested by running a CI job pointing to a branch containing this patch. > > > Thanks, > > Vinod Kone > >
