From b85ae70ebd95a349e6a5e134c9844851e51085b4 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Fri, 7 Aug 2026 23:43:49 +0200 Subject: [PATCH] Stop pipefail from turning grep -q SIGPIPE exits into false negatives (#6614) * Stop pipefail from turning grep -q SIGPIPE exits into false negatives grep -q exits at the first match, and when the producer is still writing it dies with SIGPIPE. Under pipefail that 141 becomes the pipeline's status, so hardware checks like lspci | grep -q read as "not found" on exactly the machines they target. The T2 defaults migration hit this and silently skipped real T2 Macs (#6608). Redirect grep to /dev/null instead of -q wherever a pipeline feeds grep in a pipefail context, so grep reads all input and the producer never gets killed. The install-time T2 checks aren't run under pipefail today but are switched too, since they're the same detection line the issue calls out. Co-Authored-By: Claude Fable 5 * Re-run the T2 defaults migration its broken hardware check skipped The SIGPIPE bug marked 1785944594 as applied without doing anything on affected T2 Macs. The original migration is idempotent, so a fresh migration can just source it now that the guard is fixed. Co-Authored-By: Claude Fable 5 * Address Copilot review: fix OCR grep pipeline and prove the T2 repair screen_contains piped tesseract into grep -Fqi under the acceptance suite's pipefail, the same SIGPIPE false negative the rest of the branch fixes. The T2 test's lspci stub now keeps writing past the pipe buffer after the match so every scenario exercises the SIGPIPE case, and a new case runs the rerun migration against fixtures a bitten install would have. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Fable 5 --- bin/omarchy-audio-tuning | 4 ++-- bin/omarchy-dev-install-ydoo | 2 +- bin/omarchy-upgrade-to-quattro | 4 ++-- install/hardware/apple/fix-t2.sh | 2 +- install/hardware/pacman.sh | 2 +- migrations/1784401744.sh | 2 +- migrations/1784809451.sh | 2 +- migrations/1785944594.sh | 2 +- migrations/1786137597.sh | 7 +++++++ test/acceptance.d/base-test.sh | 2 +- test/acceptance.d/panels-test.sh | 2 +- test/shell.d/clipboard-test.sh | 2 +- test/shell.d/t2-hardware-test.sh | 33 ++++++++++++++++++++++++++++++++ 13 files changed, 53 insertions(+), 13 deletions(-) create mode 100644 migrations/1786137597.sh diff --git a/bin/omarchy-audio-tuning b/bin/omarchy-audio-tuning index f238de46..75b301df 100755 --- a/bin/omarchy-audio-tuning +++ b/bin/omarchy-audio-tuning @@ -120,7 +120,7 @@ tuned_hardware_sink() { } tuning_present() { - pactl list sinks short 2>/dev/null | awk '{print $2}' | grep -qx "$sink_name" + pactl list sinks short 2>/dev/null | awk '{print $2}' | grep -x "$sink_name" >/dev/null } # Only real application streams may be moved. A filter-chain's own output is also @@ -151,7 +151,7 @@ tuning_downstream_sink() { } easyeffects_running() { - pactl list sinks short 2>/dev/null | awk '{print $2}' | grep -qx easyeffects_sink || + pactl list sinks short 2>/dev/null | awk '{print $2}' | grep -x easyeffects_sink >/dev/null || pgrep -u "$(id -u)" -x easyeffects >/dev/null 2>&1 || systemctl --user is-active --quiet easyeffects.service 2>/dev/null } diff --git a/bin/omarchy-dev-install-ydoo b/bin/omarchy-dev-install-ydoo index 91c4c502..8a773f97 100755 --- a/bin/omarchy-dev-install-ydoo +++ b/bin/omarchy-dev-install-ydoo @@ -12,7 +12,7 @@ if ! getent group input >/dev/null; then exit 1 fi -if ! id -nG "$USER" | tr ' ' '\n' | grep -qx input; then +if ! id -nG "$USER" | tr ' ' '\n' | grep -x input >/dev/null; then echo "Adding $USER to the input group. You may need to log out and back in before this applies." pkexec usermod -aG input "$USER" fi diff --git a/bin/omarchy-upgrade-to-quattro b/bin/omarchy-upgrade-to-quattro index ef2cb509..15408273 100755 --- a/bin/omarchy-upgrade-to-quattro +++ b/bin/omarchy-upgrade-to-quattro @@ -574,7 +574,7 @@ EOF while read -r uki; do [[ -n $uki ]] || continue as_root objcopy -O binary --only-section=.cmdline "$uki" /dev/stdout 2>/dev/null | - tr -d '\0' | grep -qE '(^|[[:space:]])root=' || missing+=("$uki") + tr -d '\0' | grep -E '(^|[[:space:]])root=' >/dev/null || missing+=("$uki") done < <(as_root find /boot/EFI/Linux -maxdepth 1 -name 'omarchy_linux*.efi' 2>/dev/null) fi @@ -714,7 +714,7 @@ install_omarchy_quattro_packages() { install_hardware_transition_packages() { local hardware_packages=() - if lspci | grep -qiE '(Multimedia audio controller|Audio device).*Intel'; then + if lspci | grep -iE '(Multimedia audio controller|Audio device).*Intel' >/dev/null; then hardware_packages+=(sof-firmware) fi diff --git a/install/hardware/apple/fix-t2.sh b/install/hardware/apple/fix-t2.sh index 904da06f..7936a794 100644 --- a/install/hardware/apple/fix-t2.sh +++ b/install/hardware/apple/fix-t2.sh @@ -1,6 +1,6 @@ # Detect T2 MacBook models using PCI IDs # Vendor: 106b (Apple), Device IDs: 1801 or 1802 (T2 Security Chip) -if lspci -nn | grep -q "106b:180[12]"; then +if lspci -nn | grep "106b:180[12]" >/dev/null; then echo "Detected MacBook with T2 chip. Installing support items..." omarchy-pkg-add \ diff --git a/install/hardware/pacman.sh b/install/hardware/pacman.sh index 9d2266bf..7533d3f3 100644 --- a/install/hardware/pacman.sh +++ b/install/hardware/pacman.sh @@ -1,6 +1,6 @@ # Hardware-specific pacman repository extensions that must survive the final # pacman.conf restore. -if lspci -nn | grep -q "106b:180[12]"; then +if lspci -nn | grep "106b:180[12]" >/dev/null; then if ! grep -q '^\[arch-mact2\]' /etc/pacman.conf; then cat >> /etc/pacman.conf <<'EOF' diff --git a/migrations/1784401744.sh b/migrations/1784401744.sh index 423c1ecf..91bd6065 100644 --- a/migrations/1784401744.sh +++ b/migrations/1784401744.sh @@ -12,7 +12,7 @@ if [[ -f $tmux_config ]]; then fi hardware_packages=() -if lspci | grep -qiE '(Multimedia audio controller|Audio device).*Intel' && omarchy-pkg-missing sof-firmware; then +if lspci | grep -iE '(Multimedia audio controller|Audio device).*Intel' >/dev/null && omarchy-pkg-missing sof-firmware; then hardware_packages+=(sof-firmware) fi if lspci | grep -iE '(VGA|Display).*Intel' >/dev/null && omarchy-pkg-missing vulkan-intel; then diff --git a/migrations/1784809451.sh b/migrations/1784809451.sh index fb59a794..bcd6c6de 100644 --- a/migrations/1784809451.sh +++ b/migrations/1784809451.sh @@ -16,7 +16,7 @@ as_root() { [[ -f $locate_config_script ]] || exit 0 if grep -q '^PRUNE_BIND_MOUNTS = "no"' "$UPDATEDB_CONF_PATH" && - grep -E '^PRUNEPATHS' "$UPDATEDB_CONF_PATH" | grep -qE '(^|[[:space:]"])/\.snapshots([[:space:]"]|$)'; then + grep -E '^PRUNEPATHS' "$UPDATEDB_CONF_PATH" | grep -E '(^|[[:space:]"])/\.snapshots([[:space:]"]|$)' >/dev/null; then exit 0 fi diff --git a/migrations/1785944594.sh b/migrations/1785944594.sh index 7f290e11..823fe952 100644 --- a/migrations/1785944594.sh +++ b/migrations/1785944594.sh @@ -1,6 +1,6 @@ echo "Update T2 Mac suspend, Touch Bar, and fan defaults" -if ! lspci -nn | grep -q "106b:180[12]"; then +if ! lspci -nn | grep "106b:180[12]" >/dev/null; then exit 0 fi diff --git a/migrations/1786137597.sh b/migrations/1786137597.sh new file mode 100644 index 00000000..b4afab94 --- /dev/null +++ b/migrations/1786137597.sh @@ -0,0 +1,7 @@ +echo "Re-run the T2 defaults migration that a broken hardware check skipped" + +# The T2 check in 1785944594 piped lspci into grep -q, which can exit 141 under +# the runner's pipefail when grep quits at the first match and lspci catches +# SIGPIPE. That marked the migration applied on the very hardware it targeted. +# The original is idempotent, so re-running it is safe everywhere. +source "$OMARCHY_PATH/migrations/1785944594.sh" diff --git a/test/acceptance.d/base-test.sh b/test/acceptance.d/base-test.sh index 4262d40a..eff2a52d 100644 --- a/test/acceptance.d/base-test.sh +++ b/test/acceptance.d/base-test.sh @@ -40,7 +40,7 @@ screen_contains() { rm -f "$snapshot" return 1 fi - tesseract "$snapshot" stdout --psm 11 2>/dev/null | grep -Fqi -- "$text" + tesseract "$snapshot" stdout --psm 11 2>/dev/null | grep -Fi -- "$text" >/dev/null local status=$? rm -f "$snapshot" return $status diff --git a/test/acceptance.d/panels-test.sh b/test/acceptance.d/panels-test.sh index c3b812bd..eff1a35a 100644 --- a/test/acceptance.d/panels-test.sh +++ b/test/acceptance.d/panels-test.sh @@ -76,7 +76,7 @@ done <<<"$panels" # The power widget intentionally disappears on desktops and VMs without a # battery. Exercise it on laptops, and verify that hardware-less sessions take # the supported no-panel path instead of treating that as a shell failure. -if upower -e | grep -q '/battery_'; then +if upower -e | grep '/battery_' >/dev/null; then if ! (trap - EXIT; open_and_capture_panel "power" "omarchy.power"); then status=1 hide_panels diff --git a/test/shell.d/clipboard-test.sh b/test/shell.d/clipboard-test.sh index d3d839ab..79ce6fb8 100644 --- a/test/shell.d/clipboard-test.sh +++ b/test/shell.d/clipboard-test.sh @@ -318,7 +318,7 @@ PATH="$TMPDIR/bin:$PATH" wl-paste --type text --watch "$current_script" text & stale_pid=$! PIDS_TO_KILL+=("$stale_pid") sleep 0.2 -pgrep -f 'wl-paste .*--watch .*/shell/plugins/clipboard/capture\.sh' | grep -qx "$stale_pid" || fail "clipboard reaper pattern matches running watchers" +pgrep -f 'wl-paste .*--watch .*/shell/plugins/clipboard/capture\.sh' | grep -x "$stale_pid" >/dev/null || fail "clipboard reaper pattern matches running watchers" kill "$stale_pid" 2>/dev/null || true wait "$stale_pid" 2>/dev/null || true pass "clipboard reaper pattern matches running watchers" diff --git a/test/shell.d/t2-hardware-test.sh b/test/shell.d/t2-hardware-test.sh index cd80468c..8b2c478b 100644 --- a/test/shell.d/t2-hardware-test.sh +++ b/test/shell.d/t2-hardware-test.sh @@ -33,9 +33,15 @@ mkdir -p "$stub_bin" cat >"$stub_bin/lspci" <<'SH' #!/bin/bash +# Chatty like real lspci: keep writing well past the pipe buffer after the T2 +# match, so a grep -q consumer would kill this stub with SIGPIPE and pipefail +# would read that as "no T2 hardware" (#6608). if (( ${T2_HARDWARE:-0} == 1 )); then echo '01:00.0 Bridge [0680]: Apple Inc. T2 Security Chip [106b:1801]' fi +for _ in {1..4096}; do + echo '02:00.0 Host bridge [0600]: Filler Device [ffff:0000]' +done SH cat >"$stub_bin/sudo" <<'SH' @@ -179,3 +185,30 @@ grep -q 'pcie_ports=compat' "$limine_conf" || fail "non-T2 Limine configuration ! grep -q '\[Fan2\]' "$fan_conf" || fail "non-T2 fan configuration is unchanged" [[ ! -s $calls ]] || fail "non-T2 systems skip the repair" "$(cat "$calls")" pass "T2 migration skips unrelated hardware" + +# The previous block left the fixtures looking like an install the SIGPIPE bug +# skipped: stale Limine parameters, one fan section, and no repair marker. The +# rerun migration must complete the repair the original was marked as done for. +rerun_migration="$ROOT/migrations/1786137597.sh" +rm -f "$repair_marker" +: >"$calls" + +PATH="$stub_bin:$PATH" \ + TEST_LOG="$calls" \ + T2_HARDWARE=1 \ + TINY_DFR_INSTALLED=0 \ + OMARCHY_PATH="$ROOT" \ + OMARCHY_T2_LIMINE_CONF="$limine_conf" \ + OMARCHY_T2_FAN_CONF="$fan_conf" \ + OMARCHY_T2_RUNNING_CMDLINE="$running_cmdline" \ + OMARCHY_T2_REPAIR_MARKER="$repair_marker" \ + bash -euo pipefail "$rerun_migration" >/dev/null + +grep -Fq 'pm_async=off mem_sleep_default=deep' "$limine_conf" || + fail "T2 rerun migration updates the Limine suspend parameters" +(( $(grep -Ec '^[[:space:]]*\[Fan2\][[:space:]]*$' "$fan_conf") == 1 )) || + fail "T2 rerun migration adds the second-fan section" +grep -Fxq 'limine-mkinitcpio' "$calls" || + fail "T2 rerun migration rebuilds the boot image" +[[ -f $repair_marker ]] || fail "T2 rerun migration records the machine-wide repair" +pass "T2 rerun migration repairs installs the broken hardware check skipped"