-
Notifications
You must be signed in to change notification settings - Fork 133
Linux: Wrapper and scripts modernizations #4982
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
base: develop-linux
Are you sure you want to change the base?
Changes from 2 commits
72a8fa3
d1c9d1d
ba2f4d8
955bb7d
33118cd
803a0fe
81f6060
7b3a236
9b7db58
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,26 @@ | ||
| #!/bin/bash | ||
| #!/usr/bin/env sh | ||
|
|
||
| # Send a URL of the form secondlife://... to Second Life. | ||
| # Send a URL of the form secondlife://... to any running viewer, if not, launch the default viewer. | ||
| # | ||
|
|
||
| URL="$1" | ||
| sl_url="$*" | ||
|
|
||
| if [ -z "$URL" ]; then | ||
| echo Usage: $0 secondlife://... | ||
| exit | ||
| echo "Got SLURL: ${sl_url}" | ||
| if [ -z "${sl_url}" ]; then | ||
| echo "Usage: $0 secondlife:// ..." | ||
| exit | ||
| fi | ||
|
|
||
| RUN_PATH=`dirname "$0" || echo .` | ||
| cd "${RUN_PATH}/.." | ||
|
|
||
| exec ./secondlife -url \'"${URL}"\' | ||
| run_path=$(dirname "$0" || echo .) | ||
|
|
||
| #Poll DBus to get a list of registered services, then look through the list for the Second Life API Service - if present, this means a viewer is running, if not, then no viewer is running and a new instance should be launched | ||
| service_name="com.secondlife.ViewerAppAPIService" #Name of Second Life DBus service. This should be the same across all viewers. | ||
| if dbus-send --print-reply --dest=org.freedesktop.DBus /org/freedesktop/DBus org.freedesktop.DBus.ListNames | grep -q "${service_name}"; then | ||
| echo "Second Life running, sending to DBus..."; | ||
| exec dbus-send --type=method_call --dest="${service_name}" /com/secondlife/ViewerAppAPI com.secondlife.ViewerAppAPI.GoSLURL string:"${sl_url}" | ||
| else | ||
| echo "Second Life not running, launching new instance..."; | ||
| cd "${run_path}"/.. || exit | ||
| #Go to .sh location (/etc), then up a directory to the viewer location | ||
| exec ./secondlife -url "${sl_url}" | ||
| fi | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,6 +3,18 @@ | |||||||||||||||||||||||||||||
| # Install the Second Life Viewer. This script can install the viewer both | ||||||||||||||||||||||||||||||
| # system-wide and for an individual user. | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| build_data_file="build_data.json" | ||||||||||||||||||||||||||||||
| if [ -f "${build_data_file}" ]; then | ||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will fail if install.sh is not executed as Better to put this part after the detection of RUN_PATH, and embed |
||||||||||||||||||||||||||||||
| version=$(sed -n 's/.*"Version"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' "${build_data_file}") | ||||||||||||||||||||||||||||||
| channel=$(sed -n 's/.*"Channel"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' "${build_data_file}") | ||||||||||||||||||||||||||||||
| installdir_name=$(echo "$channel" | tr '[:upper:]' '[:lower:]' | tr ' ' '-' )-install | ||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||
| echo "Error: File ${build_data_file} not found." >&2 | ||||||||||||||||||||||||||||||
| exit 1 | ||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| echo "Installing ${channel} version ${version}" | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| VT102_STYLE_NORMAL='\E[0m' | ||||||||||||||||||||||||||||||
| VT102_COLOR_RED='\E[31m' | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
@@ -58,8 +70,11 @@ function homedir_install() | |||||||||||||||||||||||||||||
| exit 0 | ||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| install_to_prefix "$HOME/.secondlife-install" | ||||||||||||||||||||||||||||||
| $HOME/.secondlife-install/etc/refresh_desktop_app_entry.sh | ||||||||||||||||||||||||||||||
| if [ -d "$XDG_DATA_HOME" ] ; then | ||||||||||||||||||||||||||||||
| install_to_prefix "$XDG_DATA_HOME/$installdir_name" #$XDG_DATA_HOME is a synonym for $HOME/.local/share/ unless the user has specified otherwise (unlikely). | ||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||
| install_to_prefix "$HOME/.local/share/$installdir_name" #XDG_DATA_HOME not set, so use default path as defined by XDG spec. | ||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
| if [ -d "$XDG_DATA_HOME" ] ; then | |
| install_to_prefix "$XDG_DATA_HOME/$installdir_name" #$XDG_DATA_HOME is a synonym for $HOME/.local/share/ unless the user has specified otherwise (unlikely). | |
| else | |
| install_to_prefix "$HOME/.local/share/$installdir_name" #XDG_DATA_HOME not set, so use default path as defined by XDG spec. | |
| fi | |
| if [ -n "$XDG_DATA_HOME" ] ; then | |
| local xdg_data_dir="$XDG_DATA_HOME" # $XDG_DATA_HOME is a synonym for $HOME/.local/share/ unless the user has specified otherwise. | |
| else | |
| local xdg_data_dir="$HOME/.local/share" # XDG_DATA_HOME not set, so use default path as defined by XDG spec. | |
| fi | |
| mkdir -p "$xdg_data_dir" || die "Failed to create data directory: $xdg_data_dir" | |
| install_to_prefix "$xdg_data_dir/$installdir_name" |
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message 'Failed to integrate into DE via XDG.' is somewhat unclear. Consider providing more actionable information, such as 'Failed to register desktop entry. You may need to manually add the application to your menu.' This helps users understand what failed and what they might need to do.
| "$1"/etc/refresh_desktop_app_entry.sh || echo "Failed to integrate into DE via XDG." | |
| "$1"/etc/refresh_desktop_app_entry.sh || echo "Failed to register desktop entry via XDG. You may need to manually add the application to your desktop menu." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not using function
Though it is not strictly necessary, the convention seems to be using the function keyword. In addition using function makes the function stand out more.
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent capitalization of 'SLurl' - should be 'SLURL' (all caps) to match the usage throughout the rest of the codebase and documentation, as seen in other files in this PR.
| prompt "Would you like to set Second Life as your default SLurl handler? [Y/N]: " | |
| prompt "Would you like to set Second Life as your default SLURL handler? [Y/N]: " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be return 0 instead?
exit 0 ends the script completely, meaning there are steps in root_install() that will not be executed (involving "refresh_desktop_app_entry")
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function exits the entire installation script when the user declines to set the SLURL handler (line 124). This should use 'return 0' instead of 'exit 0' to allow the installation to complete successfully even if the user chooses not to set the SLURL handler.
| exit 0 | |
| return 0 |
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable 'install_dir' is assigned but its naming is inconsistent with the rest of the codebase which uses 'installation_prefix' or 'install_prefix'. Using a consistent naming convention would improve code maintainability.
| install_dir=$1 | |
| echo | |
| prompt "Would you like to set Second Life as your default SLurl handler? [Y/N]: " | |
| if [ $? -eq 0 ]; then | |
| exit 0 | |
| fi | |
| "$install_dir"/etc/register_secondlifeprotocol.sh #Successful association comes with a notification to the user. | |
| install_prefix=$1 | |
| echo | |
| prompt "Would you like to set Second Life as your default SLurl handler? [Y/N]: " | |
| if [ $? -eq 0 ]; then | |
| exit 0 | |
| fi | |
| "$install_prefix"/etc/register_secondlifeprotocol.sh #Successful association comes with a notification to the user. |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,37 +1,69 @@ | ||||||||||||||
| #!/bin/bash | ||||||||||||||
|
|
||||||||||||||
| SCRIPTSRC=`readlink -f "$0" || echo "$0"` | ||||||||||||||
| RUN_PATH=`dirname "${SCRIPTSRC}" || echo .` | ||||||||||||||
| SCRIPTSRC=$(readlink -f "$0" || echo "$0") | ||||||||||||||
| RUN_PATH=$(dirname "${SCRIPTSRC}" || echo .) | ||||||||||||||
|
|
||||||||||||||
| install_prefix="$(realpath -- "${RUN_PATH}/..")" | ||||||||||||||
|
|
||||||||||||||
| build_data_file="${install_prefix}/build_data.json" | ||||||||||||||
| if [ -f "${build_data_file}" ]; then | ||||||||||||||
| version=$(sed -n 's/.*"Version"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' "${build_data_file}") | ||||||||||||||
| channel_base=$(sed -n 's/.*"Channel Base"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' "${build_data_file}") | ||||||||||||||
| channel=$(sed -n 's/.*"Channel"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' "${build_data_file}") | ||||||||||||||
| desktopfilename=$(echo "$channel" | tr '[:upper:]' '[:lower:]' | tr ' ' '-' )-viewer | ||||||||||||||
| else | ||||||||||||||
| echo "Error: File ${build_data_file} not found." >&2 | ||||||||||||||
| exit 1 | ||||||||||||||
| fi | ||||||||||||||
|
|
||||||||||||||
| # Check for the Release channel. This channel should not have the channel name in its launcher. | ||||||||||||||
| if [ "$channel" = "$channel_base Release" ]; then | ||||||||||||||
| launcher_name="$channel_base" | ||||||||||||||
| else | ||||||||||||||
| launcher_name=$channel | ||||||||||||||
| fi | ||||||||||||||
|
|
||||||||||||||
| function install_desktop_entry() | ||||||||||||||
| { | ||||||||||||||
| local installation_prefix="$1" | ||||||||||||||
| local desktop_entries_dir="$2" | ||||||||||||||
| local installation_prefix="${1}" | ||||||||||||||
| local desktop_entries_dir="${2}" | ||||||||||||||
|
|
||||||||||||||
| local desktop_entry="\ | ||||||||||||||
| [Desktop Entry]\n\ | ||||||||||||||
| Name=Second Life\n\ | ||||||||||||||
| Version=1.4\n\ | ||||||||||||||
| Name=${launcher_name}\n\ | ||||||||||||||
| GenericName=Second Life Viewer\n\ | ||||||||||||||
| Comment=Client for the On-line Virtual World, Second Life\n\ | ||||||||||||||
| Comment=Client for the Online Virtual World, Second Life\n\ | ||||||||||||||
| Path=${installation_prefix}\n\ | ||||||||||||||
| Exec=${installation_prefix}/secondlife\n\ | ||||||||||||||
| Icon=${installation_prefix}/secondlife_icon.png\n\ | ||||||||||||||
| Icon=${desktopfilename}\n\ | ||||||||||||||
| Terminal=false\n\ | ||||||||||||||
| Type=Application\n\ | ||||||||||||||
| Categories=Game;Simulation;\n\ | ||||||||||||||
| StartupNotify=true\n\ | ||||||||||||||
| StartupWMClass="com.secondlife.indra.viewer"\n\ | ||||||||||||||
| X-Desktop-File-Install-Version=3.0" | ||||||||||||||
| StartupWMClass=\"com.secondlife.indra.viewer\"\n\ | ||||||||||||||
| X-Desktop-File-Install-Version=3.0 | ||||||||||||||
|
||||||||||||||
| X-Desktop-File-Install-Version=3.0 | |
| X-Desktop-File-Install-Version=3.0\n\ |
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing newline escape sequence '\n' after 'PrefersNonDefaultGPU=true' will result in an empty line being added to the desktop file, which may not be intended. The backslash should be removed if you want the line to end there without continuation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does every line end with \n\ ? We can remove all of them. The quoting during printf should keep the line breaks.
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation: lines 64-66 use tabs while the rest of the function uses spaces. This should use consistent indentation (spaces) to match the rest of the file.
| xdg-icon-resource install --novendor --size 256 "${installation_prefix}/secondlife_icon.png" "${desktopfilename}" | |
| #NOTE: Above command takes the path to the icon to install && The name of the icon to be used by XDG. This should always be in the format of "x-Viewer" to avoid potential naming conflicts, as per XDG spec. | |
| xdg-desktop-menu install --novendor "${installation_prefix}"/"${desktopfilename}".desktop | |
| xdg-icon-resource install --novendor --size 256 "${installation_prefix}/secondlife_icon.png" "${desktopfilename}" | |
| #NOTE: Above command takes the path to the icon to install && The name of the icon to be used by XDG. This should always be in the format of "x-Viewer" to avoid potential naming conflicts, as per XDG spec. | |
| xdg-desktop-menu install --novendor "${installation_prefix}"/"${desktopfilename}".desktop |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,57 +1,66 @@ | ||||||||||
| #!/bin/bash | ||||||||||
| #!/bin/env sh | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see no benefits running this script with It is understandable for the actual handler to be executed by
|
||||||||||
| #!/bin/env sh | |
| #!/usr/bin/env sh |
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in comment: 'executeable' should be spelled 'executable'.
| # Ensure the handle_secondlifeprotocol.sh file is executeable (otherwise, xdg-mime won't work) | |
| # Ensure the handle_secondlifeprotocol.sh file is executable (otherwise, xdg-mime won't work) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we be using ${XDG_DATA_HOME}/applications instead?
Also, this causes breakage if installation was done using sudo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
R.E. XDG_DATA_HOME - possibly, however on most systems this isn't actually set and according to the XDG spec, if it's not set then we should assume $HOME/.local/share instead.
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable 'PWD%' on line 27 appears to have a typo with the trailing '%' character. This should likely be just 'PWD' without the '%' suffix, as the '%' has no meaning in this context and will be treated as a literal character.
| newhandler="secondlifeprotocol_$(basename "${PWD%}").desktop" | |
| newhandler="secondlifeprotocol_$(basename "${PWD}").desktop" |
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Name and Comment fields should not have quotes around their values in desktop files. According to the Desktop Entry Specification, these values should be unquoted strings. The quotes will be treated as literal characters in the display name.
| Name="Second Life URL handler" | |
| Comment="secondlife:// URL handler" | |
| Name=Second Life URL handler | |
| Comment=secondlife:// URL handler |
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message on line 29 is missing proper error handling. The 'cat' command will fail if it cannot write to the file, but the script continues execution regardless. The '|| print ...' only handles the output, but doesn't prevent the subsequent xdg-mime commands from running with a potentially invalid or missing desktop file. Consider using proper error handling with 'exit' or returning early.
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The notify-send command will fail silently if the notification daemon is not available or if notify-send is not installed. Consider checking if notify-send is available before calling it, similar to the xdg-mime checks on line 24.
| notify-send -t 5000 -i info "Second Life URL Handler" "SLURL association created" | |
| if command -v notify-send >/dev/null 2>&1; then | |
| notify-send -t 5000 -i info "Second Life URL Handler" "SLURL association created" | |
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using '$' instead of '"$@"' for capturing command-line arguments can cause issues with arguments containing spaces. The variable will contain all arguments as a single string with spaces, which means SLURLs with query parameters or special characters may not be properly preserved. Consider using '"$@"' or at least quoting '$' as '"$*"' to ensure proper argument handling.