Skip to content

Update instructions for building from source #1879

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Conversation

Onvember
Copy link
Contributor

@Onvember Onvember commented Mar 1, 2021

Fixes #1702
Fixes #1640

@Onvember Onvember requested a review from Totktonada March 1, 2021 10:44
@NickVolynkin
Copy link
Contributor

@Totktonada I've got several concerns:

  1. We don't freeze package versions here, so build environment isn't controlled. Someone can follow the instructions exactly and still face a problem. Moreover, we recommend exact versions for some packages in the instructions, but then don't repeat them in the code snippet.
  2. Package lists and build instructions have no connection with our tests for the corresponding platforms. We will have to update instructions, and we will forget to do so.
  3. Package lists and build instructions aren't in the form of code.

@Totktonada
Copy link
Member

  1. We don't freeze package versions here, so build environment isn't controlled. Someone can follow the instructions exactly and still face a problem. Moreover, we recommend exact versions for some packages in the instructions, but then don't repeat them in the code snippet.

Packages are installed from the distribution specific repositories. Any binary distribution must ensure binary compatibility of packages within a distribution version. A user should not meet any problems related to packages version on a distribution we support.

I don't see frozen versions or recommendation to use it aside of PyYAML (the mentioned version is quite old anyway). I don't know why the instruction repeats test-run/requirements.txt. It should not.

@Totktonada
Copy link
Member

  1. Package lists and build instructions have no connection with our tests for the corresponding platforms. We will have to update instructions, and we will forget to do so.

Do you propose to extract dependencies from rpm/tarantool.spec or debian/control? That's the good idea, but requires some research. Consider this and this.

@Totktonada
Copy link
Member

  1. Package lists and build instructions aren't in the form of code.

Is that about formatting on the web page?

Comment on lines +41 to +45
* `pyyaml <https://pypi.python.org/pypi/PyYAML>`_ version 3.10
* `argparse <https://pypi.python.org/pypi/argparse>`_ version 1.1
* `msgpack-python <https://pypi.python.org/pypi/msgpack-python>`_ version 0.4.6
* `gevent <https://pypi.python.org/pypi/gevent>`_ version 1.1.2
* `six <https://pypi.python.org/pypi/six>`_ version 1.8.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • argparse — it is built-in in Python 2.6+ AFAIR.
  • msgpack-python is a submdoule of test-run, so it is not necessary to install it separately.
  • All other versions seems obsoleted. And there is not any sense to list them, they're listed in test-run/requirements.txt in the format that pip understands. One who want to install it using a package manager can find related package names in rpm/tarantol.spec or debian/control (but I would left this exercise to a user, just mention where to find it).

autoconf automake libtool zlib-devel \
readline-devel ncurses-devel openssl-devel \
libunwind-devel libicu-devel \
python python-pip python-setuptools python-devel \
python-msgpack python-yaml python-argparse python-six python-gevent
python-msgpack python-yaml python-six python-gevent
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's is not so trivial considering that it is Python 2 packages on CentOS 7, but Python 3 ones on CentOS 8 / Fedora. I would just suggest to use pip --user -r test-run/requirements.txt or pip -r test-run/requirements.txt from a virtualenv environments. We can mention that the same can be installed with packages, but, to be honest, if one want, (s)he will do it anyway based on the requirements.txt content. So, let's stick to pip for simplicity.

@Totktonada
Copy link
Member

Please, also eliminate mentions of 2.2 branch. It is not supported for a long time and useless for a user in fact.

@Totktonada
Copy link
Member

  1. Run the test suite.

This section is something really strance. You should just have all dependencies available, build tarantool and run make test. I don't know all those symlinks and PATH tweaks are here. Maybe they were required a long time ago (more than 4 years).

@Totktonada
Copy link
Member

dpkg-buildpackage or rpmbuild

To be honest, I'm not sure those tools will works seamlessly. We use packpack and it is the only reliable way to build tarantool package ATM.

I would just refer this instruction.

@Totktonada
Copy link
Member

$ # if you installed tarantool locally after build
$ tarantool

It is either when we're about installing to some custom destdir and setting PATH (which is not actually necessary AFAIU). Or regarding installing directly to /usr or /usr/local bypassing a system package manager, which is not what I would suggest for anyone.

@Totktonada
Copy link
Member

  • Release -- used only if the highest performance is required

We don't deploy and don't verify such packages. And the point regarding better performance comparing to RelWithDebInfo is dubious. So I would remove mention of the Release build type.

@Totktonada
Copy link
Member

  • OpenSSL <https://www.openssl.org>_ library, version 1.0.1+

There are no problems with 1.1.* known to me.

@Totktonada
Copy link
Member

$ # if you installed tarantool locally after build
$ tarantool

It is either when we're about installing to some custom destdir and setting PATH (which is not actually necessary AFAIU). Or regarding installing directly to /usr or /usr/local bypassing a system package manager, which is not what I would suggest for anyone.

However, okay, if it is /usr/local, it is more or less okay. Still requires root priveleges and dubious suggestion. Debatable topic.

@Onvember Onvember linked an issue Mar 23, 2021 that may be closed by this pull request
3 tasks
@github-actions github-actions bot temporarily deployed to release March 25, 2021 14:28 Inactive
@Onvember
Copy link
Contributor Author

Onvember commented Apr 6, 2021

Outdated. See #1989

@Onvember Onvember closed this Apr 6, 2021
@Onvember Onvember deleted the onvember/gh-1640 branch April 9, 2021 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[2pt] mopinion: mistake in building instruction [1pt] feedback: Building from source
3 participants