Skip to content

Commit 817cb4b

Browse files
chore(parser_app): address PR #303 follow-up review comments (#309)
prasanna-anchorage approved #303 with four follow-up asks. This commit addresses all of them: 1. Restore dynamic chain discovery via $(wildcard) wrapped in $(sort). The hardcoded list required editing two places (Makefile + Cargo.toml [features]) for every new chain crate. The sorted wildcard gives the same stable ordering the depth-2 pair generator relied on, with no manual sync. 2. Swap `cargo build` for `cargo check` in both narrow-build-check and feature-matrix-check. The regression class these targets guard (unconditional chain crate references in parser_app) is compile-level, so `check` is sufficient. Roughly 2x faster on per-PR runs, and the feature-matrix target dir grows by ~2GB instead of ~28GB. 3. Add `--no-default-features --features diagnostics` (no chains) to feature-matrix-check so the diagnostics axis is symmetric. 4. Tighten the narrow-build-check comment to reflect the auto-pickup behavior the dynamic variable provides; rewrite the CI cache note so it's accurate (parser_app re-type-checks under each feature set while workspace deps reuse the cache). Verified locally: make narrow-build-check (6 checks + 6 clippy, exit 0); make feature-matrix-check (31 checks across the matrix, exit 0, +~2GB in src/target).
1 parent 5d3b717 commit 817cb4b

1 file changed

Lines changed: 47 additions & 26 deletions

File tree

src/Makefile

Lines changed: 47 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,18 @@ PARSER_INNER_SOCKET_PATH ?= /tmp/inner_parser.sock
77

88
ROOT := $(shell git rev-parse --show-toplevel)
99

10-
# Chains parser_app can be built with. Single source of truth for both
11-
# narrow-build-check and feature-matrix-check below; keep in sync with
12-
# the [features] block in parser_app/Cargo.toml. Listed alphabetically
13-
# so the depth-2 pair generator in feature-matrix-check produces stable,
14-
# unique pairs.
15-
PARSER_APP_CHAINS := ethereum solana sui tron unspecified
10+
# Chains parser_app can be built with -- derived from the
11+
# `chain_parsers/visualsign-<name>/` directory layout that the workspace
12+
# already uses as its source of truth. `$(sort ...)` deduplicates and
13+
# alphabetizes so the depth-2 pair generator in feature-matrix-check
14+
# produces stable, unique pairs across machines (filesystem-listing
15+
# order isn't guaranteed otherwise).
16+
#
17+
# Convention: every `chain_parsers/visualsign-<name>/` directory has a
18+
# matching `<name>` feature in parser_app/Cargo.toml. A chain crate added
19+
# without that feature wired in fails loudly at narrow-build-check on
20+
# the PR that introduces it -- which is the right place to catch it.
21+
PARSER_APP_CHAINS := $(sort $(patsubst visualsign-%,%,$(notdir $(wildcard chain_parsers/visualsign-*))))
1622

1723
.PHONY: all
1824
all: generated
@@ -55,55 +61,70 @@ lint:
5561
narrow-build-check:
5662
@# Guards parser_app's per-chain feature gating. If a future change
5763
@# re-introduces an unconditional reference to a chain crate from
58-
@# parser_app, one of these narrow builds will fail to compile.
64+
@# parser_app, one of these narrow checks will fail to compile.
5965
@# Wired into main CI; runs on every PR. Driven by
60-
@# $(PARSER_APP_CHAINS) above -- when a chain is added to
61-
@# parser_app/Cargo.toml, extend that variable and the matrix picks
62-
@# the new chain up.
66+
@# $(PARSER_APP_CHAINS) above, which auto-extends when a new
67+
@# chain_parsers/visualsign-<name>/ directory is added.
6368
@#
64-
@# Includes clippy under each narrow feature set so dead-code /
69+
@# Uses `cargo check` rather than `cargo build`: the failure modes
70+
@# we want to catch here are compile-level (unconditional crate
71+
@# references that should have been cfg-gated). `check` produces no
72+
@# linker output, runs ~2x faster, and matches what clippy needs.
73+
@#
74+
@# Clippy runs under each narrow feature set so dead-code /
6575
@# unused-import lints that only trigger when a chain is OFF are
6676
@# enforced (make lint above only covers default features).
67-
cargo build -p parser_app --no-default-features
77+
@#
78+
@# Note: parser_app re-type-checks under each distinct feature set,
79+
@# but workspace dependencies are reused from the cache that earlier
80+
@# CI steps (make test / make lint) already populated -- so the
81+
@# marginal CI cost is dominated by parser_app's own check time.
82+
cargo check -p parser_app --no-default-features
6883
cargo clippy -p parser_app --no-default-features --all-targets -- -D warnings
6984
@set -e; for c in $(PARSER_APP_CHAINS); do \
7085
echo "==> features: $$c"; \
71-
cargo build -p parser_app --no-default-features --features $$c; \
86+
cargo check -p parser_app --no-default-features --features $$c; \
7287
cargo clippy -p parser_app --no-default-features --features $$c --all-targets -- -D warnings; \
7388
done
7489

7590
.PHONY: feature-matrix-check
7691
feature-matrix-check:
77-
@# Deep coverage of parser_app's feature gating: every chain alone,
78-
@# every chain + diagnostics, every depth-2 pair, and every depth-2
79-
@# pair + diagnostics. Hand-rolled (no cargo-hack dependency); matrix
80-
@# is generated from $(PARSER_APP_CHAINS) above.
92+
@# Deep coverage of parser_app's feature gating: diagnostics alone
93+
@# (no chains), every chain alone, every chain + diagnostics, every
94+
@# depth-2 pair, and every depth-2 pair + diagnostics. Hand-rolled
95+
@# (no cargo-hack dependency); matrix is generated from
96+
@# $(PARSER_APP_CHAINS) above.
8197
@#
8298
@# Note: `diagnostics` propagates to the always-on `visualsign` core
8399
@# crate, and weakly to `visualsign-solana` via the `?` syntax. The
84100
@# non-solana + diagnostics combos still exercise the weak-dep path
85101
@# (resolving to a no-op for visualsign-solana).
86102
@#
87-
@# LOCAL ONLY -- not wired into CI. The full matrix accumulates ~28GB
88-
@# in the cargo target dir; ensure ~30GB free in src/target before
89-
@# running. Use before releases or when touching feature definitions;
90-
@# the per-PR narrow-build-check above covers the common regression
91-
@# class on every change.
103+
@# Uses `cargo check` for the same reason as narrow-build-check
104+
@# above -- the goal is compile-level regression detection, not
105+
@# producing binaries.
106+
@#
107+
@# LOCAL ONLY -- not wired into CI. The full matrix grows the cargo
108+
@# target dir by a few GB. Run before releases or when touching
109+
@# feature definitions; the per-PR narrow-build-check above covers
110+
@# the common regression class on every change.
111+
echo "==> features: diagnostics"
112+
cargo check -p parser_app --no-default-features --features diagnostics
92113
@set -e; for c in $(PARSER_APP_CHAINS); do \
93114
echo "==> features: $$c"; \
94-
cargo build -p parser_app --no-default-features --features $$c; \
115+
cargo check -p parser_app --no-default-features --features $$c; \
95116
echo "==> features: $$c diagnostics"; \
96-
cargo build -p parser_app --no-default-features --features "$$c diagnostics"; \
117+
cargo check -p parser_app --no-default-features --features "$$c diagnostics"; \
97118
done
98119
@set -e; i=0; for c1 in $(PARSER_APP_CHAINS); do \
99120
i=$$((i+1)); j=0; \
100121
for c2 in $(PARSER_APP_CHAINS); do \
101122
j=$$((j+1)); \
102123
if [ $$j -gt $$i ]; then \
103124
echo "==> features: $$c1 $$c2"; \
104-
cargo build -p parser_app --no-default-features --features "$$c1 $$c2"; \
125+
cargo check -p parser_app --no-default-features --features "$$c1 $$c2"; \
105126
echo "==> features: $$c1 $$c2 diagnostics"; \
106-
cargo build -p parser_app --no-default-features --features "$$c1 $$c2 diagnostics"; \
127+
cargo check -p parser_app --no-default-features --features "$$c1 $$c2 diagnostics"; \
107128
fi; \
108129
done; \
109130
done

0 commit comments

Comments
 (0)