Harden the provider per Momus review and prove it with offline fixtures

bin/sync-upstream self-test swaps the two network fetches in
helpers/upstream-github.sh for fixture readers and runs the production code
paths: fallback past a quarantined release, draft/prerelease filtering, the
deliberate bypass, unchanged-version and all-quarantined no-update paths,
unusable tags/timestamps and missing checksums failing the sync, {tag} and
{pkgver} asset templates with ./ and * manifest prefixes across both
architectures, the min_release_age backstop verdicts (now a testable
release_age_status function), the duration parser, and manifest validation.

Also fixes from the review: the duration parser forces base-10 arithmetic
(leading zeros no longer parse as octal) and bounds values to nine digits so
no suffix can overflow; jq // treating false as absent can no longer let
"min_release_age": false or "upstream": false slip through as unset; the
release feed page grew to the API maximum of 100 with the bounded search
documented; and the README package-metadata section documents the upstream
block, min_release_age, the bypass, and provider-versus-hook exclusivity.
This commit is contained in:
Ryan Hughes
2026-08-24 20:07:10 -04:00
parent 699261471a
commit 5777573a84
4 changed files with 294 additions and 35 deletions
+40 -3
View File
@@ -258,8 +258,41 @@ bin/sync-upstream openai-codex-desktop # Update specific packages
Some vendors publish a release feed of their own that is faster and more precise Some vendors publish a release feed of their own that is faster and more precise
than the AUR packaging of it. Those packages are `source: local` — Omarchy owns than the AUR packaging of it. Those packages are `source: local` — Omarchy owns
the PKGBUILD — and provide `.omarchy/upstream.sh`, a hook that reports the newest the PKGBUILD — and declare where releases come from in one of two ways.
upstream release as JSON on stdout:
A vendor shipping tagged GitHub releases with a checksum manifest asset is pure
data, declared as `upstream` in `.omarchy/package.json` with no code at all:
```json
"upstream": {
"github": "jdx/mise",
"checksums": "SHASUMS256.txt",
"assets": {
"x86_64": "mise-{tag}-linux-x64.tar.xz",
"aarch64": "mise-{tag}-linux-arm64.tar.xz"
}
}
```
`{tag}` and `{pkgver}` interpolate into asset names; a leading `v` on the tag is
stripped for `pkgver`; drafts and prereleases are ignored. Only the 100 most
recent releases are considered. The provider fails closed on anything it cannot
read — an unusable tag, timestamp, or checksum stops the sync rather than being
skipped.
A package may also declare `"min_release_age": "24h"` (`s`/`m`/`h`/`d` suffix or
bare seconds) to quarantine fresh releases until maintainers have had time to
pull a bad or compromised one. The newest release that has cleared the window
ships, so a fast release cadence cannot starve updates. The window is enforced
centrally: whatever reports the release must prove its age via `published_at`,
or the sync fails. A maintainer deliberately shipping inside the window runs
`BYPASS_MIN_RELEASE_AGE=1 bin/sync-upstream <package>` locally and merges the
result through a normal PR; scheduled automation never sets the bypass.
A vendor whose feed fits no convention (a Debian package index, a bare
version.txt) instead provides `.omarchy/upstream.sh`, a hook that reports the
newest upstream release as JSON on stdout — declaring both an `upstream` block
and a hook is an error:
```json ```json
{ {
@@ -281,7 +314,11 @@ back cannot walk the repository backwards.
Hooks should read checksums from whatever manifest the vendor publishes rather Hooks should read checksums from whatever manifest the vendor publishes rather
than downloading the artifacts — see `pkgbuilds/openai-codex-desktop/.omarchy/upstream.sh`, than downloading the artifacts — see `pkgbuilds/openai-codex-desktop/.omarchy/upstream.sh`,
which reads OpenAI's Debian package index and never fetches the 750 MB of debs which reads OpenAI's Debian package index and never fetches the 750 MB of debs
it describes. it describes. Hooks honoring `min_release_age` receive the window as
`MIN_RELEASE_AGE_SECONDS` and report `published_at` alongside `pkgver`.
`bin/sync-upstream self-test` runs offline fixture tests over the release
selection, quarantine backstop, duration parsing, and manifest validation.
### Sync Rebuild Triggers ### Sync Rebuild Triggers
+205 -16
View File
@@ -48,8 +48,12 @@ the bypass, so the resulting change still goes through a reviewed PR.
Arguments: Arguments:
PACKAGE One or more package names to update (optional) PACKAGE One or more package names to update (optional)
Commands:
self-test Run the offline fixture tests for release selection, the
quarantine backstop, and metadata parsing
Examples: Examples:
$0 # Update every package with an upstream hook $0 # Update every package with an upstream source
$0 openai-codex-desktop # Update specific packages $0 openai-codex-desktop # Update specific packages
EOF EOF
} }
@@ -157,6 +161,22 @@ set_pkgbuild_array() {
mv "$rewritten" "$pkgbuild" mv "$rewritten" "$pkgbuild"
} }
# Backstop verdict for a reported release against min_release_age. Returns 0
# when old enough (or no policy is set, or the bypass is deliberate), 1 when
# the release is younger than the window, 2 when the report carries no usable
# published_at and the age cannot be established at all.
release_age_status() {
local release="$1" min_age="$2"
(( min_age > 0 )) || return 0
[[ "${BYPASS_MIN_RELEASE_AGE:-}" != "1" ]] || return 0
local published_at published_epoch
published_at=$(jq -r '.published_at // empty' <<<"$release")
if [[ -z "$published_at" ]] || ! published_epoch=$(date --date="$published_at" +%s 2>/dev/null); then
return 2
fi
(( $(date +%s) - published_epoch >= min_age )) || return 1
}
# pacman's own comparator, because nothing else agrees with it at the corners: # pacman's own comparator, because nothing else agrees with it at the corners:
# sort -V calls 1.0a newer than 1.0, vercmp calls it older, and pacman is what # sort -V calls 1.0a newer than 1.0, vercmp calls it older, and pacman is what
# decides whether a published package is an upgrade. # decides whether a published package is an upgrade.
@@ -371,23 +391,23 @@ sync_package() {
return 0 return 0
fi fi
# Backstop for min_release_age: the hook already selects within the window, # Backstop for min_release_age: the selection already honors the window,
# but a hook bug must not be able to ship a release younger than the policy. # but a provider or hook bug must not be able to ship a release younger
if (( min_age > 0 )) && [[ "${BYPASS_MIN_RELEASE_AGE:-}" != "1" ]]; then # than the policy.
local published_at published_epoch age local age_status=0
published_at=$(jq -r '.published_at // empty' <<<"$release") release_age_status "$release" "$min_age" || age_status=$?
if [[ -z "$published_at" ]] || ! published_epoch=$(date --date="$published_at" +%s 2>/dev/null); then case "$age_status" in
print_error "min_release_age is set for $package but its hook reported no usable published_at; refusing an unverifiable release" 1)
((++FAILED)) print_warning " Reported release is inside the ${min_age}s minimum release age; leaving it alone"
return 0
fi
age=$(( $(date +%s) - published_epoch ))
if (( age < min_age )); then
print_warning " Hook reported a release only $((age / 3600))h old, inside the ${min_age}s minimum age; leaving it alone"
((++SKIPPED)) ((++SKIPPED))
return 0 return 0
fi ;;
fi 2)
print_error "min_release_age is set for $package but its source reported no usable published_at; refusing an unverifiable release"
((++FAILED))
return 0
;;
esac
local pkgver current_pkgver local pkgver current_pkgver
pkgver=$(jq -r '.pkgver' <<<"$release") pkgver=$(jq -r '.pkgver' <<<"$release")
@@ -421,6 +441,175 @@ sync_package() {
((++UPDATED)) ((++UPDATED))
} }
# Offline fixture tests: the network fetches in helpers/upstream-github.sh
# are swapped for fixture readers, everything else runs the production code
# paths. Covers release selection (fallback past quarantined releases,
# draft/prerelease filtering, bypass, unchanged version), failure paths
# (unusable tags/timestamps, missing checksums), checksum template mapping
# for both architectures, the min_release_age backstop, the duration parser,
# and manifest validation.
cmd_self_test() {
local failures=0
check() {
local desc="$1" expected="$2" got="$3"
if [[ "$expected" == "$got" ]]; then
echo " ok: $desc"
else
echo " FAIL: $desc (expected '$expected', got '$got')"
failures=$((failures + 1))
fi
}
local pkg="$TEMP_DIR/selftest-pkg"
mkdir -p "$pkg/.omarchy"
printf 'pkgver=1.0.0\npkgrel=1\n' > "$pkg/PKGBUILD"
cat > "$pkg/.omarchy/package.json" <<'EOF'
{
"source": "local",
"min_release_age": "24h",
"upstream": {
"github": "example/tool",
"checksums": "SHASUMS256.txt",
"assets": {
"x86_64": "tool-{tag}-x64.tar.xz",
"aarch64": "tool-v{pkgver}-arm64.tar.xz"
}
}
}
EOF
local young old2d old3d
young=$(date -u -d '1 hour ago' +%Y-%m-%dT%H:%M:%SZ)
old2d=$(date -u -d '2 days ago' +%Y-%m-%dT%H:%M:%SZ)
old3d=$(date -u -d '3 days ago' +%Y-%m-%dT%H:%M:%SZ)
# v2.0.0 is inside the 24h window; v1.9.9/v1.9.8 are a prerelease and a
# draft that would outrank v1.9.0 if the filters failed.
local sum_x19 sum_a19 sum_x20 sum_a20
sum_x19=$(printf 'a%.0s' {1..64})
sum_a19=$(printf 'b%.0s' {1..64})
sum_x20=$(printf 'c%.0s' {1..64})
sum_a20=$(printf 'd%.0s' {1..64})
FIXTURE_RELEASES=$(jq -n --arg young "$young" --arg old2 "$old2d" --arg old3 "$old3d" '[
{tag_name: "v2.0.0", published_at: $young, draft: false, prerelease: false},
{tag_name: "v1.9.9", published_at: $old2, draft: false, prerelease: true},
{tag_name: "v1.9.8", published_at: $old2, draft: true, prerelease: false},
{tag_name: "v1.9.0", published_at: $old2, draft: false, prerelease: false},
{tag_name: "v1.8.0", published_at: $old3, draft: false, prerelease: false}
]')
FIXTURE_CHECKSUMS=$(printf '%s\n' \
"$sum_x19 ./tool-v1.9.0-x64.tar.xz" \
"$sum_a19 tool-v1.9.0-arm64.tar.xz" \
"$sum_x20 *tool-v2.0.0-x64.tar.xz" \
"$sum_a20 tool-v2.0.0-arm64.tar.xz")
github_fetch_releases() { printf '%s' "$FIXTURE_RELEASES"; }
github_fetch_checksums() { printf '%s\n' "$FIXTURE_CHECKSUMS"; }
echo "Release selection:"
local out
out=$(github_upstream_release "$pkg" 86400 2>/dev/null) || out="<error>"
check "quarantine falls back past the young v2.0.0" "1.9.0" "$(jq -r '.pkgver // "<none>"' <<<"$out")"
check "selected release reports its published_at" "$old2d" "$(jq -r '.published_at // "<none>"' <<<"$out")"
check "x86_64 checksum via {tag} template and ./ prefix" "$sum_x19" "$(jq -r '.sha256sums.x86_64[0] // "<none>"' <<<"$out")"
check "aarch64 checksum via {pkgver} template" "$sum_a19" "$(jq -r '.sha256sums.aarch64[0] // "<none>"' <<<"$out")"
out=$(github_upstream_release "$pkg" 0 2>/dev/null) || out="<error>"
check "no policy selects the newest stable release" "2.0.0" "$(jq -r '.pkgver // "<none>"' <<<"$out")"
check "prerelease v1.9.9 and draft v1.9.8 are never selected" "" "$(jq -r 'select(.pkgver == "1.9.9" or .pkgver == "1.9.8") | .pkgver' <<<"$out")"
out=$(BYPASS_MIN_RELEASE_AGE=1 github_upstream_release "$pkg" 86400 2>/dev/null) || out="<error>"
check "bypass lifts the quarantine" "2.0.0" "$(jq -r '.pkgver // "<none>"' <<<"$out")"
check "x86_64 checksum via * binary-mode prefix" "$sum_x20" "$(jq -r '.sha256sums.x86_64[0] // "<none>"' <<<"$out")"
out=$(github_upstream_release "$pkg" 8640000 2>/dev/null) || out="<error>"
check "everything quarantined reports no update" "{}" "$(jq -c . <<<"$out")"
printf 'pkgver=1.9.0\npkgrel=1\n' > "$pkg/PKGBUILD"
out=$(github_upstream_release "$pkg" 86400 2>/dev/null) || out="<error>"
check "already checked in reports no update" "{}" "$(jq -c . <<<"$out")"
printf 'pkgver=1.0.0\npkgrel=1\n' > "$pkg/PKGBUILD"
echo "Failure paths:"
local rc
FIXTURE_RELEASES=$(jq -n '[{tag_name: "v1.9.0", published_at: "not-a-date", draft: false, prerelease: false}]')
rc=0; github_upstream_release "$pkg" 86400 >/dev/null 2>&1 || rc=$?
check "invalid published_at fails the sync" "1" "$rc"
FIXTURE_RELEASES=$(jq -n --arg old "$old2d" '[{tag_name: "release 1.9!", published_at: $old, draft: false, prerelease: false}]')
rc=0; github_upstream_release "$pkg" 86400 >/dev/null 2>&1 || rc=$?
check "unusable tag fails the sync" "1" "$rc"
FIXTURE_RELEASES=$(jq -n --arg old "$old2d" '[{tag_name: "v1.9.0", published_at: $old, draft: false, prerelease: false}]')
FIXTURE_CHECKSUMS="$sum_x19 ./tool-v1.9.0-x64.tar.xz"
rc=0; github_upstream_release "$pkg" 86400 >/dev/null 2>&1 || rc=$?
check "missing aarch64 checksum fails the sync" "1" "$rc"
echo "Quarantine backstop:"
local rel st
rel=$(jq -n --arg p "$old2d" '{pkgver: "1.9.0", published_at: $p, sha256sums: {}}')
st=0; release_age_status "$rel" 86400 || st=$?
check "old enough passes" "0" "$st"
rel=$(jq -n --arg p "$young" '{pkgver: "2.0.0", published_at: $p, sha256sums: {}}')
st=0; release_age_status "$rel" 86400 || st=$?
check "too young is held" "1" "$st"
st=0; release_age_status "$rel" 0 || st=$?
check "no policy passes anything" "0" "$st"
st=0; BYPASS_MIN_RELEASE_AGE=1 release_age_status "$rel" 86400 || st=$?
check "deliberate bypass passes" "0" "$st"
rel=$(jq -n '{pkgver: "2.0.0", sha256sums: {}}')
st=0; release_age_status "$rel" 86400 || st=$?
check "missing published_at is unprovable" "2" "$st"
echo "Duration parser:"
local agepkg="$TEMP_DIR/selftest-age"
mkdir -p "$agepkg/.omarchy"
check_age() {
local json_value="$1" expected="$2" got
jq -n "{source: \"local\", min_release_age: $json_value}" > "$agepkg/.omarchy/package.json"
got=$(package_min_release_age_seconds "$agepkg") || got="<reject>"
check "min_release_age $json_value" "$expected" "$got"
}
check_age '"24h"' 86400
check_age '"90m"' 5400
check_age '"2d"' 172800
check_age '3600' 3600
check_age '"600s"' 600
check_age '"010h"' 36000
check_age '"abc"' "<reject>"
check_age '"24hh"' "<reject>"
check_age 'false' "<reject>"
check_age '"9999999999"' "<reject>"
echo "Manifest validation:"
printf 'pkgver=1.0.0\n' > "$agepkg/PKGBUILD"
local vst
echo '{"source": "local", "upstream": false}' > "$agepkg/.omarchy/package.json"
vst=0; validate_package_metadata "$agepkg" >/dev/null || vst=$?
check "upstream: false is rejected" "1" "$vst"
echo '{"source": "local", "upstream": {"github": "example/tool"}}' > "$agepkg/.omarchy/package.json"
vst=0; validate_package_metadata "$agepkg" >/dev/null || vst=$?
check "upstream without checksums/assets is rejected" "1" "$vst"
cp "$pkg/.omarchy/package.json" "$agepkg/.omarchy/package.json"
vst=0; validate_package_metadata "$agepkg" >/dev/null || vst=$?
check "the real declaration shape is accepted" "0" "$vst"
echo ""
if [[ "$failures" -eq 0 ]]; then
print_success "Self-test passed"
else
print_error "$failures self-test failure(s)"
exit 1
fi
}
if [[ ${#SPECIFIC_PACKAGES[@]} -gt 0 && "${SPECIFIC_PACKAGES[0]}" == "self-test" ]]; then
cmd_self_test
exit 0
fi
if [[ ${#SPECIFIC_PACKAGES[@]} -gt 0 ]]; then if [[ ${#SPECIFIC_PACKAGES[@]} -gt 0 ]]; then
SPECIFIC_MODE=true SPECIFIC_MODE=true
for package in "${SPECIFIC_PACKAGES[@]}"; do for package in "${SPECIFIC_PACKAGES[@]}"; do
+26 -10
View File
@@ -82,17 +82,29 @@ package_is_fast_ring() {
# Quarantine window for upstream releases, in seconds. Accepts a bare number # Quarantine window for upstream releases, in seconds. Accepts a bare number
# of seconds or a number suffixed s/m/h/d ("24h", "2d"). Unset means 0 (no # of seconds or a number suffixed s/m/h/d ("24h", "2d"). Unset means 0 (no
# hold); an unparseable value returns 1 so callers fail closed instead of # hold); an unparseable value -- including a non-string/non-number JSON type
# silently dropping the hold. # like false -- returns 1 so callers fail closed instead of silently dropping
# the hold. At most 9 digits: enough for three decades in seconds, and small
# enough that no suffix multiplication can overflow 64-bit arithmetic.
package_min_release_age_seconds() { package_min_release_age_seconds() {
local pkgdir="$1" raw local pkgdir="$1" metadata raw
raw=$(package_metadata_value "$pkgdir" '.min_release_age' "") metadata=$(metadata_file_for_dir "$pkgdir")
if [[ ! -f "$metadata" ]]; then
echo 0
return 0
fi
raw=$(jq -r '
if has("min_release_age") then
.min_release_age | if type == "string" or type == "number" then tostring else "unparseable" end
else "" end
' "$metadata")
if [[ -z "$raw" ]]; then if [[ -z "$raw" ]]; then
echo 0 echo 0
return 0 return 0
fi fi
[[ "$raw" =~ ^([0-9]+)([smhd]?)$ ]] || return 1 [[ "$raw" =~ ^([0-9]{1,9})([smhd]?)$ ]] || return 1
local n=${BASH_REMATCH[1]} # Forced base 10: bash arithmetic would otherwise read "010" as octal.
local n=$((10#${BASH_REMATCH[1]}))
case "${BASH_REMATCH[2]}" in case "${BASH_REMATCH[2]}" in
""|s) echo "$n" ;; ""|s) echo "$n" ;;
m) echo $((n * 60)) ;; m) echo $((n * 60)) ;;
@@ -167,7 +179,8 @@ package_has_upstream_hook() {
package_has_upstream_provider() { package_has_upstream_provider() {
local pkgdir="$1" local pkgdir="$1"
[[ -n "$(package_metadata_value "$pkgdir" '.upstream.github' "")" ]] # `objects` drops a non-object upstream value instead of erroring jq.
[[ -n "$(package_metadata_value "$pkgdir" '(.upstream? | objects | .github)' "")" ]]
} }
packages_for_upstream_sync() { packages_for_upstream_sync() {
@@ -346,15 +359,18 @@ validate_package_metadata() {
return 1 return 1
fi fi
# `has` rather than `// {}`: jq's // treats false as absent, which would
# let "upstream": false slip through as an empty declaration.
if ! jq -e ' if ! jq -e '
(.upstream // {}) | type == "object" if has("upstream") | not then true
and (if . == {} then true else elif (.upstream | type) != "object" then false
else .upstream |
((.github // "") | type == "string" and test("\\A[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+\\z")) ((.github // "") | type == "string" and test("\\A[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+\\z"))
and ((.checksums // "") | type == "string" and length > 0) and ((.checksums // "") | type == "string" and length > 0)
and ((.assets // {}) | type == "object" and length > 0 and (to_entries | all( and ((.assets // {}) | type == "object" and length > 0 and (to_entries | all(
(.key | test("\\A[a-z0-9_]+\\z")) and (.value | type == "string" and length > 0) (.key | test("\\A[a-z0-9_]+\\z")) and (.value | type == "string" and length > 0)
))) )))
end) end
' "$metadata" >/dev/null; then ' "$metadata" >/dev/null; then
echo "invalid upstream for $(basename "$pkgdir"): needs github owner/repo, checksums asset name, and an assets arch->name map" echo "invalid upstream for $(basename "$pkgdir"): needs github owner/repo, checksums asset name, and an assets arch->name map"
return 1 return 1
+23 -6
View File
@@ -21,7 +21,24 @@
package_upstream_github_repo() { package_upstream_github_repo() {
local pkgdir="$1" local pkgdir="$1"
package_metadata_value "$pkgdir" '.upstream.github' "" # `objects` drops a non-object upstream value (validation rejects those
# separately) instead of erroring the jq pipeline.
package_metadata_value "$pkgdir" '(.upstream? | objects | .github)' ""
}
# Fetches sit behind functions so the self-test can replace them with fixture
# readers; everything below the fetch is deterministic and testable offline.
# Only the 100 most recent releases are considered -- a bounded search, not
# pagination. A feed whose entire first page is drafts, prereleases, or
# quarantined releases reports no update and waits for the next run.
github_fetch_releases() {
local repo="$1"
curl -fsSL "https://api.github.com/repos/$repo/releases?per_page=100"
}
github_fetch_checksums() {
local repo="$1" tag="$2" asset="$3"
curl -fsSL "https://github.com/$repo/releases/download/$tag/$asset"
} }
# Emits the newest qualifying release as hook-contract JSON. min_release_age # Emits the newest qualifying release as hook-contract JSON. min_release_age
@@ -35,18 +52,18 @@ github_upstream_release() {
local metadata repo checksums_name local metadata repo checksums_name
metadata=$(metadata_file_for_dir "$package_dir") metadata=$(metadata_file_for_dir "$package_dir")
repo=$(jq -r '.upstream.github // ""' "$metadata") repo=$(jq -r '(.upstream? | objects | .github) // ""' "$metadata")
if [[ ! "$repo" =~ ^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$ ]]; then if [[ ! "$repo" =~ ^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$ ]]; then
echo "invalid upstream.github repository: '${repo:-<empty>}'" >&2 echo "invalid upstream.github repository: '${repo:-<empty>}'" >&2
return 1 return 1
fi fi
checksums_name=$(jq -r '.upstream.checksums // ""' "$metadata") checksums_name=$(jq -r '(.upstream? | objects | .checksums) // ""' "$metadata")
if [[ -z "$checksums_name" ]]; then if [[ -z "$checksums_name" ]]; then
echo "upstream.checksums names the checksum manifest asset and is required" >&2 echo "upstream.checksums names the checksum manifest asset and is required" >&2
return 1 return 1
fi fi
local arches local arches
mapfile -t arches < <(jq -r '.upstream.assets // {} | keys[]' "$metadata") mapfile -t arches < <(jq -r '(.upstream? | objects | .assets) // {} | keys[]' "$metadata")
if [[ ${#arches[@]} -eq 0 ]]; then if [[ ${#arches[@]} -eq 0 ]]; then
echo "upstream.assets must map at least one architecture to an asset name" >&2 echo "upstream.assets must map at least one architecture to an asset name" >&2
return 1 return 1
@@ -54,7 +71,7 @@ github_upstream_release() {
local releases now local releases now
now=$(date +%s) now=$(date +%s)
if ! releases=$(curl -fsSL "https://api.github.com/repos/$repo/releases?per_page=20"); then if ! releases=$(github_fetch_releases "$repo"); then
echo "could not fetch the release feed for $repo" >&2 echo "could not fetch the release feed for $repo" >&2
return 1 return 1
fi fi
@@ -108,7 +125,7 @@ github_upstream_release() {
fi fi
local checksums local checksums
if ! checksums=$(curl -fsSL "https://github.com/$repo/releases/download/$best_tag/$checksums_name"); then if ! checksums=$(github_fetch_checksums "$repo" "$best_tag" "$checksums_name"); then
echo "could not fetch $checksums_name for $repo $best_tag" >&2 echo "could not fetch $checksums_name for $repo $best_tag" >&2
return 1 return 1
fi fi