From 981b1ab77632ca02c790ed87badeb6ceabdef332 Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Sun, 22 Sep 2024 19:11:09 +0100 Subject: [PATCH 01/16] check_whence.py: use consistent naming Namely: file(name) and sym(link) ... which were swapped in a few places. One of which by yours truly :facepalm: Signed-off-by: Emil Velikov --- check_whence.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/check_whence.py b/check_whence.py index fd74a56a..c270ee60 100755 --- a/check_whence.py +++ b/check_whence.py @@ -99,7 +99,7 @@ def main(): sys.stderr.write("E: %s listed in WHENCE as File, but is directory\n" % name) ret = 1 - for name in set(fw for fw in whence_files if whence_files.count(fw) > 1): + for name in set(name for name in whence_files if whence_files.count(name) > 1): sys.stderr.write("E: %s listed in WHENCE twice\n" % name) ret = 1 @@ -107,7 +107,7 @@ def main(): sys.stderr.write("E: %s listed in WHENCE twice\n" % name) ret = 1 - for name in set(link for link in whence_files if os.path.islink(link)): + for name in set(file for file in whence_files if os.path.islink(file)): sys.stderr.write("E: %s listed in WHENCE as File, but is a symlink\n" % name) ret = 1 @@ -131,10 +131,10 @@ def main(): break valid_targets.add(dirname) - for name, target in sorted(links_list): + for link, target in sorted(links_list): if target not in valid_targets: sys.stderr.write( - "E: target %s of link %s in WHENCE" " does not exist\n" % (target, name) + "E: target %s of link %s in WHENCE" " does not exist\n" % (target, link) ) ret = 1 From 8a507494f3febe8f6b4e933fc951fdb16e3932df Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Sun, 22 Sep 2024 17:31:24 +0100 Subject: [PATCH 02/16] check_whence.py: ban link-to-a-link Using link-to-a-link reduces legibility and changes to the root link, also changes the leafs. Where the latter may be undesired and in some cases just wrong. We have a couple of instances in-tree, so fix them up and ban the combination. One particularly good example, why we'd want this is: https://gitlab.com/kernel-firmware/linux-firmware/-/merge_requests/272 In there we have the following: File: ti/tas2781/TAS2XXX1EB30.bin [snip] Link: TAS2XXX1EB3.bin -> ti/tas2781/TAS2XXX1EB3.bin Link: ti/tas2781/TAS2XXX1EB3.bin -> TAS2XXX1EB30.bin Link: TAS2XXX1EB30.bin -> ti/tas2781/TAS2XXX1EB30.bin Signed-off-by: Emil Velikov --- WHENCE | 14 +++++++------- check_whence.py | 12 ++++++++++-- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/WHENCE b/WHENCE index e3e085b8..9678e670 100644 --- a/WHENCE +++ b/WHENCE @@ -3699,7 +3699,7 @@ File: ath10k/WCN3990/hw1.0/qcm2290/wlanmdsp.mbn Version: WLAN.HL.3.3.7.c2-00931-QCAHLSWMTPLZ-1 Link: ath10k/WCN3990/hw1.0/qrb4210/wlanmdsp.mbn -> ../qcm2290/wlanmdsp.mbn Link: qcom/qcm2290/wlanmdsp.mbn -> ../../ath10k/WCN3990/hw1.0/qcm2290/wlanmdsp.mbn -Link: qcom/qrb4210/wlanmdsp.mbn -> ../../ath10k/WCN3990/hw1.0/qrb4210/wlanmdsp.mbn +Link: qcom/qrb4210/wlanmdsp.mbn -> ../../ath10k/WCN3990/hw1.0/qcm2290/wlanmdsp.mbn File: ath10k/WCN3990/hw1.0/qcm2290/firmware-5.bin Link: ath10k/WCN3990/hw1.0/qrb4210/firmware-5.bin -> ../qcm2290/firmware-5.bin @@ -5227,11 +5227,11 @@ Link: nvidia/gp104/acr/bl.bin -> ../../gp102/acr/bl.bin Link: nvidia/gp104/acr/ucode_load.bin -> ../../gp102/acr/ucode_load.bin Link: nvidia/gp104/acr/ucode_unload.bin -> ../../gp102/acr/ucode_unload.bin Link: nvidia/gp104/acr/unload_bl.bin -> ../../gp102/acr/unload_bl.bin -Link: nvidia/gp104/gr/fecs_bl.bin -> ../../gp102/gr/fecs_bl.bin +Link: nvidia/gp104/gr/fecs_bl.bin -> ../../gm200/gr/fecs_bl.bin File: nvidia/gp104/gr/fecs_data.bin File: nvidia/gp104/gr/fecs_inst.bin File: nvidia/gp104/gr/fecs_sig.bin -Link: nvidia/gp104/gr/gpccs_bl.bin -> ../../gp102/gr/gpccs_bl.bin +Link: nvidia/gp104/gr/gpccs_bl.bin -> ../../gm200/gr/gpccs_bl.bin File: nvidia/gp104/gr/gpccs_data.bin File: nvidia/gp104/gr/gpccs_inst.bin File: nvidia/gp104/gr/gpccs_sig.bin @@ -5250,11 +5250,11 @@ Link: nvidia/gp106/acr/bl.bin -> ../../gp102/acr/bl.bin Link: nvidia/gp106/acr/ucode_load.bin -> ../../gp102/acr/ucode_load.bin Link: nvidia/gp106/acr/ucode_unload.bin -> ../../gp102/acr/ucode_unload.bin Link: nvidia/gp106/acr/unload_bl.bin -> ../../gp102/acr/unload_bl.bin -Link: nvidia/gp106/gr/fecs_bl.bin -> ../../gp102/gr/fecs_bl.bin +Link: nvidia/gp106/gr/fecs_bl.bin -> ../../gm200/gr/fecs_bl.bin File: nvidia/gp106/gr/fecs_data.bin Link: nvidia/gp106/gr/fecs_inst.bin -> ../../gp102/gr/fecs_inst.bin File: nvidia/gp106/gr/fecs_sig.bin -Link: nvidia/gp106/gr/gpccs_bl.bin -> ../../gp102/gr/gpccs_bl.bin +Link: nvidia/gp106/gr/gpccs_bl.bin -> ../../gm200/gr/gpccs_bl.bin File: nvidia/gp106/gr/gpccs_data.bin Link: nvidia/gp106/gr/gpccs_inst.bin -> ../../gp102/gr/gpccs_inst.bin File: nvidia/gp106/gr/gpccs_sig.bin @@ -5998,10 +5998,10 @@ File: netronome/flower/nic_AMDA0096.nffw File: netronome/flower/nic_AMDA0097.nffw File: netronome/flower/nic_AMDA0058.nffw Link: netronome/flower/nic_AMDA0081.nffw -> nic_AMDA0097.nffw -Link: netronome/flower/nic_AMDA0081-0001_1x40.nffw -> nic_AMDA0081.nffw +Link: netronome/flower/nic_AMDA0081-0001_1x40.nffw -> nic_AMDA0097.nffw Link: netronome/flower/nic_AMDA0097-0001_2x40.nffw -> nic_AMDA0097.nffw Link: netronome/flower/nic_AMDA0099-0001_2x10.nffw -> nic_AMDA0099.nffw -Link: netronome/flower/nic_AMDA0081-0001_4x10.nffw -> nic_AMDA0081.nffw +Link: netronome/flower/nic_AMDA0081-0001_4x10.nffw -> nic_AMDA0097.nffw Link: netronome/flower/nic_AMDA0097-0001_4x10_1x40.nffw -> nic_AMDA0097.nffw Link: netronome/flower/nic_AMDA0099-0001_2x25.nffw -> nic_AMDA0099.nffw Link: netronome/flower/nic_AMDA0096-0001_2x10.nffw -> nic_AMDA0096.nffw diff --git a/check_whence.py b/check_whence.py index c270ee60..d7742f1e 100755 --- a/check_whence.py +++ b/check_whence.py @@ -115,12 +115,20 @@ def main(): sys.stderr.write("E: %s listed in WHENCE as Link, is in tree\n" % name) ret = 1 + invalid_targets = set(link[0] for link in links_list) + for link, target in sorted(links_list): + if target in invalid_targets: + sys.stderr.write( + "E: target %s of link %s is also a link\n" % (target, link) + ) + ret = 1 + for name in sorted(list(known_files - git_files)): sys.stderr.write("E: %s listed in WHENCE does not exist\n" % name) ret = 1 - # A link can point to another link, or to a file... - valid_targets = set(link[0] for link in links_list) | git_files + # A link can point to a file... + valid_targets = set(git_files) # ... or to a directory for target in set(valid_targets): From 1dbbc8fb826775b6ad8e77aa76dc19f67c6aff97 Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Sun, 22 Sep 2024 17:29:49 +0100 Subject: [PATCH 03/16] check_whence.py: LC_ALL=C sort -u the filelist Sort the file list, so it's easier to manage. Signed-off-by: Emil Velikov --- check_whence.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/check_whence.py b/check_whence.py index d7742f1e..dbbb68f7 100755 --- a/check_whence.py +++ b/check_whence.py @@ -73,23 +73,23 @@ def main(): whence_links = list(zip(*links_list))[0] known_files = set(name for name in whence_list if not name.endswith("/")) | set( [ - ".gitignore", ".codespell.cfg", + ".gitignore", ".gitlab-ci.yml", ".pre-commit-config.yaml", + "Dockerfile", + "Makefile", + "README.md", + "WHENCE", "build_packages.py", "check_whence.py", "configure", - "Makefile", - "README.md", - "copy-firmware.sh", - "WHENCE", - "Dockerfile", + "contrib/process_linux_firmware.py", "contrib/templates/debian.changelog", "contrib/templates/debian.control", "contrib/templates/debian.copyright", "contrib/templates/rpm.spec", - "contrib/process_linux_firmware.py", + "copy-firmware.sh", ] ) known_prefixes = set(name for name in whence_list if name.endswith("/")) From 737e6f37c06570bcf4ed3707baccbcd38a6c03c4 Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Sun, 22 Sep 2024 20:44:59 +0100 Subject: [PATCH 04/16] check_whence.py: annotate replacement strings as raw Otherwise python 3.12 throws warnings like below: .../check_whence.py:40: SyntaxWarning: invalid escape sequence '\ ' yield match.group(1).replace("\ ", " ").replace('"', "") Signed-off-by: Emil Velikov --- check_whence.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/check_whence.py b/check_whence.py index dbbb68f7..64dc1335 100755 --- a/check_whence.py +++ b/check_whence.py @@ -37,7 +37,7 @@ def list_whence_files(): for line in whence: match = re.match(r"(?:RawFile|File):\s*(.*)", line) if match: - yield match.group(1).replace("\ ", " ").replace('"', "") + yield match.group(1).replace(r"\ ", " ").replace('"', "") continue @@ -48,8 +48,8 @@ def list_links_list(): if match: linkname, target = match.group(1).split("->") - linkname = linkname.strip().replace("\ ", " ").replace('"', "") - target = target.strip().replace("\ ", " ").replace('"', "") + linkname = linkname.strip().replace(r"\ ", " ").replace('"', "") + target = target.strip().replace(r"\ ", " ").replace('"', "") # Link target is relative to the link target = os.path.join(os.path.dirname(linkname), target) From a924bda835d1096ecc296e4daaab11eeacc4ea9c Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Sun, 22 Sep 2024 16:02:38 +0100 Subject: [PATCH 05/16] editorconfig: add initial config file It's a simple format handled by practically every supported platform or program out there. Add an initial configuration file, so we reduce the style variation of the files in-tree. Signed-off-by: Emil Velikov --- .editorconfig | 21 +++++++++++++++++++++ check_whence.py | 1 + 2 files changed, 22 insertions(+) create mode 100644 .editorconfig diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 00000000..100bbceb --- /dev/null +++ b/.editorconfig @@ -0,0 +1,21 @@ +# To use this config on you editor, follow the instructions at: +# http://editorconfig.org + +root = true + +[*] +charset = utf-8 +insert_final_newline = true +tab_width = 8 +max_line_length = 90 + +[Makefile] +indent_style = tab + +[*.{sh,py}] +indent_style = space +indent_size = 4 + +[*.{yml,yaml}] +indent_style = space +indent_size = 2 diff --git a/check_whence.py b/check_whence.py index 64dc1335..afe97a76 100755 --- a/check_whence.py +++ b/check_whence.py @@ -74,6 +74,7 @@ def main(): known_files = set(name for name in whence_list if not name.endswith("/")) | set( [ ".codespell.cfg", + ".editorconfig", ".gitignore", ".gitlab-ci.yml", ".pre-commit-config.yaml", From cc4c1fccb0d8f5ac675e16d89a1ab756da671961 Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Sun, 22 Sep 2024 16:06:44 +0100 Subject: [PATCH 06/16] Style update yaml files Signed-off-by: Emil Velikov --- .gitlab-ci.yml | 2 +- .pre-commit-config.yaml | 38 +++++++++++++++++++------------------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index ae96f7d8..e05ef3bb 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -11,7 +11,7 @@ check-commits: stage: test image: registry.gitlab.com/kernel-firmware/linux-firmware rules: - - if: $CI_MERGE_REQUEST_ID + - if: $CI_MERGE_REQUEST_ID script: - ci-fairy check-commits --signed-off-by --textwidth=0 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 7a49d063..370a2970 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,30 +1,30 @@ default_stages: [commit] repos: -- repo: https://github.com/pre-commit/pre-commit-hooks + - repo: https://github.com/pre-commit/pre-commit-hooks rev: v4.0.1 hooks: - - id: check-executables-have-shebangs - - id: forbid-new-submodules - - id: check-yaml - - id: check-symlinks - - id: destroyed-symlinks -- repo: https://github.com/shellcheck-py/shellcheck-py + - id: check-executables-have-shebangs + - id: forbid-new-submodules + - id: check-yaml + - id: check-symlinks + - id: destroyed-symlinks + - repo: https://github.com/shellcheck-py/shellcheck-py rev: v0.9.0.5 hooks: - - id: shellcheck -- repo: https://github.com/ambv/black + - id: shellcheck + - repo: https://github.com/ambv/black rev: 22.6.0 hooks: - - id: black -- repo: https://github.com/igorshubovych/markdownlint-cli + - id: black + - repo: https://github.com/igorshubovych/markdownlint-cli rev: v0.33.0 hooks: - - id: markdownlint - args: ['--fix'] -- repo: local + - id: markdownlint + args: ['--fix'] + - repo: local hooks: - - id: check-whence - name: Check whence - files: 'WHENCE' - language: script - entry: ./check_whence.py + - id: check-whence + name: Check whence + files: 'WHENCE' + language: script + entry: ./check_whence.py From ee8c336ab3ab434908866c9a5e6dbbc555a80f39 Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Sun, 22 Sep 2024 16:21:44 +0100 Subject: [PATCH 07/16] copy-firmware.sh: flesh out and fix dedup-firmware.sh Flesh out the de-duplication logic in separate script. The copy-firmware.sh is already complex enough and de-duplication doesn't really fit in there. In the process we migrate away from the open-coded `ln --relative`. We also avoid touching symlinks, which are not created by rdfind. Otherwise we end up "fixing" the folder to folder symlinks (created earlier in the process) and things explode. As result we also get a few bonuses: - the COPYOPTS shell injection is gone - the variable was never used - people can dedup as separate step if/when they choose to do so Aside: based on the noise in git log and around distros ... I'm wondering if having the de-duplication as opt-in, would have been better. Is it too late to change or the ship has sailed? Signed-off-by: Emil Velikov --- Makefile | 9 ++++---- check_whence.py | 1 + copy-firmware.sh | 23 --------------------- dedup-firmware.sh | 52 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 58 insertions(+), 27 deletions(-) create mode 100755 dedup-firmware.sh diff --git a/Makefile b/Makefile index ac43c97f..216dd488 100644 --- a/Makefile +++ b/Makefile @@ -26,21 +26,22 @@ deb: rpm: ./build_packages.py --rpm -install: - install -d $(DESTDIR)$(FIRMWAREDIR) - ./copy-firmware.sh $(COPYOPTS) $(DESTDIR)$(FIRMWAREDIR) +install: install-nodedup + ./dedup-firmware.sh $(DESTDIR)$(FIRMWAREDIR) install-nodedup: install -d $(DESTDIR)$(FIRMWAREDIR) - ./copy-firmware.sh --ignore-duplicates $(DESTDIR)$(FIRMWAREDIR) + ./copy-firmware.sh $(DESTDIR)$(FIRMWAREDIR) install-xz: install -d $(DESTDIR)$(FIRMWAREDIR) ./copy-firmware.sh --xz $(DESTDIR)$(FIRMWAREDIR) + ./dedup-firmware.sh $(DESTDIR)$(FIRMWAREDIR) install-zst: install -d $(DESTDIR)$(FIRMWAREDIR) ./copy-firmware.sh --zstd $(DESTDIR)$(FIRMWAREDIR) + ./dedup-firmware.sh $(DESTDIR)$(FIRMWAREDIR) clean: rm -rf release dist diff --git a/check_whence.py b/check_whence.py index afe97a76..09dbdd54 100755 --- a/check_whence.py +++ b/check_whence.py @@ -91,6 +91,7 @@ def main(): "contrib/templates/debian.copyright", "contrib/templates/rpm.spec", "copy-firmware.sh", + "dedup-firmware.sh", ] ) known_prefixes = set(name for name in whence_list if name.endswith("/")) diff --git a/copy-firmware.sh b/copy-firmware.sh index 6c557f23..344f478d 100755 --- a/copy-firmware.sh +++ b/copy-firmware.sh @@ -9,7 +9,6 @@ prune=no # shellcheck disable=SC2209 compress=cat compext= -skip_dedup=0 while test $# -gt 0; do case $1 in @@ -45,11 +44,6 @@ while test $# -gt 0; do shift ;; - --ignore-duplicates) - skip_dedup=1 - shift - ;; - -*) if test "$compress" = "cat"; then echo "ERROR: unknown command-line option: $1" @@ -75,13 +69,6 @@ if [ -z "$destdir" ]; then exit 1 fi -if ! command -v rdfind >/dev/null; then - if [ "$skip_dedup" != 1 ]; then - echo "ERROR: rdfind is not installed. Pass --ignore-duplicates to skip deduplication" - exit 1 - fi -fi - # shellcheck disable=SC2162 # file/folder name can include escaped symbols grep -E '^(RawFile|File):' WHENCE | sed -E -e 's/^(RawFile|File): */\1 /;s/"//g' | while read k f; do test -f "$f" || continue @@ -95,16 +82,6 @@ grep -E '^(RawFile|File):' WHENCE | sed -E -e 's/^(RawFile|File): */\1 /;s/"//g' fi done -if [ "$skip_dedup" != 1 ] ; then - $verbose "Finding duplicate files" - rdfind -makesymlinks true -makeresultsfile false "$destdir" >/dev/null - find "$destdir" -type l | while read -r l; do - target="$(realpath "$l")" - $verbose "Correcting path for $l" - ln -fs "$(realpath --relative-to="$(dirname "$(realpath -s "$l")")" "$target")" "$l" - done -fi - # shellcheck disable=SC2162 # file/folder name can include escaped symbols grep -E '^Link:' WHENCE | sed -e 's/^Link: *//g;s/-> //g' | while read f d; do if test -L "$f$compext"; then diff --git a/dedup-firmware.sh b/dedup-firmware.sh new file mode 100755 index 00000000..2bbd637f --- /dev/null +++ b/dedup-firmware.sh @@ -0,0 +1,52 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# +# Deduplicate files in a given destdir +# + +err() { + echo "ERROR: $*" + exit 1 +} + +verbose=: +destdir= +while test $# -gt 0; do + case $1 in + -v | --verbose) + # shellcheck disable=SC2209 + verbose=echo + ;; + *) + if test -n "$destdir"; then + err "unknown command-line options: $*" + fi + + destdir="$1" + shift + ;; + esac +done + +if test -z "$destdir"; then + err "destination directory was not specified." +fi + +if ! test -d "$destdir"; then + err "provided directory does not exit." +fi + +if ! command -v rdfind >/dev/null; then + err "rdfind is not installed." +fi + +$verbose "Finding duplicate files" +rdfind -makesymlinks true -makeresultsfile true "$destdir" >/dev/null + +grep DUPTYPE_WITHIN_SAME_TREE results.txt | grep -o "$destdir.*" | while read -r l; do + target="$(realpath "$l")" + $verbose "Correcting path for $l" + ln --force --symbolic --relative "$target" "$l" +done + +rm results.txt From e8f8537768aaa8d991477155ac00f7826457515e Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Sun, 22 Sep 2024 16:31:23 +0100 Subject: [PATCH 08/16] Revert "copy-firmware: Support additional compressor options" This reverts commit 2bad80e7edd3c0f84718545d9338c8edfed16513. The commit effectively added accidental command injection, while it was aiming to control the compression flags. In practise you'd want to use ZSTD_CLEVEL and ZSTD_NBTHREADS for zstd. As documented in zstd(1) it allows for up-to level 19, which is fine since the kernel does not support higher levels. Arch, Alpine and likely other distributions have been using this approach since day one. The other compressors like xz have equivalent. Cc: Juerg Haefliger Signed-off-by: Emil Velikov --- copy-firmware.sh | 8 -------- 1 file changed, 8 deletions(-) diff --git a/copy-firmware.sh b/copy-firmware.sh index 344f478d..21ed20d5 100755 --- a/copy-firmware.sh +++ b/copy-firmware.sh @@ -44,14 +44,6 @@ while test $# -gt 0; do shift ;; - -*) - if test "$compress" = "cat"; then - echo "ERROR: unknown command-line option: $1" - exit 1 - fi - compress="$compress $1" - shift - ;; *) if test "x$destdir" != "x"; then echo "ERROR: unknown command-line options: $*" From a3446bc1a7bd4824177e5765ce7a5e7c6489a23a Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Mon, 23 Sep 2024 12:09:08 +0100 Subject: [PATCH 09/16] copy-firmware.sh: reset and consistently handle destdir Currently we don't reset/override the destdir variable, so we end up inheriting whatever the caller's environment has for it. While it may work, it's not particularly consistent (be that within the script or other tools) nor is it obvious. While in here, ensure we handle the variable with test -z/-n instead of varying other constructs. Signed-off-by: Emil Velikov --- copy-firmware.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/copy-firmware.sh b/copy-firmware.sh index 21ed20d5..65a1b39b 100755 --- a/copy-firmware.sh +++ b/copy-firmware.sh @@ -9,6 +9,7 @@ prune=no # shellcheck disable=SC2209 compress=cat compext= +destdir= while test $# -gt 0; do case $1 in @@ -45,7 +46,7 @@ while test $# -gt 0; do ;; *) - if test "x$destdir" != "x"; then + if test -n "$destdir"; then echo "ERROR: unknown command-line options: $*" exit 1 fi @@ -56,7 +57,7 @@ while test $# -gt 0; do esac done -if [ -z "$destdir" ]; then +if test -z "$destdir"; then echo "ERROR: destination directory was not specified" exit 1 fi From 6360b4a1d7d4972e5cb68ccd3aed66abd0c3a538 Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Sun, 22 Sep 2024 16:42:48 +0100 Subject: [PATCH 10/16] copy-firmware.sh: fix indentation Signed-off-by: Emil Velikov --- copy-firmware.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/copy-firmware.sh b/copy-firmware.sh index 65a1b39b..f83b4775 100755 --- a/copy-firmware.sh +++ b/copy-firmware.sh @@ -58,8 +58,8 @@ while test $# -gt 0; do done if test -z "$destdir"; then - echo "ERROR: destination directory was not specified" - exit 1 + echo "ERROR: destination directory was not specified" + exit 1 fi # shellcheck disable=SC2162 # file/folder name can include escaped symbols @@ -115,9 +115,9 @@ done # Verify no broken symlinks if test "$(find "$destdir" -xtype l | wc -l)" -ne 0 ; then - echo "ERROR: Broken symlinks found:" - find "$destdir" -xtype l - exit 1 + echo "ERROR: Broken symlinks found:" + find "$destdir" -xtype l + exit 1 fi exit 0 From 97d200d75b94b1984760e0474c08be042d4d0d33 Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Sun, 22 Sep 2024 16:48:35 +0100 Subject: [PATCH 11/16] copy-firmware.sh: add err() helper v2: - use printf instead of echo -e Signed-off-by: Emil Velikov --- copy-firmware.sh | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/copy-firmware.sh b/copy-firmware.sh index f83b4775..381bde29 100755 --- a/copy-firmware.sh +++ b/copy-firmware.sh @@ -11,6 +11,11 @@ compress=cat compext= destdir= +err() { + printf "ERROR: %s\n" "$*" + exit 1 +} + while test $# -gt 0; do case $1 in -v | --verbose) @@ -26,8 +31,7 @@ while test $# -gt 0; do --xz) if test "$compext" = ".zst"; then - echo "ERROR: cannot mix XZ and ZSTD compression" - exit 1 + err "cannot mix XZ and ZSTD compression" fi compress="xz --compress --quiet --stdout --check=crc32" compext=".xz" @@ -36,8 +40,7 @@ while test $# -gt 0; do --zstd) if test "$compext" = ".xz"; then - echo "ERROR: cannot mix XZ and ZSTD compression" - exit 1 + err "cannot mix XZ and ZSTD compression" fi # shellcheck disable=SC2209 compress="zstd --compress --quiet --stdout" @@ -47,8 +50,7 @@ while test $# -gt 0; do *) if test -n "$destdir"; then - echo "ERROR: unknown command-line options: $*" - exit 1 + err "unknown command-line options: $*" fi destdir="$1" @@ -58,8 +60,7 @@ while test $# -gt 0; do done if test -z "$destdir"; then - echo "ERROR: destination directory was not specified" - exit 1 + err "destination directory was not specified" fi # shellcheck disable=SC2162 # file/folder name can include escaped symbols @@ -115,9 +116,7 @@ done # Verify no broken symlinks if test "$(find "$destdir" -xtype l | wc -l)" -ne 0 ; then - echo "ERROR: Broken symlinks found:" - find "$destdir" -xtype l - exit 1 + err "Broken symlinks found:\\n$(find "$destdir" -xtype l)" fi exit 0 From 32f71d6d456e0a0cd89f9fa03c62d984f3afa05d Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Sun, 22 Sep 2024 17:00:01 +0100 Subject: [PATCH 12/16] copy-firmware.sh: warn if the destination folder is not empty If the user provides an existing non-empty folder (their /usr/lib/firmware/ or otherwise) there is a high chance we'll silently overwrite existing files. That may or may not be what they wanted, so throw a warning so highlight that. v2: - use printf instead of echo Signed-off-by: Emil Velikov --- copy-firmware.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/copy-firmware.sh b/copy-firmware.sh index 381bde29..fa83492e 100755 --- a/copy-firmware.sh +++ b/copy-firmware.sh @@ -16,6 +16,10 @@ err() { exit 1 } +warn() { + printf "WARNING: %s\n" "$*" +} + while test $# -gt 0; do case $1 in -v | --verbose) @@ -63,6 +67,10 @@ if test -z "$destdir"; then err "destination directory was not specified" fi +if test -d "$destdir"; then + find "$destdir" -type d -empty >/dev/null || warn "destination folder is not empty." +fi + # shellcheck disable=SC2162 # file/folder name can include escaped symbols grep -E '^(RawFile|File):' WHENCE | sed -E -e 's/^(RawFile|File): */\1 /;s/"//g' | while read k f; do test -f "$f" || continue From 6edd0fd36acb93c8075bbd700ad91f53fc810c0e Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Sun, 22 Sep 2024 16:44:25 +0100 Subject: [PATCH 13/16] copy-firmware.sh: call ./check_whence.py before parsing the file Currently ./check_whence.py is used when submitting new firmware, while copy-firmware.sh when the firmware is to be consumed. Since the latter does (very little) validation, having a malformed WHENCE file can lead to all sorted of problems. From the obvious, where it errors out, to more serious one where it overwrites or executes something it should not have. Just call check_whence.py and error out. It takes 0.2s on my 5 year old mid-range laptop, so the overhead is negligible. Signed-off-by: Emil Velikov --- copy-firmware.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/copy-firmware.sh b/copy-firmware.sh index fa83492e..0f21022a 100755 --- a/copy-firmware.sh +++ b/copy-firmware.sh @@ -71,6 +71,9 @@ if test -d "$destdir"; then find "$destdir" -type d -empty >/dev/null || warn "destination folder is not empty." fi +$verbose "Checking that WHENCE file is formatted properly" +./check_whence.py || err "check_whence.py has detected errors." + # shellcheck disable=SC2162 # file/folder name can include escaped symbols grep -E '^(RawFile|File):' WHENCE | sed -E -e 's/^(RawFile|File): */\1 /;s/"//g' | while read k f; do test -f "$f" || continue From 25d5e6352ba5dd8d8d75cb80fc905f65b13c4c44 Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Sun, 22 Sep 2024 17:03:53 +0100 Subject: [PATCH 14/16] copy-firmware.sh: remove no longer reachable test -f With previous commit we call check_whence.py, which ensures that all files listed are available. Drop the now dead code. Signed-off-by: Emil Velikov --- copy-firmware.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/copy-firmware.sh b/copy-firmware.sh index 0f21022a..da3d9620 100755 --- a/copy-firmware.sh +++ b/copy-firmware.sh @@ -76,7 +76,6 @@ $verbose "Checking that WHENCE file is formatted properly" # shellcheck disable=SC2162 # file/folder name can include escaped symbols grep -E '^(RawFile|File):' WHENCE | sed -E -e 's/^(RawFile|File): */\1 /;s/"//g' | while read k f; do - test -f "$f" || continue install -d "$destdir/$(dirname "$f")" $verbose "copying/compressing file $f$compext" if test "$compress" != "cat" && test "$k" = "RawFile"; then From 9ce7dac098621af796ee59ff3177b37d70c96dc5 Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Sun, 22 Sep 2024 17:10:01 +0100 Subject: [PATCH 15/16] copy-firmware.sh: remove no longer reachable test -L The check_whence.py script ensures that links defined in WHENCE are not in-tree. Since we're calling the script, we no longer need the convoluted path and associated --prune tag. Signed-off-by: Emil Velikov --- copy-firmware.sh | 46 ++++++++-------------------------------------- 1 file changed, 8 insertions(+), 38 deletions(-) diff --git a/copy-firmware.sh b/copy-firmware.sh index da3d9620..66c8d83a 100755 --- a/copy-firmware.sh +++ b/copy-firmware.sh @@ -5,7 +5,6 @@ # verbose=: -prune=no # shellcheck disable=SC2209 compress=cat compext= @@ -28,11 +27,6 @@ while test $# -gt 0; do shift ;; - -P | --prune) - prune=yes - shift - ;; - --xz) if test "$compext" = ".zst"; then err "cannot mix XZ and ZSTD compression" @@ -88,39 +82,15 @@ done # shellcheck disable=SC2162 # file/folder name can include escaped symbols grep -E '^Link:' WHENCE | sed -e 's/^Link: *//g;s/-> //g' | while read f d; do - if test -L "$f$compext"; then - test -f "$destdir/$f$compext" && continue - $verbose "copying link $f$compext" - install -d "$destdir/$(dirname "$f")" - cp -d "$f$compext" "$destdir/$f$compext" - - if test "x$d" != "x"; then - target="$(readlink "$f")" - - if test "x$target" != "x$d"; then - $verbose "WARNING: inconsistent symlink target: $target != $d" - else - if test "x$prune" != "xyes"; then - $verbose "WARNING: unneeded symlink detected: $f" - else - $verbose "WARNING: pruning unneeded symlink $f" - rm -f "$f$compext" - fi - fi - else - $verbose "WARNING: missing target for symlink $f" - fi + directory="$destdir/$(dirname "$f")" + install -d "$directory" + target="$(cd "$directory" && realpath -m -s "$d")" + if test -e "$target"; then + $verbose "creating link $f -> $d" + ln -s "$d" "$destdir/$f" else - directory="$destdir/$(dirname "$f")" - install -d "$directory" - target="$(cd "$directory" && realpath -m -s "$d")" - if test -e "$target"; then - $verbose "creating link $f -> $d" - ln -s "$d" "$destdir/$f" - else - $verbose "creating link $f$compext -> $d$compext" - ln -s "$d$compext" "$destdir/$f$compext" - fi + $verbose "creating link $f$compext -> $d$compext" + ln -s "$d$compext" "$destdir/$f$compext" fi done From a89349c33c603beeef553163341b01f2c1b30cf9 Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Sun, 22 Sep 2024 17:14:34 +0100 Subject: [PATCH 16/16] copy-firmware.sh: rename variables in symlink hanlding Currently we use f(ile) and d(irectory), over the more common ones t(arget) and l(ink). Rename things appropriately. Signed-off-by: Emil Velikov --- copy-firmware.sh | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/copy-firmware.sh b/copy-firmware.sh index 66c8d83a..dcb3b92c 100755 --- a/copy-firmware.sh +++ b/copy-firmware.sh @@ -81,16 +81,16 @@ grep -E '^(RawFile|File):' WHENCE | sed -E -e 's/^(RawFile|File): */\1 /;s/"//g' done # shellcheck disable=SC2162 # file/folder name can include escaped symbols -grep -E '^Link:' WHENCE | sed -e 's/^Link: *//g;s/-> //g' | while read f d; do - directory="$destdir/$(dirname "$f")" +grep -E '^Link:' WHENCE | sed -e 's/^Link: *//g;s/-> //g' | while read l t; do + directory="$destdir/$(dirname "$l")" install -d "$directory" - target="$(cd "$directory" && realpath -m -s "$d")" + target="$(cd "$directory" && realpath -m -s "$t")" if test -e "$target"; then - $verbose "creating link $f -> $d" - ln -s "$d" "$destdir/$f" + $verbose "creating link $l -> $t" + ln -s "$t" "$destdir/$l" else - $verbose "creating link $f$compext -> $d$compext" - ln -s "$d$compext" "$destdir/$f$compext" + $verbose "creating link $l$compext -> $t$compext" + ln -s "$t$compext" "$destdir/$l$compext" fi done