Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions .github/workflows/build-plugin.yml
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,6 @@ jobs:
with:
name: unraid-api
path: ${{ github.workspace }}/plugin/api/
- name: Download PNPM Store
uses: actions/download-artifact@v4
with:
name: packed-node-modules
path: ${{ github.workspace }}/plugin/node-modules-archive/
- name: Extract Unraid API
run: |
mkdir -p ${{ github.workspace }}/plugin/source/dynamix.unraid.net/usr/local/unraid-api
Expand All @@ -129,11 +124,6 @@ jobs:
exit 1
fi

if [ ! -f ./deploy/*.tar.xz ]; then
echo "Error: .tar.xz file not found in plugin/deploy/"
exit 1
fi

- name: Upload to GHA
uses: actions/upload-artifact@v4
with:
Expand Down
5 changes: 0 additions & 5 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -185,11 +185,6 @@ jobs:
with:
name: unraid-api
path: ${{ github.workspace }}/api/deploy/unraid-api.tgz
- name: Upload Node Modules to Github artifacts
uses: actions/upload-artifact@v4
with:
name: packed-node-modules
path: ${{ github.workspace }}/api/deploy/node-modules-archive/packed-node-modules.tar.xz

build-unraid-ui-webcomponents:
name: Build Unraid UI Library (Webcomponent Version)
Expand Down
7 changes: 0 additions & 7 deletions api/scripts/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,6 @@ try {
}
}

const compressionLevel = process.env.WATCH_MODE ? '-1' : '-5';
await $`XZ_OPT=${compressionLevel} tar -cJf packed-node-modules.tar.xz node_modules`;
// Create a subdirectory for the node modules archive
await mkdir('../node-modules-archive', { recursive: true });
await $`mv packed-node-modules.tar.xz ../node-modules-archive/`;
await $`rm -rf node_modules`;

// Clean the release directory
await $`rm -rf ../release/*`;

Expand Down
23 changes: 23 additions & 0 deletions justfile
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,29 @@ setup:

pnpm install

# Removes all node_modules directories in the repository
clean-modules:
#!/usr/bin/env bash
echo "Finding and removing all node_modules directories..."
find . -name "node_modules" -type d -not -path "*/node_modules/*" | while read dir; do
echo "Removing: $dir"
rm -rf "$dir"
done
echo "All node_modules directories have been removed."

# Removes all build artifacts (dist, .nuxt, .output, coverage, etc.)
clean-build:
#!/usr/bin/env bash
echo "Finding and removing build artifacts..."
build_dirs=("dist" ".nuxt" ".output" "coverage" "deploy")
for build_dir in "${build_dirs[@]}"; do
find . -name "$build_dir" -type d | while read dir; do
echo "Removing: $dir"
rm -rf "$dir"
done
done
echo "All build artifacts have been removed."

# restore notification files under api/dev
restore-notifications:
git checkout ./api/dev/notifications
Expand Down
6 changes: 1 addition & 5 deletions plugin/builder/build-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { $ } from "zx";
import { escape as escapeHtml } from "html-sloppy-escaper";
import { dirname, join } from "node:path";
import { getTxzName, pluginName, startingDir, defaultArch, defaultBuild } from "./utils/consts";
import { getAssetUrl, getPluginUrl } from "./utils/bucket-urls";
import { getPluginUrl } from "./utils/bucket-urls";
import { getMainTxzUrl } from "./utils/bucket-urls";
import {
deployDir,
Expand All @@ -12,7 +12,6 @@ import {
} from "./utils/paths";
import { PluginEnv, setupPluginEnv } from "./cli/setup-plugin-environment";
import { cleanupPluginFiles } from "./utils/cleanup";
import { bundleVendorStore, getVendorBundleName } from "./build-vendor-store";

/**
* Check if git is available
Expand Down Expand Up @@ -76,8 +75,6 @@ const buildPlugin = async ({
txz_url: getMainTxzUrl({ baseUrl, apiVersion, tag }),
txz_sha256: txzSha256,
txz_name: getTxzName(apiVersion),
vendor_store_url: getAssetUrl({ baseUrl, tag }, getVendorBundleName(apiVersion)),
vendor_store_filename: getVendorBundleName(apiVersion),
...(tag ? { tag } : {}),
};

Expand Down Expand Up @@ -123,7 +120,6 @@ const main = async () => {

await buildPlugin(validatedEnv);
await moveTxzFile(validatedEnv);
await bundleVendorStore(validatedEnv.apiVersion);
} catch (error) {
console.error(error);
process.exit(1);
Expand Down
2 changes: 1 addition & 1 deletion plugin/builder/build-txz.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ const storeVendorArchiveInfo = async (version: string, vendorUrl: string, vendor

// Create a JSON config file with vendor information
const configData = {
vendor_store_url: vendorUrl,
// vendor_store_url: vendorUrl,
vendor_store_path: vendorFullPath,
api_version: version
};
Expand Down
1 change: 1 addition & 0 deletions plugin/builder/build-vendor-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ async function createNodeModulesTarball(outputPath: string): Promise<void> {
* After this operation, the vendored node_modules will be available inside the `deployDir`.
*
* @param apiVersion Required API version to use for the vendor bundle
* @deprecated vendored node_modules are now included in the API slackware package
*/
export async function bundleVendorStore(apiVersion: string): Promise<void> {
// Ensure deploy directory exists
Expand Down
1 change: 0 additions & 1 deletion plugin/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ services:
- ../unraid-ui/dist-wc:/app/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/unraid-components/uui
- ../web/.nuxt/nuxt-custom-elements/dist/unraid-components:/app/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/unraid-components/nuxt
- ../api/deploy/release/:/app/source/dynamix.unraid.net/usr/local/unraid-api # Use the release dir instead of pack to allow watcher to not try to build with node_modules
- ../api/deploy/node-modules-archive:/app/node-modules-archive
stdin_open: true # equivalent to -i
tty: true # equivalent to -t
environment:
Expand Down
10 changes: 3 additions & 7 deletions plugin/plugins/dynamix.unraid.net.plg
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
<!ENTITY txz_sha256 "">
<!ENTITY txz_url "">
<!ENTITY txz_name "">
<!ENTITY vendor_store_url "">
<!ENTITY vendor_store_filename "">
<!ENTITY arch "x86_64">
<!ENTITY build "1">
<!ENTITY tag "">
Expand Down Expand Up @@ -47,10 +45,6 @@ exit 0
]]>
</INLINE>
</FILE>

<FILE Name="/boot/config/plugins/dynamix.my.servers/&vendor_store_filename;">
<URL>&vendor_store_url;</URL>
</FILE>

<!-- download main txz -->
<FILE Name="&source;">
Expand Down Expand Up @@ -223,7 +217,6 @@ exit 0
PKG_FILE="&source;" # Full path to the package file including .txz extension
PKG_URL="&txz_url;" # URL where package was downloaded from
PKG_NAME="&txz_name;" # Name of the package file
VENDOR_ARCHIVE="/boot/config/plugins/dynamix.my.servers/&vendor_store_filename;"
<![CDATA[
# Install the Slackware package
echo "Installing package..."
Expand All @@ -235,6 +228,9 @@ for txz_file in /boot/config/plugins/dynamix.my.servers/dynamix.unraid.net-*.txz
fi
done

# Clean up any old node_modules archives (on the boot drive) that don't match our current version
/etc/rc.d/rc.unraid-api cleanup-dependencies

Comment on lines +231 to +233
Copy link
Contributor

@coderabbitai coderabbitai bot Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Guard rc.unraid-api cleanup-dependencies with existence & exit-status checks

/etc/rc.d/rc.unraid-api might be missing or return a non-zero status (e.g., corrupted script, permission issue).
Right now the installer would sail past the failure silently and continue, leaving stale archives behind or, worse, terminating later with a less obvious error.

-# Clean up any old node_modules archives (on the boot drive) that don't match our current version
-/etc/rc.d/rc.unraid-api cleanup-dependencies
+# Clean up any old node_modules archives (on the boot drive) that don't match our current version
+if [ -x "/etc/rc.d/rc.unraid-api" ]; then
+  /etc/rc.d/rc.unraid-api cleanup-dependencies || \
+    echo "⚠️  Dependency cleanup failed, continuing install anyway"
+else
+  echo "⚠️  /etc/rc.d/rc.unraid-api not found or not executable, skipping dependency cleanup"
+fi

Minimal defensive code prevents broken upgrades on edge systems without adding much complexity.

🤖 Prompt for AI Agents
In plugin/plugins/dynamix.unraid.net.plg around lines 234 to 236, the call to
/etc/rc.d/rc.unraid-api cleanup-dependencies lacks checks for the script's
existence and its exit status. To fix this, first verify that
/etc/rc.d/rc.unraid-api exists and is executable before running the cleanup
command. Then, after running it, check the exit status and handle any non-zero
result appropriately, such as logging an error or aborting the installation to
prevent silent failures and stale archives.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file is bundled with the plugin, so it will always exist

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pujitm Got it—since the plugin ships its own /etc/rc.d/rc.unraid-api, the file’s absence isn’t a realistic scenario. Thanks for clarifying.

If you’re comfortable relying on that guarantee, feel free to keep the current line as-is. For belt-and-suspenders robustness you could still echo a warning when the cleanup command itself returns non-zero, but that’s purely optional:

# Optional: surface a failure while still proceeding
/etc/rc.d/rc.unraid-api cleanup-dependencies || \
  echo "⚠️  Dependency cleanup failed (non-zero exit), continuing install"

Totally fine to leave it out if you’d rather keep things concise. Let me know if you’d like me to drop the suggestion entirely.


✏️ Learnings added
Learnt from: pujitm
PR: unraid/api#1415
File: plugin/plugins/dynamix.unraid.net.plg:234-236
Timestamp: 2025-06-11T14:14:30.305Z
Learning: For the Unraid Connect plugin, the script `/etc/rc.d/rc.unraid-api` is bundled with the plugin package itself, so its presence on the target system is guaranteed during installation.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

# Remove existing node_modules directory
echo "Cleaning up existing node_modules directory..."
if [ -d "/usr/local/unraid-api/node_modules" ]; then
Expand Down
7 changes: 3 additions & 4 deletions plugin/source/dynamix.unraid.net/etc/rc.d/rc.unraid-api
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,10 @@ uninstall() {
start() {
echo "Starting Unraid API service..."

# Ensure dependencies are installed
# Restore vendored API plugins if they were installed
if [ -x "$scripts_dir/dependencies.sh" ]; then
"$scripts_dir/dependencies.sh" ensure || {
echo "Failed to install dependencies – aborting start."
return 1
"$scripts_dir/dependencies.sh" restore || {
echo "Failed to restore API plugin dependencies! Continuing with start, but API plugins may be unavailable."
}
else
echo "Warning: dependencies.sh script not found or not executable"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ get_api_version() {
get_archive_information() {
# Define all local variables at the top
local api_version=""
local vendor_store_url=""
local vendor_store_path=""

if [ ! -f "$CONFIG_FILE" ]; then
Expand All @@ -39,7 +38,6 @@ get_archive_information() {
# Read values from JSON config using jq
if command -v jq >/dev/null 2>&1; then
api_version=$(jq -r '.api_version' "$CONFIG_FILE")
vendor_store_url=$(jq -r '.vendor_store_url' "$CONFIG_FILE")
vendor_store_path=$(jq -r '.vendor_store_path' "$CONFIG_FILE")
else
echo "jq not found, can't parse config file" >&2
Expand All @@ -52,19 +50,13 @@ get_archive_information() {
return 1
fi

if [ -z "$vendor_store_url" ] || [ "$vendor_store_url" = "null" ]; then
echo "Invalid or missing vendor_store_url in config file" >&2
return 1
fi

if [ -z "$vendor_store_path" ] || [ "$vendor_store_path" = "null" ]; then
echo "Invalid or missing vendor_store_path in config file" >&2
return 1
fi

# Return the values
echo "$api_version"
echo "$vendor_store_url"
echo "$vendor_store_path"
return 0
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ cleanup() {
fi

api_version="${info[0]}"
vendor_store_path="${info[2]}"
vendor_store_path="${info[1]}"

echo "Cleaning up node_modules archives that don't match current API version: $api_version"

Expand Down Expand Up @@ -62,48 +62,16 @@ cleanup() {
# Args:
# $1 - Path to vendor archive to download (ignored, kept for backward compatibility)
redownload_vendor_archive() {
# Define all local variables at the top
local info
local api_version=""
local vendor_store_url=""
local vendor_store_path=""

# Get archive information
if ! mapfile -t info < <(get_archive_information); then
echo "Error: Failed to get vendor archive information. Cannot proceed." >&2
return 1
fi

api_version="${info[0]}"
vendor_store_url="${info[1]}"
vendor_store_path="${info[2]}"

echo "Attempting to download vendor archive for version $api_version"

# Create directory if it doesn't exist
mkdir -p "$(dirname "$vendor_store_path")"

# Attempt to download the vendor archive
echo "Downloading vendor archive from $vendor_store_url to $vendor_store_path"
if curl -f -L "$vendor_store_url" -o "$vendor_store_path"; then
echo "Successfully downloaded vendor archive to $vendor_store_path"
# Return the path to the downloaded archive
echo "$vendor_store_path"
return 0
else
echo "Failed to download vendor archive from URL"
return 1
fi
echo "Error: Download functionality not available - vendor store URL not configured" >&2
return 1
}

# Function to ensure vendor archive is available
# This tries to locate or download the appropriate vendor archive
# Returns the path to the vendor archive or empty string if not available
ensure_vendor_archive() {
# Define all local variables at the top
local info
local api_version=""
local vendor_store_url=""
local vendor_store_path=""

# Get archive information
Expand All @@ -113,8 +81,7 @@ ensure_vendor_archive() {
fi

api_version="${info[0]}"
vendor_store_url="${info[1]}"
vendor_store_path="${info[2]}"
vendor_store_path="${info[1]}"

echo "Looking for vendor archive at $vendor_store_path" >&2

Expand All @@ -124,28 +91,41 @@ ensure_vendor_archive() {
return 0
fi

# Expected archive is missing, attempt to download
echo "Expected vendor archive missing at $vendor_store_path. Attempting to download..." >&2
downloaded_archive=$(redownload_vendor_archive)
if [ -n "$downloaded_archive" ] && [ -f "$downloaded_archive" ]; then
echo "$downloaded_archive"
return 0
fi

# No vendor archive available
echo "No vendor archive available" >&2
echo "No vendor archive available at $vendor_store_path" >&2
return 1
}

# Restores the node_modules directory from a backup file
# Args:
# $1 - Path to the backup file (tar.xz format)
# $1 - Path to the backup file (tar.xz format) [optional - if not provided, uses vendor store path]
# Returns:
# 0 on success, 1 on failure
# Note: Requires 1.5x the backup size in free space for safe extraction
restore_dependencies() {
backup_file="$1"

# If no backup file provided, get it from vendor store path
if [ -z "$backup_file" ]; then
local info
local vendor_store_path=""

# Get archive information
if ! mapfile -t info < <(get_archive_information); then
echo "Error: Failed to get vendor archive information. Skipping restore." >&2
return 0
fi

vendor_store_path="${info[1]}"

if [ -z "$vendor_store_path" ]; then
echo "Vendor store path is undefined. Skipping restore."
return 0
fi

backup_file="$vendor_store_path"
fi

# Check if backup file exists
if [ ! -f "$backup_file" ]; then
echo "Backup file not found at '$backup_file'. Skipping restore."
Expand Down Expand Up @@ -191,7 +171,6 @@ archive_dependencies() {
# Define all local variables at the top
local info
local api_version=""
local vendor_store_url=""
local vendor_store_path=""
local source_dir="$DEPENDENCIES_DIR"
local archive_file=""
Expand All @@ -202,8 +181,7 @@ archive_dependencies() {
return 1
else
api_version="${info[0]}"
vendor_store_url="${info[1]}"
vendor_store_path="${info[2]}"
vendor_store_path="${info[1]}"

archive_file="$vendor_store_path"
fi
Expand Down Expand Up @@ -260,13 +238,8 @@ ensure() {
# Main logic
case "$1" in
'restore')
if [ -n "$2" ]; then
restore_dependencies "$2"
exit $?
else
echo "Usage: $0 restore <archive_path>"
exit 1
fi
restore_dependencies "$2"
exit $?
;;
'archive')
archive_dependencies
Expand All @@ -291,7 +264,7 @@ case "$1" in
fi
;;
*)
echo "Usage: $0 {restore|archive|ensure|cleanup|redownload}"
echo "Usage: $0 {restore [archive_path]|archive|ensure|cleanup|redownload}"
exit 1
;;
esac
Loading