{"action":"created","issue":{"url":"https://api.github.com/repos/ceph/ceph/issues/10208","repository_url":"https://api.github.com/repos/ceph/ceph","labels_url":"https://api.github.com/repos/ceph/ceph/issues/10208/labels{/name}","comments_url":"https://api.github.com/repos/ceph/ceph/issues/10208/comments","events_url":"https://api.github.com/repos/ceph/ceph/issues/10208/events","html_url":"https://github.com/ceph/ceph/pull/10208","id":164513088,"number":10208,"title":"Include Python 3 bindings into the cmake build and make packages for them","user":{"login":"BlaXpirit","id":371383,"avatar_url":"https://avatars.githubusercontent.com/u/371383?v=3","gravatar_id":"","url":"https://api.github.com/users/BlaXpirit","html_url":"https://github.com/BlaXpirit","followers_url":"https://api.github.com/users/BlaXpirit/followers","following_url":"https://api.github.com/users/BlaXpirit/following{/other_user}","gists_url":"https://api.github.com/users/BlaXpirit/gists{/gist_id}","starred_url":"https://api.github.com/users/BlaXpirit/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/BlaXpirit/subscriptions","organizations_url":"https://api.github.com/users/BlaXpirit/orgs","repos_url":"https://api.github.com/users/BlaXpirit/repos","events_url":"https://api.github.com/users/BlaXpirit/events{/privacy}","received_events_url":"https://api.github.com/users/BlaXpirit/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/ceph/ceph/labels/build/ops","name":"build/ops","color":"eb6420"},{"url":"https://api.github.com/repos/ceph/ceph/labels/feature","name":"feature","color":"009800"},{"url":"https://api.github.com/repos/ceph/ceph/labels/pybind","name":"pybind","color":"bfd4f2"}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":20,"created_at":"2016-07-08T11:37:36Z","updated_at":"2016-08-10T14:31:57Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/ceph/ceph/pulls/10208","html_url":"https://github.com/ceph/ceph/pull/10208","diff_url":"https://github.com/ceph/ceph/pull/10208.diff","patch_url":"https://github.com/ceph/ceph/pull/10208.patch"},"body":"## cmake\r\n\r\n#### FindPython3Interp and FindPython3Libs modules have been added\r\nMultiple alternatives have been considered on how to include multiple versions of Python into the build process.\r\n\r\n- The first thing that comes to mind is to reuse the standard module *FindPythonInterp* twice with different inputs, however CMake makes this very difficult because this module saves globally cached state and still gives its initial outputs even when ran with different arguments. An effort was made to clear specific cache variables in the middle of the build process to be able to reuse the module, and it seemed to mostly work, even if it was a mess, but in the end it was discovered that Ubuntu 14.04 ships an old version of CMake where *FindPythonLibs* simply does not acknowledge Python 3 yet, even by itself.\r\n\r\nThe two other alternatives that were dismissed, for hopefully obvious reasons:\r\n- Drop *FindPythonXXX* modules and run hardcoded Python binaries\r\n- Run the whole configure+build process twice with a different flag for the Python version\r\n\r\nSo, the final decision was to copy the latest versions of the official CMake modules *FindPythonInterp*, *FindPythonLibs* and replace every variable name inside them from `Python`, `PYTHON_` etc. to `Python3`, `PYTHON3_` etc. So even though these modules have global state, the state is now separate for Python 2 and 3.\r\n\r\n#### Python bindings are built for Python 2 and 3\r\nThis is achieved by looping over two values of a global variable `${PYTHON_VERSION}` &ndash; `\"\"` and `\"3\"`. *src/pybind/CMakeLists.txt* introduces this variable, calls `find_package(Python${PYTHON_VERSION}Interp ...` and includes the subdirectories, which use the *Distutils* module, which is now aware of this `PYTHON_VERSION` variable.\r\n\r\n#### FindCython module has been dropped\r\nThe only effect this module did was to fail when a `cython` executable was not found. However, running Cython itself was done through `python setup.py` in a different module, which is a bit of an inconsistency and these two things succeeding are not necessarily correlated. The situation is made worse by the fact that now we need Cython specifically for Python 2 and separately for Python 3.\r\n\r\nI acknowledge that there needs to be a way to fail if Cython is not present &ndash; during the configuration phase rather than during the build. Looking for feedback on alternatives. The simple way to do the check would be to run `${PYTHON_EXECUTABLE} -m cython`\r\n\r\n#### Summary\r\n\r\nThis change makes the CMake-based build additionally produce files like *build/lib/cython_modules/lib.linux-x86_64-3.4/`{`rados,cephfs,rbd`}`.cpython-34m.so*\r\n\r\nThe weakest point of this change is about *FindCython* (discussed above). Other than that, it has been tested on Debian and produces working libraries for both Python versions.\r\n\r\n\r\n## debian\r\n\r\nThis is quite straightforward, three new packages have been created: *python3-`{`rados,cephfs,rbd`}`* (based on existing ones). Because Python libraries are installed into a path like */usr/lib/python3.4/dist-packages/*, these *install* files utilize this like so: `usr/lib/python3*/dist-packages/rados.so`. Additionally, the file lists for Python 2 needed to be changed to include a *2* so they don't pick up files for both versions of Python.\r\nThe newly added packages get a line [`Depends: ${python3:Depends}`](https://wiki.debian.org/Python/LibraryStyleGuide#Python_versions)\r\n\r\nSome build dependencies were added: [*python3-dev*](https://packages.debian.org/search?keywords=python3-dev), [*python3-setuptools*](https://packages.debian.org/search?keywords=python3-setuptools), [*cython3*](https://packages.debian.org/search?keywords=cython3).\r\n\r\nThis change has **not** been confirmed working, but to the best of my knowledge there should be no problems.\r\n\r\n\r\n## rpm\r\n\r\nThree new packages have been created: *python34-`{`rados,cephfs,rbd`}`* (based on existing ones). Well, the version *34* is not specified anywhere, but my understanding is that the `%{python3_pkgversion}` macro expands to *34* on CentOS and to *3* on Fedora. `%{python3_sitearch}` and `%{python3_sitelib}` were utilized. [*python3-numpy.spec*](http://pkgs.fedoraproject.org/cgit/rpms/python3-numpy.git/tree/python3-numpy.spec?h=epel7) served as an example of these techniques.\r\n\r\nAdded dependencies:\r\n\r\n- RHEL/CentOS:\r\n[*python34*](https://dl.fedoraproject.org/pub/epel/7/x86_64/p/python34-3.4.3-4.el7.x86_64.rpm)\r\n    - build:\r\n[*python34-devel*](https://dl.fedoraproject.org/pub/epel/7/x86_64/p/python34-devel-3.4.3-4.el7.x86_64.rpm), [*python34-setuptools*](https://dl.fedoraproject.org/pub/epel/7/x86_64/p/python34-setuptools-19.2-3.el7.noarch.rpm), [*python34-Cython*](https://dl.fedoraproject.org/pub/epel/7/x86_64/p/python34-Cython-0.23.5-1.el7.x86_64.rpm)\r\n- Fedora:\r\n[*python3*](https://admin.fedoraproject.org/pkgdb/package/rpms/python3/)\r\n    - build:\r\n[*python3-devel*](https://apps.fedoraproject.org/packages/python3-devel), [*python3-setuptools*](https://apps.fedoraproject.org/packages/python3-setuptools), [*python3-Cython*](https://apps.fedoraproject.org/packages/python3-Cython)\r\n- OpenSUSE:\r\n[*python3*](http://download.opensuse.org/tumbleweed/repo/oss/suse/x86_64/python3-3.5.1-3.4.x86_64.rpm)\r\n    - build:\r\n[*python3-devel*](http://download.opensuse.org/tumbleweed/repo/oss/suse/x86_64/python3-devel-3.5.1-3.3.x86_64.rpm), [*python3-setuptools*](http://download.opensuse.org/tumbleweed/repo/oss/suse/noarch/python3-setuptools-23.1.0-1.1.noarch.rpm), [*python3-cython*](http://download.opensuse.org/tumbleweed/repo/oss/suse/x86_64/python3-Cython-0.24-2.2.x86_64.rpm)\r\n\r\nThis change has **not** been tested at all.\r\n\r\nThe big problem with it is that on CentOS all mentioned Python3-related packages come from EPEL (not readily available to install by default). And they are mandatory for the build, so it may become generally more difficult to build packages."},"comment":{"url":"https://api.github.com/repos/ceph/ceph/issues/comments/238885244","html_url":"https://github.com/ceph/ceph/pull/10208#issuecomment-238885244","issue_url":"https://api.github.com/repos/ceph/ceph/issues/10208","id":238885244,"user":{"login":"BlaXpirit","id":371383,"avatar_url":"https://avatars.githubusercontent.com/u/371383?v=3","gravatar_id":"","url":"https://api.github.com/users/BlaXpirit","html_url":"https://github.com/BlaXpirit","followers_url":"https://api.github.com/users/BlaXpirit/followers","following_url":"https://api.github.com/users/BlaXpirit/following{/other_user}","gists_url":"https://api.github.com/users/BlaXpirit/gists{/gist_id}","starred_url":"https://api.github.com/users/BlaXpirit/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/BlaXpirit/subscriptions","organizations_url":"https://api.github.com/users/BlaXpirit/orgs","repos_url":"https://api.github.com/users/BlaXpirit/repos","events_url":"https://api.github.com/users/BlaXpirit/events{/privacy}","received_events_url":"https://api.github.com/users/BlaXpirit/received_events","type":"User","site_admin":false},"created_at":"2016-08-10T14:31:56Z","updated_at":"2016-08-10T14:31:56Z","body":"Yeah, @tchaikov, if I had to guess, I would say this wouldn't solve the problem.\r\n\r\nAnyhow, just to confirm, with that commit [reverted](https://github.com/ceph/ceph/commits/58906461047baa2c79279eaa454d7cc8ec4be49f) it [builds fine](http://gitbuilder.sepia.ceph.com/gitbuilder-ceph-rpm-centos7-amd64-basic/log.cgi?log=58906461047baa2c79279eaa454d7cc8ec4be49f)"}}
{"actor":{"display_login":"BlaXpirit"}}