Skip to content

Working Python 2.7 support under Microsoft Windows #2

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.

Sign up for GitHub

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

Closed
clach04 wants to merge 4 commits into from
Closed

Working Python 2.7 support under Microsoft Windows #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3dc6a7c
Working Python 2.7 support under Microsoft Windows
clach04 Jul 26, 2022
72f3a37
Fix two thirds of review comments
clach04 Jul 22, 2023
54af7a7
Fix issues with setup.py - possibly Windows specific
clach04 Jul 22, 2023
b5cbbd1
Include pyreadline in setup.py to match requirements.txt
clach04 Jul 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Pebble Tool

To install, from a checkout, issue:

python -m pip install -r requirements.txt

or:

python setup.py develop
python setup.py install


Definitely works with Python 2.7.

4 changes: 3 additions & 1 deletion requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
libpebble2==0.0.26
libpebble2==0.0.28
enum34==1.0.4
pyparsing==2.4.7
httplib2==0.19.0
oauth2client==1.4.12
progressbar2==2.7.3
Expand All @@ -16,3 +17,4 @@ websocket-client==0.32.0
wheel==0.24.0
colorama==0.3.3
packaging==16.7
pyreadline==2.1
Copy link
Member

@Katharine Katharine Jul 22, 2023

Choose a reason for hiding this comment

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

A quick look suggests that pyreadline has been unmaintained for longer than this tool, and does not work under the latest version of Python. pyreadline3 might be a better choice, though it's very obscure.

Does installing this without importing it do anything?

Copy link
Author

@clach04 clach04 Jul 22, 2023

Choose a reason for hiding this comment

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

I'll need to get back to you on this, been a while since I looked at it.

Copy link
Author

@clach04 clach04 Jul 22, 2023

Choose a reason for hiding this comment

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

A quick look suggests that pyreadline has been unmaintained for longer than this tool, and does not work under the latest version of Python. pyreadline3 might be a better choice, though it's very obscure.

Do you mean https://pypi.org/project/pyreadline3 ? That doesn't look appropriate, it does NOT support Python 2.7 :-(, only some 3.x

Does installing this without importing it do anything?

Are you asking if the install runs some code in the background I'd need to look at the setup.py to know? I'm not 100% sure what you are asking. It's defintely being imported by commands/repl.py

pebble-tool/pebble_tool/commands/repl.py

Line 5 in 92561f5

import readline

Copy link
Member

@Katharine Katharine Jul 22, 2023

Choose a reason for hiding this comment

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

Supporting Python 2.7 is not really a goal at this point, given that Python 2.7 has been dead for years. That said, it is still necessary given that while this runs (or should run) fine under Python 3, the build chain does not. (For that matter, supporting Windows outside WSL is also a non-goal...)

Is editing requirements.txt even the right thing to be doing? Selectively installing things might be better. What, if anything, happens if this is installed on Linux or macOS?

Copy link
Author

@clach04 clach04 Jul 22, 2023

Choose a reason for hiding this comment

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

Hmmm, maybe that's the way to tackle this. Scratch this PR, and I open a new one that's ONLY a readme change with a note for py2.7 (and Windows) so it's a manual step (or a separate requirements file) so the current known platforms work the way they always have.

Let me try that out real quick.

Copy link
Member

@Katharine Katharine Jul 22, 2023

Choose a reason for hiding this comment

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

It should be possible to selectively install things in setup.py, if one uses that mechanism instead of requirements.txt.

Copy link
Author

@clach04 clach04 Jul 22, 2023

Choose a reason for hiding this comment

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

I'm definitely down for that :-)

I just posted a hopefully easier to accept PR #4 to fix things on all platforms, with Windows to follow. I already tested out requirements windows file and it works great, but a conditional in setup.py would be great too. My main goal is to get something merged so it's in a central place, for a clean working version.

6 changes: 4 additions & 2 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
from setuptools import setup, find_packages

requires = [
'libpebble2==0.0.26',
'httplib2==0.9.1',
'pyparsing==2.4.7',
'libpebble2==0.0.28',
'httplib2==0.19.0',
'oauth2client==1.4.12',
'progressbar2==2.7.3',
'pyasn1==0.1.8',
Expand All @@ -21,6 +22,7 @@
'wheel==0.24.0',
'colorama==0.3.3',
'packaging==16.7',
'pyreadline==2.1',
]

if sys.version_info < (3, 4, 0):
Expand Down