[SRU] displaycal 3.9.11-2 needs patch to work with Python 3.12

[Andrea Ieri]

[Impact]

DisplayCAL, when released, was only compatible with Python 3.8-3.11. However, all it needed was WxWidgets 4.1.1 and a few patches to make it work with Python 3.12. Without, it would simply fail. Namely, it needed to be patched to remove the Python version check upon launch.

[Test Case]

 * Install displaycal
 * Run displaycal

Expected: Run normally
Result:

Traceback (most recent call last):
  File "/usr/bin/displaycal", line 4, in <module>
    from DisplayCAL.main import main
  File "/usr/lib/python3/dist-packages/DisplayCAL/main.py", line 27, in <module>
    raise RuntimeError(
RuntimeError: Need Python version >= 3.8 <= 3.11, got 3.12.2

[What could go wrong]

The DisplayCAL developers do not yet support Python 3.12 until the release of WxWidgets 4.2.0 since it could crash upon closure. I (Erich Eickmeyer) have not experienced this in my testing, however, it is plausible. Additionally, the patches that were used to make this work were developed after the release of DisplayCAL 3.9.12, but still applied cleanly to 3.9.11 and made it function correctly.

To be clear, the issue with WxWidgets is a separate bug and outside the scope of this bug.

[Other Information]

Currently the tests have to be skipped otherwise they segfault with Python 3.12 and the entire application fails to run, hence the inclusion of the patch `05_skip-test-wioth-python-3.12.patch`. I would, however, argue that the application running trumps its internal tests segfaulting. Furthermore, the upstream developers have yet to figure out why those tests are segfaulting on Python 3.12 and therefore don't consider it a priority. The goal here is to make the application run.

Original bug report:

I have upgraded to the noble beta today, and displaycal fails to start.

The package dependencies request python 3.12, but the actual software needs 3.11 (or older).

```
$ apt show displaycal | grep Depends

WARNING: apt does not have a stable CLI interface. Use with caution in scripts.

Depends: libc6 (>= 2.34), libx11-6, libxinerama1 (>= 2:1.1.4), libxrandr2 (>= 2:1.2.0), libxxf86vm1, python3 (<< 3.13), python3 (>= 3.12~), python3-build, python3-certifi, python3-dbus, python3-distro, python3-numpy, python3-pil, python3-pychromecast, python3-send2trash, python3-wxgtk4.0, python3-zeroconf, python3:any, dbus-x11, argyll, libjs-jquery, python3-gi, libsdl2-mixer-2.0-0

$ displaycal --help
Traceback (most recent call last):
  File "/usr/bin/displaycal", line 4, in <module>
    from DisplayCAL.main import main
  File "/usr/lib/python3/dist-packages/DisplayCAL/main.py", line 27, in <module>
    raise RuntimeError(
RuntimeError: Need Python version >= 3.8 <= 3.11, got 3.12.2

```

ProblemType: Bug
DistroRelease: Ubuntu 24.04
Package: displaycal 3.9.11-2
ProcVersionSignature: Ubuntu 6.8.0-22.22-generic 6.8.1
Uname: Linux 6.8.0-22-generic x86_64
NonfreeKernelModules: zfs
ApportVersion: 2.28.0-0ubuntu1
Architecture: amd64
CasperMD5CheckResult: unknown
CurrentDesktop: ubuntu:GNOME
Date: Sun Apr 14 15:16:10 2024
ProcEnviron:
 LANG=en_US.UTF-8
 PATH=(custom, no user)
 SHELL=/bin/zsh
 TERM=xterm-256color
 XDG_RUNTIME_DIR=<set>
SourcePackage: displaycal-py3
UpgradeStatus: Upgraded to noble on 2024-04-14 (0 days ago)
mtime.conffile..etc.xdg.autostart.z-displaycal-apply-profiles.desktop: 2023-10-22T21:28:16.420795

[Andrea Ieri (aieri)]

It's also worth noting that displaycal-py3 does not support python 3.12 yet. See for example https://github.com/eoyilmaz/displaycal-py3/issues/335#issuecomment-2016761349

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Erich Eickmeyer (eeickmeyer)]

There's a potential clue on fixing this in https://github.com/eoyilmaz/displaycal-py3/issues/341, but I'm not 100% sure on this. I'm definitely keeping an eye on this.

[Erich Eickmeyer (eeickmeyer)]

Looks like what we're really waiting on is https://github.com/wxWidgets/Phoenix/issues/2553.

[Erich Eickmeyer (eeickmeyer)]

So I guess this is an upstream issue they are certainly aware of but it's currently out of their hands. I guess all we can do is wait and see. In the meantime, once we get a viable 3.12 version, I'll try to get it into 24.04 one way or another, hopefully just by patching this version. If not, a wholesale backport of the new version might be in order as it might have changes to be relevant.

[Erich Eickmeyer (eeickmeyer)]

Actually, the version of wxwidgets supports 3.12. Currently seeing what a backport of the version in oracular, which contains no new features, does. If it works, I'll try to SRU it into noble.

[Erich Eickmeyer (eeickmeyer)]

Heh, it won't. It simply needs this commit: https://github.com/eoyilmaz/displaycal-py3/commit/0025940335eb1bd56f03f742aa095028d7ffa06f. I'll try simply patching and see if that works.

[Erich Eickmeyer (eeickmeyer)]

Unfortunately, that one patch didn't work and it needs more work. Got the following:

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/DisplayCAL/main.py", line 549, in main
    _main(module, name, app_lock_file_name)
  File "/usr/lib/python3/dist-packages/DisplayCAL/main.py", line 466, in _main
    from DisplayCAL.wxwindows import BaseApp
  File "/usr/lib/python3/dist-packages/DisplayCAL/wxwindows.py", line 28, in <module>
    from DisplayCAL import ICCProfile as ICCP
  File "/usr/lib/python3/dist-packages/DisplayCAL/ICCProfile.py", line 51, in <module>
    from DisplayCAL import edid
  File "/usr/lib/python3/dist-packages/DisplayCAL/edid.py", line 43, in <module>
    from DisplayCAL import RealDisplaySizeMM as RDSMM
  File "/usr/lib/python3/dist-packages/DisplayCAL/RealDisplaySizeMM.py", line 47, in <module>
    _GetXRandROutputXID = GetXRandROutputXID
                          ^^^^^^^^^^^^^^^^^^
NameError: name 'GetXRandROutputXID' is not defined

[Erich Eickmeyer (eeickmeyer)]

Ok, we have success built in my PPA at https://launchpad.net/~eeickmeyer/+archive/ubuntu/displaycaltest. I'll convert this bug into an SRU now.

[Erich Eickmeyer (eeickmeyer)]

Ok, the fix is now uploaded for Oracular and Noble, and is awaiting SRU approval for Noble. This will take a minimum of one week for the SRU process. For more information, see https://wiki.ubuntu.com/StableReleaseUpdates

[Launchpad Janitor (janitor)]

This bug was fixed in the package displaycal-py3 - 3.9.12-1ubuntu1

---------------
displaycal-py3 (3.9.12-1ubuntu1) oracular; urgency=medium

  * Patch to fix runtime on python 3.12 (LP: #2061329)

 -- Erich Eickmeyer <email address hidden> Thu, 30 May 2024 10:39:07 -0700

[Robie Basak (racb)]

Thank you for working on this!

I think this is really a different upstream bug though. The current fix is just kicking the can down the road. We don't hardcode versioned dependencies for what we ship with, because that would lead to brittle packaging that breaks every package when Python is updated in the archive. Doing so within the code itself is equivalently problematic.

The real issue here is that there's a hardcoded version check which is inappropriate for our packaging. Instead, this check should be removed. Where behaviour must change based on the Python version, then that's fine, but it should arrange to use the latest behaviour for future versions. For example:

# Using additional redundant checks to make my point in
# this review clear; I wouldn't do this in real code
if dependency_version < 3:
    do_original_behaviour()
elif 3 <= dependency_version < 5:
    do_intermediate_behaviour()
elif dependency_version >= 5:
    do_latest_behaviour()

This way, the code and therefore the final package will continue to work with newer dependency versions without requiring any further intervention, which is suitable for a distribution that contains thousands of packages.

I see a concrete historical real world example where following this pattern would have caused problems. Python 3.11 was added to 22.04 in an SRU into universe. If a depending package was hardcoded to require the Python 3.10 that 22.04 originally exclusively shipped, then it would unnecessarily break against Python 3.11 on 22.04 after that SRU. If all depending packages were brittle in this way, the SRU wouldn't have worked at all without being forced to individually tweak all of those packages, for no good reason.

I would suggest to upstream that they follow this pattern to prevent issues for distribution packagers. It will also save them work every time a dependency version is bumped. Whether they agree for their case or not, our distribution packaging must be less brittle than this.

Please:

1) Fix the development release to not be brittle in this way, so it won't unnecessarily break if Python is updated in a way that doesn't break its API in a way that matters to this package.

2) Fix the SRU upload to also not be brittle in this way, so it won't unnecessarily break against a newer version of Python were we to add it to 24.04 during its lifetime, as we did for 22.04. For example, you could the patch you're adding so it works for all future Python versions, not just 3.12.

SRU process requires the development release also be fixed, and the current fix will automatically break the moment the development release moves to Python 3.13 currently in oracular-proposed; hence my requirement for 1 above, before we can land 2.

> [e-mail](reason="Test segfaults with python 3.12 - further investigation
required.")

3) This also needs some further explanation, please. Is this related to "since it could crash upon closure", or something else? What's the status of this? Are you proposing this SRU on the basis that it's definitely better than the current state? All of this should be documented, please.

[Robie Basak (racb)]

An upload of displaycal-py3 to noble-proposed has been rejected from the upload queue for the following reason: "See https://bugs.launchpad.net/ubuntu/+source/displaycal-py3/+bug/2061329/comments/13".

[Launchpad Janitor (janitor)]

This bug was fixed in the package displaycal-py3 - 3.9.12-1ubuntu2

---------------
displaycal-py3 (3.9.12-1ubuntu2) oracular; urgency=medium

  * Patch to remove Python version check (LP: #2061329)

 -- Erich Eickmeyer <email address hidden> Wed, 19 Jun 2024 09:40:04 -0700

[Erich Eickmeyer (eeickmeyer)]

> 1) Fix the development release to not be brittle in this way, so it won't unnecessarily break if Python is updated in a way that doesn't break its API in a way that matters to this package.

Done. It no longer tests for python versions at all, but still needs to grab the python version for internal functions.

> 2) Fix the SRU upload to also not be brittle in this way, so it won't unnecessarily break against a newer version of Python were we to add it to 24.04 during its lifetime, as we did for 22.04. For example, you could the patch you're adding so it works for all future Python versions, not just 3.12.

Done in the same fashion.

> 3) This also needs some further explanation, please. Is this related to "since it could crash upon closure", or something else? What's the status of this? Are you proposing this SRU on the basis that it's definitely better than the current state? All of this should be documented, please.

The "since it could crash upon closure" is related to WxPython and not related to this bug at all. As stated, this has not occurred in my testing of the actual binary in my PPA at https://launchpad.net/~eeickmeyer/+archive/ubuntu/displaycaltest.

> > > [e-mail](reason="Test segfaults with python 3.12 - further investigation
required.")

This is related to an internal test not during build, but during runtime upon launch. Upstream has not yet figured out why it segfaults at this point (upstream is now maintaining abandoned code and is not the original author), so they're working diligently on it. However, as stated in the description, I argue that having a functioning application is better than a flaky test that crashes the entire application. As far as I can see, the application functions normally without the test.

[Robie Basak (racb)]

Thank you for the update!

> ...but still needs to grab the python version for internal functions.

I guess this is:

> ++ elif sys.version_info[:2] == (3, 12):
> ++ from DisplayCAL.lib64.python312.RealDisplaySizeMM import *

I'm not happy about this. If the CPython API doesn't change, then the package should correctly be able to rebuild against a newer Python without needing any changes to the source tree. Every other Python extension package achieves this and I see no exceptional reason why this cannot be done here. But I looked at the details and it seems like it'd be too invasive to do in an SRU (setup.py seems unnecessarily complex and seems to have the effect of removing this functionality that we'd get for free from something simpler), so I'll accept as-is.

It'd be nice to file a bug upstream asking for this quality issue to be fixed, though, and or fixing it upstream and/or the development release.

> @@ -526,7 +532,7 @@ dispcalgui (1.0.7.6-1) unstable; urgency=low
> (Closes: #675680).
> * Don't package uneeded /etc/udev directory.
> * Of course, should depends on argyll.
> - *
> + *
>
> -- Christian Marillat <email address hidden> Sun, 24 Jun 2012 11:02:13 +0200

Random whitespace error? Seems not worth holding this SRU up further just for this though.

> However, as stated in the description, I argue that having a functioning application is better than a flaky test that crashes the entire application. As far as I can see, the application functions normally without the test.

OK, trade-off accepted. Thank you for documenting this reasoning!

> +-py_minversion = (3, 8)
> +-py_maxversion = (3, 11)

FWIW, I'd have made this patch more minimal and removed or disabled the final check only. That would make future conflicts less likely and easier to resolve. Maintainability for distro patches is quite different from what would be most maintainable upstream!

[Robie Basak (racb)]

Hello Andrea, or anyone else affected,

Accepted displaycal-py3 into noble-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/displaycal-py3/3.9.11-2ubuntu0.24.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-noble to verification-done-noble. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-noble. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Erich Eickmeyer (eeickmeyer)]

$ apt-cache policy displaycal
displaycal:
  Installed: 3.9.11-2ubuntu0.24.04.1
  Candidate: 3.9.11-2ubuntu0.24.04.1
  Version table:
 *** 3.9.11-2ubuntu0.24.04.1 100
        100 http://us.archive.ubuntu.com/ubuntu noble-proposed/universe amd64 Packages
        100 /var/lib/dpkg/status
     3.9.11-2 500
        500 http://us.archive.ubuntu.com/ubuntu noble/universe amd64 Packages

* Ran displaycal
* No issues, application ran normally and as expected. Didn't crash on close, either. :)

[Launchpad Janitor (janitor)]

This bug was fixed in the package displaycal-py3 - 3.9.11-2ubuntu0.24.04.1

---------------
displaycal-py3 (3.9.11-2ubuntu0.24.04.1) noble; urgency=medium

  * Patch drop Python runtime version check (LP: #2061329)

 -- Erich Eickmeyer <email address hidden> Thu, 30 May 2024 13:00:27 -0700

[Andreas Hasenack (ahasenack)]

The verification of the Stable Release Update for displaycal-py3 has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.