Skip to content

feat: allow multiple architectures from netinstall-cli 7.16 #5

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

Closed
clorichel wants to merge 2 commits into from
Closed

feat: allow multiple architectures from netinstall-cli 7.16 #5

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3c46b82
feat: allow multiple architectures from netinstall-cli 7.16
clorichel Oct 9, 2024
27c5574
chore: improve readability
clorichel Oct 9, 2024
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
24 changes: 17 additions & 7 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
ARCH ?= arm
ARCH ?= arm arm64
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

With a multi-item default like ARCH ?= arm arm64, any recursive $(MAKE) ... ARCH=$(ARCH) invocations will be parsed by the shell as multiple words (e.g., make ... ARCH=arm arm64 ...), causing arm64 to be treated as an extra make goal and dropping the intended multi-arch value. Quote/escape the assignment in recursive make calls (e.g., ARCH='$(ARCH)') or avoid space-separated lists for ARCH in those paths.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

Changing the default from a single ARCH to arm arm64 increases the default downloads/work (multiple routeros/all_packages artifacts) and makes the README’s documented defaults (e.g., ARCH ?= arm) inaccurate. If the new default is intentional, the documentation/config guidance should be updated; otherwise consider keeping a single-arch default and letting users opt into multi-arch explicitly.

Copilot uses AI. Check for mistakes.
PKGS ?= wifi-qcom-ac
CHANNEL ?= stable
OPTS ?= -b -r
Expand All @@ -10,18 +10,28 @@ URLVER ?= https://upgrade.mikrotik.com/routeros/NEWESTa7
channel_ver = $(firstword $(shell wget -q -O - $(URLVER).$(1)))
VER ?= $(call channel_ver,$(CHANNEL))
VER_NETINSTALL ?= $(call channel_ver,$(CHANNEL))
PKGS_FILES := $(foreach pkg, $(PKGS), $(pkg)-$(VER)-$(ARCH).npk)
PKGS_FILES := $(foreach somearch, $(ARCH), $(foreach pkg, $(PKGS), $(pkg)-$(VER)-$(somearch).npk))

QEMU ?= ./i386
PLATFORM ?= $(shell uname -m)
FIRST_MULTIARCH_NETINSTALL_VER ?= 7.16

.PHONY: run all service download clean nothing dump extra-packages stable long-term testing arm arm64 mipsbe mmips smips ppc tile x86
.SUFFIXES:

define compare_versions
$(if $(findstring $(word 1,$(sort $(1) $(2))),$(2)),true,false)
endef
Comment on lines +22 to +24
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

compare_versions returns the literal strings true/false. In GNU make, $(if ...) treats any non-empty string (including false) as truthy, so the version gate at run will never take the error branch. Also, using sort + findstring is not a reliable version comparator (e.g., 7.10 vs 7.9, and substring cases like 7.1 vs 7.16). Consider returning an empty string for the false case and using a real version comparison (e.g., sort -V in a $(shell ...) helper) that can handle rc/beta suffixes correctly.

Copilot uses AI. Check for mistakes.

run: all
$(eval PKGS_FILES := $(shell for file in $(PKGS_FILES); do if [ -e "./$$file" ]; then echo "$$file"; fi; done))
@echo starting netinstall... PLATFORM=$(PLATFORM) ARCH=$(ARCH) VER=$(VER) OPTS="$(OPTS)" NET_OPTS="$(NET_OPTS)" PKGS=$(PKGS)
@echo using $(PKGS_FILES)

$(if $(call compare_versions,$(VER_NETINSTALL),$(FIRST_MULTIARCH_NETINSTALL_VER)), , \
$(if $(findstring ,$(wordlist 2,2,$(ARCH))),, \
$(error "You cannot have multiple ARCH items if you use netinstall-cli < 7.16")))
Comment on lines +31 to +33
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

This check for a second ARCH item is broken: $(findstring ,$(wordlist 2,2,$(ARCH))) uses an empty search string, which evaluates empty and will not correctly detect multi-item ARCH values. Use a non-empty test like $(word 2,$(ARCH)) (or $(words $(ARCH)) > 1) so the error only triggers when multiple architectures are actually configured.

Copilot uses AI. Check for mistakes.

$(if $(findstring x86_64, $(PLATFORM)), , $(QEMU)) ./netinstall-cli-$(VER_NETINSTALL) $(OPTS) $(NET_OPTS) routeros-$(VER)-$(ARCH).npk $(PKGS_FILES) $(PKGS_CUSTOM)
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

routeros-$(VER)-$(ARCH).npk will break when ARCH contains multiple items (it expands to a space-separated string, which the shell splits into multiple arguments and invalid filenames). If multi-arch netinstall is intended, build a list of routeros NPKs similar to PKGS_FILES and pass that list instead of interpolating the raw ARCH variable into a single filename.

Copilot uses AI. Check for mistakes.

service: all
Expand All @@ -30,7 +40,7 @@ service: all
download: all
@echo use 'make' to run netinstall after connecting $(IFACE) or $(CLIENTIP) to router

all: routeros-$(VER)-$(ARCH).npk netinstall-cli-$(VER_NETINSTALL) all_packages-$(ARCH)-$(VER).zip
all: $(foreach somearch,$(ARCH),routeros-$(VER)-$(somearch).npk netinstall-cli-$(VER_NETINSTALL) all_packages-$(somearch)-$(VER).zip)
@echo finished download ARCH=$(ARCH) VER=$(VER) PKGS=$(PKGS) PLATFORM=$(PLATFORM)
Comment on lines +43 to 44
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

all prerequisites are generated with a foreach that includes netinstall-cli-$(VER_NETINSTALL) inside the loop, duplicating it once per architecture. It’s cleaner and less error-prone to list netinstall-cli-$(VER_NETINSTALL) once, plus per-arch routeros/all_packages prerequisites separately.

Copilot uses AI. Check for mistakes.

dump:
Expand All @@ -49,12 +59,12 @@ netinstall-cli-$(VER_NETINSTALL): netinstall-$(VER_NETINSTALL).tar.gz
mv netinstall-cli netinstall-cli-$(VER_NETINSTALL)
touch netinstall-cli-$(VER_NETINSTALL)

routeros-$(VER)-$(ARCH).npk:
routeros-$(VER)-%.npk:
wget https://download.mikrotik.com/routeros/$(VER)/$@

all_packages-$(ARCH)-$(VER).zip:
all_packages-%-$(VER).zip:
wget https://download.mikrotik.com/routeros/$(VER)/$@
unzip -o all_packages-$(ARCH)-$(VER).zip
unzip -o all_packages-$*-$(VER).zip

stable long-term testing:
$(MAKE) $(filter-out $@,$(MAKECMDGOALS)) CHANNEL=$@ ARCH=$(ARCH)
Expand All @@ -63,4 +73,4 @@ arm arm64 mipsbe mmips smips ppc tile x86:
$(MAKE) $(filter-out $@,$(MAKECMDGOALS)) CHANNEL=$(CHANNEL) ARCH=$@

nothing:
while :; do sleep 3600; done
while :; do sleep 3600; done