RNS-Things/reticulum-meshchatX
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases 8 Wiki Activity

Legacy support #23

Merged
Sudo-Ivan merged 7 commits from legacy-support into master 2025-12-06 05:34:20 +00:00
Conversation 3 Commits 7 Files Changed 4 +110 -11
4 changed files with 110 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

61
.github/workflows/build.yml vendored
View File

@@ -76,24 +76,48 @@ jobs:
- name: windows
os: windows-latest
node: 22
python: "3.12"
python: "3.13"
release_artifacts: "dist/*-win-installer.exe,dist/*-win-portable.exe"
build_input: build_windows
dist_script: dist-prebuilt
variant: standard
electron_version: "39.2.4"
- name: mac
os: macos-14
node: 18
python: "3.11"
node: 22
python: "3.13"
release_artifacts: "dist/*-mac-*.dmg"
build_input: build_mac
dist_script: dist:mac-universal
variant: standard
electron_version: "39.2.4"
- name: linux
os: ubuntu-latest
node: 22
python: "3.12"
python: "3.13"
release_artifacts: "dist/*-linux.AppImage,dist/*-linux.deb,python-dist/*.whl"
build_input: build_linux
dist_script: dist-prebuilt
variant: standard
electron_version: "39.2.4"
- name: windows-legacy
os: windows-latest
node: 18
python: "3.11"
release_artifacts: "dist/*-win-installer*.exe,dist/*-win-portable*.exe"
build_input: build_windows
dist_script: dist-prebuilt
variant: legacy
electron_version: "30.0.8"
- name: linux-legacy
os: ubuntu-latest
node: 18
python: "3.11"
release_artifacts: "dist/*-linux*.AppImage,dist/*-linux*.deb,python-dist/*.whl"
build_input: build_linux
dist_script: dist-prebuilt
variant: legacy
electron_version: "30.0.8"
permissions:
contents: write
steps:
@@ -103,6 +127,16 @@ jobs:
(github.event_name == 'workflow_dispatch' && inputs[matrix.build_input] == true)
uses: actions/checkout@50fbc622fc4ef5163becd7fab6573eac35f8462e # v1
- name: Set legacy Electron version
if: |
matrix.variant == 'legacy' &&
(github.event_name == 'push' ||
(github.event_name == 'workflow_dispatch' && inputs[matrix.build_input] == true))
shell: bash
run: |
node -e "const fs=require('fs');const pkg=require('./package.json');pkg.devDependencies.electron='${{ matrix.electron_version }}';fs.writeFileSync('package.json', JSON.stringify(pkg,null,2));"
if [ -f package-lock.json ]; then rm package-lock.json; fi
- name: Install NodeJS
if: |
github.event_name == 'push' ||
@@ -160,14 +194,14 @@ jobs:
- name: Install patchelf
if: |
matrix.name == 'linux' &&
startsWith(matrix.name, 'linux') &&
(github.event_name == 'push' ||
(github.event_name == 'workflow_dispatch' && inputs[matrix.build_input] == true))
run: sudo apt-get update && sudo apt-get install -y patchelf
- name: Build Python wheel
if: |
matrix.name == 'linux' &&
startsWith(matrix.name, 'linux') &&
(github.event_name == 'push' ||
(github.event_name == 'workflow_dispatch' && inputs[matrix.build_input] == true))
run: |
@@ -182,6 +216,13 @@ jobs:
(github.event_name == 'workflow_dispatch' && inputs[matrix.build_input] == true)
coderabbitai[bot] commented 2025-12-06 05:17:51 +00:00 (Migrated from github.com)
Review
Copy Link

🛠️ Refactor suggestion | 🟠 Major

Artifact renaming logic duplicates Makefile implementation; extract to shared script for maintainability.

The "Rename artifacts for legacy build" step duplicates the renaming logic from the Makefile's build-appimage-legacy and build-exe-legacy targets. The workflow version:

  • Uses bash shopt -s nullglob for safe glob matching (good practice).
  • Renames all legacy artifacts with a -legacy suffix.

The Makefile version does the same but is called only if the Makefile targets are invoked directly (which the workflow does not do).

Recommendation: Extract the renaming logic into a shared shell script (e.g., scripts/rename_legacy_artifacts.sh) and invoke it from both the Makefile and the workflow:

# scripts/rename_legacy_artifacts.sh
#!/bin/bash
set -e
shopt -s nullglob
for f in dist/*-win-installer.exe dist/*-win-portable.exe dist/*-linux.AppImage dist/*-linux.deb; do
  [ -e "$f" ] || continue
  dir=$(dirname "$f")
  base=$(basename "$f")
  ext="${base##*.}"
  name="${base%.$ext}"
  mv "$f" "$dir/${name}-legacy.${ext}"
done

Then in the workflow (line 225–234):

run: bash scripts/rename_legacy_artifacts.sh

And in the Makefile (lines 54–58, 63–67):

	bash scripts/rename_legacy_artifacts.sh

This eliminates duplication and makes future updates easier.

🤖 Prompt for AI Agents 
.github/workflows/build.yml lines 219-234: the workflow duplicates Makefile
artifact-renaming logic—extract that loop into a shared executable script (e.g.,
scripts/rename_legacy_artifacts.sh) containing the bash renaming logic, make it
executable, replace the inline run block in this workflow with a call to execute
that script, and update the corresponding Makefile targets to call the same
script instead of duplicating the loop so both the CI workflow and Makefile
reuse the single canonical implementation.

âś… Addressed in commits bf8c22c to d97676a

_🛠️ Refactor suggestion_ | _🟠 Major_ **Artifact renaming logic duplicates Makefile implementation; extract to shared script for maintainability.** The "Rename artifacts for legacy build" step duplicates the renaming logic from the Makefile's `build-appimage-legacy` and `build-exe-legacy` targets. The workflow version: - Uses bash `shopt -s nullglob` for safe glob matching (good practice). - Renames all legacy artifacts with a `-legacy` suffix. The Makefile version does the same but is called only if the Makefile targets are invoked directly (which the workflow does not do). **Recommendation:** Extract the renaming logic into a shared shell script (e.g., `scripts/rename_legacy_artifacts.sh`) and invoke it from both the Makefile and the workflow: ```bash # scripts/rename_legacy_artifacts.sh #!/bin/bash set -e shopt -s nullglob for f in dist/*-win-installer.exe dist/*-win-portable.exe dist/*-linux.AppImage dist/*-linux.deb; do [ -e "$f" ] || continue dir=$(dirname "$f") base=$(basename "$f") ext="${base##*.}" name="${base%.$ext}" mv "$f" "$dir/${name}-legacy.${ext}" done ``` Then in the workflow (line 225–234): ```yaml run: bash scripts/rename_legacy_artifacts.sh ``` And in the Makefile (lines 54–58, 63–67): ```makefile bash scripts/rename_legacy_artifacts.sh ``` This eliminates duplication and makes future updates easier. <details> <summary>🤖 Prompt for AI Agents</summary> ``` .github/workflows/build.yml lines 219-234: the workflow duplicates Makefile artifact-renaming logic—extract that loop into a shared executable script (e.g., scripts/rename_legacy_artifacts.sh) containing the bash renaming logic, make it executable, replace the inline run block in this workflow with a call to execute that script, and update the corresponding Makefile targets to call the same script instead of duplicating the loop so both the CI workflow and Makefile reuse the single canonical implementation. ``` </details> <!-- fingerprinting:phantom:triton:mongoose --> <!-- This is an auto-generated comment by CodeRabbit --> ✅ Addressed in commits bf8c22c to d97676a
run: npm run ${{ matrix.dist_script }}
- name: Rename artifacts for legacy build
if: |
matrix.variant == 'legacy' &&
(github.event_name == 'push' ||
(github.event_name == 'workflow_dispatch' && inputs[matrix.build_input] == true))
run: ./scripts/rename_legacy_artifacts.sh
- name: Upload build artifacts
if: |
github.event_name == 'push' ||
@@ -190,11 +231,11 @@ jobs:
with:
name: build-${{ matrix.name }}
path: |
dist/*-win-installer.exe
dist/*-win-portable.exe
dist/*-win-installer*.exe
dist/*-win-portable*.exe
dist/*-mac-*.dmg
dist/*-linux.AppImage
dist/*-linux.deb
dist/*-linux*.AppImage
dist/*-linux*.deb
python-dist/*.whl
if-no-files-found: ignore

22
.github/workflows/dependency-review.yml vendored Normal file
View File

@@ -0,0 +1,22 @@
name: 'Dependency review'
on:
pull_request:
branches: [ "master" ]
permissions:
contents: read
pull-requests: write
jobs:
dependency-review:
runs-on: ubuntu-latest
steps:
- name: 'Checkout repository'
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
- name: 'Dependency Review'
uses: actions/dependency-review-action@3c4e3dcb1aa7874d2c16be7d79418e9b7efd6261 # v4
with:
comment-summary-in-pr: always

17
Makefile
View File

@@ -1,8 +1,9 @@
.PHONY: install run develop clean build build-appimage build-exe dist sync-version wheel node_modules python build-docker run-docker
.PHONY: install run develop clean build build-appimage build-exe dist sync-version wheel node_modules python build-docker run-docker electron-legacy build-appimage-legacy build-exe-legacy
PYTHON ?= python
POETRY = $(PYTHON) -m poetry
NPM = npm
LEGACY_ELECTRON_VERSION ?= 30.0.8
DOCKER_COMPOSE_CMD ?= docker compose
DOCKER_COMPOSE_FILE ?= docker-compose.yml
@@ -44,6 +45,20 @@ build-exe: build
coderabbitai[bot] commented 2025-12-06 05:17:51 +00:00 (Migrated from github.com)
Review
Copy Link

🛠️ Refactor suggestion | 🟠 Major

Artifact renaming logic is duplicated between Makefile and workflow; clarify intended usage.

The artifact renaming logic (bash pattern matching to append -legacy suffix) appears identically in both:

  • Makefile: lines 54–58 (AppImage/deb) and 63–67 (Windows exe)
  • Workflow: lines 226–234 (all platforms)

The workflow does not invoke these Makefile targets; it runs npm commands directly and performs its own renaming. This duplication complicates maintenance—if the renaming logic needs to change, updates must be made in two places.

Recommendation: If the Makefile targets are meant only for local development, add a comment documenting this. Alternatively, consider extracting the renaming logic into a shared script that both the Makefile and workflow invoke, or remove the Makefile targets if they are unused in the actual CI/CD pipeline.

For now, consider adding a comment above the legacy targets:

+# Legacy build targets for local development.
+# Note: The GitHub Actions workflow does not invoke these targets;
+# it runs npm commands directly and performs artifact renaming inline.
 electron-legacy:
 	$(NPM) install --no-save electron@$(LEGACY_ELECTRON_VERSION)

Also, consider handling the case where no files match the glob pattern. The workflow uses shopt -s nullglob to avoid errors if no files match, but the Makefile targets do not. Add the nullglob option to the Makefile loops:

 build-appimage-legacy: build electron-legacy
 	$(NPM) run electron-postinstall
 	$(NPM) run dist -- --linux AppImage
-	@set -e; for f in dist/*-linux.AppImage dist/*-linux.deb; do \
+	@set -e; shopt -s nullglob; for f in dist/*-linux.AppImage dist/*-linux.deb; do \
 		[ -e "$$f" ] || continue; \
 		dir=$$(dirname "$$f"); base=$$(basename "$$f"); ext=$${base##*.}; name=$${base%.$$ext}; \
 		mv "$$f" "$$dir/$${name}-legacy.$$ext"; \
 	done

Note: The [ -e "$$f" ] check makes the nullglob redundant, so this is a minor improvement for clarity. The current code is safe.

đź“ť 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.

# Legacy build targets for local development.
# Note: The GitHub Actions workflow does not invoke these targets;
# it runs npm commands directly and performs artifact renaming inline.
electron-legacy:
	$(NPM) install --no-save electron@$(LEGACY_ELECTRON_VERSION)

build-appimage-legacy: build electron-legacy
	$(NPM) run electron-postinstall
	$(NPM) run dist -- --linux AppImage
	@set -e; shopt -s nullglob; for f in dist/*-linux.AppImage dist/*-linux.deb; do \
		[ -e "$$f" ] || continue; \
		dir=$$(dirname "$$f"); base=$$(basename "$$f"); ext=$${base##*.}; name=$${base%.$$ext}; \
		mv "$$f" "$$dir/$${name}-legacy.$$ext"; \
	done

build-exe-legacy: build electron-legacy
	$(NPM) run electron-postinstall
	$(NPM) run dist -- --win portable
	@set -e; shopt -s nullglob; for f in dist/*-win-installer.exe dist/*-win-portable.exe; do \
		[ -e "$$f" ] || continue; \
		dir=$$(dirname "$$f"); base=$$(basename "$$f"); ext=$${base##*.}; name=$${base%.$$ext}; \
		mv "$$f" "$$dir/$${name}-legacy.$$ext"; \
	done
🤖 Prompt for AI Agents 
In Makefile around lines 48–67 the artifact-renaming bash loops for legacy
builds duplicate logic that also exists in the CI workflow and lack an explicit
note about intended usage; update this section by adding a brief comment that
these legacy targets are intended for local/manual use (or remove them if unused
in CI), and refactor the renaming into a single shared script (e.g.,
scripts/rename-legacy-artifacts.sh) that both the Makefile targets and the
workflow invoke to avoid duplication; additionally, make the Makefile loop
robust by enabling nullglob in the shell invocation or otherwise ensuring no-op
behavior when globs match nothing (the existing [ -e "$$f" ] guard is acceptable
but document the choice).

âś… Addressed in commits bf8c22c to d97676a

_🛠️ Refactor suggestion_ | _🟠 Major_ **Artifact renaming logic is duplicated between Makefile and workflow; clarify intended usage.** The artifact renaming logic (bash pattern matching to append `-legacy` suffix) appears identically in both: - Makefile: lines 54–58 (AppImage/deb) and 63–67 (Windows exe) - Workflow: lines 226–234 (all platforms) The workflow does not invoke these Makefile targets; it runs npm commands directly and performs its own renaming. This duplication complicates maintenance—if the renaming logic needs to change, updates must be made in two places. **Recommendation:** If the Makefile targets are meant only for local development, add a comment documenting this. Alternatively, consider extracting the renaming logic into a shared script that both the Makefile and workflow invoke, or remove the Makefile targets if they are unused in the actual CI/CD pipeline. For now, consider adding a comment above the legacy targets: ```diff +# Legacy build targets for local development. +# Note: The GitHub Actions workflow does not invoke these targets; +# it runs npm commands directly and performs artifact renaming inline. electron-legacy: $(NPM) install --no-save electron@$(LEGACY_ELECTRON_VERSION) ``` **Also,** consider handling the case where no files match the glob pattern. The workflow uses `shopt -s nullglob` to avoid errors if no files match, but the Makefile targets do not. Add the nullglob option to the Makefile loops: ```diff build-appimage-legacy: build electron-legacy $(NPM) run electron-postinstall $(NPM) run dist -- --linux AppImage - @set -e; for f in dist/*-linux.AppImage dist/*-linux.deb; do \ + @set -e; shopt -s nullglob; for f in dist/*-linux.AppImage dist/*-linux.deb; do \ [ -e "$$f" ] || continue; \ dir=$$(dirname "$$f"); base=$$(basename "$$f"); ext=$${base##*.}; name=$${base%.$$ext}; \ mv "$$f" "$$dir/$${name}-legacy.$$ext"; \ done ``` Note: The `[ -e "$$f" ]` check makes the nullglob redundant, so this is a minor improvement for clarity. The current code is safe. <!-- suggestion_start --> <details> <summary>📝 Committable suggestion</summary> > ‼️ **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. ```suggestion # Legacy build targets for local development. # Note: The GitHub Actions workflow does not invoke these targets; # it runs npm commands directly and performs artifact renaming inline. electron-legacy: $(NPM) install --no-save electron@$(LEGACY_ELECTRON_VERSION) build-appimage-legacy: build electron-legacy $(NPM) run electron-postinstall $(NPM) run dist -- --linux AppImage @set -e; shopt -s nullglob; for f in dist/*-linux.AppImage dist/*-linux.deb; do \ [ -e "$$f" ] || continue; \ dir=$$(dirname "$$f"); base=$$(basename "$$f"); ext=$${base##*.}; name=$${base%.$$ext}; \ mv "$$f" "$$dir/$${name}-legacy.$$ext"; \ done build-exe-legacy: build electron-legacy $(NPM) run electron-postinstall $(NPM) run dist -- --win portable @set -e; shopt -s nullglob; for f in dist/*-win-installer.exe dist/*-win-portable.exe; do \ [ -e "$$f" ] || continue; \ dir=$$(dirname "$$f"); base=$$(basename "$$f"); ext=$${base##*.}; name=$${base%.$$ext}; \ mv "$$f" "$$dir/$${name}-legacy.$$ext"; \ done ``` </details> <!-- suggestion_end --> <details> <summary>🤖 Prompt for AI Agents</summary> ``` In Makefile around lines 48–67 the artifact-renaming bash loops for legacy builds duplicate logic that also exists in the CI workflow and lack an explicit note about intended usage; update this section by adding a brief comment that these legacy targets are intended for local/manual use (or remove them if unused in CI), and refactor the renaming into a single shared script (e.g., scripts/rename-legacy-artifacts.sh) that both the Makefile targets and the workflow invoke to avoid duplication; additionally, make the Makefile loop robust by enabling nullglob in the shell invocation or otherwise ensuring no-op behavior when globs match nothing (the existing [ -e "$$f" ] guard is acceptable but document the choice). ``` </details> <!-- fingerprinting:phantom:triton:mongoose --> <!-- This is an auto-generated comment by CodeRabbit --> ✅ Addressed in commits bf8c22c to d97676a
dist: build-appimage
electron-legacy:
$(NPM) install --no-save electron@$(LEGACY_ELECTRON_VERSION)
# Legacy targets intended for manual/local builds; CI uses workflow jobs.
build-appimage-legacy: build electron-legacy
$(NPM) run electron-postinstall
$(NPM) run dist -- --linux AppImage
./scripts/rename_legacy_artifacts.sh
build-exe-legacy: build electron-legacy
$(NPM) run electron-postinstall
$(NPM) run dist -- --win portable
./scripts/rename_legacy_artifacts.sh
clean:
rm -rf node_modules
rm -rf build

21
scripts/rename_legacy_artifacts.sh Executable file
View File

@@ -0,0 +1,21 @@
#!/usr/bin/env bash
set -euo pipefail
shopt -s nullglob
patterns=(
"dist/*-win-installer.exe"
"dist/*-win-portable.exe"
"dist/*-linux.AppImage"
"dist/*-linux.deb"
)
for pattern in "${patterns[@]}"; do
for f in $pattern; do
dir=$(dirname "$f")
base=$(basename "$f")
ext="${base##*.}"
name="${base%.$ext}"
mv "$f" "$dir/${name}-legacy.${ext}"
done
done