Skip to content

Scan for boot files at run time.#2

Open
ali1234 wants to merge 5 commits intoraspberrypi:masterfrom
ali1234:master
Open

Scan for boot files at run time.#2
ali1234 wants to merge 5 commits intoraspberrypi:masterfrom
ali1234:master

Conversation

@ali1234
Copy link
Copy Markdown

@ali1234 ali1234 commented Dec 14, 2017

This patch allows usbbootgui to dynamically load boot files at run
time. XDG standard directories are searched. Usually this means:

/usr/share/rpiboot
/usr/local/share/rpiboot
~/.local/share/rpiboot

The code first checks if the rpiboot/ subdirectory exists. If it
does, every subdirectory is checked for a bootcode.bin file. If
found, the directory is added to the list store.

If a description.txt is found in the directory, it is used as the
"friendly name" of the image. If an icon.png is found, it is loaded
as the icon. If not found, the directory name and a default icon
are used instead.

Both the description and path of the image are displayed in the GUI.
This is to distinguish between the same image in /usr/share and
/usr/local/share for example.

Images are now always refered to by their full paths. I have not
checked how this interacts with the "always use image" code, but I
think it should work. Old config files might have to be deleted.

This patch also allows the TreeView to have a scrollbar, and fixes
the "expand" property on a few items in the GUI, so that only the
TreeView expands when the window is resized.

Signed-off-by: Alistair Buxton a.j.buxton@gmail.com

This patch allows usbbootgui to dynamically load boot files at run
time. XDG standard directories are searched. Usually this means:

    /usr/share/rpiboot
    /usr/local/share/rpiboot
    ~/.local/share/rpiboot

The code first checks if the rpiboot/ subdirectory exists. If it
does, every subdirectory is checked for a bootcode.bin file. If
found, the directory is added to the list store.

If a description.txt is found in the directory, it is used as the
"friendly name" of the image. If an icon.png is found, it is loaded
as the icon. If not found, the directory name and a default icon
are used instead.

Both the description and path of the image are displayed in the GUI.
This is to distinguish between the same image in /usr/share and
/usr/local/share for example.

Images are now always refered to by their full paths. I have not
checked how this interacts with the "always use image" code, but I
think it should work. Old config files might have to be deleted.

This patch also allows the TreeView to have a scrollbar, and fixes
the "expand" property on a few items in the GUI, so that only the
TreeView expands when the window is resized.

Signed-off-by: Alistair Buxton <a.j.buxton@gmail.com>
@ali1234
Copy link
Copy Markdown
Author

ali1234 commented Dec 14, 2017

Note: this code probably needs more thorough testing than what I have done, particularly around the "always use image" functionality - I'm not really sure exactly how that is supposed to work.

@ali1234
Copy link
Copy Markdown
Author

ali1234 commented Dec 14, 2017

screenshot_2017-12-14_00-29-46

Signed-off-by: Alistair Buxton <a.j.buxton@gmail.com>
As well as being cleaner and more portable, this also fixes any
double separators in displayed strings.

Signed-off-by: Alistair Buxton <a.j.buxton@gmail.com>
They just cause Gtk warnings, so remove them.
Signed-off-by: Alistair Buxton <a.j.buxton@gmail.com>
@maxnet
Copy link
Copy Markdown
Collaborator

maxnet commented Dec 18, 2017

Some random thoughts.

XDG standard directories are searched. Usually this means:

/usr/share/rpiboot
/usr/local/share/rpiboot
~/.local/share/rpiboot

Don't think searching the home directory for custom images is a good idea.
Keep in mind that usbbootgui essentially runs under the privileges of the root user.

If I was an unprivileged user on a multi-user setup, and wanted to view the contents of a file I am not supposed to have access to, all I would need to do is something along the lines of:

mkdir -p ~/.local/share/rpiboot/x
touch ~/.local/share/rpiboot/x/bootcode.bin
ln -s /etc/shadow ~/.local/share/rpiboot/x/description.txt

Plug in a Pi zero, and your code will show the contents of the file I symlinked description.txt to in the GUI.

While that could be made harder by testing if the file is a regular file and not a link -instead of just that it exists like you do now-, also keep in mind that rpiboot is run as root as well, and can also be tricked to serve arbitrary files to the Pi zero with a bit of creativity.

While I can understand the desire to have the option to dynamically add extra items to the menu, I think it would make things simpler if discovery was limited to /usr/share/rpiboot instead of multiple locations.

==

Might want to consider using .desktop style files instead of plain text files for description.
Has the advantage that it allows localization:

Description=English description
Description[nl]=Dutch description
Icon=icon.png

And leaves the option open of adding extra parameters in the future, if there is any need for that.

Can be parsed with the standard g_key_file_* glib functions, which we currently already use to parse the settings file that stores always use image.

https://specifications.freedesktop.org/desktop-entry-spec/desktop-entry-spec-1.1.html
https://developer.gnome.org/glib/stable/glib-Key-value-file-parser.html

In that case I would suggest also creating those files for the standard images like gpioexpander, instead of having them hard coded like you currently have.

==

Not sure if showing the full path in the GUI isn't too much detailed information for the average user.
If that information needs to be available, you may want to consider putting it in a tooltip, or hide it under right mouse cursor.

@ali1234
Copy link
Copy Markdown
Author

ali1234 commented Dec 18, 2017

If you don't want to scan the files in the user's home, it will suffice to remove the four lines at:

15d1d2c#diff-b53da094f10b03fa5984b61cf99f0cf3R572

However, since the system XDG dirs also come from an environment variable, the user can still modify those to scan other paths. So you would have to hard code it.

Perhaps a better approach would be to check for symlinks and ignore them. That should be fairly easy.

As for the full path. That is shown to prevent confusion between /usr/share/rpiboot and the user's local installed files. If you limit scanning to one directory, it isn't necessary.

I agree about .desktop files. That would be a better way to store the metadata, and would make translation a lot easier.

In the long term, usbbootgui should not be running as root at all. It should make the call to rpiboot with pkexec or over dbus. That could end up being quite complex since it is launched from udev, but it would make everything much safer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants