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

Migrate off SafeConfigParser #196

Merged

Conversation

luca-della-vedova
Copy link
Contributor

@luca-della-vedova luca-della-vedova commented Jun 18, 2024

SafeConfigParser has been deprecated since Python 3.2. There has been a shim which has been finally removed in Python 3.11 in this commit in favor of simply ConfigParser.

This means that this utility won't work if users are on Python >= 3.11 (which is the case for Ubuntu Noble).
In this PR I migrated to ConfigParser and took the liberty to remove the try except that made the package work for Python 2 since Python 2 is been EOL for more than 4 years now (this fix would have been more boilerplate otherwise).

readfp was also deprecated in favor of read_file.

Test it!

Try building a package on Ubuntu 22.04, then try on Ubuntu 24.04. For example, when
trying to build colcon-cmake using the publish-python utility, the output under Ubuntu 24.04 is:

~/colcon-cmake$ ../publish-python/bin/publish-python 
[CUT]
== Creating 'stdeb' artifacts ...

-- Building sdist_dsc and bdist_deb package ...
$ /usr/bin/python3 setup.py --command-packages=stdeb.command sdist_dsc --source python3-colcon-cmake --with-python3 true --with-python2 false --force-x-python3-version bdist_deb 
running sdist_dsc
running egg_info
writing colcon_cmake.egg-info/PKG-INFO
writing dependency_links to colcon_cmake.egg-info/dependency_links.txt
writing entry points to colcon_cmake.egg-info/entry_points.txt
writing requirements to colcon_cmake.egg-info/requires.txt
writing top-level names to colcon_cmake.egg-info/top_level.txt
reading manifest file 'colcon_cmake.egg-info/SOURCES.txt'
adding license file 'LICENSE'
writing manifest file 'colcon_cmake.egg-info/SOURCES.txt'
Traceback (most recent call last):
  File "/home/lucadv/colcon-cmake/setup.py", line 6, in <module>
    setup()
  File "/usr/lib/python3/dist-packages/setuptools/__init__.py", line 107, in setup
    return distutils.core.setup(**attrs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/setuptools/_distutils/core.py", line 185, in setup
    return run_commands(dist)
           ^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/setuptools/_distutils/core.py", line 201, in run_commands
    dist.run_commands()
  File "/usr/lib/python3/dist-packages/setuptools/_distutils/dist.py", line 969, in run_commands
    self.run_command(cmd)
  File "/usr/lib/python3/dist-packages/setuptools/dist.py", line 1233, in run_command
    super().run_command(command)
  File "/usr/lib/python3/dist-packages/setuptools/_distutils/dist.py", line 988, in run_command
    cmd_obj.run()
  File "/usr/lib/python3/dist-packages/stdeb/command/sdist_dsc.py", line 30, in run
    debinfo = self.get_debinfo()
              ^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/stdeb/command/common.py", line 197, in get_debinfo
    debinfo = DebianInfo(
              ^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/stdeb/util.py", line 802, in __init__
    check_cfg_files(cfg_files, module_name)
  File "/usr/lib/python3/dist-packages/stdeb/util.py", line 733, in check_cfg_files
    cfg = ConfigParser.SafeConfigParser()
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: module 'configparser' has no attribute 'SafeConfigParser'. Did you mean: 'RawConfigParser'?
Traceback (most recent call last):
  File "/home/lucadv/colcon-cmake/../publish-python/bin/publish-python", line 24, in <module>
    rc = main()
         ^^^^^^
  File "/home/lucadv/publish-python/publish_python/cli.py", line 77, in main
    artifacts = artifact_handlers[artifact_type](config=artifact.config)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/lucadv/publish-python/publish_python/artifact/stdeb.py", line 44, in create_deb
    subprocess.check_call(cmd, env=dict(os.environ, **add_env))
  File "/usr/lib/python3.12/subprocess.py", line 413, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['/usr/bin/python3', 'setup.py', '--command-packages=stdeb.command', 'sdist_dsc', '--source', 'python3-colcon-cmake', '--with-python3', 'true', '--with-python2', 'false', '--force-x-python3-version', 'bdist_deb']' returned non-zero exit status 1.

Now try the PR code:

  • sudo apt remove python3-stdeb.
  • pip install git+https://github.com/luca-della-vedova/stdeb@luca/migrate_configparser --break-system-packages
  • rebuild the package
  • It works!

@luca-della-vedova
Copy link
Contributor Author

@nuclearsandwich @cottsay As people adopt distros with Python >= 3.11 this will become increasingly more common (i.e. #197 that was just opened), can you take a look? The main "controversial" part is that in its current form the change would drop Python 2 compatibility (which imho is OK, but I'm happy to put the extra work to keep it if it's needed)

@ampledata
Copy link

@nuclearsandwich @cottsay As people adopt distros with Python >= 3.11 this will become increasingly more common (i.e. #197 that was just opened), can you take a look? The main "controversial" part is that in its current form the change would drop Python 2 compatibility (which imho is OK, but I'm happy to put the extra work to keep it if it's needed)

1_Wguhwyawnz7QV-LtsAgLHg

Copy link
Collaborator

@nuclearsandwich nuclearsandwich left a comment

Choose a reason for hiding this comment

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

Thanks @luca-della-vedova

I'm not going to merge this right away because I want to include it in a more formal dropping of Python 2 support.

Signed-off-by: Luca Della Vedova <[email protected]>
Signed-off-by: Luca Della Vedova <[email protected]>
@nuclearsandwich nuclearsandwich force-pushed the luca/migrate_configparser branch from 6b1098d to 7730361 Compare November 14, 2024 20:57
@nuclearsandwich nuclearsandwich merged commit 3a09257 into astraw:master Nov 14, 2024
5 checks passed
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