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

Windows encoding of __init__.py breaks setup with certain locales #89

Closed
glaubitz opened this issue Jun 2, 2017 · 2 comments · Fixed by #91

Comments

@glaubitz
Copy link

@glaubitz glaubitz commented Jun 2, 2017

The setup.py script parses the version number from the adal/__init__.py. This can fail with certain locales on Linux when Python3 is being used:

glaubitz@suse-laptop:~/upstream/azure-activedirectory-library-for-python> LANG=C python3 setup.py build
Traceback (most recent call last):
  File "setup.py", line 35, in <module>
    open('adal/__init__.py').read()).group(1)
  File "/usr/lib64/python3.6/encodings/ascii.py", line 26, in decode
    return codecs.ascii_decode(input, self.errors)[0]
UnicodeDecodeError: 'ascii' codec can't decode byte 0xef in position 0: ordinal not in range(128)
glaubitz@suse-laptop:~/upstream/azure-activedirectory-library-for-python>

This is because of the Windows encoding of __init__.py contains a byte-order mark at the beginning of the file:

<U+FEFF>#------------------------------------------------------------------------------

simply converting __init__.py to standard Unix format fixes the problem:

glaubitz@suse-laptop:~/upstream/azure-activedirectory-library-for-python> dos2unix adal/__init__.py 
dos2unix: converting file adal/__init__.py to Unix format...
glaubitz@suse-laptop:~/upstream/azure-activedirectory-library-for-python> LANG=C python3 setup.py build
running build
running build_py
creating build
creating build/lib
creating build/lib/adal
copying adal/constants.py -> build/lib/adal
copying adal/adal_error.py -> build/lib/adal
copying adal/cache_driver.py -> build/lib/adal
copying adal/mex.py -> build/lib/adal
copying adal/xmlutil.py -> build/lib/adal
copying adal/authority.py -> build/lib/adal
copying adal/authentication_context.py -> build/lib/adal
copying adal/wstrust_response.py -> build/lib/adal
copying adal/self_signed_jwt.py -> build/lib/adal
copying adal/authentication_parameters.py -> build/lib/adal
copying adal/util.py -> build/lib/adal
copying adal/argument.py -> build/lib/adal
copying adal/user_realm.py -> build/lib/adal
copying adal/oauth2_client.py -> build/lib/adal
copying adal/token_cache.py -> build/lib/adal
copying adal/log.py -> build/lib/adal
copying adal/wstrust_request.py -> build/lib/adal
copying adal/token_request.py -> build/lib/adal
copying adal/__init__.py -> build/lib/adal
copying adal/code_request.py -> build/lib/adal
glaubitz@suse-laptop:~/upstream/azure-activedirectory-library-for-python>

This problem also affects the building of RPM packages. In fact, I stumbled over this issue when trying to build the python3 RPM package for ADAL.

Please consider plain UTF-8 for the source code encoding.

@rayluo

This comment has been minimized.

Copy link
Contributor

@rayluo rayluo commented Jun 2, 2017

Hey @glaubitz thanks for your report! Different locale support has always been a hard-to-test problem.

In this particular case, we are already using UTF-8 for source code encoding, which is the Python default encoding. Besides, there is no practical way to ensure dos2unix would be run at every time after we modify our source file.

I believe the right solution would be to explicitly use UTF-8 when attempting to read the file. I create a tentative fix for that, and it passes my local simulation test based on my theory. However I don't exactly have your locale environment to reproduce the issue in the first place, so I would like to have your help to test it out. Would you mind pull my experimental branch enforcing-utf8-on-version-detection into your environment and build your RPM again? If it works well, let me know and I will merge my branch.

@glaubitz

This comment has been minimized.

Copy link
Author

@glaubitz glaubitz commented Jun 4, 2017

Hi @rayluo

Yes, your suggested patch fixes the problem for me and I can build the RPM package just fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.