Hi Alexander, thanks for your review, I’ve made the changes you suggested.   >Понедельник, 23 марта 2020, 15:11 +03:00 от Alexander Turenko : >  >Looked briefly. > >It looks as a bunch of fixes for different problems. It seems they >should be splitted to properly described separately. > >WBR, Alexander Turenko. Right, I’ve moved all the changes for mac mini to separate patch. > >On Mon, Mar 23, 2020 at 02:49:24PM +0300, Alexander V. Tikhonov wrote: >> Set in use Python 2 latest commit from tapped local >> formula, since Python 2 is EOL. >> >> Set 'var' directory for test-run tool to the shorter >> path name to avoid of issues with long names. >> --- >> >> Github: https://github.com/tarantool/tarantool/tree/avtikhon/osx_depends-full-ci >> First commit from Github: https://github.com/tarantool/tarantool/tree/avtikhon/osx_on_mini14-full-ci >> Which already has LGTM: >> https://lists.tarantool.org/pipermail/tarantool-patches/2020-March/014734.html > >> diff --git a/.gitlab.mk b/.gitlab.mk >> index b39c5c651..d804d3cf6 100644 >> --- a/.gitlab.mk >> +++ b/.gitlab.mk >> @@ -14,7 +14,7 @@ git_submodule_update: >> git submodule update --recursive --init >> >> # Pass *_no_deps goals to .travis.mk. >> -test_%_no_deps: git_submodule_update >> +test_%: git_submodule_update >> ${TRAVIS_MAKE} $@ > >This looks suspectful. Will not it break Linux builds? > >The comment still states that this goal is about *_no_deps. It has no any affect on any other jobs/tasks, so it can be used, anyway I’’ve removed this change from this patch. > >> # ####################################################### >> diff --git a/.travis.mk b/.travis.mk >> index 42969ff56..2a4ff8046 100644 >> --- a/.travis.mk >> +++ b/.travis.mk >> @@ -5,6 +5,7 @@ >> DOCKER_IMAGE?=packpack/packpack:debian-stretch >> TEST_RUN_EXTRA_PARAMS?= >> MAX_FILES?=65534 >> +MAX_PROC?=2500 > >It seems you fixed some other problem, but it is not described neither >in an issue, nor in the commit message. Right, the MAX_PROC limit value should be set too, it was found running OSX on hosts w/o previous changes. > >> @@ -127,13 +128,24 @@ test_asan_debian: deps_debian deps_buster_clang_8 test_asan_debian_no_deps >> # OSX # >> ####### >> >> +# since Python 2 is EOL it's latest commit from tapped local formula is used >> +OSX_PKGS=openssl readline curl icu4c libiconv zlib autoconf automake libtool \ >> + cmake file://${PWD}/tools/brew_taps/tntpython2.rb >> + >> deps_osx: >> - brew update >> - brew install openssl readline curl icu4c libiconv zlib autoconf automake libtool --force >> - python2 -V || brew install python2 --force >> + # install brew using command from Homebrew repository instructions: >> + # https://github.com/Homebrew/install >> + # NOTE: 'echo' command below is required since brew installation >> + # script obliges the one to enter a newline for confirming the >> + # installation via Ruby script. >> + brew update || echo | /usr/bin/ruby -e \ >> + "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install )" > >So you added brew installation when it is not installed. Add this to the >commit message at least. > >Is it possible at all for a machine that is enabled into CI? Aren't you >use brew to install gitlab runner? I’ve added the commit message about it. This change is really needed to be sure that our customers can use this script too. > >> + # try to install the packages either upgrade it to avoid of fails >> + # if the package already exists with the previous version >> + brew install --force ${OSX_PKGS} || brew upgrade ${OSX_PKGS} >> curl --silent --show-error --retry 5 https://bootstrap.pypa.io/get-pip.py >get-pip.py >> - python get-pip.py --user >> - pip install --user --force-reinstall -r test-run/requirements.txt >> + sudo python get-pip.py > >Why do we need get-pip.py if you already install python2 from tap? Right, tapped formula for python2 installs pip — I’ve removed this code from the make rule at all. > >> + pip install --force-reinstall -r test-run/requirements.txt > >> @@ -148,11 +160,16 @@ test_osx_no_deps: build_osx >> # call as tests runs call. >> # Tests: Temporary excluded replication/ suite with some tests >> # from other suites by issues #4357 and #4370 >> - echo tarantool | sudo -S launchctl limit maxfiles ${MAX_FILES} || : ; \ >> + sudo -S launchctl limit maxfiles ${MAX_FILES} || : ; \ >> launchctl limit maxfiles || : ; \ >> ulimit -n ${MAX_FILES} || : ; \ >> ulimit -n ; \ >> - cd test && ./test-run.py --force $(TEST_RUN_EXTRA_PARAMS) \ >> + sudo -S launchctl limit maxproc ${MAX_PROC} || : ; \ >> + launchctl limit maxproc || : ; \ >> + ulimit -u ${MAX_PROC} || : ; \ >> + ulimit -u ; \ >> + rm -rf /tmp/tnt ; \ >> + cd test && ./test-run.py --vardir /tmp/tnt --force $(TEST_RUN_EXTRA_PARAMS) \ >> app/ app-tap/ box/ box-py/ box-tap/ engine/ engine_long/ long_run-py/ luajit-tap/ \ >> replication-py/ small/ sql/ sql-tap/ swim/ unit/ vinyl/ wal_off/ xlog/ xlog-py/ > >The problem with 'var' directory is not related to the new way to Python >2 installation? Why everything is in one commit? This commit is about OSX host setup for building and testing, so I think it is ok to set it here.   -- Alexander Tikhonov