Skip to content
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

Python3 build error: unicode problem in pip_installed_packages() #23822

Closed
koffie opened this issue Sep 10, 2017 · 11 comments
Closed

Python3 build error: unicode problem in pip_installed_packages() #23822

koffie opened this issue Sep 10, 2017 · 11 comments

Comments

@koffie
Copy link

koffie commented Sep 10, 2017

The python 3 build was broken by #23615 because of incorrect unicode usage causing the building of sage failing in the following way

chapoton@icj-laptop:~/sage3$ ./sage -br
cd . && export                                    \
    SAGE_ROOT=/doesnotexist                               \
    SAGE_SRC=/doesnotexist                                \
    SAGE_SRC_ROOT=/doesnotexist                           \
    SAGE_DOC_SRC=/doesnotexist                            \
    SAGE_BUILD_DIR=/doesnotexist                          \
    SAGE_PKGS=/home/chapoton/sage3/build/pkgs                \
    SAGE_CYTHONIZED=/home/chapoton/sage3/src/build/cythonized      \
&& sage-python23 -u setup.py --no-user-cfg build install
************************************************************************
Traceback (most recent call last):
  File "setup.py", line 69, in <module>
    from module_list import ext_modules, library_order, aliases
  File "/home/chapoton/sage3/src/module_list.py", line 166, in <module>
    from sage_setup.optional_extension import OptionalExtension
  File "/home/chapoton/sage3/src/sage_setup/optional_extension.py", line 24, in <module>
    all_packages = list_packages(local=True)
  File "/home/chapoton/sage3/src/sage/misc/package.py", line 226, in list_packages
    installed = installed_packages(exclude_pip)
  File "/home/chapoton/sage3/src/sage/misc/package.py", line 286, in installed_packages
    installed.update(pip_installed_packages())
  File "/home/chapoton/sage3/src/sage/misc/package.py", line 148, in pip_installed_packages
    return {package['name'].lower():package['version'] for package in json.loads(stdout)}
  File "/home/chapoton/sage3/local/lib/python3.6/json/__init__.py", line 354, in loads
    return _default_decoder.decode(s)
  File "/home/chapoton/sage3/local/lib/python3.6/json/decoder.py", line 339, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/home/chapoton/sage3/local/lib/python3.6/json/decoder.py", line 357, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)
************************************************************************
Error building the Sage library
************************************************************************
Makefile:34 : la recette pour la cible « sage » a échouée
make: *** [sage] Erreur 1

Component: python3

Author: Maarten Derickx

Branch/Commit: f3e5282

Reviewer: Frédéric Chapoton

Issue created by migration from https://trac.sagemath.org/ticket/23822

@koffie koffie added this to the sage-8.1 milestone Sep 10, 2017
@koffie
Copy link
Author

koffie commented Sep 10, 2017

New commits:

f3e5282Trac #23822: fix python3 build error: unicode problem in pip_installed_packages()

@koffie
Copy link
Author

koffie commented Sep 10, 2017

Commit: f3e5282

@koffie
Copy link
Author

koffie commented Sep 10, 2017

Branch: u/mderickx/23822

@koffie
Copy link
Author

koffie commented Sep 10, 2017

Author: Maarten Derickx

@fchapoton
Copy link
Contributor

comment:3

ok, let it be

@fchapoton
Copy link
Contributor

Reviewer: Frédéric Chapoton

@jhpalmieri
Copy link
Member

comment:4

Do you need the .decode, or can you just use

stdout = proc.communicate()[0]

?

@koffie
Copy link
Author

koffie commented Sep 18, 2017

comment:5

In python 2 it is not needed but good praxis. In python 3 it is needed since stdout is a bytestring before decoding and the json library only takes unicode strings and not bytestrings as input.

@jhpalmieri
Copy link
Member

comment:6

When I was looking at #23876, I came across this bug and didn't use .decode, and it seemed to work with Python 3. That is, before making any changes, Sage crashed on startup, complaining about this problem, but when I switched to stdout = proc.communicate()[0], it didn't complain about that. (It crashed elsewhere, as described at #23876.) So I am not convinced that it is needed with Python 3.

Okay, I checked the Python documentation. I agree that stdout should be a binary stream, but according to the json documentation, json.loads can take something of type bytes as input.

Edit: this behavior of json is new as of Python 3.6.

@koffie
Copy link
Author

koffie commented Sep 18, 2017

comment:7

I could have sworn that I only added the decode() because it was failing otherwise. But sage was already on 3.6 when I wrote this. I dont think either solution is clearly better then the other. But the one with a decode already has positive review so I propose to leave it as is.

@vbraun
Copy link
Member

vbraun commented Sep 20, 2017

Changed branch from u/mderickx/23822 to f3e5282

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants