[tarantool-patches] [PATCH v2 1/2] Gitlab-ci updated after reiview

Tarantool development patches archive
 help / color / mirror / Atom feed

* [tarantool-patches] [PATCH v2 1/2] Gitlab-ci updated after reiview - w/o perf
@ 2019-06-15  4:46 Alexander V. Tikhonov
  2019-06-18 11:07 ` [tarantool-patches] " Alexander Turenko
  0 siblings, 1 reply; 3+ messages in thread
From: Alexander V. Tikhonov @ 2019-06-15  4:46 UTC (permalink / raw)
  To: Alexander Turenko; +Cc: Alexander V. Tikhonov, tarantool-patches

---

Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4156-gitlab-ci-testing
Issue: https://github.com/tarantool/tarantool/issues/4156

 .gitlab-ci.yml           | 328 +++++++++++++++++++++++++++++++++++++++
 .gitlab.mk               |  57 +++++++
 .travis.mk               | 101 +++++++++---
 .travis.yml              |   3 +
 images/Dockerfile.debian |   4 +
 images/Dockerfile.lto    |   5 +
 6 files changed, 475 insertions(+), 23 deletions(-)
 create mode 100644 .gitlab-ci.yml
 create mode 100644 .gitlab.mk
 create mode 100644 images/Dockerfile.debian
 create mode 100644 images/Dockerfile.lto

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
new file mode 100644
index 000000000..01c4eacea
--- /dev/null
+++ b/.gitlab-ci.yml
@@ -0,0 +1,328 @@
+stages:
+  - bootstrap
+  - test
+  - perf_deploy
+
+variables:
+  VERSION_SUFFIX: "master"
+  IMAGE_TEST: "${CI_REGISTRY}/${CI_PROJECT_PATH}:debian-stretch_${VERSION_SUFFIX}"
+  IMAGE_TEST_LTO: "${CI_REGISTRY}/${CI_PROJECT_PATH}:debian-buster_${VERSION_SUFFIX}"
+  FLAG_LTO: "-DENABLE_LTO=ON"
+  GITLAB_MAKE: "make -f .gitlab.mk"
+
+# Bootstraps
+
+debian9:
+  stage: bootstrap
+  tags:
+    - bootstrap
+  variables:
+    IMAGE: ${IMAGE_TEST}
+    DOCKERFILE_SUFFIX: "debian"
+  script:
+    - ${GITLAB_MAKE} docker_bootstrap
+
+debian10_lto:
+  only:
+    refs:
+      - master
+      - /.*/
+  stage: bootstrap
+  tags:
+    - bootstrap
+  variables:
+    IMAGE: ${IMAGE_TEST_LTO}
+    DOCKERFILE_SUFFIX: "lto"
+  script:
+    - ${GITLAB_MAKE} docker_bootstrap
+
+# Tests
+
+release:
+  image: ${IMAGE_TEST}
+  stage: test
+  tags:
+    - docker_test
+  script:
+    - ${GITLAB_MAKE} runtest_ubuntu
+
+debug:
+  image: ${IMAGE_TEST}
+  stage: test
+  tags:
+    - docker_test
+  script:
+    - ${GITLAB_MAKE} runtest_cov_ubuntu
+
+release_clang:
+  image: ${IMAGE_TEST}
+  stage: test
+  tags:
+    - docker_test
+  variables:
+    CC: clang
+    CXX: clang++
+  script:
+    - ${GITLAB_MAKE} runtest_ubuntu
+
+release_lto:
+  only:
+    refs:
+      - master
+      - /.*/
+  image: ${IMAGE_TEST_LTO}
+  stage: test
+  tags:
+    - docker_test
+  variables:
+    CMAKE_EXTRA_PARAMS: ${FLAG_LTO}
+  script:
+    - ${GITLAB_MAKE} runtest_ubuntu
+
+release_lto_clang8:
+  only:
+    refs:
+      - master
+      - /.*/
+  image: ${IMAGE_TEST_LTO}
+  stage: test
+  tags:
+    - docker_test
+  variables:
+    CC: clang-8
+    CXX: clang++-8
+    CMAKE_EXTRA_PARAMS: ${FLAG_LTO}
+  script:
+    - ${GITLAB_MAKE} runtest_ubuntu
+
+osx13_release:
+  only:
+    refs:
+      - master
+      - /.*/
+  stage: test
+  tags:
+    - vms_sierra
+  variables:
+    VMS_NAME: 'Sierra'
+    VMS_USER: 'tarantool'
+    VMS_PORT: '2212'
+  before_script:
+    - ${GITLAB_MAKE} vms_start
+  script:
+    - export
+    - ${GITLAB_MAKE} vms_test_osx
+  after_script:
+    - ${GITLAB_MAKE} vms_shutdown
+
+osx14_release:
+  stage: test
+  tags:
+    - vms_mojave
+  variables:
+    VMS_NAME: 'Mojave'
+    VMS_USER: 'tarantool'
+    VMS_PORT: '2222'
+  before_script:
+    - ${GITLAB_MAKE} vms_start
+  script:
+    - export
+    - ${GITLAB_MAKE} vms_test_osx
+  after_script:
+    - ${GITLAB_MAKE} vms_shutdown
+
+osx14_release_lto:
+  only:
+    refs:
+      - master
+      - /.*/
+  stage: test
+  tags:
+    - vms_mojave
+  variables:
+    EXTRA_ENV: "export CMAKE_EXTRA_PARAMS=${FLAG_LTO} ;"
+    VMS_NAME: 'Mojave'
+    VMS_USER: 'tarantool'
+    VMS_PORT: '2222'
+  before_script:
+    - ${GITLAB_MAKE} vms_start
+  script:
+    - export
+    - ${GITLAB_MAKE} vms_test_osx
+  after_script:
+    - ${GITLAB_MAKE} vms_shutdown
+
+freebsd_release:
+  stage: test
+  tags:
+    - vms_freebsd
+  variables:
+    VMS_NAME: 'Freebsd'
+    VMS_USER: 'vagrant'
+    VMS_PORT: '2232'
+    TRAVIS_MAKE: 'gmake -f .travis.mk'
+  before_script:
+    - ${GITLAB_MAKE} vms_start
+  script:
+    - export
+    - ${GITLAB_MAKE} vms_test_freebsd
+  after_script:
+    - ${GITLAB_MAKE} vms_shutdown
+
+# Packs
+
+pack_cent6:
+  only:
+    refs:
+      - master
+      - /.*/
+  image: packpack/packpack:el-6
+  stage: perf_deploy
+  tags:
+    - deploy
+  script:
+    - ${GITLAB_MAKE} pack_run_cent6
+
+pack_cent7:
+  only:
+    refs:
+      - master
+      - /.*/
+  image: packpack/packpack:el-7
+  stage: perf_deploy
+  tags:
+    - deploy_test
+  script:
+    - ${GITLAB_MAKE} pack_run
+
+pack_fedora28:
+  only:
+    refs:
+      - master
+      - /.*/
+  image: packpack/packpack:fedora-28
+  stage: perf_deploy
+  tags:
+    - deploy_test
+  script:
+    - ${GITLAB_MAKE} pack_run
+
+pack_fedora29:
+  only:
+    refs:
+      - master
+      - /.*/
+  image: packpack/packpack:fedora-29
+  stage: perf_deploy
+  tags:
+    - deploy_test
+  script:
+    - ${GITLAB_MAKE} pack_run
+
+pack_fedora30:
+  only:
+    refs:
+      - master
+      - /.*/
+  image: packpack/packpack:fedora-30
+  stage: perf_deploy
+  tags:
+    - deploy_test
+  script:
+    - ${GITLAB_MAKE} pack_run
+
+pack_ubuntu14:
+  only:
+    refs:
+      - master
+      - /.*/
+  image: packpack/packpack:ubuntu-trusty
+  stage: perf_deploy
+  tags:
+    - deploy
+  script:
+    - ${GITLAB_MAKE} pack_run
+
+pack_ubuntu16:
+  only:
+    refs:
+      - master
+      - /.*/
+  image: packpack/packpack:ubuntu-xenial
+  stage: perf_deploy
+  tags:
+    - deploy
+  script:
+    - ${GITLAB_MAKE} pack_run
+
+pack_ubuntu18:
+  only:
+    refs:
+      - master
+      - /.*/
+  image: packpack/packpack:ubuntu-bionic
+  stage: perf_deploy
+  tags:
+    - deploy
+  script:
+    - ${GITLAB_MAKE} pack_run
+
+pack_ubuntu1810:
+  only:
+    refs:
+      - master
+      - /.*/
+  image: packpack/packpack:ubuntu-cosmic
+  stage: perf_deploy
+  tags:
+    - deploy
+  script:
+    - ${GITLAB_MAKE} pack_run
+
+pack_ubuntu19:
+  only:
+    refs:
+      - master
+      - /.*/
+  image: packpack/packpack:ubuntu-disco
+  stage: perf_deploy
+  tags:
+    - deploy
+  script:
+    - ${GITLAB_MAKE} pack_run
+
+pack_debian8:
+  only:
+    refs:
+      - master
+      - /.*/
+  image: packpack/packpack:debian-jessie
+  stage: perf_deploy
+  tags:
+    - deploy
+  script:
+    - ${GITLAB_MAKE} pack_run
+
+pack_debian9:
+  only:
+    refs:
+      - master
+      - /.*/
+  image: packpack/packpack:debian-stretch
+  stage: perf_deploy
+  tags:
+    - deploy
+  script:
+    - ${GITLAB_MAKE} pack_run
+
+pack_debian10:
+  only:
+    refs:
+      - master
+      - /.*/
+  image: packpack/packpack:debian-buster
+  stage: perf_deploy
+  tags:
+    - deploy
+  script:
+    - ${GITLAB_MAKE} pack_run
diff --git a/.gitlab.mk b/.gitlab.mk
new file mode 100644
index 000000000..00a75a389
--- /dev/null
+++ b/.gitlab.mk
@@ -0,0 +1,57 @@
+GITLAB_MAKE?=make -f .gitlab.mk
+TRAVIS_MAKE?=make -f .travis.mk
+GIT_SUBMODULES_UPDATE?=git submodule update --recursive --init
+DOCKER_BUILD=docker build --network=host
+
+# docker images bootstrap
+docker_login:
+	docker login -u ${CI_REGISTRY_USER} -p ${CI_REGISTRY_PASSWORD} ${CI_REGISTRY}
+
+docker_bootstrap: docker_login
+	cp .travis.mk images/
+	${DOCKER_BUILD} -t ${IMAGE} -f images/Dockerfile.${DOCKERFILE_SUFFIX} images
+	docker push ${IMAGE}
+
+# build sources with different flags and test
+git_submodules_update:
+	${GIT_SUBMODULES_UPDATE} --force
+
+git_submodules_update_cent6:
+	${GIT_SUBMODULES_UPDATE}
+
+runtest_%: git_submodules_update
+	${TRAVIS_MAKE} $@
+
+# build sources with different flags and test under VBox virtual machines
+vms_start:
+	VBoxManage controlvm ${VMS_NAME} poweroff || true
+	VBoxManage snapshot ${VMS_NAME} restore ${VMS_NAME}
+	VBoxManage startvm ${VMS_NAME} --type headless
+
+vms_test_%:
+	ssh -A ${VMS_USER}@127.0.0.1 -p ${VMS_PORT} "/bin/bash -c \
+		'${EXTRA_ENV} \
+		export PATH=/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin ; \
+		cd tarantool && \
+		git fetch -p && \
+		git checkout --force ${CI_BUILD_REF} && \
+		${GITLAB_MAKE} git_submodules_update && \
+		${TRAVIS_MAKE} $(subst vms_,,$@)'"
+
+vms_shutdown:
+	VBoxManage controlvm ${VMS_NAME} poweroff
+
+# build packs and test
+pack_clone:
+	git clone https://github.com/packpack/packpack.git packpack
+
+pack_make:
+	# to avoid of issue with too long names build path is short
+	make -f packpack/pack/Makefile -C . BUILDDIR=/builds/pack -j
+
+pack_build: pack_clone pack_make
+
+pack_run: git_submodules_update pack_build
+
+pack_run_cent6: git_submodules_update_cent6 pack_build
+
diff --git a/.travis.mk b/.travis.mk
index 6d0c42207..7d61c218b 100644
--- a/.travis.mk
+++ b/.travis.mk
@@ -3,6 +3,7 @@
 #
 
 DOCKER_IMAGE?=packpack/packpack:debian-stretch
+NO_PARALLEL_TESTING?=
 
 all: package
 
@@ -33,8 +34,12 @@ docker_%:
 		${DOCKER_IMAGE} \
 		make -f .travis.mk $(subst docker_,,$@)
 
+#########
+# Linux #
+#########
+
 deps_ubuntu:
-	sudo apt-get update && apt-get install -y -f \
+	apt-get -y update && apt-get install -y -f \
 		build-essential cmake coreutils sed \
 		libreadline-dev libncurses5-dev libyaml-dev libssl-dev \
 		libcurl4-openssl-dev libunwind-dev libicu-dev \
@@ -42,34 +47,29 @@ deps_ubuntu:
 		python-msgpack python-yaml python-argparse python-six python-gevent \
 		lcov ruby clang llvm llvm-dev
 
-test_ubuntu: deps_ubuntu
+deps_buster_clang_8:
+	echo "deb http://apt.llvm.org/buster/ llvm-toolchain-buster-8 main" > /etc/apt/sources.list.d/clang_8.list
+	echo "deb-src http://apt.llvm.org/buster/ llvm-toolchain-buster-8 main" >> /etc/apt/sources.list.d/clang_8.list
+	wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | sudo apt-key add -
+	apt-get -y update
+	apt-get -y install clang-8 llvm-8 llvm-8-dev
+
+build_ubuntu:
 	cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON ${CMAKE_EXTRA_PARAMS}
-	make -j8
-	cd test && /usr/bin/python test-run.py --force -j 1
+	make -j
 
-deps_osx:
-	brew update
-	brew install openssl readline curl icu4c --force
-	python2 -V || brew install python2 --force
-	curl --silent --show-error --retry 5 https://bootstrap.pypa.io/get-pip.py | python
-	pip install -r test-run/requirements.txt
+runtest_ubuntu: build_ubuntu
+	cd test && /usr/bin/python test-run.py --force $(NO_PARALLEL_TESTING)
 
-test_osx: deps_osx
-	cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON ${CMAKE_EXTRA_PARAMS}
-	# Increase the maximum number of open file descriptors on macOS
-	sudo sysctl -w kern.maxfiles=20480 || :
-	sudo sysctl -w kern.maxfilesperproc=20480 || :
-	sudo launchctl limit maxfiles 20480 || :
-	ulimit -S -n 20480 || :
-	ulimit -n
-	make -j8
-	cd test && ./test-run.py --force -j 1 unit/ app/ app-tap/ box/ box-tap/
+test_ubuntu: deps_ubuntu runtest_ubuntu
 
-coverage_ubuntu: deps_ubuntu
+build_cov_ubuntu:
 	cmake . -DCMAKE_BUILD_TYPE=Debug -DENABLE_GCOV=ON
-	make -j8
+	make -j
+
+runtest_cov_ubuntu: build_cov_ubuntu
 	# Enable --long tests for coverage
-	cd test && /usr/bin/python test-run.py --force -j 1 --long
+	cd test && /usr/bin/python test-run.py --force $(NO_PARALLEL_TESTING) --long
 	lcov --compat-libtool --directory src/ --capture --output-file coverage.info.tmp
 	lcov --compat-libtool --remove coverage.info.tmp 'tests/*' 'third_party/*' '/usr/*' \
 		--output-file coverage.info
@@ -81,6 +81,61 @@ coverage_ubuntu: deps_ubuntu
 		coveralls-lcov --service-name travis-ci --service-job-id $(TRAVIS_JOB_ID) --repo-token $(COVERALLS_TOKEN) coverage.info; \
 	fi;
 
+coverage_ubuntu: deps_ubuntu runtest_cov_ubuntu
+
+#######
+# OSX #
+#######
+
+deps_osx:
+	brew update
+	brew install openssl readline curl icu4c --force
+	python2 -V || brew install python2 --force
+	curl --silent --show-error --retry 5 https://bootstrap.pypa.io/get-pip.py >get-pip.py
+	python get-pip.py --user
+	pip install --user --ignore-installed -r test-run/requirements.txt
+
+build_osx:
+	cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON ${CMAKE_EXTRA_PARAMS}
+	make -j
+
+runtest_osx: build_osx
+	# Increase the maximum number of open file descriptors on macOS
+	sudo sysctl -w kern.maxfiles=20480 || :
+	sudo sysctl -w kern.maxfilesperproc=20480 || :
+	sudo launchctl limit maxfiles 20480 || :
+	ulimit -S -n 20480 || :
+	ulimit -n
+	cd test && ./test-run.py --force $(NO_PARALLEL_TESTING) unit/ app/ app-tap/ box/ box-tap/
+
+test_osx: deps_osx runtest_osx
+
+###########
+# FreeBSD #
+###########
+
+deps_freebsd:
+	# don't use bad py27-msgpack-python
+	sudo pkg install -y sudo git cmake gmake gcc coreutils \
+	readline ncurses libyaml openssl curl libunwind icu \
+	python27 py27-pip py27-setuptools py27-daemon \
+	py27-yaml py27-argparse py27-six py27-gevent \
+	gdb bash vim
+
+build_freebsd:
+	cmake .
+	gmake
+	sudo make install
+
+runtest_freebsd: build_freebsd
+	cd test && /usr/bin/python test-run.py --force $(NO_PARALLEL_TESTING) || true
+
+test_freebsd: deps_freebsd runtest_freebsd
+
+###################
+# Sources & Packs #
+###################
+
 source:
 	git clone https://github.com/packpack/packpack.git packpack
 	TARBALL_COMPRESSOR=gz packpack/packpack tarball
diff --git a/.travis.yml b/.travis.yml
index e94d02ef5..a22197845 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -17,6 +17,9 @@ cache:
 git:
     depth: 100500
 
+env: >
+    NO_PARALLEL_TESTING="-j 1"
+
 jobs:
     include:
       # Testing targets (just run tests on Debian Stretch or OS X).
diff --git a/images/Dockerfile.debian b/images/Dockerfile.debian
new file mode 100644
index 000000000..eb5278343
--- /dev/null
+++ b/images/Dockerfile.debian
@@ -0,0 +1,4 @@
+FROM packpack/packpack:debian-stretch
+
+COPY .travis.mk .
+RUN make -f .travis.mk deps_ubuntu
diff --git a/images/Dockerfile.lto b/images/Dockerfile.lto
new file mode 100644
index 000000000..54c585acc
--- /dev/null
+++ b/images/Dockerfile.lto
@@ -0,0 +1,5 @@
+FROM packpack/packpack:debian-buster
+
+COPY .travis.mk .
+RUN make -f .travis.mk deps_ubuntu
+RUN make -f .travis.mk deps_buster_clang_8
-- 
2.17.1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [tarantool-patches] Re: [PATCH v2 1/2] Gitlab-ci updated after reiview - w/o perf
  2019-06-15  4:46 [tarantool-patches] [PATCH v2 1/2] Gitlab-ci updated after reiview - w/o perf Alexander V. Tikhonov
@ 2019-06-18 11:07 ` Alexander Turenko
  2019-06-19  6:52   ` [tarantool-patches] " Alexander Tikhonov
  0 siblings, 1 reply; 3+ messages in thread
From: Alexander Turenko @ 2019-06-18 11:07 UTC (permalink / raw)
  To: Alexander V. Tikhonov; +Cc: tarantool-patches

Thanks, Alexander!

test_foo runs runtest_foo, which runs test-run.py. This looks confusing
for me. We can use names like `test_ubuntu` and `test_ubuntu_no_deps`.

Comments below are generally re decreasing number of goals that I think
will simplify the configuration at whole.

WBR, Alexander Turenko.

On Sat, Jun 15, 2019 at 07:46:23AM +0300, Alexander V. Tikhonov wrote:
> ---
> 
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4156-gitlab-ci-testing
> Issue: https://github.com/tarantool/tarantool/issues/4156
> 
>  .gitlab-ci.yml           | 328 +++++++++++++++++++++++++++++++++++++++
>  .gitlab.mk               |  57 +++++++
>  .travis.mk               | 101 +++++++++---
>  .travis.yml              |   3 +
>  images/Dockerfile.debian |   4 +
>  images/Dockerfile.lto    |   5 +
>  6 files changed, 475 insertions(+), 23 deletions(-)
>  create mode 100644 .gitlab-ci.yml
>  create mode 100644 .gitlab.mk
>  create mode 100644 images/Dockerfile.debian
>  create mode 100644 images/Dockerfile.lto
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> new file mode 100644
> index 000000000..01c4eacea
> --- /dev/null
> +++ b/.gitlab-ci.yml
> @@ -0,0 +1,328 @@
> +stages:
> +  - bootstrap
> +  - test
> +  - perf_deploy

Nit: Maybe perf_and_deploy?

However I would name it just 'deploy' within this patch.

> +
> +variables:
> +  VERSION_SUFFIX: "master"
> +  IMAGE_TEST: "${CI_REGISTRY}/${CI_PROJECT_PATH}:debian-stretch_${VERSION_SUFFIX}"
> +  IMAGE_TEST_LTO: "${CI_REGISTRY}/${CI_PROJECT_PATH}:debian-buster_${VERSION_SUFFIX}"

This suffix does not resolve any problems with changed dependencies. So,
we should either remove it for now or really resolve the problem: say,
use $(md5sum .travis.mk | awk '{ print $1 }') as a tag name.

BTW, I would prefer to use tags instead of suffixes; gitlab ci supports
it.

> +  FLAG_LTO: "-DENABLE_LTO=ON"

I think -DENABLE_LTO-ON is more self-descriptive then this variable
name. There is no need to extract it to a variable.

> +  GITLAB_MAKE: "make -f .gitlab.mk"
> +
> +# Bootstraps
> +
> +debian9:
> +  stage: bootstrap
> +  tags:
> +    - bootstrap
> +  variables:
> +    IMAGE: ${IMAGE_TEST}
> +    DOCKERFILE_SUFFIX: "debian"
> +  script:
> +    - ${GITLAB_MAKE} docker_bootstrap
> +
> +debian10_lto:
> +  only:
> +    refs:
> +      - master
> +      - /.*/

As we discusssed voicely: /.*/ should not be here in a final patch. We
however can provide an ability to run full testing on a branch different
from master. Say, match /.*-full/.

(The same is applicable for all such patterns below.)

> +  stage: bootstrap
> +  tags:
> +    - bootstrap
> +  variables:
> +    IMAGE: ${IMAGE_TEST_LTO}
> +    DOCKERFILE_SUFFIX: "lto"
> +  script:
> +    - ${GITLAB_MAKE} docker_bootstrap
> +
> +# Tests
> +
> +release:
> +  image: ${IMAGE_TEST}
> +  stage: test
> +  tags:
> +    - docker_test
> +  script:
> +    - ${GITLAB_MAKE} runtest_ubuntu
> +
> +debug:
> +  image: ${IMAGE_TEST}
> +  stage: test
> +  tags:
> +    - docker_test
> +  script:
> +    - ${GITLAB_MAKE} runtest_cov_ubuntu
> +
> +release_clang:
> +  image: ${IMAGE_TEST}
> +  stage: test
> +  tags:
> +    - docker_test
> +  variables:
> +    CC: clang
> +    CXX: clang++
> +  script:
> +    - ${GITLAB_MAKE} runtest_ubuntu
> +
> +release_lto:
> +  only:
> +    refs:
> +      - master
> +      - /.*/
> +  image: ${IMAGE_TEST_LTO}
> +  stage: test
> +  tags:
> +    - docker_test
> +  variables:
> +    CMAKE_EXTRA_PARAMS: ${FLAG_LTO}
> +  script:
> +    - ${GITLAB_MAKE} runtest_ubuntu
> +
> +release_lto_clang8:
> +  only:
> +    refs:
> +      - master
> +      - /.*/
> +  image: ${IMAGE_TEST_LTO}
> +  stage: test
> +  tags:
> +    - docker_test
> +  variables:
> +    CC: clang-8
> +    CXX: clang++-8
> +    CMAKE_EXTRA_PARAMS: ${FLAG_LTO}
> +  script:
> +    - ${GITLAB_MAKE} runtest_ubuntu
> +
> +osx13_release:
> +  only:
> +    refs:
> +      - master
> +      - /.*/
> +  stage: test
> +  tags:
> +    - vms_sierra
> +  variables:
> +    VMS_NAME: 'Sierra'
> +    VMS_USER: 'tarantool'
> +    VMS_PORT: '2212'
> +  before_script:
> +    - ${GITLAB_MAKE} vms_start
> +  script:
> +    - export

Forgotten debug? Below too.

> +    - ${GITLAB_MAKE} vms_test_osx
> +  after_script:
> +    - ${GITLAB_MAKE} vms_shutdown
> +
> +osx14_release:
> +  stage: test
> +  tags:
> +    - vms_mojave
> +  variables:
> +    VMS_NAME: 'Mojave'
> +    VMS_USER: 'tarantool'
> +    VMS_PORT: '2222'
> +  before_script:
> +    - ${GITLAB_MAKE} vms_start
> +  script:
> +    - export
> +    - ${GITLAB_MAKE} vms_test_osx
> +  after_script:
> +    - ${GITLAB_MAKE} vms_shutdown
> +
> +osx14_release_lto:
> +  only:
> +    refs:
> +      - master
> +      - /.*/
> +  stage: test
> +  tags:
> +    - vms_mojave
> +  variables:
> +    EXTRA_ENV: "export CMAKE_EXTRA_PARAMS=${FLAG_LTO} ;"
> +    VMS_NAME: 'Mojave'
> +    VMS_USER: 'tarantool'
> +    VMS_PORT: '2222'
> +  before_script:
> +    - ${GITLAB_MAKE} vms_start
> +  script:
> +    - export
> +    - ${GITLAB_MAKE} vms_test_osx
> +  after_script:
> +    - ${GITLAB_MAKE} vms_shutdown
> +
> +freebsd_release:
> +  stage: test
> +  tags:
> +    - vms_freebsd
> +  variables:
> +    VMS_NAME: 'Freebsd'

I would include a version number into the virtual machine name (and the
job name too).

> +    VMS_USER: 'vagrant'
> +    VMS_PORT: '2232'
> +    TRAVIS_MAKE: 'gmake -f .travis.mk'
> +  before_script:
> +    - ${GITLAB_MAKE} vms_start
> +  script:
> +    - export
> +    - ${GITLAB_MAKE} vms_test_freebsd
> +  after_script:
> +    - ${GITLAB_MAKE} vms_shutdown
> +
> +# Packs
> +
> +pack_cent6:

Why cent, not centos?

> +  only:
> +    refs:
> +      - master
> +      - /.*/
> +  image: packpack/packpack:el-6
> +  stage: perf_deploy
> +  tags:
> +    - deploy
> +  script:
> +    - ${GITLAB_MAKE} pack_run_cent6
> +
> +pack_cent7:
> +  only:
> +    refs:
> +      - master
> +      - /.*/
> +  image: packpack/packpack:el-7
> +  stage: perf_deploy
> +  tags:
> +    - deploy_test
> +  script:
> +    - ${GITLAB_MAKE} pack_run
> +
> +pack_fedora28:
> +  only:
> +    refs:
> +      - master
> +      - /.*/
> +  image: packpack/packpack:fedora-28
> +  stage: perf_deploy
> +  tags:
> +    - deploy_test
> +  script:
> +    - ${GITLAB_MAKE} pack_run
> +
> +pack_fedora29:
> +  only:
> +    refs:
> +      - master
> +      - /.*/
> +  image: packpack/packpack:fedora-29
> +  stage: perf_deploy
> +  tags:
> +    - deploy_test
> +  script:
> +    - ${GITLAB_MAKE} pack_run
> +
> +pack_fedora30:
> +  only:
> +    refs:
> +      - master
> +      - /.*/
> +  image: packpack/packpack:fedora-30
> +  stage: perf_deploy
> +  tags:
> +    - deploy_test
> +  script:
> +    - ${GITLAB_MAKE} pack_run
> +
> +pack_ubuntu14:
> +  only:
> +    refs:
> +      - master
> +      - /.*/
> +  image: packpack/packpack:ubuntu-trusty
> +  stage: perf_deploy
> +  tags:
> +    - deploy
> +  script:
> +    - ${GITLAB_MAKE} pack_run
> +
> +pack_ubuntu16:
> +  only:
> +    refs:
> +      - master
> +      - /.*/
> +  image: packpack/packpack:ubuntu-xenial
> +  stage: perf_deploy
> +  tags:
> +    - deploy
> +  script:
> +    - ${GITLAB_MAKE} pack_run
> +
> +pack_ubuntu18:
> +  only:
> +    refs:
> +      - master
> +      - /.*/
> +  image: packpack/packpack:ubuntu-bionic
> +  stage: perf_deploy
> +  tags:
> +    - deploy
> +  script:
> +    - ${GITLAB_MAKE} pack_run
> +
> +pack_ubuntu1810:
> +  only:
> +    refs:
> +      - master
> +      - /.*/
> +  image: packpack/packpack:ubuntu-cosmic
> +  stage: perf_deploy
> +  tags:
> +    - deploy
> +  script:
> +    - ${GITLAB_MAKE} pack_run
> +
> +pack_ubuntu19:

I would name it 'package_ubuntu_19_04'.

> +  only:
> +    refs:
> +      - master
> +      - /.*/
> +  image: packpack/packpack:ubuntu-disco
> +  stage: perf_deploy
> +  tags:
> +    - deploy
> +  script:
> +    - ${GITLAB_MAKE} pack_run
> +
> +pack_debian8:
> +  only:
> +    refs:
> +      - master
> +      - /.*/
> +  image: packpack/packpack:debian-jessie
> +  stage: perf_deploy
> +  tags:
> +    - deploy
> +  script:
> +    - ${GITLAB_MAKE} pack_run
> +
> +pack_debian9:
> +  only:
> +    refs:
> +      - master
> +      - /.*/
> +  image: packpack/packpack:debian-stretch
> +  stage: perf_deploy
> +  tags:
> +    - deploy
> +  script:
> +    - ${GITLAB_MAKE} pack_run
> +
> +pack_debian10:
> +  only:
> +    refs:
> +      - master
> +      - /.*/
> +  image: packpack/packpack:debian-buster
> +  stage: perf_deploy
> +  tags:
> +    - deploy
> +  script:
> +    - ${GITLAB_MAKE} pack_run
> diff --git a/.gitlab.mk b/.gitlab.mk
> new file mode 100644
> index 000000000..00a75a389
> --- /dev/null
> +++ b/.gitlab.mk
> @@ -0,0 +1,57 @@
> +GITLAB_MAKE?=make -f .gitlab.mk
> +TRAVIS_MAKE?=make -f .travis.mk
> +GIT_SUBMODULES_UPDATE?=git submodule update --recursive --init

Nit: I would name it GIT_SUBMODULE_UPDATE (w/o 'S'), because the command
is named so.

> +DOCKER_BUILD=docker build --network=host

I think there is no much reason to extract it from 'docker_bootstrap'
goal.

> +
> +# docker images bootstrap
> +docker_login:
> +	docker login -u ${CI_REGISTRY_USER} -p ${CI_REGISTRY_PASSWORD} ${CI_REGISTRY}

Why do we need to extract this goal? It is used only in
docker_bootstrap.

> +
> +docker_bootstrap: docker_login
> +	cp .travis.mk images/
> +	${DOCKER_BUILD} -t ${IMAGE} -f images/Dockerfile.${DOCKERFILE_SUFFIX} images

We can give . (a current directory) as a context, so we'll not need to
copy .travis.mk.

Also we can generate a dockerfile on-the-fly: I don't see much need to
keep it in the repository.

If a docker version allows we can even pass a dockerfile to stdin:
https://stackoverflow.com/a/42559367/1598057

> +	docker push ${IMAGE}
> +
> +# build sources with different flags and test

It is the title for the set of goals below? Looks as a comment for one
goal. Also here I see that bootstrap, submodules update and runnign
tests are doing, but not build sources. It seems the comment is not
precise.

> +git_submodules_update:
> +	${GIT_SUBMODULES_UPDATE} --force

Nit: The same as for GIT_SUBMODULES_UPDATE -> GIT_SUBMODULE_UPDATE:
git_submodules_update -> git_submodule_update.

Is there real need in this flag (some specific case when it is needed)?
What will going on CentOS 6 if we'll run into this case?

If it is really needed and we don't want to handle it on CentOS 6 (that
is strange), can we just run it like so?

$ git submodule update --recursive --init --force || git submodule update --recursive --init

It will allow to reduce amount of goals and so simplify the file.

> +
> +git_submodules_update_cent6:
> +	${GIT_SUBMODULES_UPDATE}
> +
> +runtest_%: git_submodules_update
> +	${TRAVIS_MAKE} $@
> +
> +# build sources with different flags and test under VBox virtual machines
> +vms_start:
> +	VBoxManage controlvm ${VMS_NAME} poweroff || true
> +	VBoxManage snapshot ${VMS_NAME} restore ${VMS_NAME}
> +	VBoxManage startvm ${VMS_NAME} --type headless
> +
> +vms_test_%:
> +	ssh -A ${VMS_USER}@127.0.0.1 -p ${VMS_PORT} "/bin/bash -c \

Should we really expose host keys into a virtual environment (-A
option)?

> +		'${EXTRA_ENV} \
> +		export PATH=/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin ; \
> +		cd tarantool && \
> +		git fetch -p && \
> +		git checkout --force ${CI_BUILD_REF} && \
> +		${GITLAB_MAKE} git_submodules_update && \
> +		${TRAVIS_MAKE} $(subst vms_,,$@)'"
> +
> +vms_shutdown:
> +	VBoxManage controlvm ${VMS_NAME} poweroff
> +
> +# build packs and test
> +pack_clone:
> +	git clone https://github.com/packpack/packpack.git packpack
> +
> +pack_make:
> +	# to avoid of issue with too long names build path is short
> +	make -f packpack/pack/Makefile -C . BUILDDIR=/builds/pack -j

Why do we need to split 'pack_build' into 'pack_clone' and 'pack_make'?

> +
> +pack_build: pack_clone pack_make
> +
> +pack_run: git_submodules_update pack_build
> +
> +pack_run_cent6: git_submodules_update_cent6 pack_build
> +

Nit: Extra empty line at the of the file.

I would squash all pack goals into one w/o deps (just 'package') and
update submodules just with a command. I guess --force is not really
needed and the command can be the same on all OSes.

Yep, we'll duplicate the command in +vms_test_%, but the structure of
the file will be simpler and one can read a certain sequince of commands
w/o look into dependent goals tree.

> diff --git a/.travis.mk b/.travis.mk
> index 6d0c42207..7d61c218b 100644
> --- a/.travis.mk
> +++ b/.travis.mk
> @@ -3,6 +3,7 @@
>  #
>  
>  DOCKER_IMAGE?=packpack/packpack:debian-stretch
> +NO_PARALLEL_TESTING?=

I would name it TEST_RUN_EXTRA_PARAMS (we already have
CMAKE_EXTRA_PARAMS).

>  
>  all: package
>  
> @@ -33,8 +34,12 @@ docker_%:
>  		${DOCKER_IMAGE} \
>  		make -f .travis.mk $(subst docker_,,$@)
>  
> +#########
> +# Linux #
> +#########
> +
>  deps_ubuntu:
> -	sudo apt-get update && apt-get install -y -f \
> +	apt-get -y update && apt-get install -y -f \
>  		build-essential cmake coreutils sed \
>  		libreadline-dev libncurses5-dev libyaml-dev libssl-dev \
>  		libcurl4-openssl-dev libunwind-dev libicu-dev \
> @@ -42,34 +47,29 @@ deps_ubuntu:
>  		python-msgpack python-yaml python-argparse python-six python-gevent \
>  		lcov ruby clang llvm llvm-dev
>  
> -test_ubuntu: deps_ubuntu
> +deps_buster_clang_8:
> +	echo "deb http://apt.llvm.org/buster/ llvm-toolchain-buster-8 main" > /etc/apt/sources.list.d/clang_8.list
> +	echo "deb-src http://apt.llvm.org/buster/ llvm-toolchain-buster-8 main" >> /etc/apt/sources.list.d/clang_8.list
> +	wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | sudo apt-key add -

You removed 'sudo' above, but added it here. Is it the typo?

> +	apt-get -y update
> +	apt-get -y install clang-8 llvm-8 llvm-8-dev

Are llvm headers really needed? BTW, the same question re llvm-dev
above.

> +
> +build_ubuntu:
>  	cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON ${CMAKE_EXTRA_PARAMS}
> -	make -j8
> -	cd test && /usr/bin/python test-run.py --force -j 1
> +	make -j
>  
> -deps_osx:
> -	brew update
> -	brew install openssl readline curl icu4c --force
> -	python2 -V || brew install python2 --force
> -	curl --silent --show-error --retry 5 https://bootstrap.pypa.io/get-pip.py | python
> -	pip install -r test-run/requirements.txt
> +runtest_ubuntu: build_ubuntu
> +	cd test && /usr/bin/python test-run.py --force $(NO_PARALLEL_TESTING)
>  
> -test_osx: deps_osx
> -	cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON ${CMAKE_EXTRA_PARAMS}
> -	# Increase the maximum number of open file descriptors on macOS
> -	sudo sysctl -w kern.maxfiles=20480 || :
> -	sudo sysctl -w kern.maxfilesperproc=20480 || :
> -	sudo launchctl limit maxfiles 20480 || :
> -	ulimit -S -n 20480 || :
> -	ulimit -n
> -	make -j8
> -	cd test && ./test-run.py --force -j 1 unit/ app/ app-tap/ box/ box-tap/
> +test_ubuntu: deps_ubuntu runtest_ubuntu
>  
> -coverage_ubuntu: deps_ubuntu
> +build_cov_ubuntu:

I would use the word 'coverage' everywhere instead of 'coverage' and
'cov' here and there.

>  	cmake . -DCMAKE_BUILD_TYPE=Debug -DENABLE_GCOV=ON
> -	make -j8
> +	make -j
> +
> +runtest_cov_ubuntu: build_cov_ubuntu
>  	# Enable --long tests for coverage
> -	cd test && /usr/bin/python test-run.py --force -j 1 --long
> +	cd test && /usr/bin/python test-run.py --force $(NO_PARALLEL_TESTING) --long
>  	lcov --compat-libtool --directory src/ --capture --output-file coverage.info.tmp
>  	lcov --compat-libtool --remove coverage.info.tmp 'tests/*' 'third_party/*' '/usr/*' \
>  		--output-file coverage.info
> @@ -81,6 +81,61 @@ coverage_ubuntu: deps_ubuntu
>  		coveralls-lcov --service-name travis-ci --service-job-id $(TRAVIS_JOB_ID) --repo-token $(COVERALLS_TOKEN) coverage.info; \
>  	fi;
>  
> +coverage_ubuntu: deps_ubuntu runtest_cov_ubuntu
> +
> +#######
> +# OSX #
> +#######
> +
> +deps_osx:
> +	brew update
> +	brew install openssl readline curl icu4c --force
> +	python2 -V || brew install python2 --force
> +	curl --silent --show-error --retry 5 https://bootstrap.pypa.io/get-pip.py >get-pip.py
> +	python get-pip.py --user
> +	pip install --user --ignore-installed -r test-run/requirements.txt
> +
> +build_osx:
> +	cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON ${CMAKE_EXTRA_PARAMS}
> +	make -j
> +
> +runtest_osx: build_osx
> +	# Increase the maximum number of open file descriptors on macOS
> +	sudo sysctl -w kern.maxfiles=20480 || :
> +	sudo sysctl -w kern.maxfilesperproc=20480 || :
> +	sudo launchctl limit maxfiles 20480 || :
> +	ulimit -S -n 20480 || :
> +	ulimit -n
> +	cd test && ./test-run.py --force $(NO_PARALLEL_TESTING) unit/ app/ app-tap/ box/ box-tap/
> +
> +test_osx: deps_osx runtest_osx
> +
> +###########
> +# FreeBSD #
> +###########
> +
> +deps_freebsd:
> +	# don't use bad py27-msgpack-python

Can you link exact problem description here or in the commit message?

> +	sudo pkg install -y sudo git cmake gmake gcc coreutils \

> sudo pkg install -y sudo

If sudo is not installed, then this command will fail. If it is
installed, then we don't need to add it to the package list.

> +	readline ncurses libyaml openssl curl libunwind icu \
> +	python27 py27-pip py27-setuptools py27-daemon \
> +	py27-yaml py27-argparse py27-six py27-gevent \
> +	gdb bash vim

Installing vim in a testing environment is the strange thing.

> +
> +build_freebsd:
> +	cmake .

I would explicitly set a build type (I guess it worth to test
RelWithDebInfo), set -DENABLE_WERROR=ON and pass ${CMAKE_EXTRA_PARAMS}
here (even despite we don't use it right now).

> +	gmake

Use -j as in other make invocations?

> +	sudo make install

Why do we need to install tarantool into a system? test-run able to
detect in-source build.

> +
> +runtest_freebsd: build_freebsd
> +	cd test && /usr/bin/python test-run.py --force $(NO_PARALLEL_TESTING) || true

Volkswagen mode?

> +
> +test_freebsd: deps_freebsd runtest_freebsd
> +
> +###################
> +# Sources & Packs #
> +###################

I would say 'Source tarballs', because we have no such separate things
as 'source' and 'pack'. I mean, it is possible to undestand the header
in a wrong way.

> +
>  source:
>  	git clone https://github.com/packpack/packpack.git packpack
>  	TARBALL_COMPRESSOR=gz packpack/packpack tarball
> diff --git a/.travis.yml b/.travis.yml
> index e94d02ef5..a22197845 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -17,6 +17,9 @@ cache:
>  git:
>      depth: 100500
>  
> +env: >
> +    NO_PARALLEL_TESTING="-j 1"
> +
>  jobs:
>      include:
>        # Testing targets (just run tests on Debian Stretch or OS X).
> diff --git a/images/Dockerfile.debian b/images/Dockerfile.debian
> new file mode 100644
> index 000000000..eb5278343
> --- /dev/null
> +++ b/images/Dockerfile.debian
> @@ -0,0 +1,4 @@
> +FROM packpack/packpack:debian-stretch
> +
> +COPY .travis.mk .
> +RUN make -f .travis.mk deps_ubuntu
> diff --git a/images/Dockerfile.lto b/images/Dockerfile.lto
> new file mode 100644
> index 000000000..54c585acc
> --- /dev/null
> +++ b/images/Dockerfile.lto
> @@ -0,0 +1,5 @@
> +FROM packpack/packpack:debian-buster
> +
> +COPY .travis.mk .
> +RUN make -f .travis.mk deps_ubuntu
> +RUN make -f .travis.mk deps_buster_clang_8

I guess we can remove these files, se the comment to 'docker_bootstrap'
goal above.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [tarantool-patches] Re: [tarantool-patches] Re: [PATCH v2 1/2] Gitlab-ci updated after reiview - w/o perf
  2019-06-18 11:07 ` [tarantool-patches] " Alexander Turenko
@ 2019-06-19  6:52   ` Alexander Tikhonov
  0 siblings, 0 replies; 3+ messages in thread
From: Alexander Tikhonov @ 2019-06-19  6:52 UTC (permalink / raw)
  To: tarantool-patches, Alexander Turenko

[-- Attachment #1: Type: text/plain, Size: 20359 bytes --]




>Вторник, 18 июня 2019, 14:08 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:
>Thanks, Alexander!
>
>test_foo runs runtest_foo, which runs test-run.py. This looks confusing
>for me. We can use names like `test_ubuntu` and `test_ubuntu_no_deps`.
>
>Comments below are generally re decreasing number of goals that I think
>will simplify the configuration at whole.
>
>WBR, Alexander Turenko.
>
Fixed.
>
>On Sat, Jun 15, 2019 at 07:46:23AM +0300, Alexander V. Tikhonov wrote:
>> ---
>> 
>> Github:  https://github.com/tarantool/tarantool/tree/avtikhon/gh-4156-gitlab-ci-testing
>> Issue:  https://github.com/tarantool/tarantool/issues/4156
>> 
>> .gitlab-ci.yml | 328 +++++++++++++++++++++++++++++++++++++++
>> .gitlab.mk | 57 +++++++
>> .travis.mk | 101 +++++++++---
>> .travis.yml | 3 +
>> images/Dockerfile.debian | 4 +
>> images/Dockerfile.lto | 5 +
>> 6 files changed, 475 insertions(+), 23 deletions(-)
>> create mode 100644 .gitlab-ci.yml
>> create mode 100644 .gitlab.mk
>> create mode 100644 images/Dockerfile.debian
>> create mode 100644 images/Dockerfile.lto
>> 
>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
>> new file mode 100644
>> index 000000000..01c4eacea
>> --- /dev/null
>> +++ b/.gitlab-ci.yml
>> @@ -0,0 +1,328 @@
>> +stages:
>> + - bootstrap
>> + - test
>> + - perf_deploy
>
>Nit: Maybe perf_and_deploy?
>
>However I would name it just 'deploy' within this patch.
>
Fixed.
>
>> +
>> +variables:
>> + VERSION_SUFFIX: "master"
>> + IMAGE_TEST: "${CI_REGISTRY}/${CI_PROJECT_PATH}:debian-stretch_${VERSION_SUFFIX}"
>> + IMAGE_TEST_LTO: "${CI_REGISTRY}/${CI_PROJECT_PATH}:debian-buster_${VERSION_SUFFIX}"
>
>This suffix does not resolve any problems with changed dependencies. So,
>we should either remove it for now or really resolve the problem: say,
>use $(md5sum .travis.mk | awk '{ print $1 }') as a tag name.
>
Not possible to use scripts for variables, neither use API to remove images from registry. Not resolved.
>
>BTW, I would prefer to use tags instead of suffixes; gitlab ci supports
>it.
>
Fixed.
>
>> + FLAG_LTO: "-DENABLE_LTO=ON"
>
>I think -DENABLE_LTO-ON is more self-descriptive then this variable
>name. There is no need to extract it to a variable.
>
Fixed, set name to: 'DENABLE_LTO_ON'
>
>> + GITLAB_MAKE: "make -f .gitlab.mk"
>> +
>> +# Bootstraps
>> +
>> +debian9:
>> + stage: bootstrap
>> + tags:
>> + - bootstrap
>> + variables:
>> + IMAGE: ${IMAGE_TEST}
>> + DOCKERFILE_SUFFIX: "debian"
>> + script:
>> + - ${GITLAB_MAKE} docker_bootstrap
>> +
>> +debian10_lto:
>> + only:
>> + refs:
>> + - master
>> + - /.*/
>
>As we discusssed voicely: /.*/ should not be here in a final patch. We
>however can provide an ability to run full testing on a branch different
>from master. Say, match /.*-full/.
>
>(The same is applicable for all such patterns below.)
>
Working on.
>
>> + stage: bootstrap
>> + tags:
>> + - bootstrap
>> + variables:
>> + IMAGE: ${IMAGE_TEST_LTO}
>> + DOCKERFILE_SUFFIX: "lto"
>> + script:
>> + - ${GITLAB_MAKE} docker_bootstrap
>> +
>> +# Tests
>> +
>> +release:
>> + image: ${IMAGE_TEST}
>> + stage: test
>> + tags:
>> + - docker_test
>> + script:
>> + - ${GITLAB_MAKE} runtest_ubuntu
>> +
>> +debug:
>> + image: ${IMAGE_TEST}
>> + stage: test
>> + tags:
>> + - docker_test
>> + script:
>> + - ${GITLAB_MAKE} runtest_cov_ubuntu
>> +
>> +release_clang:
>> + image: ${IMAGE_TEST}
>> + stage: test
>> + tags:
>> + - docker_test
>> + variables:
>> + CC: clang
>> + CXX: clang++
>> + script:
>> + - ${GITLAB_MAKE} runtest_ubuntu
>> +
>> +release_lto:
>> + only:
>> + refs:
>> + - master
>> + - /.*/
>> + image: ${IMAGE_TEST_LTO}
>> + stage: test
>> + tags:
>> + - docker_test
>> + variables:
>> + CMAKE_EXTRA_PARAMS: ${FLAG_LTO}
>> + script:
>> + - ${GITLAB_MAKE} runtest_ubuntu
>> +
>> +release_lto_clang8:
>> + only:
>> + refs:
>> + - master
>> + - /.*/
>> + image: ${IMAGE_TEST_LTO}
>> + stage: test
>> + tags:
>> + - docker_test
>> + variables:
>> + CC: clang-8
>> + CXX: clang++-8
>> + CMAKE_EXTRA_PARAMS: ${FLAG_LTO}
>> + script:
>> + - ${GITLAB_MAKE} runtest_ubuntu
>> +
>> +osx13_release:
>> + only:
>> + refs:
>> + - master
>> + - /.*/
>> + stage: test
>> + tags:
>> + - vms_sierra
>> + variables:
>> + VMS_NAME: 'Sierra'
>> + VMS_USER: 'tarantool'
>> + VMS_PORT: '2212'
>> + before_script:
>> + - ${GITLAB_MAKE} vms_start
>> + script:
>> + - export
>
>Forgotten debug? Below too.
Fixed.
>
>
>> + - ${GITLAB_MAKE} vms_test_osx
>> + after_script:
>> + - ${GITLAB_MAKE} vms_shutdown
>> +
>> +osx14_release:
>> + stage: test
>> + tags:
>> + - vms_mojave
>> + variables:
>> + VMS_NAME: 'Mojave'
>> + VMS_USER: 'tarantool'
>> + VMS_PORT: '2222'
>> + before_script:
>> + - ${GITLAB_MAKE} vms_start
>> + script:
>> + - export
>> + - ${GITLAB_MAKE} vms_test_osx
>> + after_script:
>> + - ${GITLAB_MAKE} vms_shutdown
>> +
>> +osx14_release_lto:
>> + only:
>> + refs:
>> + - master
>> + - /.*/
>> + stage: test
>> + tags:
>> + - vms_mojave
>> + variables:
>> + EXTRA_ENV: "export CMAKE_EXTRA_PARAMS=${FLAG_LTO} ;"
>> + VMS_NAME: 'Mojave'
>> + VMS_USER: 'tarantool'
>> + VMS_PORT: '2222'
>> + before_script:
>> + - ${GITLAB_MAKE} vms_start
>> + script:
>> + - export
>> + - ${GITLAB_MAKE} vms_test_osx
>> + after_script:
>> + - ${GITLAB_MAKE} vms_shutdown
>> +
>> +freebsd_release:
>> + stage: test
>> + tags:
>> + - vms_freebsd
>> + variables:
>> + VMS_NAME: 'Freebsd'
>
>I would include a version number into the virtual machine name (and the
>job name too).
Fixed.
>
>
>> + VMS_USER: 'vagrant'
>> + VMS_PORT: '2232'
>> + TRAVIS_MAKE: 'gmake -f .travis.mk'
>> + before_script:
>> + - ${GITLAB_MAKE} vms_start
>> + script:
>> + - export
>> + - ${GITLAB_MAKE} vms_test_freebsd
>> + after_script:
>> + - ${GITLAB_MAKE} vms_shutdown
>> +
>> +# Packs
>> +
>> +pack_cent6:
>
>Why cent, not centos?
Fixed.
>
>
>> + only:
>> + refs:
>> + - master
>> + - /.*/
>> + image: packpack/packpack:el-6
>> + stage: perf_deploy
>> + tags:
>> + - deploy
>> + script:
>> + - ${GITLAB_MAKE} pack_run_cent6
>> +
>> +pack_cent7:
>> + only:
>> + refs:
>> + - master
>> + - /.*/
>> + image: packpack/packpack:el-7
>> + stage: perf_deploy
>> + tags:
>> + - deploy_test
>> + script:
>> + - ${GITLAB_MAKE} pack_run
>> +
>> +pack_fedora28:
>> + only:
>> + refs:
>> + - master
>> + - /.*/
>> + image: packpack/packpack:fedora-28
>> + stage: perf_deploy
>> + tags:
>> + - deploy_test
>> + script:
>> + - ${GITLAB_MAKE} pack_run
>> +
>> +pack_fedora29:
>> + only:
>> + refs:
>> + - master
>> + - /.*/
>> + image: packpack/packpack:fedora-29
>> + stage: perf_deploy
>> + tags:
>> + - deploy_test
>> + script:
>> + - ${GITLAB_MAKE} pack_run
>> +
>> +pack_fedora30:
>> + only:
>> + refs:
>> + - master
>> + - /.*/
>> + image: packpack/packpack:fedora-30
>> + stage: perf_deploy
>> + tags:
>> + - deploy_test
>> + script:
>> + - ${GITLAB_MAKE} pack_run
>> +
>> +pack_ubuntu14:
>> + only:
>> + refs:
>> + - master
>> + - /.*/
>> + image: packpack/packpack:ubuntu-trusty
>> + stage: perf_deploy
>> + tags:
>> + - deploy
>> + script:
>> + - ${GITLAB_MAKE} pack_run
>> +
>> +pack_ubuntu16:
>> + only:
>> + refs:
>> + - master
>> + - /.*/
>> + image: packpack/packpack:ubuntu-xenial
>> + stage: perf_deploy
>> + tags:
>> + - deploy
>> + script:
>> + - ${GITLAB_MAKE} pack_run
>> +
>> +pack_ubuntu18:
>> + only:
>> + refs:
>> + - master
>> + - /.*/
>> + image: packpack/packpack:ubuntu-bionic
>> + stage: perf_deploy
>> + tags:
>> + - deploy
>> + script:
>> + - ${GITLAB_MAKE} pack_run
>> +
>> +pack_ubuntu1810:
>> + only:
>> + refs:
>> + - master
>> + - /.*/
>> + image: packpack/packpack:ubuntu-cosmic
>> + stage: perf_deploy
>> + tags:
>> + - deploy
>> + script:
>> + - ${GITLAB_MAKE} pack_run
>> +
>> +pack_ubuntu19:
>
>I would name it 'package_ubuntu_19_04'.
Fixed like 'ubuntu_19_04'.
>
>
>> + only:
>> + refs:
>> + - master
>> + - /.*/
>> + image: packpack/packpack:ubuntu-disco
>> + stage: perf_deploy
>> + tags:
>> + - deploy
>> + script:
>> + - ${GITLAB_MAKE} pack_run
>> +
>> +pack_debian8:
>> + only:
>> + refs:
>> + - master
>> + - /.*/
>> + image: packpack/packpack:debian-jessie
>> + stage: perf_deploy
>> + tags:
>> + - deploy
>> + script:
>> + - ${GITLAB_MAKE} pack_run
>> +
>> +pack_debian9:
>> + only:
>> + refs:
>> + - master
>> + - /.*/
>> + image: packpack/packpack:debian-stretch
>> + stage: perf_deploy
>> + tags:
>> + - deploy
>> + script:
>> + - ${GITLAB_MAKE} pack_run
>> +
>> +pack_debian10:
>> + only:
>> + refs:
>> + - master
>> + - /.*/
>> + image: packpack/packpack:debian-buster
>> + stage: perf_deploy
>> + tags:
>> + - deploy
>> + script:
>> + - ${GITLAB_MAKE} pack_run
>> diff --git a/.gitlab.mk b/.gitlab.mk
>> new file mode 100644
>> index 000000000..00a75a389
>> --- /dev/null
>> +++ b/.gitlab.mk
>> @@ -0,0 +1,57 @@
>> +GITLAB_MAKE?=make -f .gitlab.mk
>> +TRAVIS_MAKE?=make -f .travis.mk
>> +GIT_SUBMODULES_UPDATE?=git submodule update --recursive --init
>
>Nit: I would name it GIT_SUBMODULE_UPDATE (w/o 'S'), because the command
>is named so.
>
Fixed.
>
>> +DOCKER_BUILD=docker build --network=host
>
>I think there is no much reason to extract it from 'docker_bootstrap'
>goal.
>
Fixed.
>
>> +
>> +# docker images bootstrap
>> +docker_login:
>> + docker login -u ${CI_REGISTRY_USER} -p ${CI_REGISTRY_PASSWORD} ${CI_REGISTRY}
>
>Why do we need to extract this goal? It is used only in
>docker_bootstrap.
Fixed.
>
>
>> +
>> +docker_bootstrap: docker_login
>> + cp .travis.mk images/
>> + ${DOCKER_BUILD} -t ${IMAGE} -f images/Dockerfile.${DOCKERFILE_SUFFIX} images
>
>We can give . (a current directory) as a context, so we'll not need to
>copy .travis.mk.
Fixed.
>
>
>Also we can generate a dockerfile on-the-fly: I don't see much need to
>keep it in the repository.
>
>If a docker version allows we can even pass a dockerfile to stdin:
>https://stackoverflow.com/a/42559367/1598057
>
Fixed.
>
>> + docker push ${IMAGE}
>> +
>> +# build sources with different flags and test
>
>It is the title for the set of goals below? Looks as a comment for one
>goal. Also here I see that bootstrap, submodules update and runnign
>tests are doing, but not build sources. It seems the comment is not
>precise.
>
Right, the comment for a set of goals there below.
>
>> +git_submodules_update:
>> + ${GIT_SUBMODULES_UPDATE} --force
>
>Nit: The same as for GIT_SUBMODULES_UPDATE -> GIT_SUBMODULE_UPDATE:
>git_submodules_update -> git_submodule_update.
>
Fixed.
>
>Is there real need in this flag (some specific case when it is needed)?
>What will going on CentOS 6 if we'll run into this case?
>
Once I got the broken repository some how, '--force' flag fixed it, on CentOS 6 will need to fix it manually.
>
>If it is really needed and we don't want to handle it on CentOS 6 (that
>is strange), can we just run it like so?
>
>$ git submodule update --recursive --init --force || git submodule update --recursive --init
>
>It will allow to reduce amount of goals and so simplify the file.
>
Fixed.
>
>> +
>> +git_submodules_update_cent6:
>> + ${GIT_SUBMODULES_UPDATE}
>> +
>> +runtest_%: git_submodules_update
>> + ${TRAVIS_MAKE} $@
>> +
>> +# build sources with different flags and test under VBox virtual machines
>> +vms_start:
>> + VBoxManage controlvm ${VMS_NAME} poweroff || true
>> + VBoxManage snapshot ${VMS_NAME} restore ${VMS_NAME}
>> + VBoxManage startvm ${VMS_NAME} --type headless
>> +
>> +vms_test_%:
>> + ssh -A ${VMS_USER}@127.0.0.1 -p ${VMS_PORT} "/bin/bash -c \
>
>Should we really expose host keys into a virtual environment (-A
>option)?
>
Fixed.
>
>> + '${EXTRA_ENV} \
>> + export PATH=/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin ; \
>> + cd tarantool && \
>> + git fetch -p && \
>> + git checkout --force ${CI_BUILD_REF} && \
>> + ${GITLAB_MAKE} git_submodules_update && \
>> + ${TRAVIS_MAKE} $(subst vms_,,$@)'"
>> +
>> +vms_shutdown:
>> + VBoxManage controlvm ${VMS_NAME} poweroff
>> +
>> +# build packs and test
>> +pack_clone:
>> + git clone  https://github.com/packpack/packpack.git packpack
>> +
>> +pack_make:
>> + # to avoid of issue with too long names build path is short
>> + make -f packpack/pack/Makefile -C . BUILDDIR=/builds/pack -j
>
>Why do we need to split 'pack_build' into 'pack_clone' and 'pack_make'?
>
Fixed.
>
>> +
>> +pack_build: pack_clone pack_make
>> +
>> +pack_run: git_submodules_update pack_build
>> +
>> +pack_run_cent6: git_submodules_update_cent6 pack_build
>> +
>
>Nit: Extra empty line at the of the file.
>
Fixed.
>
>I would squash all pack goals into one w/o deps (just 'package') and
>update submodules just with a command. I guess --force is not really
>needed and the command can be the same on all OSes.
>
Once I got the broken repository some how, '--force' flag fixed it.
Fixed, but left update submodules call.
>Yep, we'll duplicate the command in +vms_test_%, but the structure of
>the file will be simpler and one can read a certain sequince of commands
>w/o look into dependent goals tree.
>
I don't think so, once I already got this issue with this wrong command and spent
time to check all places where it was used, better to use the standalone target.
>
>> diff --git a/.travis.mk b/.travis.mk
>> index 6d0c42207..7d61c218b 100644
>> --- a/.travis.mk
>> +++ b/.travis.mk
>> @@ -3,6 +3,7 @@
>> #
>> 
>> DOCKER_IMAGE?=packpack/packpack:debian-stretch
>> +NO_PARALLEL_TESTING?=
>
>I would name it TEST_RUN_EXTRA_PARAMS (we already have
>CMAKE_EXTRA_PARAMS).
>
Fixed.
>
>> 
>> all: package
>> 
>> @@ -33,8 +34,12 @@ docker_%:
>> ${DOCKER_IMAGE} \
>> make -f .travis.mk $(subst docker_,,$@)
>> 
>> +#########
>> +# Linux #
>> +#########
>> +
>> deps_ubuntu:
>> - sudo apt-get update && apt-get install -y -f \
>> + apt-get -y update && apt-get install -y -f \
>> build-essential cmake coreutils sed \
>> libreadline-dev libncurses5-dev libyaml-dev libssl-dev \
>> libcurl4-openssl-dev libunwind-dev libicu-dev \
>> @@ -42,34 +47,29 @@ deps_ubuntu:
>> python-msgpack python-yaml python-argparse python-six python-gevent \
>> lcov ruby clang llvm llvm-dev
>> 
>> -test_ubuntu: deps_ubuntu
>> +deps_buster_clang_8:
>> + echo "deb  http://apt.llvm.org/buster/ llvm-toolchain-buster-8 main" > /etc/apt/sources.list.d/clang_8.list
>> + echo "deb-src  http://apt.llvm.org/buster/ llvm-toolchain-buster-8 main" >> /etc/apt/sources.list.d/clang_8.list
>> + wget -O -  https://apt.llvm.org/llvm-snapshot.gpg.key | sudo apt-key add -
>
>You removed 'sudo' above, but added it here. Is it the typo?
Fixed.
>
>
>> + apt-get -y update
>> + apt-get -y install clang-8 llvm-8 llvm-8-dev
>
>Are llvm headers really needed? BTW, the same question re llvm-dev
>above.
>
Fixed.
>
>> +
>> +build_ubuntu:
>> cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON ${CMAKE_EXTRA_PARAMS}
>> - make -j8
>> - cd test && /usr/bin/python test-run.py --force -j 1
>> + make -j
>> 
>> -deps_osx:
>> - brew update
>> - brew install openssl readline curl icu4c --force
>> - python2 -V || brew install python2 --force
>> - curl --silent --show-error --retry 5  https://bootstrap.pypa.io/get-pip.py | python
>> - pip install -r test-run/requirements.txt
>> +runtest_ubuntu: build_ubuntu
>> + cd test && /usr/bin/python test-run.py --force $(NO_PARALLEL_TESTING)
>> 
>> -test_osx: deps_osx
>> - cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON ${CMAKE_EXTRA_PARAMS}
>> - # Increase the maximum number of open file descriptors on macOS
>> - sudo sysctl -w kern.maxfiles=20480 || :
>> - sudo sysctl -w kern.maxfilesperproc=20480 || :
>> - sudo launchctl limit maxfiles 20480 || :
>> - ulimit -S -n 20480 || :
>> - ulimit -n
>> - make -j8
>> - cd test && ./test-run.py --force -j 1 unit/ app/ app-tap/ box/ box-tap/
>> +test_ubuntu: deps_ubuntu runtest_ubuntu
>> 
>> -coverage_ubuntu: deps_ubuntu
>> +build_cov_ubuntu:
>
>I would use the word 'coverage' everywhere instead of 'coverage' and
>'cov' here and there.
>
Fixed.
>
>> cmake . -DCMAKE_BUILD_TYPE=Debug -DENABLE_GCOV=ON
>> - make -j8
>> + make -j
>> +
>> +runtest_cov_ubuntu: build_cov_ubuntu
>> # Enable --long tests for coverage
>> - cd test && /usr/bin/python test-run.py --force -j 1 --long
>> + cd test && /usr/bin/python test-run.py --force $(NO_PARALLEL_TESTING) --long
>> lcov --compat-libtool --directory src/ --capture --output-file coverage.info.tmp
>> lcov --compat-libtool --remove coverage.info.tmp 'tests/*' 'third_party/*' '/usr/*' \
>> --output-file coverage.info
>> @@ -81,6 +81,61 @@ coverage_ubuntu: deps_ubuntu
>> coveralls-lcov --service-name travis-ci --service-job-id $(TRAVIS_JOB_ID) --repo-token $(COVERALLS_TOKEN) coverage.info; \
>> fi;
>> 
>> +coverage_ubuntu: deps_ubuntu runtest_cov_ubuntu
>> +
>> +#######
>> +# OSX #
>> +#######
>> +
>> +deps_osx:
>> + brew update
>> + brew install openssl readline curl icu4c --force
>> + python2 -V || brew install python2 --force
>> + curl --silent --show-error --retry 5  https://bootstrap.pypa.io/get-pip.py >get-pip.py
>> + python get-pip.py --user
>> + pip install --user --ignore-installed -r test-run/requirements.txt
>> +
>> +build_osx:
>> + cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON ${CMAKE_EXTRA_PARAMS}
>> + make -j
>> +
>> +runtest_osx: build_osx
>> + # Increase the maximum number of open file descriptors on macOS
>> + sudo sysctl -w kern.maxfiles=20480 || :
>> + sudo sysctl -w kern.maxfilesperproc=20480 || :
>> + sudo launchctl limit maxfiles 20480 || :
>> + ulimit -S -n 20480 || :
>> + ulimit -n
>> + cd test && ./test-run.py --force $(NO_PARALLEL_TESTING) unit/ app/ app-tap/ box/ box-tap/
>> +
>> +test_osx: deps_osx runtest_osx
>> +
>> +###########
>> +# FreeBSD #
>> +###########
>> +
>> +deps_freebsd:
>> + # don't use bad py27-msgpack-python
>
>Can you link exact problem description here or in the commit message?
Removed, obvious message.
>
>
>> + sudo pkg install -y sudo git cmake gmake gcc coreutils \
>
>> sudo pkg install -y sudo
>
>If sudo is not installed, then this command will fail. If it is
>installed, then we don't need to add it to the package list.
Fixed.
>
>
>> + readline ncurses libyaml openssl curl libunwind icu \
>> + python27 py27-pip py27-setuptools py27-daemon \
>> + py27-yaml py27-argparse py27-six py27-gevent \
>> + gdb bash vim
>
>Installing vim in a testing environment is the strange thing.
Fixed.
>
>
>> +
>> +build_freebsd:
>> + cmake .
>
>I would explicitly set a build type (I guess it worth to test
>RelWithDebInfo), set -DENABLE_WERROR=ON and pass ${CMAKE_EXTRA_PARAMS}
>here (even despite we don't use it right now).
Fixed.
>
>
>> + gmake
>
>Use -j as in other make invocations?
Fixed.
>
>
>> + sudo make install
>
>Why do we need to install tarantool into a system? test-run able to
>detect in-source build.
>
Fixed.
>
>> +
>> +runtest_freebsd: build_freebsd
>> + cd test && /usr/bin/python test-run.py --force $(NO_PARALLEL_TESTING) || true
>
>Volkswagen mode?
Currently FreeBSD testing is not ready and will fail pipeline, the issue in github exists,
but we really need the test result to check the new fails.
>
>
>> +
>> +test_freebsd: deps_freebsd runtest_freebsd
>> +
>> +###################
>> +# Sources & Packs #
>> +###################
>
>I would say 'Source tarballs', because we have no such separate things
>as 'source' and 'pack'. I mean, it is possible to undestand the header
>in a wrong way.
>
Fixed.
>
>> +
>> source:
>> git clone  https://github.com/packpack/packpack.git packpack
>> TARBALL_COMPRESSOR=gz packpack/packpack tarball
>> diff --git a/.travis.yml b/.travis.yml
>> index e94d02ef5..a22197845 100644
>> --- a/.travis.yml
>> +++ b/.travis.yml
>> @@ -17,6 +17,9 @@ cache:
>> git:
>> depth: 100500
>> 
>> +env: >
>> + NO_PARALLEL_TESTING="-j 1"
>> +
>> jobs:
>> include:
>> # Testing targets (just run tests on Debian Stretch or OS X).
>> diff --git a/images/Dockerfile.debian b/images/Dockerfile.debian
>> new file mode 100644
>> index 000000000..eb5278343
>> --- /dev/null
>> +++ b/images/Dockerfile.debian
>> @@ -0,0 +1,4 @@
>> +FROM packpack/packpack:debian-stretch
>> +
>> +COPY .travis.mk .
>> +RUN make -f .travis.mk deps_ubuntu
>> diff --git a/images/Dockerfile.lto b/images/Dockerfile.lto
>> new file mode 100644
>> index 000000000..54c585acc
>> --- /dev/null
>> +++ b/images/Dockerfile.lto
>> @@ -0,0 +1,5 @@
>> +FROM packpack/packpack:debian-buster
>> +
>> +COPY .travis.mk .
>> +RUN make -f .travis.mk deps_ubuntu
>> +RUN make -f .travis.mk deps_buster_clang_8
>
>I guess we can remove these files, se the comment to 'docker_bootstrap'
>goal above.
>
Fixed.
>
>


-- 
Alexander Tikhonov


[-- Attachment #2: Type: text/html, Size: 34671 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-06-19  6:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-15  4:46 [tarantool-patches] [PATCH v2 1/2] Gitlab-ci updated after reiview - w/o perf Alexander V. Tikhonov
2019-06-18 11:07 ` [tarantool-patches] " Alexander Turenko
2019-06-19  6:52   ` [tarantool-patches] " Alexander Tikhonov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox