Skip to content

Conversation

mreid-tt
Copy link
Contributor

@mreid-tt mreid-tt commented Feb 10, 2025

Description

This PR includes the following:

  1. Update to beets 2.3.1
  2. Migrate to Python 3.12

Fixes #

Checklist

  • Build rule all-supported completed successfully
  • New installation of package completed successfully
  • Package upgrade completed successfully (Manually install the package again)
  • Package functionality was tested
  • Any needed documentation is updated/created

Type of change

  • Package update

@mreid-tt mreid-tt self-assigned this Feb 10, 2025
@mreid-tt

This comment was marked as outdated.

@mreid-tt mreid-tt changed the title Beets: Update to v2.2.0 and Python 3.12 Beets: Migrate to Python 3.12 Mar 23, 2025
@mreid-tt mreid-tt force-pushed the beets-update branch 3 times, most recently from c01a32b to 5200a17 Compare March 24, 2025 14:22
@mreid-tt mreid-tt changed the title Beets: Migrate to Python 3.12 Beets: Update to v2.3.1 and migrate to Python 3.12 May 20, 2025
@th0ma7
Copy link
Contributor

th0ma7 commented Jun 6, 2025

I believe issue might be solved 🚀
I also encountered an issue with numpy that would fail due to using WHEEL_NAME in its Makefile. This 63f080a should be pushed outside from this PR as it fixes an unrelated issue and will trigger other unneeded packages to build.

@th0ma7
Copy link
Contributor

th0ma7 commented Jun 6, 2025

Looking at the code, you should really not enable llvmlite as this is probably for lower-level c++ bindings and will invoke a full build of LLVM which takes hours and is too heavy in disk space. It's also probably not that much interest unless being a beets developer (I'm presuming here).

@mreid-tt
Copy link
Contributor Author

mreid-tt commented Jun 6, 2025

Looking at the code, you should really not enable llvmlite as this is probably for lower-level c++ bindings and will invoke a full build of LLVM which takes hours and is too heavy in disk space. It's also probably not that much interest unless being a beets developer (I'm presuming here).

I’m open to any workable approach for bundling llvmlite—it’s needed by numba, which in turn is required by librosa and resampy. These wheels are essential for the Autobpm plugin.

If integrating LLVM proves impossible, we’ll limit the update to the other dependencies noted in #6421 (comment) and flag the Autobpm plugin as unsupported for this release.

@th0ma7
Copy link
Contributor

th0ma7 commented Jun 6, 2025

If integrating LLVM proves impossible, we’ll limit the update to the other dependencies noted in #6421 (comment) and flag the Autobpm plugin as unsupported for this release.

It's not impossible... it's just something that will take a hell a lot of time to figure out. It requires the exact subset of LLVM which is a massive beast to play with. Last time I played with that for OpenCL acceleration it took me months to figure that out.

I suggest considering investigating that into an independent PR later-on and indeed flag Autobpm plugin as unsupported for this release.

@mreid-tt
Copy link
Contributor Author

mreid-tt commented Jun 6, 2025

I suggest considering investigating that into an independent PR later-on and indeed flag Autobpm plugin as unsupported for this release.

Agreed, that would shrink our crossenv requirements for the remaining plugins to:

charset_normalizer==3.4.2
dbus_python==1.4.0
jellyfish==1.2.0
lap==0.5.12
lxml==5.4.0
MarkupSafe==3.0.2
numpy==2.2.6
pillow==11.2.1
pycairo==1.28.0
pygobject==3.52.3
PyYAML==6.0.2

As you will note above, wheels like scipy and scikit_learn are no longer required.

EDIT: I'll push a commit to update the requirements files shortly so we are working with the same objective.

- install cross wheels as prebuilt whl files (to install pygobject without pycairo dependency)
- install pure python wheels with requirements-pure.txt
@hgy59
Copy link
Contributor

hgy59 commented Jun 15, 2025

@mreid-tt latest version installs without pycairo.

beet --version --plugins=aura,beatport,bpd,chroma,embedart,embyupdate,fetchart,kodiupdate,lastgenre,lastimport,lyrics,metasync,mpdstats,plexupdate,scrub,sonosupdate,thumbnails,web

beets version 2.3.1
Python version 3.12.10
plugins: aura, beatport, bpd, chroma, embedart, emby, fetchart, kodi, lastgenre, lastimport, lyrics, metasync, mpdstats, plexupdate, scrub, sonosupdate, thumbnails, web

you can not add discogs plugin in the command above because it requires authentication:

discogs: 
To authenticate with Discogs, visit:
https://www.discogs.com/oauth/authorize?oauth_token=SQevFZtkezFTrpPCXRyPIrUsIIsyesUiPmOCAYLx
Enter the code:

hgy59
hgy59 previously requested changes Jun 15, 2025
Copy link
Contributor

@hgy59 hgy59 left a comment

Choose a reason for hiding this comment

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

We should remove support for the following plugins

and their associated dependencies and requirements:

This requires clear documentation of the requirements and dependencies for each plugin supported by the beets package.

We provide beets as a CLI tool without a service.
Therefore, all plugins that provide a service should be removed:

  • aura (server implementation of the AURA specification using the Flask framework)
  • bpd (runs as a daemon and implements the MPD protocol)
  • web (an experimental web-based GUI for beets)

We could create separate packages for each of these plugins, but aura and web are still experimental.
Integration with DSM (port, firewall rules, service users) requires separate server packages.

Other plugins whose support should be removed:

  • mpdstats
    This plugin must run continuously (like a service) to collect statistics.
  • thumbnails
    This plugin creates thumbnails for Linux desktop systems (it works on freedesktop.org-compatible file managers like Nautilus or Thunar), so it's not useful for DSM.

Validation

We need to verify that the chroma plugin works with the installed chromaprint package.
This was requested at #2927.

Update Documentation

Update https://github.com/SynoCommunity/spksrc/wiki/FAQ-beets and document the supported (working) plugins.

@mreid-tt
Copy link
Contributor Author

pycairo is not in the requierments file, but while processing pygobjects in requirements file, pip still finds the pycairo dependency.

Should we consider re-enabling the Cairo dependencies and corresponding tests in the PyGObject build? That way, if the resulting wheel is reused by other packages in the future, it will be fully featured rather than the minimal version we currently produce (which lacks the “Cairo integration” layer).

you can not add discogs plugin in the command above because it requires authentication

While the plugin requires authentication, I’ve already tested it successfully. Registering for a free Discogs account is trivial, so it doesn’t present a meaningful barrier to verification.

We provide beets as a CLI tool without a service. Therefore, all plugins that provide a service should be removed:

  • aura (server implementation of the AURA specification using the Flask framework)
  • bpd (runs as a daemon and implements the MPD protocol)
  • web (an experimental web-based GUI for beets)
  • mpdstats
    This plugin must run continuously (like a service) to collect statistics.
  • thumbnails
    This plugin creates thumbnails for Linux desktop systems (it works on freedesktop.org-compatible file managers like Nautilus or Thunar), so it's not useful for DSM.

I’ll prepare an updated requirements.txt file that excludes these, as they don’t align with our usage context.

We need to verify that the chroma plugin works with the installed chromaprint package.

Is there a specific test case you'd like to run for the chroma plugin? I’ve only verified that it loads successfully.

@mreid-tt
Copy link
Contributor Author

@hgy59, I disagree with your approach to exclude python/pycairo dependency (again). This adds custom code to the service-setup.sh taking us away from the standard deployment for our Python-based packages. Additionally we are limiting the functionality of the PyGObject. I am going to revert this change and build the full wheel.

@hgy59
Copy link
Contributor

hgy59 commented Jun 15, 2025

@hgy59, I disagree with your approach to exclude python/pycairo dependency (again). This adds custom code to the service-setup.sh taking us away from the standard deployment for our Python-based packages. Additionally we are limiting the functionality of the PyGObject. I am going to revert this change and build the full wheel.

I agree with pycairo for pygobject (even it enlarges the package needlessly).

BUT I don't agree with avoiding "custom code" in service setup just for the sake of a "standard".
Python packages maintained by me (homeassistant, octoprint) use this approach, because I do not want to download wheels from index that are already available in the wheelhouse of the package (and it speeds up installation, especially on slow networks).
And I am working on matter-server in #6481 that needs a patched version of a wheel and this works only with this approach.

@hgy59
Copy link
Contributor

hgy59 commented Jun 15, 2025

While the plugin requires authentication, I’ve already tested it successfully. Registering for a free Discogs account is trivial, so it doesn’t present a meaningful barrier to verification.

I have no problem with the discog plugin. When it asks for the authentication code the plugin obviously works.
It is just not possible to run "beet version" with all plugins without configured discog plugin.

@hgy59
Copy link
Contributor

hgy59 commented Jun 15, 2025

@mreid-tt I guess you will add "autobpm" to unsupported plugins - or do you still plan to add wheels for numba, scikit_learn, scipy, llvmlite, ...?

@mreid-tt
Copy link
Contributor Author

BUT I don't agree with avoiding "custom code" in service setup just for the sake of a "standard".
Python packages maintained by me (homeassistant, octoprint) use this approach, because I do not want to download wheels from index that are already available in the wheelhouse of the package (and it speeds up installation, especially on slow networks).

I think we may have different perspectives on this, which is understandable. For my part, I advocate for maintaining standardized code because it helps keep our development process as lightweight and accessible as possible. It reduces the length and complexity of our wiki pages on these topics and makes it significantly easier for new contributors to get up to speed.

This was the core motivation behind standardizing our service-setup.sh scripts across all the *arr-type .NET apps, as well as many of the PHP-based apps I modernized. Your requirements-gathering script, in particular, was a fantastic piece of work—it allowed me to engage meaningfully with the Python-based apps and is the main reason I now try to build them all using a consistent approach.

That said, I do agree with your point regarding how our installer currently defaults to retrieving wheels from the index rather than using what’s already included in the package. That behavior does feel a bit outdated, and I think it’s worth re-evaluating. Perhaps we should reverse the logic: rather than requiring a Makefile flag to include the bundled wheels, we make that the default and use a flag to fall back to the index when necessary.

I know the original rationale was to reduce server storage, but I agree that in today’s context, installation speed is more valuable. With that in mind, my preference is to stick with a standardized approach and adjust our defaults so the change benefits all packages going forward.

@mreid-tt
Copy link
Contributor Author

mreid-tt commented Jun 15, 2025

@mreid-tt I guess you will add "autobpm" to unsupported plugins - or do you still plan to add wheels for numba, scikit_learn, scipy, llvmlite, ...?

Yes, as agreed with @th0ma7, autobpm is a significant piece of work and the one plugin we’ve mutually agreed to exclude from this release.

@hgy59
Copy link
Contributor

hgy59 commented Jun 15, 2025

@mreid-tt I already evaluated plugin related requirements (it does not mention the autobpm plugin yet).
requirements-crossenv.txt
requirements-pure.txt

@hgy59
Copy link
Contributor

hgy59 commented Jun 15, 2025

Is there a specific test case you'd like to run for the chroma plugin? I’ve only verified that it loads successfully.

In the linked request, there was an issue with fpcalc tool of chromaprint package.
On my testsystem I don't have chromaprint installed, and beet --version --plugins=chroma does not report an error. So yes, it needs further testing.
The chromaprint package has SPK_COMMANDS = bin/fpcalc so the original issue seems fixed.

Since I don't know beets and chromaprint details, I can't provide a test case.
You will need a beet import with configured chroma plugin. Since the import plugin is not (yet) supported, I don't know, whether such a test is possible. You might try to reach the author of the original request, or @ymartin59 or the author of #4136.

@mreid-tt
Copy link
Contributor Author

@mreid-tt I already evaluated plugin related requirements (it does not mention the autobpm plugin yet). requirements-crossenv.txt requirements-pure.txt

Thanks for this analysis. Your comments have been integrated into the last commit.

@mreid-tt
Copy link
Contributor Author

Update https://github.com/SynoCommunity/spksrc/wiki/FAQ-beets and document the supported (working) plugins.

Done.

@mreid-tt
Copy link
Contributor Author

mreid-tt commented Jun 15, 2025

Since I don't know beets and chromaprint details, I can't provide a test case.
You will need a beet import with configured chroma plugin. Since the import plugin is not (yet) supported, I don't know, whether such a test is possible. You might try to reach the author of the original request, or @ymartin59 or the author of #4136.

I believe I was able to test this successfully with the following:

$ beet import /volume1/downloads/complete/Music

/volume1/downloads/complete/Music/Katy.Perry-1432-WEB-2024-MARR (15 items)

  Match (99.7%):
  Katy Perry - 1432
  ≠ tracks
  MusicBrainz, Digital Media, 2024, None, Capitol Records, None, explicit
  https://musicbrainz.org/release/4ee79f07-e72f-4514-bb01-d0654ea34e48
  * Artist: Katy Perry
  * Album: 1432
     ≠ (#1) Woman�s World (2:43) -> (#1) WOMAN'S WORLD (2:43)
     ≠ (#2) Gimme Gimme (Feat. 21 Savage) (2:57) -> (#2) GIMME GIMME (2:57)
     ≠ (#3) Gorgeous (Feat. Kim Petras) (3:17) -> (#3) GORGEOUS (3:17)
     ≠ (#4) I'm H is, H e' s M   (3:18) -> (#4) I'M H IS, H E' S M INE (3:18)
            ine (Feat. Doechii)                                              
     ≠ (#5) Crush (2:57) -> (#5) CRUSH (2:57)
     ≠ (#6) Lifetimes (3:12) -> (#6) LIFETIMES (3:12)
     ≠ (#7) All The Love (3:15) -> (#7) ALL THE LOVE (3:15)
     ≠ (#8) Nirvana (2:51) -> (#8) NIRVANA (2:51)
     ≠ (#9) Artificial (Feat. JID) (2:43) -> (#9) ARTIFICIAL (2:43)
     ≠ (#10) Truth (2:57) -> (#10) TRUTH (2:57)
     ≠ (#11) Wonder (3:24) -> (#11) WONDER (3:24)
     ≠ (#12) I Woke Up (2:28) -> (#12) I WOKE UP (2:28)
     ≠ (#13) Has A Heart (2:47) -> (#13) HAS A HEART (2:47)
     ≠ (#14) No T ears F or N ew (3:22) -> (#14) NO T EARS F OR N EW Y (3:22)
             Y ear's                             EAR’S                       
     ≠ (#15) Ok (2:38) -> (#15) OK (2:38)
lyrics: LyricsPlugin: Disabling Google source: no API key configured.
plexupdate: Update failed.
kodi: Kodi update failed: 404 Client Error: Not Found for url: http://localhost:8080/jsonrpc
emby: Provide at least Emby password or apikey.

There were expected failures due to non-configured plugins: lyrics, plexupdate, kodi, emby. There was one plugin that had timeouts contacting its server (Connection to oauth-api.beatport.com timed out): beatport.

However, for chroma, with Chromaprint installed we are able to see that songs are fingerprinted successfully:

$ beet fingerprint
chroma: /volume1/music/Katy Perry/1432/01 WOMAN'S WORLD.mp3: fingerprint exists, skipping
chroma: /volume1/music/Katy Perry/1432/05 CRUSH.mp3: fingerprint exists, skipping
chroma: /volume1/music/Katy Perry/1432/06 LIFETIMES.mp3: fingerprint exists, skipping
chroma: /volume1/music/Katy Perry/1432/07 ALL THE LOVE.mp3: fingerprint exists, skipping
chroma: /volume1/music/Katy Perry/1432/08 NIRVANA.mp3: fingerprint exists, skipping
chroma: /volume1/music/Katy Perry/1432/10 TRUTH.mp3: fingerprint exists, skipping
chroma: /volume1/music/Katy Perry/1432/11 WONDER.mp3: fingerprint exists, skipping
chroma: /volume1/music/Katy Perry/1432/12 I WOKE UP.mp3: fingerprint exists, skipping
chroma: /volume1/music/Katy Perry/1432/13 HAS A HEART.mp3: fingerprint exists, skipping
chroma: /volume1/music/Katy Perry/1432/14 NO TEARS FOR NEW YEAR’S.mp3: fingerprint exists, skipping
chroma: /volume1/music/Katy Perry/1432/15 OK.mp3: fingerprint exists, skipping
chroma: /volume1/music/Katy Perry/1432/02 GIMME GIMME.mp3: fingerprint exists, skipping
chroma: /volume1/music/Katy Perry/1432/09 ARTIFICIAL.mp3: fingerprint exists, skipping
chroma: /volume1/music/Katy Perry/1432/03 GORGEOUS.mp3: fingerprint exists, skipping
chroma: /volume1/music/Katy Perry/1432/04 I'M HIS, HE'S MINE.mp3: fingerprint exists, skipping

@mreid-tt
Copy link
Contributor Author

@hgy59, I believe the package is ready to go::

$ beet --version
beets version 2.3.1
Python version 3.12.10
plugins: beatport, chroma, discogs, embedart, emby, fetchart, kodi, lastgenre, lastimport, lyrics, metasync, plexupdate, replaygain, scrub, sonosupdate

@mreid-tt mreid-tt requested a review from hgy59 June 15, 2025 19:58
@hgy59 hgy59 dismissed their stale review June 15, 2025 20:35

Requested changes are done.
Additional verificatation for obsolete requirements pending.

@mreid-tt
Copy link
Contributor Author

Additional verificatation for obsolete requirements pending.

@hgy59, I'm not sure what else is to be done here.

Copy link
Contributor

@hgy59 hgy59 left a comment

Choose a reason for hiding this comment

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

LGTM

validated on DS115j (armv7-7.1)

@hgy59
Copy link
Contributor

hgy59 commented Jun 15, 2025

@hgy59, I'm not sure what else is to be done here.

just validated that no obsolete requirement exists (good work!).

@mreid-tt mreid-tt merged commit 04788b4 into SynoCommunity:master Jun 15, 2025
15 checks passed
@mreid-tt mreid-tt deleted the beets-update branch June 15, 2025 21:29
@th0ma7
Copy link
Contributor

th0ma7 commented Jun 15, 2025

Wow you've been busy over the weekend, good job!

@mreid-tt mreid-tt added status/published Published and activated (may take up to 48h until visible in DSM package manager) and removed status/ready-to-merge labels Jun 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/published Published and activated (may take up to 48h until visible in DSM package manager)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants