future3 - allow custom username #2132

AlvinSchiller · 2023-12-02T20:39:53Z

Allow custom usernames others then 'pi'

Removed the hard references on user 'pi'.
Removed install check for username (Installation as different user (not user pi) #1785)
Changed CI test to run with user 'testuser:testusergroup' instead of 'pi:pi'.

The docker setup has still some hardcoded references to the pi home. I didnt changes that so far.

pabera

Thank you. Looks good so far. I had a few minor comments

pabera · 2023-12-04T06:30:48Z

CONTRIBUTING.md

@@ -63,7 +63,7 @@ For bug fixes and improvements just open an issue or PR as described below. If y
 * By default this will get you to the `future3/main` branch. You will move to the `future3/develop` branch, do this:

 ~~~bash
-cd /home/pi/RPi-Jukebox-RFID
+cd /home/$USER/RPi-Jukebox-RFID


Could we also use cd ~/RPi-Jukebox-RFID?
I wonder if this is sufficient. It seems we have avoided this notation in the past...

There are a few places in the docs and comments that state to not use '~' .
In fact i dont know the problems that led to this statements.

Here it should be not a problem, but dont know for the config files.

I remember in an old project we had the problem that ~ wouldn't resolve to the correct path under some circumstances, but unfortunately I don't remember the details anymore.

But if we tested it and it works I see no problem using it.

Maybe its a problem, if a service is run as root and not as user?
Do we still have those?

Not really, I'd say. We do everything within the user, even in Docker. Should be ok. But must be tested of course.

I think we should check this and if it makes definition easier, but in another PR.
What do you think?

pabera · 2023-12-04T06:38:00Z

documentation/developers/known-issues.md

@@ -8,6 +8,6 @@ browser for now.
 ## Configuration

 In `jukebox.yaml` (and all other config files): do not use relative paths with `~/some/dir`.
-Always use entire explicit path, e.g. `/home/pi/some/dir`.
+Always use relativ path from settingsfile '../../'


My comment here would be similar to the one above. There is a also a context flaw here:

In line 10, we say: "do not use relative paths"

In line 11, we say: "Always use relativ path from" -> there is also a typo ;-)

See above.

is says dont use relative paths with '~'. But it can be misleading. i rephrased it.

pabera · 2023-12-04T06:39:31Z

installation/install-jukebox.sh

-# currently the user 'pi' is mandatory
-# https://github.com/MiczFlor/RPi-Jukebox-RFID/issues/1785
 _check_user() {
-  if [ "${CURRENT_USER}" != "pi" ]; then
-    echo
-    echo "ERROR: User must be 'pi'!"
-    echo "       Other usernames are currently not supported."
-    echo "       Please check the wiki for further information"
-    exit 2
-  fi
-
-  if [ "${HOME_PATH}" != "/home/pi" ]; then
-    echo
-    echo "ERROR: HomeDir must be '/home/pi'!"
-    echo "       Other usernames are currently not supported."
-    echo "       Please check the wiki for further information"
-    exit 2
-  fi
-
  if [ ! -d "${HOME_PATH}" ]; then
    echo
    echo "ERROR: HomeDir ${HOME_PATH} does not exist."


Do we need this at all now? We can expect that there is a home directory available.

It should be present, but doesn't harm to make sure ^^

I think we should remove it. It's a default expectation that a home directory exists. You also don't check if a kernel exists :-) We can anticipate this now and we only had this because we wanted to avoid people using custom usernames. since this is now resolved, we should remove the check. In 1 year from now, if we only look at this code we don't know anymore why this was added

Less code is always better ;-)

removed.
The installation will already abort in the next step if the home_path is not present ^^

RPi-Jukebox-RFID/installation/install-jukebox.sh

Line 97 in 3a40e84

cd "${HOME_PATH}" || exit_on_error "ERROR: Changing to home dir failed."

pabera · 2023-12-04T06:40:44Z

resources/default-settings/jukebox.default.yaml

@@ -1,5 +1,5 @@
 # IMPORTANT:
-# Do not use paths with '~/some/dir' - always use '/home/pi/some/dir'
+# Do not use paths with '~/some/dir' - always use relativ path from settingsfile '../../'


Typo: "relative"

pabera · 2023-12-04T22:07:19Z

.github/workflows/test_docker_debian_codename_sub_v3.yml

-    TEST_USER_NAME: hans
-    TEST_USER_GROUP: wurst
+    TEST_USER_NAME: testuser
+    TEST_USER_GROUP: testusergroup


That's a sad change 😄

* removed hardcoded references to user "pi" * removed check userNotPi * change testuser in workflow and container * updated docs * updated comments and docs * updated comments and docs * remove extra home_path check * changes testuser name and group

AlvinSchiller added 3 commits December 2, 2023 21:27

removed hardcoded references to user "pi"

a60e657

removed check userNotPi

1dad4e5

change testuser in workflow and container

9ffbb25

AlvinSchiller requested a review from pabera December 2, 2023 20:40

s-martin added enhancement future3 Relates to future3 development labels Dec 2, 2023

updated docs

a77b326

AlvinSchiller marked this pull request as ready for review December 2, 2023 22:27

AlvinSchiller requested a review from s-martin December 2, 2023 22:29

s-martin approved these changes Dec 3, 2023

View reviewed changes

This was referenced Dec 3, 2023

Installation as different user (not user pi) #1785

Closed

🐛 | DietPi Bullseye - no RFID Reader #1783

Closed

s-martin linked an issue Dec 3, 2023 that may be closed by this pull request

Installation as different user (not user pi) #1785

Closed

pabera reviewed Dec 4, 2023

View reviewed changes

AlvinSchiller added 4 commits December 4, 2023 09:44

updated comments and docs

fa57351

updated comments and docs

602206f

remove extra home_path check

3a40e84

changes testuser name and group

69ed6bd

pabera reviewed Dec 4, 2023

View reviewed changes

pabera approved these changes Dec 4, 2023

View reviewed changes

AlvinSchiller merged commit cf5aa6c into MiczFlor:future3/develop Dec 4, 2023
12 checks passed

AlvinSchiller deleted the future3/feature/username branch December 4, 2023 22:23

s-martin linked an issue Dec 5, 2023 that may be closed by this pull request

🚀 | Phniebox running as non root user #1259

Closed

s-martin mentioned this pull request Dec 5, 2023

🚀 | Phniebox running as non root user #1259

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

future3 - allow custom username #2132

future3 - allow custom username #2132

AlvinSchiller commented Dec 2, 2023 •

edited

Loading

pabera left a comment

pabera Dec 4, 2023

AlvinSchiller Dec 4, 2023

s-martin Dec 4, 2023

AlvinSchiller Dec 4, 2023 •

edited

Loading

pabera Dec 4, 2023

AlvinSchiller Dec 4, 2023

pabera Dec 4, 2023

AlvinSchiller Dec 4, 2023

pabera Dec 4, 2023

AlvinSchiller Dec 4, 2023

pabera Dec 4, 2023

pabera Dec 4, 2023

AlvinSchiller Dec 4, 2023 •

edited

Loading

pabera Dec 4, 2023

pabera Dec 4, 2023

future3 - allow custom username #2132

future3 - allow custom username #2132

Conversation

AlvinSchiller commented Dec 2, 2023 • edited Loading

pabera left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlvinSchiller Dec 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlvinSchiller Dec 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlvinSchiller commented Dec 2, 2023 •

edited

Loading

AlvinSchiller Dec 4, 2023 •

edited

Loading

AlvinSchiller Dec 4, 2023 •

edited

Loading