

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