* bug#63205: quodlibet fails to build after python 3.10 update
@ 2023-05-01 13:48 Zack Weinberg
2023-05-05 13:31 ` Alice BRENON
2023-06-12 8:47 ` Remco van 't Veer
0 siblings, 2 replies; 5+ messages in thread
From: Zack Weinberg @ 2023-05-01 13:48 UTC (permalink / raw)
To: 63205
[-- 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 --]
^ permalink raw reply [flat|nested] 5+ messages in thread* bug#63205: quodlibet fails to build after python 3.10 update 2023-05-01 13:48 bug#63205: quodlibet fails to build after python 3.10 update Zack Weinberg @ 2023-05-05 13:31 ` Alice BRENON 2023-05-05 13:58 ` Alice BRENON 2023-06-12 8:47 ` Remco van 't Veer 1 sibling, 1 reply; 5+ messages in thread From: Alice BRENON @ 2023-05-05 13:31 UTC (permalink / raw) To: 63205 [-- 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* bug#63205: quodlibet fails to build after python 3.10 update 2023-05-05 13:31 ` Alice BRENON @ 2023-05-05 13:58 ` Alice BRENON 0 siblings, 0 replies; 5+ messages in thread From: Alice BRENON @ 2023-05-05 13:58 UTC (permalink / raw) To: 63205 [-- 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* bug#63205: quodlibet fails to build after python 3.10 update 2023-05-01 13:48 bug#63205: quodlibet fails to build after python 3.10 update Zack Weinberg 2023-05-05 13:31 ` Alice BRENON @ 2023-06-12 8:47 ` Remco van 't Veer 2023-06-13 6:48 ` Efraim Flashner 1 sibling, 1 reply; 5+ messages in thread From: Remco van 't Veer @ 2023-06-12 8:47 UTC (permalink / raw) To: guix-devel; +Cc: 63205, alice.brenon 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#63205: quodlibet fails to build after python 3.10 update 2023-06-12 8:47 ` Remco van 't Veer @ 2023-06-13 6:48 ` Efraim Flashner 0 siblings, 0 replies; 5+ messages in thread From: Efraim Flashner @ 2023-06-13 6:48 UTC (permalink / raw) To: Remco van 't Veer; +Cc: alice.brenon, 63205-done [-- 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 --] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-06-13 20:45 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-05-01 13:48 bug#63205: quodlibet fails to build after python 3.10 update Zack Weinberg 2023-05-05 13:31 ` Alice BRENON 2023-05-05 13:58 ` Alice BRENON 2023-06-12 8:47 ` Remco van 't Veer 2023-06-13 6:48 ` Efraim Flashner
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/guix.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).