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

[Tests] we need to use system file encoding, if on windows #437

Merged
merged 1 commit into from Sep 13, 2017

Conversation

Projects
None yet
2 participants
@vanadium23
Contributor

vanadium23 commented Sep 7, 2017

Problem was, because we decode latin-1 string with utf-8.

Fixes #403

@vanadium23

This comment has been minimized.

Contributor

vanadium23 commented Sep 7, 2017

@nixjdm I've found root cause, that os.path returns path in latin-1 on win32, but don't know how to fix it properly. Can you help me?

@nixjdm

This comment has been minimized.

Member

nixjdm commented Sep 7, 2017

@vanadium23 You're talking about that there is still this error, right?

_________________________ test_unicode_project_folder _________________________
tests\test_unicode.py:20: in test_unicode_project_folder
    pad, builder = get_unicode_builder(tmpdir)
tests\test_unicode.py:13: in get_unicode_builder
    env = Environment(proj)
lektor\environment.py:402: in __init__
    self.root_path = os.path.abspath(project.tree)
E   AttributeError: 'NoneType' object has no attribute 'tree'

Let me try to think through this with you. I think this is happening because https://github.com/lektor/lektor/blob/master/tests/test_unicode.py#L11 is passing in a unicode path, but the sys.getfilesystemencoding() is returning latin1. What does https://github.com/lektor/lektor/blob/master/lektor/project.py#L44 set the path to, when the argument is unicode and the os is using latin1? I'm wondering if you're getting down to the caught OSError and returning None.

I don't have a solution for you at the moment. I hope that helps.

@vanadium23

This comment has been minimized.

Contributor

vanadium23 commented Sep 10, 2017

@nixjdm finally solved it, this is more jinja2 problem and currently I've added workaround to fix it.

@nixjdm

This comment has been minimized.

Member

nixjdm commented Sep 12, 2017

It took me a while but I think I basically understand this now. I do have a question though. Is this really just a problem on PY2? Wouldn't the default encoding be latin1 on PY3 as well, and we'd run into the same problem? Maybe I should just merge this and see. If PY3 fails, appveyor will catch it and we can remove the PY2 bit here. What do you think @vanadium23 ?

@vanadium23

This comment has been minimized.

Contributor

vanadium23 commented Sep 13, 2017

@nixjdm I've next PR #442 and in it's build you see different errors for PY3 on windows. Also I don't reproduce this error with python3 on local machine (:

@nixjdm

This comment has been minimized.

Member

nixjdm commented Sep 13, 2017

Sounds good to me. Good work @vanadium23 !

@nixjdm nixjdm merged commit 55e38a0 into lektor:master Sep 13, 2017

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment