Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Windows fix for popen and unicode_literals + import_project CLI #1067

Closed

Conversation

mchubby
Copy link

@mchubby mchubby commented Apr 11, 2016

Fixes subprocess.call() on Windows due to use of future unicode_literals (see nephila/djangocms-installer#228 and jazzband/pip-tools#209), when invoking manage.py migrate.

Also fixes get_matching_files() in import_project.py with glob.glob() returning paths with forward slashes.

EDIT: squashed two more commits for another glob() and os.path.realpath() calls.

@mchubby mchubby force-pushed the windows-subprocess-import-fixes branch from e95fc3e to 09223e2 Compare April 11, 2016 22:20
second fix for glob-related windows bugs

fix for realpath() and windows backslashes
@mchubby mchubby force-pushed the windows-subprocess-import-fixes branch from 09223e2 to 85795df Compare April 11, 2016 22:39
nijel added a commit that referenced this pull request Apr 12, 2016
Always use / as path separator even on Windows, see issue #1067

Signed-off-by: Michal Čihař <michal@cihar.com>
nijel added a commit that referenced this pull request Apr 12, 2016
This will allow us to do some sanity checking there.

Issue #1067

Signed-off-by: Michal Čihař <michal@cihar.com>
nijel added a commit that referenced this pull request Apr 12, 2016
It can not handle ascii unicode objects unlike other platforms.

Issue #1067

Signed-off-by: Michal Čihař <michal@cihar.com>
@nijel
Copy link
Member

nijel commented Apr 12, 2016

Thanks for your patch, however I've decided to fix this differently. Main problems with your approach:

  • using str() all over the code makes it harder to read and also makes it easy to forget on this with future code changes
  • unconditionally replacing \ in the path can possibly break it for other platforms, where this is valid character in the path

My commits to address this issue:

  • 7c0e724 Convert environment to string objects on Python 2 on Windows
  • ab20f7f Use get_clean_env to process full environment for scripts
  • 04cdc63 Use / as path separator

@nijel nijel closed this Apr 12, 2016
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants