Skip to content

Conversation

@bennr01
Copy link
Collaborator

@bennr01 bennr01 commented Aug 5, 2025

I haven't yet had the chance to test this out (need to get a working pythonista app again first), but these changes should add a guard to prevent py2 users from updating to an incompatible branch.
I've created the py2 branch as a legacy branch that will hold the last/latest py2 compatible version. This PR ensures that both selfupdate.py and getstash.py will only update to this branch when running StaSh via py2.

TODO:

  • rebase these changes on the current master branch.
  • actually test these changes
  • change PR to target dev instead (may require reconciliation of master and dev branches)
  • Also include these changes in the py2 branch.

@cclauss
Copy link
Collaborator

cclauss commented Aug 5, 2025

Please rebase

@cclauss
Copy link
Collaborator

cclauss commented Aug 5, 2025

Python 2 died 2,043 days ago on 1/1/2020. It is beyond time to support Python 2.

Instead of doing this, we should tell users to install an older release.

@o-murphy
Copy link
Contributor

o-murphy commented Aug 5, 2025

Python 2 died 2,043 days ago on 1/1/2020. It is beyond time to support Python 2.

Instead of doing this, we should tell users to install an older release.

we cann add just getstash_legacy.py for this approach which will use some legacy branch

@bennr01
Copy link
Collaborator Author

bennr01 commented Aug 5, 2025

@cclauss wrote:

Python 2 died 2,043 days ago on 1/1/2020. It is beyond time to support Python 2.

I agree, which is why I've created this PR to implement the py2 deprecation path outlined in this comment a couple of years ago.

Instead of doing this, we should tell users to install an older release.

This is effectively what this PR does, just with the added benefit of preventing users from shooting themselves in the foot by accidentally updating to an incompatible version. At the mere cost of maintaining py2 compatibility in getstash.py (which is really barely any effort at all with the way it is currently written), we can gracefully prevent old users from updating to an incompatible version, allow new users to install the latest compatible version and keep an update path open in case of a need for an emergency patch. Effectively, the py2 branch becomes the (potential moving) latest compatible release.

we cann add just getstash_legacy.py for this approach which will use some legacy branch

This may sound like a good idea at a first glance, but is unfortunately a horrible idea due to some details of the update system. Basically, getstash.py is used by the update system too and will always be downloaded from the target branch (defaults to master). Thus, using a getstash_legacy.py file would not work for the update system. Still, thank you for the suggestion.

Another possible way would be to make getstash.py explicitly incompatible with py2, ensuring no accidental execution on an incompatible system and thus preventing the update. However, this does not bring any other of the mentioned benefit while still requiring some work.

@cclauss
Copy link
Collaborator

cclauss commented Aug 5, 2025

Note: Moving from setup.py to pyproject.toml would probably break legacy Python.

@o-murphy
Copy link
Contributor

o-murphy commented Aug 5, 2025

Note: Moving from setup.py to pyproject.toml would probably break legacy Python.

As I know we will support >=3.10 starting from the #524 PR, and also if package have wheel - the wheel in general not contain pyproject.toml, all it mostly depends from build system and pip support, not from python itself

@bennr01
Copy link
Collaborator Author

bennr01 commented Aug 5, 2025

Note: Moving from setup.py to pyproject.toml would probably break legacy Python.

I don't think so. Please keep in mind that we have two install methods: via setup (now pyproject.toml, I guess) and via ZIP. The setup/pyproject.toml-based install is only used when installing on desktop (and perhaps the tests, I guess? It's been a couple of years since I've wrote the modern StaSh test suite). When installing or updating on app, only getstash.py is used, which would not automatically break just because we switched to pyproject.toml. You are right that the setup-based install would break, but that should only affect the desktop installs (and honestly, I don't know if anyone has actually ever used that one outside of testing.)

We could, of course, simply raise a simple Exception when installing/updating under py2, but that's just a tiny bit less effort than simply forcing the target branch on py2, which has the adadded benefits outlined before.

@o-murphy
Copy link
Contributor

o-murphy commented Aug 5, 2025

Note: Moving from setup.py to pyproject.toml would probably break legacy Python.

I don't think so. Please keep in mind that we have two install methods: via setup (now pyproject.toml, I guess) and via ZIP. The setup/pyproject.toml-based install is only used when installing on desktop (and perhaps the tests, I guess? It's been a couple of years since I've wrote the modern StaSh test suite). When installing or updating on app, only getstash.py is used, which would not automatically break just because we switched to pyproject.toml. You are right that the setup-based install would break, but that should only affect the desktop installs (and honestly, I don't know if anyone has actually ever used that one outside of testing.)

We could, of course, simply raise a simple Exception when installing/updating under py2, but that's just a tiny bit less effort than simply forcing the target branch on py2, which has the adadded benefits outlined before.

about pyproject.toml, for now it works mosly for syncing dev environment on PC, but I already built wheel and sdist for it locally. Try it with uv

uv sync
.venv/bin/activate
uv build

It should work, It can be used instead of installing from zip. It have to be possible if I adjust the installation process by using pyproject.toml + setup.py at the same time and adjust 'extracting' logics

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.

3 participants