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

Make changes for Raspberry Pi OS Bookworm #2084

Merged
merged 15 commits into from
Nov 8, 2023

Conversation

s-martin
Copy link
Collaborator

@s-martin s-martin commented Oct 22, 2023

Make changes so Phoniebox 2.x runs on Bookworm (this PR replaces #2080)

Changes:

  • Use bookworm in Github Actions
  • Use "--break-system-packages" as Bookworm implemented PEP668, but 2.x is legacy anyway
  • Explicitly install netcat-traditional package

TODOs:

@s-martin
Copy link
Collaborator Author

@AlvinSchiller AlvinSchiller mentioned this pull request Oct 24, 2023
4 tasks
@s-martin
Copy link
Collaborator Author

s-martin commented Nov 3, 2023

Some more opinions on PEP 668: raspberrypi/bookworm-feedback#100

@pabera
Copy link
Collaborator

pabera commented Nov 3, 2023

Some more opinions on PEP 668: raspberrypi/bookworm-feedback#100

Do you have trouble using Python in venv?

@s-martin
Copy link
Collaborator Author

s-martin commented Nov 3, 2023

It's more my lack of knowledge of venvs, so far I avoided them pretty successful ;)

@pabera, maybe you can have a look at the changes I tried in this PR - I'm pretty sure that it's solvable, but I don't have enough experience with venvs to choose the best solution for us.

@AlvinSchiller
Copy link
Collaborator

I would try a bit too if i have time, but i have also not much experience with python and venvs.
But from the links you provided, i think we have two options:

  • use break-system-packages and leave everything as is (more of a legacy solution, but maybe the easiest for 2.x)
  • correctly adapt to "PEP 668" and create and use a venv in the RPi-Jukebox-RFID root folder. Then every python, pip or shebang path has to point to this venv folder (could be troublesome as this should be relativ path? ).

@pabera
Copy link
Collaborator

pabera commented Nov 3, 2023

It would be a very idealistic approach to solve this for V2, I agree with @AlvinSchiller. But because we rely on Python a lot more in V3, this could be a heavy lift.

@s-martin
Copy link
Collaborator Author

s-martin commented Nov 3, 2023

I also think for legacy 2.x break system packages should be fine.

@coveralls
Copy link

coveralls commented Nov 4, 2023

Pull Request Test Coverage Report for Build 6804281312

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 76.82%

Totals Coverage Status
Change from base Build 6755756729: 0.0%
Covered Lines: 633
Relevant Lines: 824

💛 - Coveralls

@AlvinSchiller
Copy link
Collaborator

AlvinSchiller commented Nov 6, 2023

Hey @s-martin,
instead of changing every python call, it is possible to set an global config entry, so theres no need to remember adding it.
Plus its backwards compatible:
sudo python3 -m pip config set global.break-system-packages true

see: https://github.com/AlvinSchiller/RPi-Jukebox-RFID/tree/test/bookworm

@AlvinSchiller
Copy link
Collaborator

rfidreader * Line 1914; there is also a check missing for test 2

You probably mean the check for package 532 and 522? I fixed them in #2092

@s-martin
Copy link
Collaborator Author

s-martin commented Nov 6, 2023

Spotify packages seem not to be available for Bookworm:

pi@phoniebox:/etc$ sudo apt install libspotify-dev
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
E: Unable to locate package libspotify-dev

pi@phoniebox:/etc$ apt search mopidy
Sorting... Done
Full Text Search... Done
gir1.2-playerctl-2.0/stable 2.4.1-2 amd64
  utility to control media players via MPRIS (gir bindings)

libplayerctl-dev/stable 2.4.1-2 amd64
  utility to control media players via MPRIS (development files)

libplayerctl-doc/stable 2.4.1-2 all
  utility to control media players via MPRIS (documentation)

libplayerctl2/stable 2.4.1-2 amd64
  utility to control media players via MPRIS (library)

mopidy/stable 3.4.2-1 all
  extensible music server

mopidy-alsamixer/stable 2.0.1-1 all
  Mopidy extension for volume control via ALSA

mopidy-beets/stable 4.0.1-1 all
  Mopidy extension for playing music from Beets' web plugin

mopidy-dleyna/stable 2.0.2-1 all
  Mopidy extension for playing music from Digital Media Servers

mopidy-doc/stable 3.4.2-1 all
  extensible music server - documentation

mopidy-internetarchive/stable 3.0.1-1 all
  Mopidy extension for playing music from the Internet Archive

mopidy-local/stable 3.2.1-1 all
  Mopidy extension for playing music from your local music archive

mopidy-mpd/stable 3.3.0-1 all
  Mopidy extension for controlling Mopidy from MPD clients

mopidy-mpris/stable 3.0.3-1 all
  Mopidy extension for controlling playback through MPRIS

mopidy-podcast/stable 3.0.1-1 all
  Mopidy extension for searching and browsing podcasts

mopidy-podcast-itunes/stable 3.0.1-1 all
  Mopidy extension for searching and browsing iTunes podcasts

mopidy-scrobbler/stable,stable 2.0.1-1 all
  Mopidy extension for scrobbling music to Last.fm

mopidy-somafm/stable 2.0.2-1 all
  Mopidy extension for playing music from SomaFM

mopidy-soundcloud/stable 3.0.2-1 all
  Mopidy extension for playing music from SoundCloud

mopidy-tunein/stable 1.1.0-1 all
  Mopidy extension for playing music from TuneIn

playerctl/stable 2.4.1-2 amd64
  utility to control media players via MPRIS

snapserver/stable 0.26.0+dfsg1-1+b2 amd64
  Snapcast server

@AlvinSchiller
Copy link
Collaborator

The "MPDConfig" block need to be moved ahead of "SPOTInstall" so the mpd service enablement is correct in the end.

@s-martin
Copy link
Collaborator Author

s-martin commented Nov 7, 2023

I fixed it by explicitly stopping and disabling mpd, when Spotify is used.

@AlvinSchiller
Copy link
Collaborator

I fixed it by explicitly stopping and disabling mpd, when Spotify is used.

Yep but Spotifyinstall and mpdconfig are not mutually exclusive. So the Spotify block will disable mpd service and the mpd block after will enable it again, which is not a correct state.

@DivineDominion
Copy link

Sorry if I'm bringing this up here: it sounds that (usage of) libspotify is dead anyway at the moment. So maybe TODO item number 3 of @s-martin's original PR comment should not be a blocker if the rest of the PR is going well?

@s-martin s-martin added this to the 2.5 milestone Nov 8, 2023
@s-martin s-martin merged commit 093cc61 into MiczFlor:develop Nov 8, 2023
28 of 31 checks passed
@s-martin s-martin mentioned this pull request Nov 8, 2023
7 tasks
AlvinSchiller pushed a commit to AlvinSchiller/RPi-Jukebox-RFID that referenced this pull request Dec 21, 2023
* fixed markdown warnings

* fix flake warnings

* * use bookworm
* use venv as Bookworm implemented PEP668
* explicitly install netcat-traditional package

* dont sudo activate

* allow breaking system packages

* fix break-system-packages, as this switch doesnt exist pre bookworm

* improve command for break system packages

* set break system packages globally

* fix permissions for mpd.conf

* revert last commit

* explictily start mpd

* explicitly stop mod when using Spotify

* use parallel for coveralls
@s-martin s-martin deleted the new_bookworm_adaptation branch January 4, 2024 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants