zrhoffman commented on a change in pull request #5004:
URL: https://github.com/apache/trafficcontrol/pull/5004#discussion_r482303017
##########
File path: pkg
##########
@@ -134,9 +152,23 @@ while (( "$#" )); do
if (( "$verbose" == 0 )); then
exec >/dev/null 2>&1
fi
+
+ # Build the project
"${COMPOSECMD[@]}" -f $COMPOSE_FILE pull $1 || exit 1
"${COMPOSECMD[@]}" -f $COMPOSE_FILE build --pull $1 || exit 1
"${COMPOSECMD[@]}" -f $COMPOSE_FILE run "${RUN_OPTIONS[@]}"
--rm $1 || exit 1
+
+ # Check for a chained compose file for this particular project.
+ # A chained compose file will be named exactly the same as main
docker-compose, with .service added,
+ # where <service> is the name of the specific service to be
chained. The file may be a symlink to another
+ # compose file, in which case the symlink will be followed
before it is processed.
+ # A "dist" symlink will be temporarily created in that
directory to facilitate the collation of artefacts.
+ [ ! -e "$COMPOSE_FILE.$1" ] || ln -s "$(dirname -- "$(realpath
-e -- "$SELF")")/dist" "$(dirname -- "$(realpath -e --
"$COMPOSE_FILE.$1")")/dist" || exit 1
+ [ ! -e "$COMPOSE_FILE.$1" ] || $SELF -f $(realpath -e
"$COMPOSE_FILE.$1") $([ "$verbose" == 0 ] || echo "-v") $([ "$quiet" == 0 ] ||
echo "-q") $([ "$debug" == 0 ] || echo "-d") $("${COMPOSECMD[@]}" -f $(realpath
-e "$COMPOSE_FILE.$1") config --services) || exit 1
+ [ ! -e "$COMPOSE_FILE.$1" ] || rm "$(dirname -- "$(realpath -e
-- "$COMPOSE_FILE.$1")")/dist"
Review comment:
This should be in an `if` statement instead of checking `! -e` 3 times
##########
File path: traffic_server/tsb/Dockerfile
##########
@@ -0,0 +1,75 @@
+# Licensed to the Apache Software Fou:qndation (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.
+#
+# SPDX-License-Identifier: Apache-2.0
+FROM centos:7
+
+RUN yum clean all \
+ && yum install -y deltarpm epel-release centos-release-scl-rh \
+ && yum-config-manager --enable rhel-server-rhscl-7-rpms \
+ && yum clean all \
+ && yum install -y \
+ autoconf \
+ automake \
+ pkgconfig \
+ libtool \
+ perl-ExtUtils-MakeMaker \
+ gcc-c++ \
+ glibc-devel \
+ tcl-devel \
+ expat-devel \
+ pcre \
+ pcre-devel \
+ libcap-devel \
+ flex \
+ hwloc-devel \
+ libuuid-devel \
+ lua-devel \
+ make \
+ git \
+ rpm-build \
+ man \
+ sudo \
+ python3 \
+ python35 \
+ ed \
+ zlib \
+ zlib-devel \
+ ncurses-devel \
+ libcurl-devel \
+ devtoolset-7 \
+ luajit-devel \
+ openssl \
+ openssl-devel \
+ hwloc \
+ perl-URI \
+ nmap-ncat \
+ perl-Digest-SHA \
+ nano \
+ ed \
+ && yum clean all
+
+ADD jansson.pic.patch /opt/src/
+ADD cjose.pic.patch /opt/src/
+ADD https://bootstrap.pypa.io/get-pip.py /
+RUN python get-pip.py
+RUN pip install --user Sphinx
+ADD run.sh /
+ADD trafficserver.spec /rpmbuilddir/SPECS/trafficserver.spec
+ADD traffic_server_jemalloc /rpmbuilddir/SOURCES/traffic_server_jemalloc
+RUN /usr/sbin/useradd -u 176 -r ats -s /sbin/nologin -d /
+CMD set -o pipefail; scl enable devtoolset-7 ./run.sh 2>&1 | tee
/rpmbuilddir/RPMS/x86_64/build.log
Review comment:
`CMD set -o pipefail` fails with some base images (e.g., `debian`), so
that command should follow the Dockerfile convention of using `&&`, not `;`.
##########
File path: traffic_server/tsb/Dockerfile
##########
@@ -0,0 +1,75 @@
+# Licensed to the Apache Software Fou:qndation (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.
+#
+# SPDX-License-Identifier: Apache-2.0
+FROM centos:7
+
+RUN yum clean all \
+ && yum install -y deltarpm epel-release centos-release-scl-rh \
+ && yum-config-manager --enable rhel-server-rhscl-7-rpms \
+ && yum clean all \
+ && yum install -y \
+ autoconf \
+ automake \
+ pkgconfig \
+ libtool \
+ perl-ExtUtils-MakeMaker \
+ gcc-c++ \
+ glibc-devel \
+ tcl-devel \
+ expat-devel \
+ pcre \
+ pcre-devel \
+ libcap-devel \
+ flex \
+ hwloc-devel \
+ libuuid-devel \
+ lua-devel \
+ make \
+ git \
+ rpm-build \
+ man \
+ sudo \
+ python3 \
+ python35 \
+ ed \
+ zlib \
+ zlib-devel \
+ ncurses-devel \
+ libcurl-devel \
+ devtoolset-7 \
+ luajit-devel \
+ openssl \
+ openssl-devel \
+ hwloc \
+ perl-URI \
+ nmap-ncat \
+ perl-Digest-SHA \
+ nano \
+ ed \
Review comment:
Is there something significant about this particular package order?
* If so, that significance should be commented.
* If not, the list should be alphabetized.
##########
File path: traffic_server/tsb/run.sh
##########
@@ -0,0 +1,64 @@
+#!/usr/bin/env bash
+
+# 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.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+die() {
+ { test -n "$@" && echo "$@"; exit 1; } >&2
Review comment:
The only command here with console output is `echo "$@"`, so the curly
braces are unnecessary.
##########
File path: traffic_server/tsb/patches.yml
##########
@@ -0,0 +1,15 @@
+ats:
+- f5ac00f3606bd826c52552e59f067d3fe3fcda33
+- 721f8ffca6ee8782573084c671e45ce392d239ef
+- d295784ba57af7e8e2dba09b85a1cbcaacf797f3
+- ef57a68f4f9d6f6dd5563cff82cddb8adcb8c28c
+- abe7a933e403b88f1b13ab7f2ebdd6ce95202b42
+- 9ac557ba3c0f28979c996cb49a827940bfb569d8
+- 72e582d3de630008641e7849f7cd42b11f6f081b
+- ad33e0fff0091093cd31fcfd429933299cb8bf5b
+- 1f13f346ef9ae4c0035f5755af4fd28b6f84f1e0
+- fc66eea820c8ba44196f2051aa41aedd7b874b0b
+- 5b38fb26737b38267615102775d81136a843075c
+openssl:
+- ec6788fb8ca0704b503c3a0030a142d9805895a6
Review comment:
We'll need a comment for each commit hash in this patch list describing
its significance
##########
File path: traffic_server/tsb/Dockerfile
##########
@@ -0,0 +1,75 @@
+# Licensed to the Apache Software Fou:qndation (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.
+#
+# SPDX-License-Identifier: Apache-2.0
+FROM centos:7
+
+RUN yum clean all \
+ && yum install -y deltarpm epel-release centos-release-scl-rh \
+ && yum-config-manager --enable rhel-server-rhscl-7-rpms \
+ && yum clean all \
+ && yum install -y \
+ autoconf \
+ automake \
+ pkgconfig \
+ libtool \
+ perl-ExtUtils-MakeMaker \
+ gcc-c++ \
+ glibc-devel \
+ tcl-devel \
+ expat-devel \
+ pcre \
+ pcre-devel \
+ libcap-devel \
+ flex \
+ hwloc-devel \
+ libuuid-devel \
+ lua-devel \
+ make \
+ git \
+ rpm-build \
+ man \
+ sudo \
+ python3 \
+ python35 \
+ ed \
Review comment:
`ed` appears in the package list twice
##########
File path: traffic_server/tsb/Dockerfile
##########
@@ -0,0 +1,75 @@
+# Licensed to the Apache Software Fou:qndation (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.
+#
+# SPDX-License-Identifier: Apache-2.0
+FROM centos:7
+
+RUN yum clean all \
+ && yum install -y deltarpm epel-release centos-release-scl-rh \
+ && yum-config-manager --enable rhel-server-rhscl-7-rpms \
+ && yum clean all \
+ && yum install -y \
+ autoconf \
+ automake \
+ pkgconfig \
+ libtool \
+ perl-ExtUtils-MakeMaker \
+ gcc-c++ \
+ glibc-devel \
+ tcl-devel \
+ expat-devel \
+ pcre \
+ pcre-devel \
+ libcap-devel \
+ flex \
+ hwloc-devel \
+ libuuid-devel \
+ lua-devel \
+ make \
+ git \
+ rpm-build \
+ man \
+ sudo \
+ python3 \
+ python35 \
+ ed \
+ zlib \
+ zlib-devel \
+ ncurses-devel \
+ libcurl-devel \
+ devtoolset-7 \
+ luajit-devel \
+ openssl \
+ openssl-devel \
+ hwloc \
+ perl-URI \
+ nmap-ncat \
+ perl-Digest-SHA \
+ nano \
+ ed \
+ && yum clean all
+
+ADD jansson.pic.patch /opt/src/
+ADD cjose.pic.patch /opt/src/
+ADD https://bootstrap.pypa.io/get-pip.py /
+RUN python get-pip.py
+RUN pip install --user Sphinx
+ADD run.sh /
+ADD trafficserver.spec /rpmbuilddir/SPECS/trafficserver.spec
+ADD traffic_server_jemalloc /rpmbuilddir/SOURCES/traffic_server_jemalloc
Review comment:
Use `COPY` for local files so the stuff that is added from a remote
source sticks out.
##########
File path: traffic_server/tsb/repos.yml
##########
@@ -0,0 +1,32 @@
+ats:
+ src: https://github.com/apache/trafficserver.git
+ branch: 8.0.x
+ head: a29528dffb8ea1c6fa57fe0c76ca1ec9d0f08694
+ extra:
+ - https://github.com/jrushford/trafficserver.git
+ - https://github.com/traeak/trafficserver.git
+cjose:
+ src: https://github.com/cisco/cjose
+ branch: master
+ head: 254ab05e04cc32d866712bea838990eb4011cbf5
+ extra: []
+jansson:
+ src: https://github.com/akheron/jansson
+ branch: master
+ head: aed855e6920923898b94a1b922fbace27a34ddf2
+ extra: []
+luacrypto:
+ src: https://github.com/mkottman/luacrypto.git
+ branch: master
+ head: 8c3f7f0caf023fe327f9e60391fc80df2d08a169
+ extra: []
+openssl:
+ src: https://github.com/openssl/openssl.git
+ branch: OpenSSL_1_1_1
+ head: 1708e3e85b4a86bae26860aa5d2913fc8eff6086
+ extra: []
+trafficcontrol:
+ src: https://github.com/apache/trafficcontrol
Review comment:
Some of these remotes end with `.git` and some do not. It should be
consistent
##########
File path: traffic_server/tsb/run.sh
##########
@@ -0,0 +1,64 @@
+#!/usr/bin/env bash
+
+# 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.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+die() {
+ { test -n "$@" && echo "$@"; exit 1; } >&2
+}
+
+mkdir /opt/build
+cp -far /opt/{src,build}/jansson
+cp -far /opt/{src,build}/cjose
+cp -far /opt/{src,build}/openssl
+cp -far /opt/{src,build}/luacrypto
Review comment:
`-r` is unnecessary if `-a` is specified, and the 4 lines can be
simplified to 1 line like
```bash
cp -fa /opt/src/{cjose,jansson,luacrypto,openssl} /opt/build
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
