Added Pd to FMA - #46335
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #46335 +/- ##
==========================================
+ Coverage 66.90% 66.91% +0.01%
==========================================
Files 2834 2837 +3
Lines 224862 225011 +149
Branches 11512 11615 +103
==========================================
+ Hits 150435 150562 +127
- Misses 60772 60784 +12
- Partials 13655 13665 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR adds Pure Data (Pd) as a managed macOS application: it introduces Homebrew input metadata and a registry entry for Pd, adds a macOS manifest for version 0.56-2 with installer URL, checksum, SQL existence/version checks, and embedded install/uninstall scripts, and adds a Pd icon component wired into the frontend icon map. Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ee/maintained-apps/outputs/pd/darwin.json`:
- Around line 19-20: The script currently can mask failures because commands
like unzip and cp may fail but the script can still exit 0 (relaunch_application
is last); add fail-fast checks: enable strict error handling at top (e.g., set
-euo pipefail) or after critical commands check their exit status and call a
common error-exit helper that logs and exits non-zero; specifically,
wrap/validate the unzip and sudo cp/mv calls (and any sudo rm -rf if relevant)
so failures in those operations cause an immediate exit with an error message;
reference functions/sections: the top-level unzip invocation, the sudo cp/mv
operations around Pd-0.56-2.app, and the relaunch_application and
quit_and_track_application flow to ensure relaunch is not relied on to surface
earlier failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0b7753f3-bfb2-4f73-b69c-736c1e8a88ee
⛔ Files ignored due to path filters (1)
website/assets/images/app-icon-pd-60x60@2x.pngis excluded by!**/*.png
📒 Files selected for processing (5)
ee/maintained-apps/inputs/homebrew/pd.jsonee/maintained-apps/outputs/apps.jsonee/maintained-apps/outputs/pd/darwin.jsonfrontend/pages/SoftwarePage/components/icons/Pd0562.tsxfrontend/pages/SoftwarePage/components/icons/index.ts
| "95ddeb15": "#!/bin/bash\n\n# variables\nAPPDIR=\"/Applications/\"\nTMPDIR=$(dirname \"$(realpath \"$INSTALLER_PATH\")\")\n# functions\n\nquit_and_track_application() {\n local bundle_id=\"$1\"\n local var_name=\"APP_WAS_RUNNING_$(echo \"$bundle_id\" | tr '.-' '__')\"\n local timeout_duration=10\n\n # check if the application is running\n local app_running\n app_running=$(osascript -e \"application id \\\"$bundle_id\\\" is running\" 2>/dev/null)\n if [[ \"$app_running\" != \"true\" ]]; then\n eval \"export $var_name=0\"\n return\n fi\n\n local console_user\n console_user=$(stat -f \"%Su\" /dev/console)\n if [[ -z \"$console_user\" || \"$console_user\" == \"root\" || \"$console_user\" == \"loginwindow\" ]]; then\n echo \"Not logged into a non-root GUI; skipping quitting application ID '$bundle_id'.\"\n eval \"export $var_name=0\"\n return\n fi\n\n # App was running, mark it for relaunch\n eval \"export $var_name=1\"\n echo \"Application '$bundle_id' was running; will relaunch after installation.\"\n\n echo \"Quitting application '$bundle_id'...\"\n\n # try to quit the application within the timeout period\n local quit_success=false\n SECONDS=0\n while (( SECONDS < timeout_duration )); do\n if osascript -e \"tell application id \\\"$bundle_id\\\" to quit\" >/dev/null 2>&1; then\n if ! pgrep -f \"$bundle_id\" >/dev/null 2>&1; then\n echo \"Application '$bundle_id' quit successfully.\"\n quit_success=true\n break\n fi\n fi\n sleep 1\n done\n\n if [[ \"$quit_success\" = false ]]; then\n echo \"Application '$bundle_id' did not quit.\"\n fi\n}\n\n\nrelaunch_application() {\n local bundle_id=\"$1\"\n local var_name=\"APP_WAS_RUNNING_$(echo \"$bundle_id\" | tr '.-' '__')\"\n local was_running\n\n # Check if the app was running before installation\n eval \"was_running=\\$$var_name\"\n if [[ \"$was_running\" != \"1\" ]]; then\n return\n fi\n\n local console_user\n console_user=$(stat -f \"%Su\" /dev/console)\n if [[ -z \"$console_user\" || \"$console_user\" == \"root\" || \"$console_user\" == \"loginwindow\" ]]; then\n echo \"Not logged into a non-root GUI; skipping relaunching application ID '$bundle_id'.\"\n return\n fi\n\n echo \"Relaunching application '$bundle_id'...\"\n\n # Launch the app in the logged-in user's GUI session. Apps launched by root\n # won't register with the user's Dock/GUI, so run 'open' as the console user.\n # Use 'launchctl asuser' to bootstrap into the console user's Mach namespace\n # and GUI session — 'sudo -u' alone doesn't do this, which can cause\n # LSOpenURLsWithRole() failures even when 'open' exits 0.\n local open_status=0\n if [[ $EUID -eq 0 ]]; then\n local console_uid\n console_uid=$(id -u \"$console_user\")\n /bin/launchctl asuser \"$console_uid\" sudo -u \"$console_user\" open -b \"$bundle_id\" >/dev/null 2>&1 || open_status=$?\n else\n open -b \"$bundle_id\" >/dev/null 2>&1 || open_status=$?\n fi\n\n if [[ $open_status -eq 0 ]]; then\n echo \"Application '$bundle_id' relaunched successfully.\"\n else\n echo \"Failed to relaunch application '$bundle_id'.\"\n fi\n}\n\n\n# extract contents\nunzip \"$INSTALLER_PATH\" -d \"$TMPDIR\"\n# copy to the applications folder\nquit_and_track_application 'org.puredata.pd.pd-gui'\nif [ -d \"$APPDIR/Pd-0.56-2.app\" ]; then\n\tsudo mv \"$APPDIR/Pd-0.56-2.app\" \"$TMPDIR/Pd-0.56-2.app.bkp\"\nfi\nsudo cp -R \"$TMPDIR/Pd-0.56-2.app\" \"$APPDIR\"\nrelaunch_application 'org.puredata.pd.pd-gui'\n", | ||
| "cec1f6f1": "#!/bin/bash\n\n# variables\nAPPDIR=\"/Applications/\"\nLOGGED_IN_USER=$(scutil <<< \"show State:/Users/ConsoleUser\" | awk '/Name :/ { print $3 }')\n# functions\n\ntrash() {\n local logged_in_user=\"$1\"\n local target_file=\"$2\"\n local timestamp=\"$(date +%Y-%m-%d-%s)\"\n local rand=\"$(jot -r 1 0 99999)\"\n\n # replace ~ with /Users/$logged_in_user\n if [[ \"$target_file\" == ~* ]]; then\n target_file=\"/Users/$logged_in_user${target_file:1}\"\n fi\n\n local trash=\"/Users/$logged_in_user/.Trash\"\n local file_name=\"$(basename \"${target_file}\")\"\n\n if [[ -e \"$target_file\" ]]; then\n echo \"removing $target_file.\"\n mv -f \"$target_file\" \"$trash/${file_name}_${timestamp}_${rand}\"\n else\n echo \"$target_file doesn't exist.\"\n fi\n}\n\nsudo rm -rf \"$APPDIR/Pd-0.56-2.app\"\ntrash $LOGGED_IN_USER '~/Library/Preferences/org.puredata.pd.pd-gui.plist'\ntrash $LOGGED_IN_USER '~/Library/Saved Application State/org.puredata.pd.pd-gui.savedState'\n" |
There was a problem hiding this comment.
Add fail-fast error handling to prevent false-success installs.
At Line 19 and Line 20, command failures in the install script can be masked (e.g., unzip/cp failure) and still return success because relaunch_application is the last command and can return 0 when the app was not previously running.
💡 Proposed hardening
#!/bin/bash
+set -euo pipefail
# variables
APPDIR="/Applications/"
TMPDIR=$(dirname "$(realpath "$INSTALLER_PATH")")
@@
-unzip "$INSTALLER_PATH" -d "$TMPDIR"
+unzip -o "$INSTALLER_PATH" -d "$TMPDIR"
+[[ -d "$TMPDIR/Pd-0.56-2.app" ]] || {
+ echo "Extracted app bundle not found: $TMPDIR/Pd-0.56-2.app"
+ exit 1
+}
@@
-sudo cp -R "$TMPDIR/Pd-0.56-2.app" "$APPDIR"
+sudo cp -R "$TMPDIR/Pd-0.56-2.app" "$APPDIR"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "95ddeb15": "#!/bin/bash\n\n# variables\nAPPDIR=\"/Applications/\"\nTMPDIR=$(dirname \"$(realpath \"$INSTALLER_PATH\")\")\n# functions\n\nquit_and_track_application() {\n local bundle_id=\"$1\"\n local var_name=\"APP_WAS_RUNNING_$(echo \"$bundle_id\" | tr '.-' '__')\"\n local timeout_duration=10\n\n # check if the application is running\n local app_running\n app_running=$(osascript -e \"application id \\\"$bundle_id\\\" is running\" 2>/dev/null)\n if [[ \"$app_running\" != \"true\" ]]; then\n eval \"export $var_name=0\"\n return\n fi\n\n local console_user\n console_user=$(stat -f \"%Su\" /dev/console)\n if [[ -z \"$console_user\" || \"$console_user\" == \"root\" || \"$console_user\" == \"loginwindow\" ]]; then\n echo \"Not logged into a non-root GUI; skipping quitting application ID '$bundle_id'.\"\n eval \"export $var_name=0\"\n return\n fi\n\n # App was running, mark it for relaunch\n eval \"export $var_name=1\"\n echo \"Application '$bundle_id' was running; will relaunch after installation.\"\n\n echo \"Quitting application '$bundle_id'...\"\n\n # try to quit the application within the timeout period\n local quit_success=false\n SECONDS=0\n while (( SECONDS < timeout_duration )); do\n if osascript -e \"tell application id \\\"$bundle_id\\\" to quit\" >/dev/null 2>&1; then\n if ! pgrep -f \"$bundle_id\" >/dev/null 2>&1; then\n echo \"Application '$bundle_id' quit successfully.\"\n quit_success=true\n break\n fi\n fi\n sleep 1\n done\n\n if [[ \"$quit_success\" = false ]]; then\n echo \"Application '$bundle_id' did not quit.\"\n fi\n}\n\n\nrelaunch_application() {\n local bundle_id=\"$1\"\n local var_name=\"APP_WAS_RUNNING_$(echo \"$bundle_id\" | tr '.-' '__')\"\n local was_running\n\n # Check if the app was running before installation\n eval \"was_running=\\$$var_name\"\n if [[ \"$was_running\" != \"1\" ]]; then\n return\n fi\n\n local console_user\n console_user=$(stat -f \"%Su\" /dev/console)\n if [[ -z \"$console_user\" || \"$console_user\" == \"root\" || \"$console_user\" == \"loginwindow\" ]]; then\n echo \"Not logged into a non-root GUI; skipping relaunching application ID '$bundle_id'.\"\n return\n fi\n\n echo \"Relaunching application '$bundle_id'...\"\n\n # Launch the app in the logged-in user's GUI session. Apps launched by root\n # won't register with the user's Dock/GUI, so run 'open' as the console user.\n # Use 'launchctl asuser' to bootstrap into the console user's Mach namespace\n # and GUI session — 'sudo -u' alone doesn't do this, which can cause\n # LSOpenURLsWithRole() failures even when 'open' exits 0.\n local open_status=0\n if [[ $EUID -eq 0 ]]; then\n local console_uid\n console_uid=$(id -u \"$console_user\")\n /bin/launchctl asuser \"$console_uid\" sudo -u \"$console_user\" open -b \"$bundle_id\" >/dev/null 2>&1 || open_status=$?\n else\n open -b \"$bundle_id\" >/dev/null 2>&1 || open_status=$?\n fi\n\n if [[ $open_status -eq 0 ]]; then\n echo \"Application '$bundle_id' relaunched successfully.\"\n else\n echo \"Failed to relaunch application '$bundle_id'.\"\n fi\n}\n\n\n# extract contents\nunzip \"$INSTALLER_PATH\" -d \"$TMPDIR\"\n# copy to the applications folder\nquit_and_track_application 'org.puredata.pd.pd-gui'\nif [ -d \"$APPDIR/Pd-0.56-2.app\" ]; then\n\tsudo mv \"$APPDIR/Pd-0.56-2.app\" \"$TMPDIR/Pd-0.56-2.app.bkp\"\nfi\nsudo cp -R \"$TMPDIR/Pd-0.56-2.app\" \"$APPDIR\"\nrelaunch_application 'org.puredata.pd.pd-gui'\n", | |
| "cec1f6f1": "#!/bin/bash\n\n# variables\nAPPDIR=\"/Applications/\"\nLOGGED_IN_USER=$(scutil <<< \"show State:/Users/ConsoleUser\" | awk '/Name :/ { print $3 }')\n# functions\n\ntrash() {\n local logged_in_user=\"$1\"\n local target_file=\"$2\"\n local timestamp=\"$(date +%Y-%m-%d-%s)\"\n local rand=\"$(jot -r 1 0 99999)\"\n\n # replace ~ with /Users/$logged_in_user\n if [[ \"$target_file\" == ~* ]]; then\n target_file=\"/Users/$logged_in_user${target_file:1}\"\n fi\n\n local trash=\"/Users/$logged_in_user/.Trash\"\n local file_name=\"$(basename \"${target_file}\")\"\n\n if [[ -e \"$target_file\" ]]; then\n echo \"removing $target_file.\"\n mv -f \"$target_file\" \"$trash/${file_name}_${timestamp}_${rand}\"\n else\n echo \"$target_file doesn't exist.\"\n fi\n}\n\nsudo rm -rf \"$APPDIR/Pd-0.56-2.app\"\ntrash $LOGGED_IN_USER '~/Library/Preferences/org.puredata.pd.pd-gui.plist'\ntrash $LOGGED_IN_USER '~/Library/Saved Application State/org.puredata.pd.pd-gui.savedState'\n" | |
| #!/bin/bash | |
| set -euo pipefail | |
| # variables | |
| APPDIR="/Applications/" | |
| TMPDIR=$(dirname "$(realpath "$INSTALLER_PATH")") | |
| # functions | |
| quit_and_track_application() { | |
| local bundle_id="$1" | |
| local var_name="APP_WAS_RUNNING_$(echo "$bundle_id" | tr '.-' '__')" | |
| local timeout_duration=10 | |
| # check if the application is running | |
| local app_running | |
| app_running=$(osascript -e "application id \"$bundle_id\" is running" 2>/dev/null) | |
| if [[ "$app_running" != "true" ]]; then | |
| eval "export $var_name=0" | |
| return | |
| fi | |
| local console_user | |
| console_user=$(stat -f "%Su" /dev/console) | |
| if [[ -z "$console_user" || "$console_user" == "root" || "$console_user" == "loginwindow" ]]; then | |
| echo "Not logged into a non-root GUI; skipping quitting application ID '$bundle_id'." | |
| eval "export $var_name=0" | |
| return | |
| fi | |
| # App was running, mark it for relaunch | |
| eval "export $var_name=1" | |
| echo "Application '$bundle_id' was running; will relaunch after installation." | |
| echo "Quitting application '$bundle_id'..." | |
| # try to quit the application within the timeout period | |
| local quit_success=false | |
| SECONDS=0 | |
| while (( SECONDS < timeout_duration )); do | |
| if osascript -e "tell application id \"$bundle_id\" to quit" >/dev/null 2>&1; then | |
| if ! pgrep -f "$bundle_id" >/dev/null 2>&1; then | |
| echo "Application '$bundle_id' quit successfully." | |
| quit_success=true | |
| break | |
| fi | |
| fi | |
| sleep 1 | |
| done | |
| if [[ "$quit_success" = false ]]; then | |
| echo "Application '$bundle_id' did not quit." | |
| fi | |
| } | |
| relaunch_application() { | |
| local bundle_id="$1" | |
| local var_name="APP_WAS_RUNNING_$(echo "$bundle_id" | tr '.-' '__')" | |
| local was_running | |
| # Check if the app was running before installation | |
| eval "was_running=\$$var_name" | |
| if [[ "$was_running" != "1" ]]; then | |
| return | |
| fi | |
| local console_user | |
| console_user=$(stat -f "%Su" /dev/console) | |
| if [[ -z "$console_user" || "$console_user" == "root" || "$console_user" == "loginwindow" ]]; then | |
| echo "Not logged into a non-root GUI; skipping relaunching application ID '$bundle_id'." | |
| return | |
| fi | |
| echo "Relaunching application '$bundle_id'..." | |
| # Launch the app in the logged-in user's GUI session. Apps launched by root | |
| # won't register with the user's Dock/GUI, so run 'open' as the console user. | |
| # Use 'launchctl asuser' to bootstrap into the console user's Mach namespace | |
| # and GUI session — 'sudo -u' alone doesn't do this, which can cause | |
| # LSOpenURLsWithRole() failures even when 'open' exits 0. | |
| local open_status=0 | |
| if [[ $EUID -eq 0 ]]; then | |
| local console_uid | |
| console_uid=$(id -u "$console_user") | |
| /bin/launchctl asuser "$console_uid" sudo -u "$console_user" open -b "$bundle_id" >/dev/null 2>&1 || open_status=$? | |
| else | |
| open -b "$bundle_id" >/dev/null 2>&1 || open_status=$? | |
| fi | |
| if [[ $open_status -eq 0 ]]; then | |
| echo "Application '$bundle_id' relaunched successfully." | |
| else | |
| echo "Failed to relaunch application '$bundle_id'." | |
| fi | |
| } | |
| # extract contents | |
| unzip -o "$INSTALLER_PATH" -d "$TMPDIR" | |
| [[ -d "$TMPDIR/Pd-0.56-2.app" ]] || { | |
| echo "Extracted app bundle not found: $TMPDIR/Pd-0.56-2.app" | |
| exit 1 | |
| } | |
| # copy to the applications folder | |
| quit_and_track_application 'org.puredata.pd.pd-gui' | |
| if [ -d "$APPDIR/Pd-0.56-2.app" ]; then | |
| sudo mv "$APPDIR/Pd-0.56-2.app" "$TMPDIR/Pd-0.56-2.app.bkp" | |
| fi | |
| sudo cp -R "$TMPDIR/Pd-0.56-2.app" "$APPDIR" | |
| relaunch_application 'org.puredata.pd.pd-gui' |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ee/maintained-apps/outputs/pd/darwin.json` around lines 19 - 20, The script
currently can mask failures because commands like unzip and cp may fail but the
script can still exit 0 (relaunch_application is last); add fail-fast checks:
enable strict error handling at top (e.g., set -euo pipefail) or after critical
commands check their exit status and call a common error-exit helper that logs
and exits non-zero; specifically, wrap/validate the unzip and sudo cp/mv calls
(and any sudo rm -rf if relevant) so failures in those operations cause an
immediate exit with an error message; reference functions/sections: the
top-level unzip invocation, the sudo cp/mv operations around Pd-0.56-2.app, and
the relaunch_application and quit_and_track_application flow to ensure relaunch
is not relied on to surface earlier failures.
|
@desmonet I am moving this to draft. This needs quite a few changes to get working correctly. For example, the zip file downloads a dmg so this will require a custom install script. It's also going to require some versioning transformation as the cask shows version 0.56-3 but osquery reports 0.56.3. Feel free to review and resubmit. You can also close this and put in a Feature Request to add Pd as a Fleet-maintained app and we can take a look at adding it. |
|
App will be added as part of this PR: #47056 |
Addition of Pd (pure data) to the Fleet-Maintained apps list.
Product information: https://puredata.info/
Homebrew formula: https://formulae.brew.sh/cask/pd#default
Thanks!
@allenhouchins
Summary by CodeRabbit
New Features
Bug Fixes