[Tarantool-patches] [PATCH v2] test: fix OSX host setup

[Tarantool-patches] [PATCH v2] test: fix OSX host setup

[Alexander V. Tikhonov]

Fixed OSX host setup for Tarantool build:
- set brew installation from Homebrew repository instructions;
- set in use Python 2 latest commit from tapped local formula,
  since Python 2 is EOL, also removed extra pip installation with
  get-pip script, because tapped formula installs pip itself;
- added upgrade packages call to avoid of fails on already
  installed packages, but with previous version.

Fixed OSX host setup for Tarantool test:
- set maximum processes limit value to 2500 for testing process;
- 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
Based on 1st 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

 .travis.mk                    |  31 +++-
 tools/brew_taps/tntpython2.rb | 306 ++++++++++++++++++++++++++++++++++
 2 files changed, 329 insertions(+), 8 deletions(-)
 create mode 100644 tools/brew_taps/tntpython2.rb

diff --git a/.travis.mk b/.travis.mk
index 42969ff56..2b21e6ff0 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
 
 all: package
 
@@ -127,13 +128,22 @@ 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
-	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
+	# 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)"
+	# 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}
+	pip install --force-reinstall -r test-run/requirements.txt
 
 build_osx:
 	cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON ${CMAKE_EXTRA_PARAMS}
@@ -148,11 +158,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/
 
diff --git a/tools/brew_taps/tntpython2.rb b/tools/brew_taps/tntpython2.rb
new file mode 100644
index 000000000..59f374030
--- /dev/null
+++ b/tools/brew_taps/tntpython2.rb
@@ -0,0 +1,306 @@
+class Tntpython2 < Formula
+  desc "Interpreted, interactive, object-oriented programming language"
+  homepage "https://www.python.org/"
+  url "https://www.python.org/ftp/python/2.7.17/Python-2.7.17.tar.xz"
+  sha256 "4d43f033cdbd0aa7b7023c81b0e986fd11e653b5248dac9144d508f11812ba41"
+  revision 1
+  head "https://github.com/python/cpython.git", :branch => "2.7"
+
+  bottle do
+    sha256 "accfaa922708f00afb69ab230199f96e6ecdddd248a1eca586ce1e5e5cfd732b" => :catalina
+    sha256 "54d3351d6be8268b2f5017894dcc8e083811dfa3812bdb9f79f989873b9a4542" => :mojave
+    sha256 "cfd5c6eeac37065d19f527bb0798a9caf1928bab3340cd545224861a3c82f219" => :high_sierra
+  end
+
+  # setuptools remembers the build flags python is built with and uses them to
+  # build packages later. Xcode-only systems need different flags.
+  pour_bottle? do
+    reason <<~EOS
+      The bottle needs the Apple Command Line Tools to be installed.
+        You can install them, if desired, with:
+          xcode-select --install
+    EOS
+    satisfy { MacOS::CLT.installed? }
+  end
+
+  depends_on "pkg-config" => :build
+  depends_on "gdbm"
+  depends_on "openssl@1.1"
+  depends_on "readline"
+  depends_on "sqlite"
+
+  resource "setuptools" do
+    url "https://files.pythonhosted.org/packages/f4/d5/a6c19dcbcbc267aca376558797f036d9bcdff344c9f785fe7d0fe9a5f2a7/setuptools-41.4.0.zip"
+    sha256 "7eae782ccf36b790c21bde7d86a4f303a441cd77036b25c559a602cf5186ce4d"
+  end
+
+  resource "pip" do
+    url "https://files.pythonhosted.org/packages/ce/ea/9b445176a65ae4ba22dce1d93e4b5fe182f953df71a145f557cffaffc1bf/pip-19.3.1.tar.gz"
+    sha256 "21207d76c1031e517668898a6b46a9fb1501c7a4710ef5dfd6a40ad9e6757ea7"
+  end
+
+  resource "wheel" do
+    url "https://files.pythonhosted.org/packages/59/b0/11710a598e1e148fb7cbf9220fd2a0b82c98e94efbdecb299cb25e7f0b39/wheel-0.33.6.tar.gz"
+    sha256 "10c9da68765315ed98850f8e048347c3eb06dd81822dc2ab1d4fde9dc9702646"
+  end
+
+  def lib_cellar
+    prefix/"Frameworks/Python.framework/Versions/2.7/lib/python2.7"
+  end
+
+  def site_packages_cellar
+    lib_cellar/"site-packages"
+  end
+
+  # The HOMEBREW_PREFIX location of site-packages.
+  def site_packages
+    HOMEBREW_PREFIX/"lib/python2.7/site-packages"
+  end
+
+  def install
+    # Unset these so that installing pip and setuptools puts them where we want
+    # and not into some other Python the user has installed.
+    ENV["PYTHONHOME"] = nil
+    ENV["PYTHONPATH"] = nil
+
+    args = %W[
+      --prefix=#{prefix}
+      --enable-ipv6
+      --datarootdir=#{share}
+      --datadir=#{share}
+      --enable-framework=#{frameworks}
+      --without-ensurepip
+    ]
+
+    # See upstream bug report from 22 Jan 2018 "Significant performance problems
+    # with Python 2.7 built with clang 3.x or 4.x"
+    # https://bugs.python.org/issue32616
+    # https://github.com/Homebrew/homebrew-core/issues/22743
+    if DevelopmentTools.clang_build_version >= 802 &&
+       DevelopmentTools.clang_build_version < 902
+      args << "--without-computed-gotos"
+    end
+
+    args << "--without-gcc" if ENV.compiler == :clang
+
+    cflags   = []
+    ldflags  = []
+    cppflags = []
+
+    if MacOS.sdk_path_if_needed
+      # Help Python's build system (setuptools/pip) to build things on SDK-based systems
+      # The setup.py looks at "-isysroot" to get the sysroot (and not at --sysroot)
+      cflags  << "-isysroot #{MacOS.sdk_path}" << "-I#{MacOS.sdk_path}/usr/include"
+      ldflags << "-isysroot #{MacOS.sdk_path}"
+      # For the Xlib.h, Python needs this header dir with the system Tk
+      # Yep, this needs the absolute path where zlib needed a path relative
+      # to the SDK.
+      cflags  << "-I#{MacOS.sdk_path}/System/Library/Frameworks/Tk.framework/Versions/8.5/Headers"
+    end
+
+    # Avoid linking to libgcc https://code.activestate.com/lists/python-dev/112195/
+    args << "MACOSX_DEPLOYMENT_TARGET=#{MacOS.version}"
+
+    # We want our readline and openssl! This is just to outsmart the detection code,
+    # superenv handles that cc finds includes/libs!
+    inreplace "setup.py" do |s|
+      s.gsub! "do_readline = self.compiler.find_library_file(lib_dirs, 'readline')",
+              "do_readline = '#{Formula["readline"].opt_lib}/libhistory.dylib'"
+      s.gsub! "/usr/local/ssl", Formula["openssl@1.1"].opt_prefix
+    end
+
+    inreplace "setup.py" do |s|
+      s.gsub! "sqlite_setup_debug = False", "sqlite_setup_debug = True"
+      s.gsub! "for d_ in inc_dirs + sqlite_inc_paths:",
+              "for d_ in ['#{Formula["sqlite"].opt_include}']:"
+
+      # Allow sqlite3 module to load extensions:
+      # https://docs.python.org/library/sqlite3.html#f1
+      s.gsub! 'sqlite_defines.append(("SQLITE_OMIT_LOAD_EXTENSION", "1"))', ""
+    end
+
+    # Allow python modules to use ctypes.find_library to find homebrew's stuff
+    # even if homebrew is not a /usr/local/lib. Try this with:
+    # `brew install enchant && pip install pyenchant`
+    inreplace "./Lib/ctypes/macholib/dyld.py" do |f|
+      f.gsub! "DEFAULT_LIBRARY_FALLBACK = [", "DEFAULT_LIBRARY_FALLBACK = [ '#{HOMEBREW_PREFIX}/lib',"
+      f.gsub! "DEFAULT_FRAMEWORK_FALLBACK = [", "DEFAULT_FRAMEWORK_FALLBACK = [ '#{HOMEBREW_PREFIX}/Frameworks',"
+    end
+
+    args << "CFLAGS=#{cflags.join(" ")}" unless cflags.empty?
+    args << "LDFLAGS=#{ldflags.join(" ")}" unless ldflags.empty?
+    args << "CPPFLAGS=#{cppflags.join(" ")}" unless cppflags.empty?
+
+    system "./configure", *args
+    system "make"
+
+    ENV.deparallelize do
+      # Tell Python not to install into /Applications
+      system "make", "install", "PYTHONAPPSDIR=#{prefix}"
+      system "make", "frameworkinstallextras", "PYTHONAPPSDIR=#{pkgshare}"
+    end
+
+    # Fixes setting Python build flags for certain software
+    # See: https://github.com/Homebrew/homebrew/pull/20182
+    # https://bugs.python.org/issue3588
+    inreplace lib_cellar/"config/Makefile" do |s|
+      s.change_make_var! "LINKFORSHARED",
+        "-u _PyMac_Error $(PYTHONFRAMEWORKINSTALLDIR)/Versions/$(VERSION)/$(PYTHONFRAMEWORK)"
+    end
+
+    # Prevent third-party packages from building against fragile Cellar paths
+    inreplace [lib_cellar/"_sysconfigdata.py",
+               lib_cellar/"config/Makefile",
+               frameworks/"Python.framework/Versions/Current/lib/pkgconfig/python-2.7.pc"],
+              prefix, opt_prefix
+
+    # Symlink the pkgconfig files into HOMEBREW_PREFIX so they're accessible.
+    (lib/"pkgconfig").install_symlink Dir[frameworks/"Python.framework/Versions/Current/lib/pkgconfig/*"]
+
+    # Remove 2to3 because Python 3 also installs it
+    rm bin/"2to3"
+
+    # A fix, because python and python@2 both want to install Python.framework
+    # and therefore we can't link both into HOMEBREW_PREFIX/Frameworks
+    # https://github.com/Homebrew/homebrew/issues/15943
+    ["Headers", "Python", "Resources"].each { |f| rm(prefix/"Frameworks/Python.framework/#{f}") }
+    rm prefix/"Frameworks/Python.framework/Versions/Current"
+
+    # Remove the site-packages that Python created in its Cellar.
+    site_packages_cellar.rmtree
+
+    (libexec/"setuptools").install resource("setuptools")
+    (libexec/"pip").install resource("pip")
+    (libexec/"wheel").install resource("wheel")
+  end
+
+  def post_install
+    # Avoid conflicts with lingering unversioned files from Python 3
+    rm_f %W[
+      #{HOMEBREW_PREFIX}/bin/easy_install
+      #{HOMEBREW_PREFIX}/bin/pip
+      #{HOMEBREW_PREFIX}/bin/wheel
+    ]
+
+    # Fix up the site-packages so that user-installed Python software survives
+    # minor updates, such as going from 2.7.0 to 2.7.1:
+
+    # Create a site-packages in HOMEBREW_PREFIX/lib/python2.7/site-packages
+    site_packages.mkpath
+
+    # Symlink the prefix site-packages into the cellar.
+    site_packages_cellar.unlink if site_packages_cellar.exist?
+    site_packages_cellar.parent.install_symlink site_packages
+
+    # Write our sitecustomize.py
+    rm_rf Dir["#{site_packages}/sitecustomize.py[co]"]
+    (site_packages/"sitecustomize.py").atomic_write(sitecustomize)
+
+    # Remove old setuptools installations that may still fly around and be
+    # listed in the easy_install.pth. This can break setuptools build with
+    # zipimport.ZipImportError: bad local file header
+    # setuptools-0.9.5-py3.3.egg
+    rm_rf Dir["#{site_packages}/setuptools*"]
+    rm_rf Dir["#{site_packages}/distribute*"]
+    rm_rf Dir["#{site_packages}/pip[-_.][0-9]*", "#{site_packages}/pip"]
+
+    setup_args = ["-s", "setup.py", "--no-user-cfg", "install", "--force",
+                  "--verbose",
+                  "--single-version-externally-managed",
+                  "--record=installed.txt",
+                  "--install-scripts=#{bin}",
+                  "--install-lib=#{site_packages}"]
+
+    (libexec/"setuptools").cd { system "#{bin}/python", *setup_args }
+    (libexec/"pip").cd { system "#{bin}/python", *setup_args }
+    (libexec/"wheel").cd { system "#{bin}/python", *setup_args }
+
+    # When building from source, these symlinks will not exist, since
+    # post_install happens after linking.
+    %w[pip pip2 pip2.7 easy_install easy_install-2.7 wheel].each do |e|
+      (HOMEBREW_PREFIX/"bin").install_symlink bin/e
+    end
+
+    # Help distutils find brewed stuff when building extensions
+    include_dirs = [HOMEBREW_PREFIX/"include", Formula["openssl@1.1"].opt_include,
+                    Formula["sqlite"].opt_include]
+    library_dirs = [HOMEBREW_PREFIX/"lib", Formula["openssl@1.1"].opt_lib,
+                    Formula["sqlite"].opt_lib]
+
+    cfg = lib_cellar/"distutils/distutils.cfg"
+    cfg.atomic_write <<~EOS
+      [install]
+      prefix=#{HOMEBREW_PREFIX}
+      [build_ext]
+      include_dirs=#{include_dirs.join ":"}
+      library_dirs=#{library_dirs.join ":"}
+    EOS
+  end
+
+  def sitecustomize
+    <<~EOS
+      # This file is created by Homebrew and is executed on each python startup.
+      # Don't print from here, or else python command line scripts may fail!
+      # <https://docs.brew.sh/Homebrew-and-Python>
+      import re
+      import os
+      import sys
+      if sys.version_info[0] != 2:
+          # This can only happen if the user has set the PYTHONPATH for 3.x and run Python 2.x or vice versa.
+          # Every Python looks at the PYTHONPATH variable and we can't fix it here in sitecustomize.py,
+          # because the PYTHONPATH is evaluated after the sitecustomize.py. Many modules (e.g. PyQt4) are
+          # built only for a specific version of Python and will fail with cryptic error messages.
+          # In the end this means: Don't set the PYTHONPATH permanently if you use different Python versions.
+          exit('Your PYTHONPATH points to a site-packages dir for Python 2.x but you are running Python ' +
+               str(sys.version_info[0]) + '.x!\\n     PYTHONPATH is currently: "' + str(os.environ['PYTHONPATH']) + '"\\n' +
+               '     You should `unset PYTHONPATH` to fix this.')
+      # Only do this for a brewed python:
+      if os.path.realpath(sys.executable).startswith('#{rack}'):
+          # Shuffle /Library site-packages to the end of sys.path and reject
+          # paths in /System pre-emptively (#14712)
+          library_site = '/Library/Python/2.7/site-packages'
+          library_packages = [p for p in sys.path if p.startswith(library_site)]
+          sys.path = [p for p in sys.path if not p.startswith(library_site) and
+                                             not p.startswith('/System')]
+          # .pth files have already been processed so don't use addsitedir
+          sys.path.extend(library_packages)
+          # the Cellar site-packages is a symlink to the HOMEBREW_PREFIX
+          # site_packages; prefer the shorter paths
+          long_prefix = re.compile(r'#{rack}/[0-9\._abrc]+/Frameworks/Python\.framework/Versions/2\.7/lib/python2\.7/site-packages')
+          sys.path = [long_prefix.sub('#{site_packages}', p) for p in sys.path]
+          # LINKFORSHARED (and python-config --ldflags) return the
+          # full path to the lib (yes, "Python" is actually the lib, not a
+          # dir) so that third-party software does not need to add the
+          # -F/#{HOMEBREW_PREFIX}/Frameworks switch.
+          try:
+              from _sysconfigdata import build_time_vars
+              build_time_vars['LINKFORSHARED'] = '-u _PyMac_Error #{opt_prefix}/Frameworks/Python.framework/Versions/2.7/Python'
+          except:
+              pass  # remember: don't print here. Better to fail silently.
+          # Set the sys.executable to use the opt_prefix
+          sys.executable = '#{opt_bin}/python2.7'
+    EOS
+  end
+
+  def caveats; <<~EOS
+    Pip and setuptools have been installed. To update them
+      pip install --upgrade pip setuptools
+    You can install Python packages with
+      pip install <package>
+    They will install into the site-package directory
+      #{site_packages}
+    See: https://docs.brew.sh/Homebrew-and-Python
+  EOS
+  end
+
+  test do
+    # Check if sqlite is ok, because we build with --enable-loadable-sqlite-extensions
+    # and it can occur that building sqlite silently fails if OSX's sqlite is used.
+    system "#{bin}/python", "-c", "import sqlite3"
+    # Check if some other modules import. Then the linked libs are working.
+    system "#{bin}/python", "-c", "import Tkinter; root = Tkinter.Tk()"
+    system "#{bin}/python", "-c", "import gdbm"
+    system "#{bin}/python", "-c", "import zlib"
+    system bin/"pip", "list", "--format=columns"
+  end
+end
-- 
2.17.1

Re: [Tarantool-patches] [PATCH v2] test: fix OSX host setup

[Sergey Bronnikov]

On 21:09 Mon 23 Mar , Alexander V. Tikhonov wrote:
> Fixed OSX host setup for Tarantool build:
> - set brew installation from Homebrew repository instructions;
> - set in use Python 2 latest commit from tapped local formula,
>   since Python 2 is EOL, also removed extra pip installation with
>   get-pip script, because tapped formula installs pip itself;
> - added upgrade packages call to avoid of fails on already
>   installed packages, but with previous version.
> 
> Fixed OSX host setup for Tarantool test:
> - set maximum processes limit value to 2500 for testing process;
> - set 'var' directory for test-run tool to the shorter path name
>   to avoid of issues with long names;

Looks good to me. One note: I suspect python brew file was copied
from somewhere. It would be better to specify resource.

Re: [Tarantool-patches] [PATCH v2] test: fix OSX host setup

[Alexander Tikhonov]

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


Sergey, thanks a lot for the review, I’ve really missed that forgot to mention about the source of the tapped formula. I’ve added the comment about the way I’ve created it.

  
>Среда, 25 марта 2020, 10:59 +03:00 от Sergey Bronnikov <sergeyb@tarantool.org>:
> 
>On 21:09 Mon 23 Mar , Alexander V. Tikhonov wrote:
>> Fixed OSX host setup for Tarantool build:
>> - set brew installation from Homebrew repository instructions;
>> - set in use Python 2 latest commit from tapped local formula,
>> since Python 2 is EOL, also removed extra pip installation with
>> get-pip script, because tapped formula installs pip itself;
>> - added upgrade packages call to avoid of fails on already
>> installed packages, but with previous version.
>>
>> Fixed OSX host setup for Tarantool test:
>> - set maximum processes limit value to 2500 for testing process;
>> - set 'var' directory for test-run tool to the shorter path name
>> to avoid of issues with long names;
>Looks good to me. One note: I suspect python brew file was copied
>from somewhere. It would be better to specify resource. 
 
 
--
Alexander Tikhonov
 

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

Re: [Tarantool-patches] [PATCH v2] test: fix OSX host setup

[Alexander Turenko]

If lack of --user (for pip) does not fail Travis CI jobs, then the whole
commit should be harmless. I cannot say that I'm happy with all changes,
but let's don't freeze on me: fix what you personally think should be
fixed and go to Kirill to push. No need to re-review with me.

WBR, Alexander Turenko.

On Mon, Mar 23, 2020 at 09:09:05PM +0300, Alexander V. Tikhonov wrote:
> Fixed OSX host setup for Tarantool build:
> - set brew installation from Homebrew repository instructions;
> - set in use Python 2 latest commit from tapped local formula,
>   since Python 2 is EOL, also removed extra pip installation with
>   get-pip script, because tapped formula installs pip itself;
> - added upgrade packages call to avoid of fails on already
>   installed packages, but with previous version.
> 
> Fixed OSX host setup for Tarantool test:
> - set maximum processes limit value to 2500 for testing process;
> - 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
> Based on 1st 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

> +	brew update || echo | /usr/bin/ruby -e \
> +		"$$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"

Why don't update formulae after installing brew?

From the previous review [1]:

> > > 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.

Are there any customer who run our testing? I don't know anyone. Even if
one doing it, (s)he prepares an environment and invoke `make test` or
`cd test && ./test-run.py`.

So it is the dead code. Personally I prefer to don't have dead code. If
you decided to do it, okay.

[1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-March/015062.html

> +	# 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}
> +	pip install --force-reinstall -r test-run/requirements.txt

--user (pip option) is not needed anymore? It was added to avoid using
of sudo or virtualenv. Is it related to a Travis CI infrastructure
change?

It looks a bit strance that pip w/o --user just works from 'travis'
user.

Please, don't do such subtle changes. At least describe a reason in the
commit message. Otherwise it will be nightmare in the future, when one
will try to understand what is made for what, what is actual, what can
be simplified / removed / refactored.

> +		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/

Why not to use --exclude or TEST_RUN_EXCLUDE?

Are there any issue about disabled tests?

Why the whole suite is going to be disabled? To save time to investigate
problems?

When we'll investigate and enable tests rather then only disable them?
The past year trend to don't enable anything back...

From the previous review [1]:

> > 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.

It sounds as 'because something going not right, when something was
changed'. Neat reason.

I cannot say neither 'ok', not 'not ok' for this. Okay, I can guess that
working directory is changed somehow, but why and where?

Anyway, it should be harmless. So kinda ok.

Re: [Tarantool-patches] [PATCH v2] test: fix OSX host setup

[Alexander Tikhonov]

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


Alexander, thanks a lot for the review, the lack of ‘--user’ did not break the testing on Travis-CI:
https://www.travis-ci.org/github/tarantool/tarantool/jobs/666683564


  
>Среда, 25 марта 2020, 14:38 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:
> 
>If lack of --user (for pip) does not fail Travis CI jobs, then the whole
>commit should be harmless. I cannot say that I'm happy with all changes,
>but let's don't freeze on me: fix what you personally think should be
>fixed and go to Kirill to push. No need to re-review with me.
>
>WBR, Alexander Turenko.
>
>On Mon, Mar 23, 2020 at 09:09:05PM +0300, Alexander V. Tikhonov wrote:
>> Fixed OSX host setup for Tarantool build:
>> - set brew installation from Homebrew repository instructions;
>> - set in use Python 2 latest commit from tapped local formula,
>> since Python 2 is EOL, also removed extra pip installation with
>> get-pip script, because tapped formula installs pip itself;
>> - added upgrade packages call to avoid of fails on already
>> installed packages, but with previous version.
>>
>> Fixed OSX host setup for Tarantool test:
>> - set maximum processes limit value to 2500 for testing process;
>> - 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
>> Based on 1st 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
>
>> + brew update || echo | /usr/bin/ruby -e \
>> + "$(curl -fsSL  https://raw.githubusercontent.com/Homebrew/install/master/install )"
>
>Why don't update formulae after installing brew?
The tapped formula does not exist in brew any more, though it can’t be updated.
>
>From the previous review [1]:
>
>> > > 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.
>
>Are there any customer who run our testing? I don't know anyone. Even if
>one doing it, (s)he prepares an environment and invoke `make test` or
>`cd test && ./test-run.py`.
>
>So it is the dead code. Personally I prefer to don't have dead code. If
>you decided to do it, okay.
>
>[1]:  https://lists.tarantool.org/pipermail/tarantool-patches/2020-March/015062.html
Actually, I’ve meant that people who run on Mac hosts, like people in our group.
>
>> + # 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}
>> + pip install --force-reinstall -r test-run/requirements.txt
>
>--user (pip option) is not needed anymore? It was added to avoid using
>of sudo or virtualenv. Is it related to a Travis CI infrastructure
>change?
Right, but in real it was added because of gitlab-ci, for now we have well configured OSX like travis’s.
>
>It looks a bit strance that pip w/o --user just works from 'travis'
>user.
You may check that testing passed with installation part without issues (check from line 8908):
https://www.travis-ci.org/github/tarantool/tarantool/jobs/666683564
>
>Please, don't do such subtle changes. At least describe a reason in the
>commit message. Otherwise it will be nightmare in the future, when one
>will try to understand what is made for what, what is actual, what can
>be simplified / removed / refactored.
Ok, sure
>
>> + 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/
>
>Why not to use --exclude or TEST_RUN_EXCLUDE?
>
>Are there any issue about disabled tests?
>
>Why the whole suite is going to be disabled? To save time to investigate
>problems?
>
>When we'll investigate and enable tests rather then only disable them?
>The past year trend to don't enable anything back…
This commit is already complete of changes and the change on the testing suites better to make in the next patch, due to the list of suites were not changed.
>
>From the previous review [1]:
>
>> > 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.
>
>It sounds as 'because something going not right, when something was
>changed'. Neat reason.
>
>I cannot say neither 'ok', not 'not ok' for this. Okay, I can guess that
>working directory is changed somehow, but why and where?
‘var’ directory lays too deep at the path and the length of the path is too long for test-run.py tool which fails because of it, the solution to fix it is to use the short link for the ‘var’ directory.
>
>Anyway, it should be harmless. So kinda ok. 
 
 
--
Alexander Tikhonov
 

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

Re: [Tarantool-patches] [PATCH v2] test: fix OSX host setup

[Alexander Turenko]

> >> + brew update || echo | /usr/bin/ruby -e \
> >> + "$(curl -fsSL  https://raw.githubusercontent.com/Homebrew/install/master/install )"
> >
> >Why don't update formulae after installing brew?
>
> The tapped formula does not exist in brew any more, though it can’t be
> updated.

After installing brew you'll have fresh brew, but no formulae, I guess.
Will the next `brew install` work? I guess, no. You need `brew update`
after this.

So it seems logical to do `[ -z "$(which brew) "] && ruby
brew_install_script; brew update` rather than `brew update || ruby
brew_install_script`.

> >From the previous review [1]:
> >
> >> > > 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.
> >
> >Are there any customer who run our testing? I don't know anyone. Even if
> >one doing it, (s)he prepares an environment and invoke `make test` or
> >`cd test && ./test-run.py`.
> >
> >So it is the dead code. Personally I prefer to don't have dead code. If
> >you decided to do it, okay.
> >
> >[1]:  https://lists.tarantool.org/pipermail/tarantool-patches/2020-March/015062.html
>
> Actually, I’ve meant that people who run on Mac hosts, like people
> in our group.

Developers doing `make test` or `cd test && ./test-run.py`. Nobody want
to run a testing script and found that something was changed in its
system. So I'm still on my view.

> >> + # 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}
> >> + pip install --force-reinstall -r test-run/requirements.txt
> >
> >--user (pip option) is not needed anymore? It was added to avoid using
> >of sudo or virtualenv. Is it related to a Travis CI infrastructure
> >change?
>
> Right, but in real it was added because of gitlab-ci, for now we have
> well configured OSX like travis’s.

'It' -- the option? Okay so. Good piece of information to add to the
commit message.

> >It looks a bit strance that pip w/o --user just works from 'travis'
> >user.
> You may check that testing passed with installation part without issues (check from line 8908):
> https://www.travis-ci.org/github/tarantool/tarantool/jobs/666683564

Okay. Yep, it seems pip assumes --user by default in Travis CI. Maybe I
wrongly remember that --user was added for Travis CI (AFAIR it was done
by Arseniy).

> >> + 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/
> >
> >Why not to use --exclude or TEST_RUN_EXCLUDE?
> >
> >Are there any issue about disabled tests?
> >
> >Why the whole suite is going to be disabled? To save time to investigate
> >problems?
> >
> >When we'll investigate and enable tests rather then only disable them?
> >The past year trend to don't enable anything back…
>
> This commit is already complete of changes and the change on the
> testing suites better to make in the next patch, due to the list of
> suites were not changed.

Let's add the info about Mac OS X testing into the relevant issue to
track disabled tests (AFAIR you have one, but it was about testing
during RPM packages build).

> >From the previous review [1]:
> >
> >> > 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.
> >
> >It sounds as 'because something going not right, when something was
> >changed'. Neat reason.
> >
> >I cannot say neither 'ok', not 'not ok' for this. Okay, I can guess that
> >working directory is changed somehow, but why and where?
>
> ‘var’ directory lays too deep at the path and the length of the path
> is too long for test-run.py tool which fails because of it, the
> solution to fix it is to use the short link for the ‘var’ directory.

Why it was okay, but becomes too long?

Re: [Tarantool-patches] [PATCH v2] test: fix OSX host setup

[Alexander Tikhonov]

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



Alexander, I’ve tried your suggestion on brew update and created the issue on testing suites list, check the following.
  
>Среда, 25 марта 2020, 15:30 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:
> 
>> >> + brew update || echo | /usr/bin/ruby -e \
>> >> + "$(curl -fsSL  https://raw.githubusercontent.com/Homebrew/install/master/install )"
>> >
>> >Why don't update formulae after installing brew?
>>
>> The tapped formula does not exist in brew any more, though it can’t be
>> updated.
>
>After installing brew you'll have fresh brew, but no formulae, I guess.
>Will the next `brew install` work? I guess, no. You need `brew update`
>after this.
I’ve tried to remove the brew and reinstalled it with this command, seems that all needed updates were tried to do during the installation:
==> Tapping homebrew/core
Cloning into '/usr/local/Homebrew/Library/Taps/homebrew/homebrew-core'...
remote: Enumerating objects: 699135, done.
remote: Total 699135 (delta 0), reused 0 (delta 0), pack-reused 699135
Receiving objects: 100% (699135/699135), 283.20 MiB | 8.49 MiB/s, done.
Resolving deltas: 100% (459609/459609), done.
Tapped 2 commands and 4942 formulae (5,205 files, 310.5MB).
Already up-to-date.

Also the next command with installation passed without any issues and installed all the needed packages.
>
>So it seems logical to do `[ -z "$(which brew) "] && ruby
>brew_install_script; brew update` rather than `brew update || ruby
>brew_install_script`.
>
>> >From the previous review [1]:
>> >
>> >> > > 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.
>> >
>> >Are there any customer who run our testing? I don't know anyone. Even if
>> >one doing it, (s)he prepares an environment and invoke `make test` or
>> >`cd test && ./test-run.py`.
>> >
>> >So it is the dead code. Personally I prefer to don't have dead code. If
>> >you decided to do it, okay.
>> >
>> >[1]:  https://lists.tarantool.org/pipermail/tarantool-patches/2020-March/015062.html
>>
>> Actually, I’ve meant that people who run on Mac hosts, like people
>> in our group.
>
>Developers doing `make test` or `cd test && ./test-run.py`. Nobody want
>to run a testing script and found that something was changed in its
>system. So I'm still on my view.
>
>> >> + # 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}
>> >> + pip install --force-reinstall -r test-run/requirements.txt
>> >
>> >--user (pip option) is not needed anymore? It was added to avoid using
>> >of sudo or virtualenv. Is it related to a Travis CI infrastructure
>> >change?
>>
>> Right, but in real it was added because of gitlab-ci, for now we have
>> well configured OSX like travis’s.
>
>'It' -- the option? Okay so. Good piece of information to add to the
>commit message.
Ok, added.
>
>> >It looks a bit strance that pip w/o --user just works from 'travis'
>> >user.
>> You may check that testing passed with installation part without issues (check from line 8908):
>>  https://www.travis-ci.org/github/tarantool/tarantool/jobs/666683564
>
>Okay. Yep, it seems pip assumes --user by default in Travis CI. Maybe I
>wrongly remember that --user was added for Travis CI (AFAIR it was done
>by Arseniy).
>
>> >> + 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/
>> >
>> >Why not to use --exclude or TEST_RUN_EXCLUDE?
>> >
>> >Are there any issue about disabled tests?
>> >
>> >Why the whole suite is going to be disabled? To save time to investigate
>> >problems?
>> >
>> >When we'll investigate and enable tests rather then only disable them?
>> >The past year trend to don't enable anything back…
>>
>> This commit is already complete of changes and the change on the
>> testing suites better to make in the next patch, due to the list of
>> suites were not changed.
>
>Let's add the info about Mac OS X testing into the relevant issue to
>track disabled tests (AFAIR you have one, but it was about testing
>during RPM packages build).
Ok, sure:
https://github.com/tarantool/tarantool/issues/4818
>
>> >From the previous review [1]:
>> >
>> >> > 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.
>> >
>> >It sounds as 'because something going not right, when something was
>> >changed'. Neat reason.
>> >
>> >I cannot say neither 'ok', not 'not ok' for this. Okay, I can guess that
>> >working directory is changed somehow, but why and where?
>>
>> ‘var’ directory lays too deep at the path and the length of the path
>> is too long for test-run.py tool which fails because of it, the
>> solution to fix it is to use the short link for the ‘var’ directory.
>
>Why it was okay, but becomes too long?
Because the name of the user on the new mac mini and its home path is really long
echo ~ | wc -c
      28
also the path of tarantool/test/var adds.
 
--
Alexander Tikhonov
 

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

Re: [Tarantool-patches] [PATCH v2] test: fix OSX host setup

[Alexander Turenko]

> >> >> + brew update || echo | /usr/bin/ruby -e \
> >> >> + "$(curl -fsSL  https://raw.githubusercontent.com/Homebrew/install/master/install )"
> >> >
> >> >Why don't update formulae after installing brew?
> >>
> >> The tapped formula does not exist in brew any more, though it can’t be
> >> updated.
> >
> >After installing brew you'll have fresh brew, but no formulae, I guess.
> >Will the next `brew install` work? I guess, no. You need `brew update`
> >after this.
>
> I’ve tried to remove the brew and reinstalled it with this command,
> seems that all needed updates were tried to do during the
> installation:

Okay so.

> >> >From the previous review [1]:
> >> >
> >> >> > 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.
> >> >
> >> >It sounds as 'because something going not right, when something was
> >> >changed'. Neat reason.
> >> >
> >> >I cannot say neither 'ok', not 'not ok' for this. Okay, I can guess that
> >> >working directory is changed somehow, but why and where?
> >>
> >> ‘var’ directory lays too deep at the path and the length of the path
> >> is too long for test-run.py tool which fails because of it, the
> >> solution to fix it is to use the short link for the ‘var’ directory.
> >
> >Why it was okay, but becomes too long?
> Because the name of the user on the new mac mini and its home path is really long
> echo ~ | wc -c
>       28
> also the path of tarantool/test/var adds.

Okay, so it worth to state something like:

> We're going to enable new Mac machines into CI and usernames on them
> are long according to internal policies. It makes a home directory to
> be long too and so a path to a unix socket created during testing can
> be longer then UNIX_PATH_MAX constant (108). Move the working
> directory for testing into <...>.

It would be also good to link the issue, which describes some details
around the path length limit:
https://github.com/tarantool/tarantool/issues/4634

Re: [Tarantool-patches] [PATCH v2] test: fix OSX host setup

[Alexander Tikhonov]

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


Alexander, I’ve corrected the patch as you suggested, thank you.

  
>Среда, 25 марта 2020, 16:38 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:
> 
>> >> >> + brew update || echo | /usr/bin/ruby -e \
>> >> >> + "$(curl -fsSL  https://raw.githubusercontent.com/Homebrew/install/master/install )"
>> >> >
>> >> >Why don't update formulae after installing brew?
>> >>
>> >> The tapped formula does not exist in brew any more, though it can’t be
>> >> updated.
>> >
>> >After installing brew you'll have fresh brew, but no formulae, I guess.
>> >Will the next `brew install` work? I guess, no. You need `brew update`
>> >after this.
>>
>> I’ve tried to remove the brew and reinstalled it with this command,
>> seems that all needed updates were tried to do during the
>> installation:
>
>Okay so.
>
>> >> >From the previous review [1]:
>> >> >
>> >> >> > 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.
>> >> >
>> >> >It sounds as 'because something going not right, when something was
>> >> >changed'. Neat reason.
>> >> >
>> >> >I cannot say neither 'ok', not 'not ok' for this. Okay, I can guess that
>> >> >working directory is changed somehow, but why and where?
>> >>
>> >> ‘var’ directory lays too deep at the path and the length of the path
>> >> is too long for test-run.py tool which fails because of it, the
>> >> solution to fix it is to use the short link for the ‘var’ directory.
>> >
>> >Why it was okay, but becomes too long?
>> Because the name of the user on the new mac mini and its home path is really long
>> echo ~ | wc -c
>>       28
>> also the path of tarantool/test/var adds.
>
>Okay, so it worth to state something like:
>
>> We're going to enable new Mac machines into CI and usernames on them
>> are long according to internal policies. It makes a home directory to
>> be long too and so a path to a unix socket created during testing can
>> be longer then UNIX_PATH_MAX constant (108). Move the working
>> directory for testing into <...>.
>
>It would be also good to link the issue, which describes some details
>around the path length limit:
>https://github.com/tarantool/tarantool/issues/4634 
 
 
--
Alexander Tikhonov
 

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

Re: [Tarantool-patches] [PATCH v2] test: fix OSX host setup

[Kirill Yukhin]

Hello,

On 23 мар 21:09, Alexander V. Tikhonov wrote:
> Fixed OSX host setup for Tarantool build:
> - set brew installation from Homebrew repository instructions;
> - set in use Python 2 latest commit from tapped local formula,
>   since Python 2 is EOL, also removed extra pip installation with
>   get-pip script, because tapped formula installs pip itself;
> - added upgrade packages call to avoid of fails on already
>   installed packages, but with previous version.
> 
> Fixed OSX host setup for Tarantool test:
> - set maximum processes limit value to 2500 for testing process;
> - 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
> Based on 1st 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

I've checked your patch into 1.10, 2.2, 2.3 and master.

--
Regards, Kirill Yukhin