sugarlabs / sugar-artwork

Sugar icons and themes
Apache License 2.0
11 stars 33 forks source link

Prefer python3 interpreter over python2 on ./autogen.sh #116

Closed srevinsaju closed 4 years ago

srevinsaju commented 4 years ago

Currently, AM_PYTHON_PATH prefers python > python2 > python3 and then rest. For Ubuntu bionic, and Focal, there is no python executable. Even if it was manually installed, it would be default to python2. Following the patch https://github.com/sugarlabs/sugar-artwork/commit/74d983ea14af84682e645765c8400ec0e0433bac , ./autogen requires empy to be detected as a Python Module of the respective PYTHON module. According to the documentation of "Setting up a Development Environment" so far tells to install python3-empy and not python2-empy. python2-empy is being removed from ubuntu's repositories.

sudo apt install python3-empy
echo $PYTHON  # make sure its not defined
which python  # make sure there is no `python` binary
python2 -m empy  # exits with non zero status
python3 -m empy  # passes!

in this case, it would be recommended to prefer a python3 exxecutable over python. Although the AM_PYTHON_PATH can overriden by PYTHON_VERSION environment variable, but this would not be recommended. As far as I have read the autotools documentation,

AM_PATH_PYTHON([3.6],[2.7],[:])

should do the trick! but I am not sure if its the best fix. Please let me know if there are any other interesting way to fix it other than that provided above!

Thanks

checking whether stripping libraries is possible... yes
checking if libtool supports shared libraries... yes
checking whether to build shared libraries... yes
checking whether to build static libraries... no
checking for xcursorgen... /usr/bin/xcursorgen
checking for python... no
checking for python2... /usr/bin/python2
checking for python version... 2.7
checking for python platform... linux2
checking for python script directory... ${prefix}/lib/python2.7/dist-packages
checking for python extension module directory... ${exec_prefix}/lib/python2.7/dist-packages
configure: error: empy not found
quozl commented 4 years ago

On Debian the build depends on the Python 3 package of empy. Therefore so will future versions of Ubuntu, since Sugar is a universe package and is only copied in without changes from Debian.

On Fedora there is no python2 present on the configuration used for building the package, and the packagers have chosen to explicitly locate em.py.

So if I understand you correctly, you've found that on Ubuntu (and therefore Debian), when both Python 2 and Python 3 are available, the AM_PATH_PYTHON in configure.ac chooses the Python 2 binary, which requires a Python 2 build of Empy, and the distribution does not provide a Python 2 build of Empy. However, a user could use pip to install a Python 2 build of Empy.

I don't think we should be making changes in sugar-artwork that are specific to distributions. While changing the version search order would seem to fix this, we have no other reason to change the version search order. It doesn't matter if sugar-artwork is built using Python 2 or Python 3, provided there is an Empy available for that version of Python.

So I don't think we should fix this; it's not a problem in sugar-artwork.

srevinsaju commented 4 years ago

I agree, a pip install would do, but in my case, I did not want to install python2-pip because that brings another 200 MB of dependencies. Maybe the documentation could be updated to install python2-empy along with python3-empy. On ubuntu, I have also heard that a python as python3 symlink exists as a Deb package. Is it possible to add that to the documentation? As that would not require the user to install empy from python2-pip. Alternatively the documentation could be updated to include an alternative method to make it easier. The problem is not with sugar-artwork, I first found it on sugar-artwork. Alternatively, this also helps me. Currently, I am doing this

PYTHON=/usr/bin/python3 ./autogen.sh

This also is an alternative, I am not sure which is the best alternative

quozl commented 4 years ago

Basically, you don't get a choice. AM_PATH_PYTHON will choose Python 2 if it is available, and that's what it is documented to do. If you have Python 2, you will need a Python 2 Empy, regardless of how you get it or how big the dependencies will be. It would be best not to have Python 2 on your system, and there's no reason to have it as far as sugar-artwork is concerned; as sugar-artwork will install fine with either Python 2 or Python 3 ... again provided there is also Empy,

What exactly do you mean by adding Ubuntu's special packaging documentation? Where would you propose to add that? Why would we want to do that? I don't want us burdened by having to maintain Ubuntu-specific stuff into the future, we just don't have time.

Yes, overriding the Python version is an alternative for the (rare) situation in which you have installed Python 2 without installing Empy for Python 2.

srevinsaju commented 4 years ago

It might be a mistake then, the sugar development environment docs doesn't ask the user to install python2-empy https://github.com/sugarlabs/sugar/blob/master/docs/development-environment.md#native-sugar, maybe we should include python2-empy also as a safer side

quozl commented 4 years ago

That's for experts. I don't think it can ever be correct, as Debian, Ubuntu and Fedora keep changing things. The problem with the instructions at the moment is that they include building for Python 2, which is critical for anyone doing Port to Python 3 for activities, since who could possibly complete a port properly if they couldn't test the activity first on Python 2 in order to notice regressions. Some systems won't have a python2-empy package, so that makes explaining it difficult. Changing sugar-artwork to prefer Python 3 just to make it easier for experts doesn't seem right. But you've not proposed a specific change yet, so I'm not sure.

srevinsaju commented 4 years ago

Thanks, you are right. Native building is for experts. I guess this issue will be a resource to them, in case if they ever fall into an error as such.