slgobinath / SafeEyes

Protect your eyes from eye strain using this simple and beautiful, yet extensible break reminder
http://slgobinath.github.io/SafeEyes/
GNU General Public License v3.0
1.41k stars 159 forks source link

Remove use of distutils #554

Closed terop closed 2 days ago

terop commented 6 months ago

As of Python 3.12 setuptools is no longer installed by default and if setuptools is not installed distutils is not found and SafeEyes fails to start.

Furthermore distutils is deprecated and should thus not be used anymore. The packaging module provides a replacement for LooseVersion and it is added in this commit.

terop commented 3 days ago

PEP 632 reference added.

deltragon commented 3 days ago

Should the new dependency also be added to the debian/control file? (Adding a dependency on https://packages.debian.org/bookworm/python3-packaging there)

terop commented 2 days ago

@deltragon That's a good question which I cannot adequately answer as I have never done Debian packaging. If you think it should be added I can certainly add it.

archisman-panigrahi commented 2 days ago

Should the new dependency also be added to the debian/control file? (Adding a dependency on https://packages.debian.org/bookworm/python3-packaging there)

Yes, we need to add python3-packaging to the build-depends field in debian/control. https://github.com/slgobinath/SafeEyes/blob/master/debian/control#L5

(I have been doing deb packaging for several years)

terop commented 2 days ago

Thanks for your input @archisman-panigrahi. I added python3-packaging per your suggestion.

archisman-panigrahi commented 2 days ago

In that case, we should bump python version requirement to 3.12+ in debian/control, and also remove python3-setuptools from the build-depends.

terop commented 2 days ago

Changes done. @archisman-panigrahi can you kindly check if the latest changes look correct?

archisman-panigrahi commented 2 days ago

I verified that the deb package builds with your PR in Ubuntu 24.04 and it runs fine. However, also please update python3 version requirement in this line.

Otherwise, I approve merging

archisman-panigrahi commented 2 days ago

Looks good, and I approve merging.

archisman-panigrahi commented 2 days ago

By the way, is this line still necessary, if setuptools is being discontinued? https://github.com/slgobinath/SafeEyes/blob/master/setup.py#L3

terop commented 2 days ago

I suppose we are not discontinuing setuptools, at least not in this PR.