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

Add support for Windows #6

Merged
merged 2 commits into from
Oct 6, 2019
Merged

Add support for Windows #6

merged 2 commits into from
Oct 6, 2019

Conversation

c-w
Copy link
Contributor

@c-w c-w commented Oct 6, 2019

This pull request makes flask-now work on Windows.

Animation showing flask-now running on Windows

This required two minor fixes:

  • The which utility doesn't exist on Windows which leads to a FileNotFoundError when trying to look up the path of the Python binary. This change adds a fallback to sys.executable which fixes the problem (as long as we can assume that the Python interpreter used to run flask-now is a valid Python3 interpreter).

  • The path to the pip executable in a virtual environment is different on Windows than Unix which leads to a FileNotFoundError. This change adds a check to os.name to adjust the pip path if flask-now is being run on Windows.

- The `which` utility doesn't exist on Windows which leads to a
  FileNotFoundError when trying to look up the path of the Python
  binary. This change adds a fallback to `sys.executable` [1] which
  fixes the problem (as long as we can assume that the Python
  interpreter used to run flask-now is a valid Python3 interpreter).

- The path to the pip executable in a virtual environment is different
  on Windows than Unix which leads to a FileNotFoundError. This change
  adds a check to `os.name` [2] to adjust the pip path if flask-now is
  being run on Windows.

[1] https://docs.python.org/3/library/sys.html#sys.executable
[2] https://docs.python.org/3/library/os.html#os.name
@ozanonurtek ozanonurtek self-requested a review October 6, 2019 19:20
@ozanonurtek
Copy link
Owner

Thanks for your contribution and opening this pull request,

  • sys.executable also works fine with MacOS and Linux. I guess we don't need to check for operating system at the beginning of the main function. I would be appreciated if you could refactor your pull request in that way.
  • install_extensions functions looks good!

Copy link
Owner

@ozanonurtek ozanonurtek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned before, sys.executable also works fine with MacOS and GNU/Linux. So could you please refactor your contribution in that way 😸

@c-w
Copy link
Contributor Author

c-w commented Oct 6, 2019

Thanks for the review @ozanonurtek. Made the change to always use sys.executable in 763cba8.

@ozanonurtek
Copy link
Owner

ozanonurtek commented Oct 6, 2019

This will be packed tomorrow with version 0.2.2. @c-w, thanks for the change!

@ozanonurtek ozanonurtek merged commit 5a072dc into ozanonurtek:master Oct 6, 2019
@c-w c-w deleted the windows-support branch October 6, 2019 23:34
@ozanonurtek
Copy link
Owner

Released under 0.2.2. I was a bit busy, sorry for latency. Thanks for your all support @c-w 🎉

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

Successfully merging this pull request may close these issues.

2 participants