search

yoctoproject / bmaptool

BMAP Tools
GNU General Public License v2.0
25 stars 13 forks source link

Remove `six` from production code #12

Closed JPEWdev closed 7 months ago

JPEWdev commented 7 months ago

Removes the six module from released production code. The old code in the test cases used for testing compatability still uses six, so it is kept as a dev dependency.

twoerner commented 7 months ago

There are a couple references to python2 in packaging/bmaptool.spec and some of the files in debian. If we're removing python2 support, should those be updated too?

JPEWdev commented 7 months ago

Probably, but I'm actually not sure what it's going to take to re-write the debian packaging stuff to work "correctly" with the new hatch build?

moto-timo commented 7 months ago

Probably, but I'm actually not sure what it's going to take to re-write the debian packaging stuff to work "correctly" with the new hatch build?

I do know a bit about debian packaging so I might be able to help out what that. The pep-517 backend stuff will be new to me, but I assume the upstream debian python packaging helper scripts know about it.

moto-timo commented 7 months ago

Seems like debian/ is ok after a quick review (I didn't try to build). packaging/bmaptool.spec is currently python2 only and will need some updates for modern Fedora and similar distributions. I am a Fedora packager, so I should be able to help.

moto-timo commented 7 months ago

Upstream Fedora will eventually need to switch over to our new repo, but the current source for the old bmap-tools.spec is https://src.fedoraproject.org/rpms/bmap-tools/blob/rawhide/f/bmap-tools.spec

twoerner commented 7 months ago

Okay, I'll merge this. But perhaps keep in mind that more needs to be done wrt debian and fedora packaging.

twoerner commented 7 months ago

hmm.... i don't think six is an optional dependency.

using a clean update/install it fails with:

$ bmaptool --version
Traceback (most recent call last):
  File "/2opt/devel/bmaptool/venv.python3.11/bin/bmaptool", line 5, in <module>
    from bmaptool.CLI import main
  File "/2opt/devel/bmaptool/venv.python3.11/lib64/python3.11/site-packages/bmaptool/CLI.py", line 42, in <module>
    from . import BmapCreate, BmapCopy, BmapHelpers, TransRead
  File "/2opt/devel/bmaptool/venv.python3.11/lib64/python3.11/site-packages/bmaptool/TransRead.py", line 36, in <module>
    from six.moves.urllib import parse as urlparse
ModuleNotFoundError: No module named 'six'
JPEWdev commented 7 months ago

Blerg sorry

moto-timo commented 7 months ago

six shouldn't need to be required, as the entire purpose is 2 to 3 transition. We probably can refactor that TransReader to not conditionally need to support python2 anymore. But easier said than done perhaps.

moto-timo commented 7 months ago

We may need to refactor for a newer urllib (urllib3 or something like that from memory).