[GitHub] marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build
marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build URL: https://github.com/apache/incubator-mxnet/pull/10485#discussion_r181140317 ## File path: docs/Jenkinsfile ## @@ -0,0 +1,82 @@ +// -*- mode: groovy -*- + +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// Jenkins pipeline +// See documents at https://jenkins.io/doc/book/pipeline/jenkinsfile/ + +// command to start a docker container +docker_run = 'tests/ci_build/ci_build.sh' Review comment: Remove this This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build
marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build URL: https://github.com/apache/incubator-mxnet/pull/10485#discussion_r181140285 ## File path: docs/Jenkinsfile ## @@ -0,0 +1,82 @@ +// -*- mode: groovy -*- + +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// Jenkins pipeline +// See documents at https://jenkins.io/doc/book/pipeline/jenkinsfile/ + +// command to start a docker container +docker_run = 'tests/ci_build/ci_build.sh' +// timeout in minutes +max_time = 1440 Review comment: Set to 60 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build
marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build URL: https://github.com/apache/incubator-mxnet/pull/10485#discussion_r181139738 ## File path: ci/docker/install/ubuntu_scala.sh ## @@ -22,10 +22,17 @@ set -ex # install libraries for mxnet's scala package on ubuntu +echo 'Installing Scala...' apt-get install -y software-properties-common add-apt-repository -y ppa:webupd8team/java apt-get update echo "oracle-java8-installer shared/accepted-oracle-license-v1-1 select true" | debconf-set-selections apt-get install -y oracle-java8-installer apt-get install -y oracle-java8-set-default -apt-get update && apt-get install -y maven \ No newline at end of file + +echo "deb https://dl.bintray.com/sbt/debian /" | tee -a /etc/apt/sources.list.d/sbt.list +apt-key adv --keyserver hkp://keyserver.ubuntu.com:80 --recv 2EE0EA64E40A89B84B2DF73499E82A75642AC823 +apt-get update && apt-get install -y \ +maven \ +sbt \ +scala Review comment: Could we pin the scala version? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build
marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build URL: https://github.com/apache/incubator-mxnet/pull/10485#discussion_r180933894 ## File path: docs/build_version_doc/build_all_version.sh ## @@ -59,16 +61,21 @@ for tag in $tag_list; do then git checkout master git pull +# Copy the latest README.md to the site root +cp README.md ../$built else git checkout "tags/$tag" fi # this gets around the Python 3 support issue in old versions of mxdoc.py -if [ $tag == '0.11.0' ] - then - git checkout master -- docs/mxdoc.py -fi -git submodule update || exit 1 -git submodule update --init --recursive + +# uncomment this if you must build in a Python 3 environment Review comment: I think this is related to our previous discussion about using the build scripts of each branch individually instead of making the master script backwards compatible, right? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build
marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build URL: https://github.com/apache/incubator-mxnet/pull/10485#discussion_r180927140 ## File path: docs/build_version_doc/build_all_version.sh ## @@ -59,16 +61,21 @@ for tag in $tag_list; do then git checkout master git pull +# Copy the latest README.md to the site root +cp README.md ../$built else git checkout "tags/$tag" fi # this gets around the Python 3 support issue in old versions of mxdoc.py -if [ $tag == '0.11.0' ] - then - git checkout master -- docs/mxdoc.py -fi -git submodule update || exit 1 -git submodule update --init --recursive + +# uncomment this if you must build in a Python 3 environment Review comment: Could we remove unused code? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build
marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build URL: https://github.com/apache/incubator-mxnet/pull/10485#discussion_r180926919 ## File path: ci/docker/install/ubuntu_docs.sh ## @@ -21,8 +21,21 @@ # the whole docker cache for the image set -ex -wget http://downloads.lightbend.com/scala/2.11.8/scala-2.11.8.deb && \ -dpkg -i scala-2.11.8.deb && rm scala-2.11.8.deb +# Install dependencies +echo 'Installing dependencies...' +apt-get install -y \ +doxygen \ +pandoc -apt-get install -y doxygen libatlas-base-dev graphviz pandoc -pip install sphinx==1.3.5 CommonMark==0.5.4 breathe mock recommonmark pypandoc beautifulsoup4 +echo 'Installing python packages...' +pip install --upgrade pip && pip install \ +beautifulsoup4 \ +breathe \ +CommonMark==0.5.4 \ +h5py \ +mock==1.0.1 \ +pypandoc \ +recommonmark==0.4.0 \ +sphinx==1.5.6 + +echo 'Dependency installation complete.' Review comment: This looks perfect now, thanks a lot! This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build
marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build URL: https://github.com/apache/incubator-mxnet/pull/10485#discussion_r180921222 ## File path: ci/docker/install/ubuntu_docs.sh ## @@ -21,8 +21,44 @@ # the whole docker cache for the image set -ex -wget http://downloads.lightbend.com/scala/2.11.8/scala-2.11.8.deb && \ -dpkg -i scala-2.11.8.deb && rm scala-2.11.8.deb +# Install dependencies +echo 'Installing dependencies...' +apt-get install -y \ +apt-transport-https \ +build-essential \ +ca-certificates \ +curl \ +doxygen \ +git \ +libatlas-base-dev \ +libjemalloc-dev \ +liblapack-dev \ +libopenblas-dev \ +libopencv-dev \ +pandoc \ +python-numpy \ +python-pip \ +software-properties-common \ +unzip \ +wget -apt-get install -y doxygen libatlas-base-dev graphviz pandoc -pip install sphinx==1.3.5 CommonMark==0.5.4 breathe mock recommonmark pypandoc beautifulsoup4 +echo 'Installing Scala...' +# Setup Scala Review comment: Wait, that's actually a good point! Where is the scala runtime actually installed? :O @nswamy do you have an idea? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build
marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build URL: https://github.com/apache/incubator-mxnet/pull/10485#discussion_r180921222 ## File path: ci/docker/install/ubuntu_docs.sh ## @@ -21,8 +21,44 @@ # the whole docker cache for the image set -ex -wget http://downloads.lightbend.com/scala/2.11.8/scala-2.11.8.deb && \ -dpkg -i scala-2.11.8.deb && rm scala-2.11.8.deb +# Install dependencies +echo 'Installing dependencies...' +apt-get install -y \ +apt-transport-https \ +build-essential \ +ca-certificates \ +curl \ +doxygen \ +git \ +libatlas-base-dev \ +libjemalloc-dev \ +liblapack-dev \ +libopenblas-dev \ +libopencv-dev \ +pandoc \ +python-numpy \ +python-pip \ +software-properties-common \ +unzip \ +wget -apt-get install -y doxygen libatlas-base-dev graphviz pandoc -pip install sphinx==1.3.5 CommonMark==0.5.4 breathe mock recommonmark pypandoc beautifulsoup4 +echo 'Installing Scala...' +# Setup Scala Review comment: Wait, that's actually a good point! Where is the scala runtime actually installed? :O This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build
marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build URL: https://github.com/apache/incubator-mxnet/pull/10485#discussion_r180921072 ## File path: ci/docker/install/ubuntu_docs.sh ## @@ -21,8 +21,44 @@ # the whole docker cache for the image set -ex -wget http://downloads.lightbend.com/scala/2.11.8/scala-2.11.8.deb && \ -dpkg -i scala-2.11.8.deb && rm scala-2.11.8.deb +# Install dependencies +echo 'Installing dependencies...' +apt-get install -y \ +apt-transport-https \ +build-essential \ +ca-certificates \ +curl \ +doxygen \ +git \ +libatlas-base-dev \ Review comment: Right, at the moment they're not versioned. The idea here is to make sure every single dependency is only defined at one place and thus giving us the ability to pin the version if required - without having to look through all the scripts and potentially missing a duplicate. Fir libatlas and libjemalloc, feel free to move them to core as they are definitely no api specific dependencies. curl, unzip and wget are required by some install scripts and the runtime as well if I'm not mistake. They're also a core dependency. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build
marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build URL: https://github.com/apache/incubator-mxnet/pull/10485#discussion_r180716560 ## File path: ci/docker/install/ubuntu_docs.sh ## @@ -21,8 +21,44 @@ # the whole docker cache for the image set -ex -wget http://downloads.lightbend.com/scala/2.11.8/scala-2.11.8.deb && \ -dpkg -i scala-2.11.8.deb && rm scala-2.11.8.deb +# Install dependencies +echo 'Installing dependencies...' +apt-get install -y \ +apt-transport-https \ +build-essential \ +ca-certificates \ +curl \ +doxygen \ +git \ +libatlas-base-dev \ Review comment: Overriding matters because this script is part of the ubuntu_cpu dockerfile pipeline (https://github.com/apache/incubator-mxnet/blob/master/ci/docker/Dockerfile.build.ubuntu_cpu#L45). I don't think we need to install libjemalloc-dev, liblapack-dev, libopenblas-dev, libopencv-dev, numpy, git and all the other libraries at this point since they should already be present in the underlying layers (see https://github.com/apache/incubator-mxnet/blob/master/ci/docker/install/ubuntu_core.sh#L27). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build
marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build URL: https://github.com/apache/incubator-mxnet/pull/10485#discussion_r180716021 ## File path: ci/docker/install/ubuntu_docs.sh ## @@ -21,8 +21,44 @@ # the whole docker cache for the image set -ex -wget http://downloads.lightbend.com/scala/2.11.8/scala-2.11.8.deb && \ -dpkg -i scala-2.11.8.deb && rm scala-2.11.8.deb +# Install dependencies +echo 'Installing dependencies...' +apt-get install -y \ +apt-transport-https \ +build-essential \ +ca-certificates \ +curl \ +doxygen \ +git \ +libatlas-base-dev \ +libjemalloc-dev \ +liblapack-dev \ +libopenblas-dev \ +libopencv-dev \ +pandoc \ +python-numpy \ +python-pip \ +software-properties-common \ +unzip \ +wget -apt-get install -y doxygen libatlas-base-dev graphviz pandoc -pip install sphinx==1.3.5 CommonMark==0.5.4 breathe mock recommonmark pypandoc beautifulsoup4 +echo 'Installing Scala...' +# Setup Scala Review comment: The problem is that this script is supposed to only install things that are related to the document generation itself. Scala, for example, is installed in https://github.com/apache/incubator-mxnet/blob/master/ci/docker/Dockerfile.build.ubuntu_cpu#L31 and no additional installation should be made at a later point in time. If there's something missing here, it's best to make the change to the install/ubuntu_scala.sh file instead. At the moment, your commands are overlapping with the scala install script and could thus cause conflicts or unexpected side effects This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build
marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build URL: https://github.com/apache/incubator-mxnet/pull/10485#discussion_r180533917 ## File path: ci/docker/install/ubuntu_docs.sh ## @@ -21,8 +21,44 @@ # the whole docker cache for the image set -ex -wget http://downloads.lightbend.com/scala/2.11.8/scala-2.11.8.deb && \ -dpkg -i scala-2.11.8.deb && rm scala-2.11.8.deb +# Install dependencies +echo 'Installing dependencies...' +apt-get install -y \ +apt-transport-https \ +build-essential \ +ca-certificates \ +curl \ +doxygen \ +git \ +libatlas-base-dev \ +libjemalloc-dev \ +liblapack-dev \ +libopenblas-dev \ +libopencv-dev \ +pandoc \ +python-numpy \ +python-pip \ +software-properties-common \ +unzip \ +wget -apt-get install -y doxygen libatlas-base-dev graphviz pandoc -pip install sphinx==1.3.5 CommonMark==0.5.4 breathe mock recommonmark pypandoc beautifulsoup4 +echo 'Installing Scala...' +# Setup Scala Review comment: Scala should already be installed This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build
marcoabreu commented on a change in pull request #10485: [MXNET-304][RFC] Jenkins docs build URL: https://github.com/apache/incubator-mxnet/pull/10485#discussion_r180533790 ## File path: ci/docker/install/ubuntu_docs.sh ## @@ -21,8 +21,44 @@ # the whole docker cache for the image set -ex -wget http://downloads.lightbend.com/scala/2.11.8/scala-2.11.8.deb && \ -dpkg -i scala-2.11.8.deb && rm scala-2.11.8.deb +# Install dependencies +echo 'Installing dependencies...' +apt-get install -y \ +apt-transport-https \ +build-essential \ +ca-certificates \ +curl \ +doxygen \ +git \ +libatlas-base-dev \ Review comment: Are all these dependencies really required? Installing them here might override other version restrictions. It would be better to run this script after the other install scripts and then see what's actually required for the document generation. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services