bug#63205: quodlibet fails to build after python 3.10 update

bug#63205: quodlibet fails to build after python 3.10 update

[Zack Weinberg]

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

After the core-updates merge (specifically after the bump to python 3.10, I would guess) quodlibet's testsuite has one failing test and therefore the package fails to build:

=================================== FAILURES ===================================
_______________________ TCoverManager.test_invalid_glob ________________________
tests/test_util_cover.py:111: in test_invalid_glob
    os.path.abspath(self._find_cover(self.song).name), f)
E   AttributeError: 'NoneType' object has no attribute 'name'
=========================== short test summary info ============================
FAILED tests/test_util_cover.py::TCoverManager::test_invalid_glob - Attribute...
===== 1 failed, 3257 passed, 53 skipped, 139 warnings in 90.85s (0:01:30) ======

Full build log is attached.

$ guix describe
Generation 19	May 01 2023 08:59:02	(current)
  guix 0c6a7e1
    repository URL: https://git.savannah.gnu.org/git/guix.git
    branch: master
    commit: 0c6a7e1045cbd06336ef3699734a803a2e4038e5

[-- Attachment #2: gj1mb1y5bchcq9pm4rsh7x8pyq0fs3-quodlibet-4.5.0.drv.gz --]
[-- Type: application/gzip, Size: 34515 bytes --]

bug#63205: quodlibet fails to build after python 3.10 update

[Alice BRENON]

[-- Attachment #1: Type: text/plain, Size: 539 bytes --]

This problem seems to have been reported and fixed upstream:

https://github.com/quodlibet/quodlibet/commit/5f55431a28509fd4f4f7b40dc246f3d34fa8549e

Since no newer release has been out, I guess we can backport the commit itself.
I've tried with the attached patch. The operon tests still fail due to some
weird utime magic, a problem also known and discussed at:

https://github.com/quodlibet/quodlibet/pull/4053/commits/06a32b319f065550efe0d2a9ff10ca6bdc32b893

So I guess we could backport that as well. Does anyone have a better fix ?

[-- Attachment #2: 0001-gnu-quodlibet-Fix-glob-failure-in-quodlibet-tests.patch --]
[-- Type: text/x-patch, Size: 6086 bytes --]

From 7e1eb6cd1c15923c7eb64b06afac9f83f6b6cfb3 Mon Sep 17 00:00:00 2001
Message-Id: <7e1eb6cd1c15923c7eb64b06afac9f83f6b6cfb3.1683293409.git.alice.brenon@ens-lyon.fr>
From: Alice BRENON <alice.brenon@ens-lyon.fr>
Date: Fri, 5 May 2023 15:26:37 +0200
Subject: [PATCH] gnu: quodlibet: Fix glob failure in quodlibet tests.

Reported in #63205.

* gnu/packages/music.scm (quodlibet): Backport patch 5f55431 from
upstream and reactivate passing tests.
---
 gnu/packages/music.scm                        |  6 +-
 .../patches/quodlibet-fix-invalid-glob.patch  | 89 +++++++++++++++++++
 2 files changed, 92 insertions(+), 3 deletions(-)
 create mode 100644 gnu/packages/patches/quodlibet-fix-invalid-glob.patch

diff --git a/gnu/packages/music.scm b/gnu/packages/music.scm
index 446580dc52..3a005b2848 100644
--- a/gnu/packages/music.scm
+++ b/gnu/packages/music.scm
@@ -7082,6 +7082,8 @@ (define-public quodlibet
              (url "https://github.com/quodlibet/quodlibet")
              (commit (string-append "release-" version))))
        (file-name (git-file-name name version))
+       (patches (search-patches
+                        "quodlibet-fix-invalid-glob.patch"))
        (sha256
         (base32 "1i5k93k3bfp7hpcwkbr865mbj9jam3jv2a5k1bazcyp4f5vdrb0v"))))
     (build-system python-build-system)
@@ -7105,9 +7107,7 @@ (define-public quodlibet
                           "--ignore=tests/test_browsers_iradio.py"
                           ;; broken upstream
                           "--disable-warnings"
-                          "--ignore=tests/quality"
-                          ;; missing legacy icons in adwaita-icon-theme
-                          "--ignore=tests/plugin/test_trayicon.py")
+                          "--ignore=tests/quality/test_flake8.py")
                   (format #t "test suite not run~%"))))
           (add-after 'install 'glib-or-gtk-wrap ; ensure icons loaded
             (assoc-ref glib-or-gtk:%standard-phases 'glib-or-gtk-wrap))
diff --git a/gnu/packages/patches/quodlibet-fix-invalid-glob.patch b/gnu/packages/patches/quodlibet-fix-invalid-glob.patch
new file mode 100644
index 0000000000..95f95d8aab
--- /dev/null
+++ b/gnu/packages/patches/quodlibet-fix-invalid-glob.patch
@@ -0,0 +1,89 @@
+From 5f55431a28509fd4f4f7b40dc246f3d34fa8549e Mon Sep 17 00:00:00 2001
+From: Christoph Reiter <reiter.christoph@gmail.com>
+Date: Sun, 26 Jun 2022 23:14:28 +0200
+Subject: [PATCH] builtin cover: fix handling of invalid glob ranges with
+ Python 3.10.5+ (#4027)
+
+Previously Python would raise if an invalid range was given
+to glob, but with 3.10.5 they fixed it to not match anything.
+https://github.com/python/cpython/issues/89973
+
+Our tests depended on the previous logic and treating the glob pattern
+as a literal file name in that case.
+
+One could argue that this is wrong since a range that doesn't contain anything
+should also not match anything, so wrap glob() to make it not match for all
+Python versions in that case and adjust the tests accordingly.
+
+This should fix the Windows CI, which is currently the only job using 3.10.5
+---
+ quodlibet/util/cover/built_in.py | 22 +++++++++++-----------
+ tests/test_util_cover.py         | 12 +++---------
+ 2 files changed, 14 insertions(+), 20 deletions(-)
+
+diff --git a/quodlibet/util/cover/built_in.py b/quodlibet/util/cover/built_in.py
+index f2a8791a2..01474c9b6 100644
+--- a/quodlibet/util/cover/built_in.py
++++ b/quodlibet/util/cover/built_in.py
+@@ -100,6 +100,15 @@ class FilesystemCover(CoverSourcePlugin):
+         base = self.song('~dirname')
+         images = []
+ 
++        def safe_glob(*args, **kwargs):
++            try:
++                return glob.glob(*args, **kwargs)
++            except sre_constants.error:
++                # https://github.com/python/cpython/issues/89973
++                # old glob would fail with invalid ranges, newer one
++                # handles it correctly.
++                return []
++
+         if config.getboolean("albumart", "force_filename"):
+             score = 100
+             for filename in config.get("albumart", "filename").split(","):
+@@ -107,17 +116,8 @@ class FilesystemCover(CoverSourcePlugin):
+                 filename = filename.strip()
+ 
+                 escaped_path = os.path.join(glob.escape(base), filename)
+-                try:
+-                    for path in glob.glob(escaped_path):
+-                        images.append((score, path))
+-                except sre_constants.error:
+-                    # Use literal filename if globbing causes errors
+-                    path = os.path.join(base, filename)
+-
+-                    # We check this here, so we can search for alternative
+-                    # files in case no preferred file was found.
+-                    if os.path.isfile(path):
+-                        images.append((score, path))
++                for path in safe_glob(escaped_path):
++                    images.append((score, path))
+ 
+                 # So names and patterns at the start are preferred
+                 score -= 1
+diff --git a/tests/test_util_cover.py b/tests/test_util_cover.py
+index db81e4d1f..71a48ad9a 100644
+--- a/tests/test_util_cover.py
++++ b/tests/test_util_cover.py
+@@ -105,15 +105,9 @@ class TCoverManager(TestCase):
+         config.set("albumart", "force_filename", str(True))
+         config.set("albumart", "filename", "[a-2].jpg")
+ 
+-        # Should match
+-        f = self.add_file("[a-2].jpg")
+-        assert path_equal(
+-            os.path.abspath(self._find_cover(self.song).name), f)
+-
+-        # Should not crash
+-        f = self.add_file("test.jpg")
+-        assert not path_equal(
+-            os.path.abspath(self._find_cover(self.song).name), f)
++        # Invalid glob range, should not match anything
++        self.add_file("a.jpg")
++        assert self._find_cover(self.song) is None
+ 
+     def test_invalid_glob_path(self):
+         config.set("albumart", "force_filename", str(True))
+-- 
+2.39.2
+

base-commit: 3efdc84bbd1819aeef6e2290aaf54bc88ba72dda
-- 
2.39.2

bug#63205: quodlibet fails to build after python 3.10 update

[Alice BRENON]

[-- Attachment #1: Type: text/plain, Size: 911 bytes --]

The second patch needs an edit because part of it doesn't apply to the
actual source (4.5.0 doesn't contain 427793aa88fb57 which applied the
@flaky markers to the test). But quodlibet builds again on my machine
with this second patch.

Le Fri, 5 May 2023 15:31:42 +0200,
Alice BRENON <alice.brenon@ens-lyon.fr> a écrit :

> This problem seems to have been reported and fixed upstream:
> 
> https://github.com/quodlibet/quodlibet/commit/5f55431a28509fd4f4f7b40dc246f3d34fa8549e
> 
> Since no newer release has been out, I guess we can backport the
> commit itself. I've tried with the attached patch. The operon tests
> still fail due to some weird utime magic, a problem also known and
> discussed at:
> 
> https://github.com/quodlibet/quodlibet/pull/4053/commits/06a32b319f065550efe0d2a9ff10ca6bdc32b893
> 
> So I guess we could backport that as well. Does anyone have a better
> fix ?


[-- Attachment #2: 0001-gnu-quodlibet-Fix-failures-in-tests.patch --]
[-- Type: text/x-patch, Size: 7970 bytes --]

From 184c009037ac04ed1c8b2892a4e111fe8b5b9540 Mon Sep 17 00:00:00 2001
Message-Id: <184c009037ac04ed1c8b2892a4e111fe8b5b9540.1683294873.git.alice.brenon@ens-lyon.fr>
From: Alice BRENON <alice.brenon@ens-lyon.fr>
Date: Fri, 5 May 2023 15:26:37 +0200
Subject: [PATCH] gnu: quodlibet: Fix failures in tests.

Reported in #63205.

* gnu/packages/music.scm (quodlibet): Backport patches 5f55431 and a
  part of 06a32b3 from upstream and reactivate passing tests.
---
 gnu/packages/music.scm                        |  7 +-
 .../patches/quodlibet-fix-invalid-glob.patch  | 89 +++++++++++++++++++
 .../patches/quodlibet-fix-mtime-tests.patch   | 39 ++++++++
 3 files changed, 132 insertions(+), 3 deletions(-)
 create mode 100644 gnu/packages/patches/quodlibet-fix-invalid-glob.patch
 create mode 100644 gnu/packages/patches/quodlibet-fix-mtime-tests.patch

diff --git a/gnu/packages/music.scm b/gnu/packages/music.scm
index 446580dc52..05fb25f1b1 100644
--- a/gnu/packages/music.scm
+++ b/gnu/packages/music.scm
@@ -7082,6 +7082,9 @@ (define-public quodlibet
              (url "https://github.com/quodlibet/quodlibet")
              (commit (string-append "release-" version))))
        (file-name (git-file-name name version))
+       (patches (search-patches
+                        "quodlibet-fix-invalid-glob.patch"
+                        "quodlibet-fix-mtime-tests.patch"))
        (sha256
         (base32 "1i5k93k3bfp7hpcwkbr865mbj9jam3jv2a5k1bazcyp4f5vdrb0v"))))
     (build-system python-build-system)
@@ -7105,9 +7108,7 @@ (define-public quodlibet
                           "--ignore=tests/test_browsers_iradio.py"
                           ;; broken upstream
                           "--disable-warnings"
-                          "--ignore=tests/quality"
-                          ;; missing legacy icons in adwaita-icon-theme
-                          "--ignore=tests/plugin/test_trayicon.py")
+                          "--ignore=tests/quality/test_flake8.py")
                   (format #t "test suite not run~%"))))
           (add-after 'install 'glib-or-gtk-wrap ; ensure icons loaded
             (assoc-ref glib-or-gtk:%standard-phases 'glib-or-gtk-wrap))
diff --git a/gnu/packages/patches/quodlibet-fix-invalid-glob.patch b/gnu/packages/patches/quodlibet-fix-invalid-glob.patch
new file mode 100644
index 0000000000..95f95d8aab
--- /dev/null
+++ b/gnu/packages/patches/quodlibet-fix-invalid-glob.patch
@@ -0,0 +1,89 @@
+From 5f55431a28509fd4f4f7b40dc246f3d34fa8549e Mon Sep 17 00:00:00 2001
+From: Christoph Reiter <reiter.christoph@gmail.com>
+Date: Sun, 26 Jun 2022 23:14:28 +0200
+Subject: [PATCH] builtin cover: fix handling of invalid glob ranges with
+ Python 3.10.5+ (#4027)
+
+Previously Python would raise if an invalid range was given
+to glob, but with 3.10.5 they fixed it to not match anything.
+https://github.com/python/cpython/issues/89973
+
+Our tests depended on the previous logic and treating the glob pattern
+as a literal file name in that case.
+
+One could argue that this is wrong since a range that doesn't contain anything
+should also not match anything, so wrap glob() to make it not match for all
+Python versions in that case and adjust the tests accordingly.
+
+This should fix the Windows CI, which is currently the only job using 3.10.5
+---
+ quodlibet/util/cover/built_in.py | 22 +++++++++++-----------
+ tests/test_util_cover.py         | 12 +++---------
+ 2 files changed, 14 insertions(+), 20 deletions(-)
+
+diff --git a/quodlibet/util/cover/built_in.py b/quodlibet/util/cover/built_in.py
+index f2a8791a2..01474c9b6 100644
+--- a/quodlibet/util/cover/built_in.py
++++ b/quodlibet/util/cover/built_in.py
+@@ -100,6 +100,15 @@ class FilesystemCover(CoverSourcePlugin):
+         base = self.song('~dirname')
+         images = []
+ 
++        def safe_glob(*args, **kwargs):
++            try:
++                return glob.glob(*args, **kwargs)
++            except sre_constants.error:
++                # https://github.com/python/cpython/issues/89973
++                # old glob would fail with invalid ranges, newer one
++                # handles it correctly.
++                return []
++
+         if config.getboolean("albumart", "force_filename"):
+             score = 100
+             for filename in config.get("albumart", "filename").split(","):
+@@ -107,17 +116,8 @@ class FilesystemCover(CoverSourcePlugin):
+                 filename = filename.strip()
+ 
+                 escaped_path = os.path.join(glob.escape(base), filename)
+-                try:
+-                    for path in glob.glob(escaped_path):
+-                        images.append((score, path))
+-                except sre_constants.error:
+-                    # Use literal filename if globbing causes errors
+-                    path = os.path.join(base, filename)
+-
+-                    # We check this here, so we can search for alternative
+-                    # files in case no preferred file was found.
+-                    if os.path.isfile(path):
+-                        images.append((score, path))
++                for path in safe_glob(escaped_path):
++                    images.append((score, path))
+ 
+                 # So names and patterns at the start are preferred
+                 score -= 1
+diff --git a/tests/test_util_cover.py b/tests/test_util_cover.py
+index db81e4d1f..71a48ad9a 100644
+--- a/tests/test_util_cover.py
++++ b/tests/test_util_cover.py
+@@ -105,15 +105,9 @@ class TCoverManager(TestCase):
+         config.set("albumart", "force_filename", str(True))
+         config.set("albumart", "filename", "[a-2].jpg")
+ 
+-        # Should match
+-        f = self.add_file("[a-2].jpg")
+-        assert path_equal(
+-            os.path.abspath(self._find_cover(self.song).name), f)
+-
+-        # Should not crash
+-        f = self.add_file("test.jpg")
+-        assert not path_equal(
+-            os.path.abspath(self._find_cover(self.song).name), f)
++        # Invalid glob range, should not match anything
++        self.add_file("a.jpg")
++        assert self._find_cover(self.song) is None
+ 
+     def test_invalid_glob_path(self):
+         config.set("albumart", "force_filename", str(True))
+-- 
+2.39.2
+
diff --git a/gnu/packages/patches/quodlibet-fix-mtime-tests.patch b/gnu/packages/patches/quodlibet-fix-mtime-tests.patch
new file mode 100644
index 0000000000..037f77d868
--- /dev/null
+++ b/gnu/packages/patches/quodlibet-fix-mtime-tests.patch
@@ -0,0 +1,39 @@
+From 06a32b319f065550efe0d2a9ff10ca6bdc32b893 Mon Sep 17 00:00:00 2001
+From: Christoph Reiter <reiter.christoph@gmail.com>
+Date: Sat, 23 Jul 2022 20:15:18 +0200
+Subject: [PATCH] operon: hopefully better fix for flaky mtime tests
+
+copy the mtime after we write everything, so there is no chance of
+it changing before we note the initial value.
+---
+ quodlibet/operon/commands.py | 8 ++++----
+ 1 files changed, 4 insertions(+), 4 deletions(-)
+
+diff --git a/quodlibet/operon/commands.py b/quodlibet/operon/commands.py
+index e0a5ef33a..af2dcfa8b 100644
+--- a/quodlibet/operon/commands.py
++++ b/quodlibet/operon/commands.py
+@@ -227,16 +227,16 @@ class EditCommand(Command):
+         # write to tmp file
+         fd, path = tempfile.mkstemp(suffix=".txt")
+ 
+-        # XXX: copy mtime here so we can test for changes in tests by
+-        # setting a out of date mtime on the source song file
+-        copy_mtime(args[0], path)
+-
+         try:
+             try:
+                 os.write(fd, dump)
+             finally:
+                 os.close(fd)
+ 
++            # XXX: copy mtime here so we can test for changes in tests by
++            # setting a out of date mtime on the source song file
++            copy_mtime(args[0], path)
++
+             # only parse the result if the editor returns 0 and the mtime has
+             # changed
+             old_mtime = mtime(path)
+-- 
+2.39.2
+

base-commit: 3efdc84bbd1819aeef6e2290aaf54bc88ba72dda
-- 
2.39.2

bug#63205: quodlibet fails to build after python 3.10 update

[Remco van 't Veer]

Hi,

Forgotten patch:

  https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63205#11

The supplied patch by Alice works and looks good to me.  Can somebody
please have a look and consider committing it?

Thanks!

Cheers,
Remco

bug#63205: quodlibet fails to build after python 3.10 update

[Efraim Flashner]

[-- Attachment #1: Type: text/plain, Size: 589 bytes --]

Looks good to me. Patch pushed!

On Mon, Jun 12, 2023 at 10:47:44AM +0200, Remco van 't Veer wrote:
> Hi,
> 
> Forgotten patch:
> 
>   https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63205#11
> 
> The supplied patch by Alice works and looks good to me.  Can somebody
> please have a look and consider committing it?
> 
> Thanks!
> 
> Cheers,
> Remco
> 
> 
> 

-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]