Skip to content

Flip counters to pre-increment to avoid bash evaluating it as 'failure' and exiting - #151

Open
mark-brannan wants to merge 1 commit into
halos-org:mainfrom
mark-brannan:main
Open

Flip counters to pre-increment to avoid bash evaluating it as 'failure' and exiting#151
mark-brannan wants to merge 1 commit into
halos-org:mainfrom
mark-brannan:main

Conversation

@mark-brannan

@mark-brannan mark-brannan commented Aug 22, 2026

Copy link
Copy Markdown

The run script was failing for me immediately after the first clone. Then it would fail immediately after the first skip.

The problem is this type of usage in bash witherrexit:

my_var=0
((my_var++)) # fails, non-zero exit status

If the variable contains a zero value, bash evaluates the line to 'Failure' (1) and the script halts.
Pre-increment sidesteps this:

my_var=0
((++my_var)) # works, value is always non-zero so exit status is zero

My original run and subsequent attempts looked like this:

$ ./run clone-repos
🔄 Checking repositories...
📥 Cloning halos-metapackages (branch: main)...
Cloning into 'halos-metapackages'...
remote: Enumerating objects: 453, done.
remote: Counting objects: 100% (19/19), done.
remote: Compressing objects: 100% (14/14), done.
remote: Total 453 (delta 3), reused 10 (delta 3), pack-reused 434 (from 2)
Receiving objects: 100% (453/453), 105.67 KiB | 1.09 MiB/s, done.
Resolving deltas: 100% (182/182), done.
✅ halos-metapackages cloned successfully

Task completed in 0m1.487s


$ ./run clone-repos
🔄 Checking repositories...
⏭️  halos-metapackages (already exists)

Task completed in 0m0.000s

But then show status would catch it:

$ ./run show-status
📊 Repository status:

📁 halos-metapackages:
## main...origin/main

❌ opencpn-docker: NOT CLONED
❌ apt.hatlabs.fi: NOT CLONED
❌ avnav-docker: NOT CLONED
❌ halos-pi-gen: NOT CLONED
❌ runtipi-docker-service: NOT CLONED
❌ cockpit-networkmanager-halos: NOT CLONED
❌ runtipi-marine-app-store: NOT CLONED
❌ halos-cockpit-config: NOT CLONED
❌ halos-core-containers: NOT CLONED
❌ cockpit-apt: NOT CLONED

Task completed in 0m0.006s

Testing the change:

./run  clone-repos
🔄 Checking repositories...
halos-metapackages + git@github.com:halos-org/halos-metapackages.git main
opencpn-docker + git@github.com:halos-org/opencpn-docker.git main
apt.hatlabs.fi + git@github.com:hatlabs/apt.hatlabs.fi.git main
avnav-docker + git@github.com:hatlabs/avnav-docker.git main
halos-pi-gen + git@github.com:halos-org/halos-pi-gen.git main
runtipi-docker-service + git@github.com:hatlabs/runtipi-docker-service.git main
cockpit-networkmanager-halos + git@github.com:halos-org/cockpit-networkmanager-halos.git main
runtipi-marine-app-store + git@github.com:hatlabs/runtipi-marine-app-store.git main
halos-cockpit-config + git@github.com:halos-org/halos-cockpit-config.git main
halos-core-containers + git@github.com:halos-org/halos-core-containers.git main
cockpit-apt + git@github.com:halos-org/cockpit-apt.git main
next: repo=halos-metapackages branch=git@github.com:halos-org/halos-metapackages.git main
⏭️  halos-metapackages (already exists)
end of loop: 0
next: repo=opencpn-docker branch=git@github.com:halos-org/opencpn-docker.git main
⏭️  opencpn-docker (already exists)
end of loop: 0
next: repo=apt.hatlabs.fi branch=git@github.com:hatlabs/apt.hatlabs.fi.git main
reading apt.hatlabs.fi ...
📥 Cloning apt.hatlabs.fi (branch: main)...
Cloning into 'apt.hatlabs.fi'...
remote: Enumerating objects: 28648, done.
remote: Counting objects: 100% (1569/1569), done.
remote: Compressing objects: 100% (797/797), done.
remote: Total 28648 (delta 1061), reused 1260 (delta 761), pack-reused 27079 (from 3)
Receiving objects: 100% (28648/28648), 419.40 MiB | 4.52 MiB/s, done.
Resolving deltas: 100% (14692/14692), done.
✅ apt.hatlabs.fi cloned successfully
end of loop: 0
next: repo=avnav-docker branch=git@github.com:hatlabs/avnav-docker.git main
reading avnav-docker ...
📥 Cloning avnav-docker (branch: main)...
Cloning into 'avnav-docker'...
...
📥 Cloning cockpit-apt (branch: main)...
Cloning into 'cockpit-apt'...
remote: Enumerating objects: 2288, done.
remote: Counting objects: 100% (151/151), done.
remote: Compressing objects: 100% (91/91), done.
remote: Total 2288 (delta 70), reused 107 (delta 55), pack-reused 2137 (from 2)
Receiving objects: 100% (2288/2288), 1.01 MiB | 2.06 MiB/s, done.
Resolving deltas: 100% (1402/1402), done.
✅ cockpit-apt cloned successfully
end of loop: 0

📊 Summary: 0 cloned, 0 skipped

Task completed in 1m59.351s


# clone now skips 'em all
./run  clone-repos
🔄 Checking repositories...
⏭️  halos-metapackages (already exists)
⏭️  opencpn-docker (already exists)
⏭️  apt.hatlabs.fi (already exists)
⏭️  avnav-docker (already exists)
⏭️  halos-pi-gen (already exists)
⏭️  runtipi-docker-service (already exists)
⏭️  cockpit-networkmanager-halos (already exists)
⏭️  runtipi-marine-app-store (already exists)
⏭️  halos-cockpit-config (already exists)
⏭️  halos-core-containers (already exists)
⏭️  cockpit-apt (already exists)

📊 Summary: 0 cloned, 11 skipped

Task completed in 0m0.001s

# remove one to see if can handle mixed case
rm -rf halos-metapackages/


$ ./run  clone-repos
🔄 Checking repositories...
reading halos-metapackages ...
📥 Cloning halos-metapackages (branch: main)...
Cloning into 'halos-metapackages'...
remote: Enumerating objects: 453, done.
remote: Counting objects: 100% (19/19), done.
remote: Compressing objects: 100% (14/14), done.
remote: Total 453 (delta 3), reused 10 (delta 3), pack-reused 434 (from 2)
Receiving objects: 100% (453/453), 105.67 KiB | 672.00 KiB/s, done.
Resolving deltas: 100% (182/182), done.
✅ halos-metapackages cloned successfully
⏭️  opencpn-docker (already exists)
⏭️  apt.hatlabs.fi (already exists)
⏭️  avnav-docker (already exists)
⏭️  halos-pi-gen (already exists)
⏭️  runtipi-docker-service (already exists)
⏭️  cockpit-networkmanager-halos (already exists)
⏭️  runtipi-marine-app-store (already exists)
⏭️  halos-cockpit-config (already exists)
⏭️  halos-core-containers (already exists)
⏭️  cockpit-apt (already exists)

📊 Summary: 1 cloned, 10 skipped

Task completed in 0m2.070s

I'm on WSL/Ubuntu on my windows machine:

bash --version
GNU bash, version 5.3.9(1)-release (x86_64-pc-linux-gnu)
Copyright (C) 2025 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Have not tried in on the shell on my mac but I usually use zsh there; not sure if there is some variation in how different bash versions treat the pre-increment and post-increment.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed repository-management counter updates so operations complete reliably when running with strict error handling.
    • Preserved existing clone and pull behavior while preventing premature termination during the first counter increment.

@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f9490ac2-737f-4317-a6ce-7b41efd18849

📥 Commits

Reviewing files that changed from the base of the PR and between e60a8df and c47be02.

📒 Files selected for processing (1)
  • run

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The run script now uses pre-increment arithmetic for repository-management counters in clone-repos and pull-all-main. Repository detection, cloning, update handling, error handling, and reporting remain unchanged.

Changes

Repository counter handling

Layer / File(s) Summary
Pre-increment repository counters
run
clone-repos pre-increments skipped and cloned. pull-all-main pre-increments missing, updated, and failed.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to c47be

The change makes repository clone and skip counters safe under Bash errexit behavior. No actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: mairas

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: replacing post-increment counters with pre-increment counters to prevent Bash errexit failures.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@mark-brannan
mark-brannan marked this pull request as ready for review August 22, 2026 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant