Skip to content

chore: gitignore the cmd modules' built binaries - #1180

Open
Amperstrand wants to merge 1 commit into
mainfrom
chore/gitignore-built-binaries
Open

Amperstrand wants to merge 1 commit into
mainfrom
chore/gitignore-built-binaries

Conversation

@Amperstrand

Copy link
Copy Markdown
Collaborator

Ends a recurring artifact class: src/cmd/tollgate-ndsctl/tollgate-ndsctl committed itself in #1140 and #1151 today (flagged in review on both). Each cmd/ module's executable lands next to its sources because the module path names the binary. One anchored line per module — a glob pattern here would swallow the modules' source files (checked: main.go and friends stay visible). tollgate-cvm was already covered; ndsctl, cli, and the vendor demo join it.

src/cmd/tollgate-ndsctl/tollgate-ndsctl committed itself twice today
(#1140, #1151) — each cmd/ module's executable lands next to its
sources because the module path names the binary. One anchored line
per module (a glob would swallow the source files); cvm was already
covered, ndsctl/cli/vendor-demo now too.
@felixfelix-bot

Copy link
Copy Markdown
Collaborator

Two small fixes stacked on your branch: #1184 — #1184 (base chore/gitignore-built-binaries, one file, +2/−4).

  1. src/cmd/tollgate-cli/tollgate-cli is listed twice — already at line 25, added again at line 117. Git honours the first, so the second is dead weight.
  2. The new comment implied the tracked ndsctl binary was committed on purpose. It was not: deps(wallet): gonuts-tollgate v0.15.0 — intents, NUT-09 restore, response binding #1151 carries it by accident (3.5 MB x86-64 ELF, A against main, not tracked upstream). Reworded without that implication.

One thing worth knowing about what this PR can and cannot do: a .gitignore rule has no effect on an already-tracked path. This stops new checkouts from re-adding the binary to the index, but the blob stays in the tree until git rm --cached runs on #1151. I raised that there separately. This PR is still correct and worth landing — just not sufficient alone, which is why the comment no longer reads as though the tracked copy is intended.

Cherry-pick, merge, or ignore — whichever suits your workflow.

@felixfelix-bot

Copy link
Copy Markdown
Collaborator

Review — PR #1180, head chore/gitignore-built-binaries

Verdict: land with #1184 applied (or take those two lines as-is). The intent is right and it closes a real hazard; as pushed it has one duplicate entry and one misleading comment.

Verified

$ grep -v '^#' .gitignore | grep -v '^$' | sort | uniq -d
.omo/
src/cmd/tollgate-cli/tollgate-cli

src/cmd/tollgate-cli/tollgate-cli now appears twice — it was already at line 25, and this PR adds it again at line 117. Git honours the first match, so the second is dead weight. (.omo/ is a pre-existing duplicate on main, not yours.)

The comment also asserts that tollgate-ndsctl "committed itself in #1140 and #1151", which reads as though the binary was tracked deliberately. It was not: #1151 carries a 3.5 MB x86-64 ELF at src/cmd/tollgate-ndsctl/tollgate-ndsctl by accident, A against main, untracked upstream. A comment that records an accident as intent is how someone eventually "restores" the binary to match the note.

The additions that are right

src/cmd/tg-vendor-ie-demo/tg-vendor-ie-demo and src/cmd/tollgate-ndsctl/tollgate-ndsctl are both correct and correctly anchored:

$ git check-ignore -v src/cmd/tollgate-ndsctl/tollgate-ndsctl
.gitignore:115:src/cmd/tollgate-ndsctl/tollgate-ndsctl	src/cmd/tollgate-ndsctl/tollgate-ndsctl

The one-line-per-module comment with "a glob here would swallow the modules' source files" is the right reasoning, and it matches what #1183 does for Rust (sidecar/*/target/) — anchored, not globbed.

Already fixed

#1184 (base chore/gitignore-built-binaries, head +2/−4) removes the duplicate and rewords the ndsctl note. I reviewed it: duplicate gone, check-ignore still matches, and it is a clean two-file-line fix. Land #1184 and this PR is complete.

Still outstanding, and outside this PR

#1151 still carries the ELF in its own history. An ignore entry does not remove a committed blob — that PR needs git rm --cached plus a rewrite (or a fresh branch). Landing this first gives the rewrite somewhere clean to go.

Not verified

Nothing behavioural to verify — .gitignore only. No router, no code.


Reviewed against PR-REVIEW.md. Evidence: duplicate scan of the merged .gitignore on this branch; git check-ignore -v on the binary path; git ls-tree confirming the ELF is untracked on main.

@felixfelix-bot

Copy link
Copy Markdown
Collaborator

Two src/merchant tests fail on this branch. It is a stale base, not your change.

Measured on this branch directly:

$ cd src/merchant && go test -count=1 -run 'TestMerchantDegraded_StorageMmap' .
--- FAIL: TestMerchantDegraded_StorageMmap_FundErrorNamesStorage
--- FAIL: TestMerchantDegraded_StorageMmap_EveryWalletOpNamesStorage

The assertion that trips:

merchant_degraded_mmap_test.go:37: Fund error should name the storage/mmap cause,
  got: wallet not initialized: no reachable mints
merchant_degraded_mmap_test.go:40: Fund error must not blame the mints when the
  mints are fine (#583), got: wallet not initialized: no reachable mints

Cause: this branch is behind main

main gained the storage-mmap repair in aa677c00 (#1150) and 8e084b9d (#1040) — notInitializedError() now routes the storage/mmap class to a filesystem-naming message instead of the historic "no reachable mints". This branch predates that, so the two tests that pin it fail.

Verified against a control rather than assumed:

$ cd src/merchant && go test -race -run 'TestMerchantDegraded_StorageMmap' -v .   # at upstream/main b4f15513
--- PASS: …FundErrorNamesStorage (0.01s)
--- PASS: …EveryWalletOpNamesStorage (0.01s)
--- PASS: …AdvertisementNamesStorage (0.23s)
--- PASS: …DegradedInfoReasons (0.01s)
ok  …/src/merchant  1.307s

All four pass on pristine main. So the red is inherited staleness, not something your diff did — and I checked this on nine open branches, of which the ones already carrying the repair pass and the ones behind fail. Exactly these two tests, always.

Fix

Rebase onto current main:

git fetch origin main && git rebase origin/main
cd src/merchant && go test -count=1 .   # expect green

No code change needed from you.

Why I am posting this rather than leaving it

A red src/merchant on your branch will look like your fault to the next reviewer, and reproducing it costs a ~5-minute -race build plus a control comparison against main before it can be cleared. Stating the measured cause once, here, saves that — and it is not a defect in the change under review.

@Amperstrand

Copy link
Copy Markdown
Collaborator Author

Local CI gate — PR #1180: chore: gitignore the cmd modules' built binaries

Run of scripts/pr-verify.sh (the local CI gate: GitHub Actions is
org-blocked and the ngit lane fires only on maintainer mirror pushes,
so a PR ref gets no CI runs — this comment is the offline verification
evidence, produced on the pinned head SHA).

  • Head: chore/gitignore-built-binaries @ 655b948f8e
  • Base: main @ 6aabea2d36
  • Gate: pr-verify 6f304ed0e72d on ai-legion, started 2026-11-05 02:56 UTC, wall 91s
  • Logs: /tmp/tollgate-pr-verify/1180-20261105T025438Z (on the verifying host)
Leg Result Detail
hygiene PASS no conflict markers in the tracked tree (0s)
contract-fences FAIL red: changelog fragments (assemble check) doc-facts contract dependency sync — see contract-fences.log; 13 checks green (4s)
packaging-sequential FAIL red (3/17): backend-api-owner-network_test.sh entry-ui-install-sequence_test.sh rebrand-literal-gutter_test.sh — logs in /tmp/tollgate-pr-verify/1180-20261105T025438Z/packaging-sequential (15s)
shell-suites FAIL red (2/23): uci-defaults-gatewayname-banner_test.sh uci-defaults-lan-private-wired_test.sh — logs in /tmp/tollgate-pr-verify/1180-20261105T025438Z/shell-suites (67s)
packaging-concurrent FAIL red (3/17): backend-api-owner-network_test.sh entry-ui-install-sequence_test.sh rebrand-literal-gutter_test.sh — logs in /tmp/tollgate-pr-verify/1180-20261105T025438Z/packaging-concurrent (5s)
go-battery FAIL 19 modules — see go-battery.log (0s)

Verdict: 5 leg(s) RED — 1 green.

What this gate does not run (no vacuous green)
  • spec-quote drift check (greatspectations tool + pinned nuts fetch)
  • reproducibility / conformance lanes (make release-check territory)
  • router-visible behaviour (PRTA / bench — unit green ≠ router works)
failing-leg log tails
contract-fences — tail

🔴 google.golang.org/genproto/googleapis/rpc — 2 versions:
   root: v0.0.0-20260928230214-8a89bd6388cc
   tollwallet: v0.0.0-20260720211330-0afa2a65878a

🔴 google.golang.org/grpc — 2 versions:
   root: v1.83.2
   tollwallet: v1.82.1

Fix: update all go.mod files to the same version.
     Run 'go mod tidy' in each sub-module after updating.
== gonuts pin fence (offline identity)
ok   an aligned tree (block + single-line carriers) passes
ok   a block-form carrier on a different version
ok   a single-line carrier left behind (the token-recovery shape)
ok   a go.sum that never recorded the truth version (half-update)
ok   a self-consistent pseudo-version integration pin passes OFFLINE (the --latest leg is the release gate for it)
ok   a manifest without the .gonuts key
ok   a tree whose manifest names a pin no carrier has

7 cases, 0 failed
== gonuts pin carriers
  manifest pin (packaging/build-inputs.json .gonuts.version): v0.14.1
  carriers discovered by glob: 5 module(s)
✅ one pin, everywhere: github.com/OpenTollGate/gonuts-tollgate v0.14.1 across 5 carrier(s).
== mirror/relay sync
ok    BLOSSOM_SERVERS: 13 copies identical (https://blossom.primal.net https://blossom.psbt.me https://blossom2.orangesync.tech)
ok    relay list: 13 copies identical (wss://relay.damus.io wss://nos.lol wss://nostr.mom wss://relay2.orangesync.tech)
== stale import paths
✅ All 358 Go source files use correct import paths.
packaging-sequential — tail
--- backend-api-owner-network_test.sh.log
ok   the fragment guards exactly :2121
ok   packaging/files/etc/nftables.d/30-backend-firewall.nft is an include fragment (no own table declaration)
ok   the chain hooks input at priority -1 (the drop is authoritative)
ok   chain 'backend_input_firewall' is declared once
== the admin-board and LuCI guards are still captive-bridge-scoped
ok   31-admin-board-not-guest-reachable.nft is scoped by $tg_portal_if (the captive bridge's define)
ok   32-luci-not-guest-reachable.nft is scoped by $tg_portal_if (the captive bridge's define)
== the recipe installs the rule
ok   local-build-ipk.sh installs 30-backend-firewall.nft
ok   packaging/Makefile installs the 30-backend-firewall.nft rule via its *.nft glob
ok   packaging/Makefile FILES_ lists /etc/nftables.d/30-backend-firewall.nft
== the fragment parses
FAIL nft -c rejects the fragment — 'nft -f' would fail the whole fw4 reload

passed=17 failed=1
--- entry-ui-install-sequence_test.sh.log

== scenario 4b: converged box, root hash emptied — 999 re-refuses the board
ok   scenario 4b: :8080 is unbound — the refused board's own pair
ok   scenario 4b: :443 is unbound — the refused board's own pair
ok   scenario 4b: exactly one section binds :8090 (main)
ok   scenario 4b: exactly one section binds :8443 (main)
ok   scenario 4b: uhttpd.main keeps at least one listener
ok   scenario 4b: uhttpd.portal keeps at least one listener
ok   scenario 4b: uhttpd.trusted keeps at least one listener
ok   scenario 4b: the admin board is deliberately refused (fail-closed gate), not torn
ok   scenario 4b: LuCI (uhttpd.main) answers :8090 on both families
ok   scenario 4b: LuCI (uhttpd.main) answers :8443 on both families
ok   scenario 4b: the board is refused — its pair is intentionally unbound

tests: 72 passed, 1 failed
--- rebrand-literal-gutter_test.sh.log

== negative control: the scan pattern sees a planted occurrence
ok   the pattern matches a planted occurrence (the gutter is not inert)
ok   the pattern does not match a near-miss brand name (it is anchored to the banned literal)
ok   scan_tree ignores the control file (it is outside the tracked tree)

== the shipped uci-defaults reference only known uhttpd sections
ok   every uhttpd.<name> in the uci-defaults' active code is a known section (or a cert/key path)

== control: the section tokenizer ignores comments, not code
ok   comment-only section names (full-line and trailing) are not flagged
ok   known sections in active code are still extracted
ok   a foreign section in active code is still extracted (the check keeps its teeth)

tests: 8 passed, 1 failed
shell-suites — tail
--- uci-defaults-gatewayname-banner_test.sh.log
ok   the FULL path's banner IS the canonical one from the shared table
ok   reinstall (same version) run exits 0
ok   the same-version reinstall took the VERIFY branch
ok   the repair path COMMITS the banner '!TollGate-OQ3Q Portal'
ok   the repair path's banner is the SAME value the full path writes
ok   no gatewayname op is left pending in the /tmp/.uci-style delta
FAIL the repair path's captive SSID converged on the same code (got '!tollgate-0GLK', want '!TollGate-OQ3Q')
FAIL the banner is exactly '<captive SSID> Portal' (got '!TollGate-OQ3Q', want '!tollgate-0GLK')
ok   the operator's PSK is untouched
ok   drifting reinstall run exits 0
ok   the drifting repair path commits the SAME banner
ok     ...and the drifted allow-list rule was repaired (the commit did fire)
ok     ...with no gatewayname op left pending

27 passed, 2 failed
--- uci-defaults-lan-private-wired_test.sh.log
ok   silent no-op: the log names the missing port
== empty source and empty target: reported as an error, nothing written
ok   empty source+target: writer returns non-zero (1)
ok   empty source+target: private_bridge still has no ports
ok   empty source+target: log reports the broken state
== bridge device section beyond index 23 is still found and moved
ok   late bridge: writer returns 0
ok   network.private_bridge lists eth1 exactly once
ok   late bridge: the source section was cleared
== negative control: a script with a no-op writer fails the assertions
ok   negative control: the writer is neutralised in the copied script
ok   negative control: without the writer the port stays on the captive bridge
ok   negative control: the captive bridge keeps its port (the move assertion would fail)

lan-private-wired: 8 check(s) FAILED, 35 passed
packaging-concurrent — tail
--- backend-api-owner-network_test.sh.log
ok   the fragment guards exactly :2121
ok   packaging/files/etc/nftables.d/30-backend-firewall.nft is an include fragment (no own table declaration)
ok   the chain hooks input at priority -1 (the drop is authoritative)
ok   chain 'backend_input_firewall' is declared once
== the admin-board and LuCI guards are still captive-bridge-scoped
ok   31-admin-board-not-guest-reachable.nft is scoped by $tg_portal_if (the captive bridge's define)
ok   32-luci-not-guest-reachable.nft is scoped by $tg_portal_if (the captive bridge's define)
== the recipe installs the rule
ok   local-build-ipk.sh installs 30-backend-firewall.nft
ok   packaging/Makefile installs the 30-backend-firewall.nft rule via its *.nft glob
ok   packaging/Makefile FILES_ lists /etc/nftables.d/30-backend-firewall.nft
== the fragment parses
FAIL nft -c rejects the fragment — 'nft -f' would fail the whole fw4 reload

passed=17 failed=1
--- entry-ui-install-sequence_test.sh.log

== scenario 4b: converged box, root hash emptied — 999 re-refuses the board
ok   scenario 4b: :8080 is unbound — the refused board's own pair
ok   scenario 4b: :443 is unbound — the refused board's own pair
ok   scenario 4b: exactly one section binds :8090 (main)
ok   scenario 4b: exactly one section binds :8443 (main)
ok   scenario 4b: uhttpd.main keeps at least one listener
ok   scenario 4b: uhttpd.portal keeps at least one listener
ok   scenario 4b: uhttpd.trusted keeps at least one listener
ok   scenario 4b: the admin board is deliberately refused (fail-closed gate), not torn
ok   scenario 4b: LuCI (uhttpd.main) answers :8090 on both families
ok   scenario 4b: LuCI (uhttpd.main) answers :8443 on both families
ok   scenario 4b: the board is refused — its pair is intentionally unbound

tests: 72 passed, 1 failed
--- rebrand-literal-gutter_test.sh.log

== negative control: the scan pattern sees a planted occurrence
ok   the pattern matches a planted occurrence (the gutter is not inert)
ok   the pattern does not match a near-miss brand name (it is anchored to the banned literal)
ok   scan_tree ignores the control file (it is outside the tracked tree)

== the shipped uci-defaults reference only known uhttpd sections
ok   every uhttpd.<name> in the uci-defaults' active code is a known section (or a cert/key path)

== control: the section tokenizer ignores comments, not code
ok   comment-only section names (full-line and trailing) are not flagged
ok   known sections in active code are still extracted
ok   a foreign section in active code is still extracted (the check keeps its teeth)

tests: 8 passed, 1 failed
go-battery — tail
=== module: src
gofmt: unformatted files (fix with: gofmt -w .):
merchant/unmeterable_session_cadence_test.go
make: *** [Makefile:13: go-battery] Error 1

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.

2 participants