Skip to content

fix: Enhance PR plugin script to handle file conflicts #2533

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.

Sign up for GitHub

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

Open
elibosley wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix: Enhance PR plugin script to handle file conflicts #2533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f2d31f5
fix: Enhance PR plugin script to handle file conflicts and improve ba…
elibosley Jan 23, 2026
54cc292
fix: Add a blank line for testing build script
elibosley Jan 23, 2026
2ea5ae8
fix: Update build workflow to include rc.d directory and streamline t…
elibosley Jan 23, 2026
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
80 changes: 77 additions & 3 deletions .github/scripts/generate-pr-plugin.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ if [ -f "$MANIFEST" ]; then
while IFS='|' read -r system_file backup_file; do
if [ "$backup_file" == "NEW" ]; then
# This was a new file from previous version, remove it
if [ -f "$system_file" ]; then
if [ -e "$system_file" ] || [ -L "$system_file" ]; then
echo "Removing PR file: $system_file"
rm -f "$system_file"
fi
Expand All @@ -95,6 +95,8 @@ if [ -f "$MANIFEST" ]; then
if [ -f "$backup_file" ]; then
echo "Restoring original: $system_file"
cp -fp "$backup_file" "$system_file"
else
echo "⚠️ Missing backup for: $system_file"
fi
fi
done < "$MANIFEST"
Expand Down Expand Up @@ -172,6 +174,76 @@ echo ""
# Get file list
tar -tzf "$TARBALL" > /tmp/plugin_files.txt

# Abort if any files are already managed by another PR plugin
OTHER_MANIFESTS="/tmp/pr_plugin_existing_files.txt"
> "$OTHER_MANIFESTS"
for plugin_dir in /boot/config/plugins/webgui-pr-*; do
if [ ! -d "$plugin_dir" ]; then
continue
fi
if [ "$plugin_dir" == "/boot/config/plugins/webgui-pr-PR_PLACEHOLDER" ]; then
continue
fi
manifest="$plugin_dir/installed_files.txt"
if [ -f "$manifest" ]; then
plugin_name=$(basename "$plugin_dir")
while IFS='|' read -r other_file _; do
if [ -n "$other_file" ]; then
echo "${other_file}|${plugin_name}" >> "$OTHER_MANIFESTS"
fi
done < "$manifest"
fi
done

if [ -s "$OTHER_MANIFESTS" ]; then
declare -A existing_files
while IFS='|' read -r existing_file existing_plugin; do
if [ -z "$existing_file" ] || [ -z "$existing_plugin" ]; then
continue
fi
if [ -n "${existing_files[$existing_file]:-}" ]; then
if [[ ",${existing_files[$existing_file]}," != *",${existing_plugin},"* ]]; then
existing_files[$existing_file]="${existing_files[$existing_file]},${existing_plugin}"
fi
else
existing_files[$existing_file]="$existing_plugin"
fi
done < "$OTHER_MANIFESTS"

conflict_found=0
declare -A conflict_plugins
while IFS= read -r file; do
# Skip directories
if [[ "$file" == */ ]]; then
continue
fi

system_file="/${file}"
if [ -n "${existing_files[$system_file]:-}" ]; then
conflict_found=1
echo "Conflict: $system_file is already managed by ${existing_files[$system_file]}"
IFS=',' read -r -a plugin_list <<< "${existing_files[$system_file]}"
for plugin in "${plugin_list[@]}"; do
conflict_plugins[$plugin]=1
done
fi
done < /tmp/plugin_files.txt

if [ "$conflict_found" -eq 1 ]; then
echo ""
echo "❌ Install aborted."
echo "One or more files are already managed by another PR plugin."
echo "Please uninstall the conflicting plugin(s) and try again:"
for plugin in "${!conflict_plugins[@]}"; do
echo " plugin remove ${plugin}.plg"
done
rm -f "$OTHER_MANIFESTS" /tmp/plugin_files.txt
exit 1
fi
fi

rm -f "$OTHER_MANIFESTS"

# Backup original files BEFORE extraction
while IFS= read -r file; do
# Skip directories
Expand Down Expand Up @@ -327,15 +399,17 @@ if [ -f "$MANIFEST" ]; then
while IFS='|' read -r system_file backup_file; do
if [ "$backup_file" == "NEW" ]; then
# This was a new file, remove it
if [ -f "$system_file" ]; then
if [ -e "$system_file" ] || [ -L "$system_file" ]; then
echo "Removing new file: $system_file"
rm -f "$system_file"
fi
else
# Restore from backup
if [ -f "$backup_file" ]; then
echo "Restoring: $system_file"
mv -f "$backup_file" "$system_file"
cp -fp "$backup_file" "$system_file"
else
echo "⚠️ Missing backup for: $system_file"
fi
fi
done < "$MANIFEST"
Expand Down
20 changes: 13 additions & 7 deletions .github/workflows/pr-plugin-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ on:
pull_request:
paths:
- 'emhttp/**'
- 'etc/**'
- 'sbin/**'
- 'share/**'
- '.github/workflows/pr-plugin-build.yml'
- '.github/scripts/**'

Expand Down Expand Up @@ -87,6 +90,9 @@ jobs:
if [ -f "$file" ]; then
# Create directory structure in build with target mappings
case "$file" in
etc/rc.d/*)
target_dir="build/usr/local/etc/rc.d"
;;
etc/*)
target_dir="build/${file%/*}"
;;
Expand All @@ -112,18 +118,18 @@ jobs:
fi
done < changed_files.txt

# Generate file list for plugin/tar (relative paths)
find build -type f | sed 's|^build/||' > file_list.txt
echo "File list for plugin:"
cat file_list.txt

# Create tarball - consistent filename for updates
cd build
echo "Creating tarball with contents:"
find usr/ -type f
tar -czf ../${{ steps.version.outputs.local_txz }} usr/
cat ../file_list.txt
tar -czf ../${{ steps.version.outputs.local_txz }} -T ../file_list.txt
Comment on lines +121 to +130
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 23, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Guard against empty file list before tar creation.

If all changed files fall into the "Skipping unsupported path" case (line 111-112), file_list.txt will be empty. While tar -T with an empty file may create an empty archive rather than failing, this likely isn't the desired behavior.

Proposed fix 
  # Generate file list for plugin/tar (relative paths)
  find build -type f | sed 's|^build/||' > file_list.txt
+ if [ ! -s file_list.txt ]; then
+   echo "Error: No files to package"
+   exit 1
+ fi
  echo "File list for plugin:"
  cat file_list.txt
📝 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.

Suggested change
# Generate file list for plugin/tar (relative paths)
find build -type f | sed 's|^build/||' > file_list.txt
echo "File list for plugin:"
cat file_list.txt
# Create tarball - consistent filename for updates
cd build
echo "Creating tarball with contents:"
find usr/ -type f
tar -czf ../${{ steps.version.outputs.local_txz }} usr/
cat ../file_list.txt
tar -czf ../${{ steps.version.outputs.local_txz }} -T ../file_list.txt
# Generate file list for plugin/tar (relative paths)
find build -type f | sed 's|^build/||' > file_list.txt
if [ ! -s file_list.txt ]; then
echo "Error: No files to package"
exit 1
fi
echo "File list for plugin:"
cat file_list.txt
# Create tarball - consistent filename for updates
cd build
echo "Creating tarball with contents:"
cat ../file_list.txt
tar -czf ../${{ steps.version.outputs.local_txz }} -T ../file_list.txt
🤖 Prompt for AI Agents 
In @.github/workflows/pr-plugin-build.yml around lines 121 - 130, The script can
produce an empty file_list.txt which would make tar create an unintended empty
archive; before running the tar command (the lines that cd build and call tar
-czf ../${{ steps.version.outputs.local_txz }} -T ../file_list.txt) add a guard
that checks file_list.txt is non-empty (e.g. test -s ../file_list.txt) and if
it's empty print a clear message and exit with non-zero status (or skip artifact
creation) so the pipeline fails loudly instead of producing an empty tar; update
the surrounding echo/logging to reflect the decision.

cd ..

# Generate file list for plugin
find build/etc build/usr/local/emhttp build/usr/local/sbin build/usr/local/share -type f | sed 's|^build||' > file_list.txt
echo "File list for plugin:"
cat file_list.txt

# Verify tarball contents
echo "Tarball contents:"
tar -tzf ${{ steps.version.outputs.local_txz }}
Expand Down
1 change: 1 addition & 0 deletions etc/rc.d/rc.udev
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# Set the path.
PATH=/sbin:/bin


# run & log functions
. /etc/rc.d/rc.runlog

Expand Down
Loading