Skip to content

bashrs lint Makefile — false positives + --fail-on error flag not honored in v6.66.1 prebuilt #209

Description

@noahgift

Hit four problems running bashrs lint Makefile against a small (~70 line) production-style Makefile in paiml/mysql-from-zero. The recipes are correct and the Make idioms are standard, but the linter flags patterns that are impossible to satisfy without breaking the recipes.

Upstream context

Full warning summary from the failing CI run:

Issues found in Makefile:

⚠ 33:11-12 [warning] MAKE003: Unquoted variable in command - may cause word splitting issues
  Fix: \"$\"

⚠ 33:65-77 [warning] MAKE003: Unquoted variable in command - may cause word splitting issues
  Fix: \"$(MYSQL_CTR)\"

⚠ 54:2-6 [warning] MAKE010: Command 'curl' missing error handling - consider adding '|| exit 1'
  Fix: curl -fsSL \"$(SAKILA_URL)\" | tar xz || { echo \"✗ Sakila download failed\"; exit 1; } || exit 1

⚠ 59:2-4 [warning] MAKE010: Command 'rm' missing error handling - consider adding '|| exit 1'
  Fix: rm -rf \"$(SAKILA_DIR)\" || { echo \"✗ rm failed\"; exit 1; } || exit 1

⚠ 39:1-111 [warning] MAKE016: Unquoted variable '$(SAKILA_DIR)' in prerequisites - may break with spaces in filenames
  Fix: sakila: \"$(SAKILA_DIR)\"/sakila-data.sql wait ## Download Sakila + load schema and data into the running container

Summary: 0 error(s), 5 warning(s), 0 info(s)
##[error]Process completed with exit code 1.

1. --fail-on error flag is documented in bashrs lint --help but the v6.66.1 prebuilt binary still exits 1 on warnings

bashrs lint --help (on v6.66.0 from cargo install, locally) advertises:

--fail-on <FAIL_ON>
    Minimum severity to trigger non-zero exit code (default: warning)
    Possible values:
    - info
    - warning
    - error

I tried bashrs lint --fail-on error Makefile in CI (commit 817e6f9 on PR #4 of paiml/mysql-from-zero) using the v6.66.1 prebuilt linux x86_64 tarball from the GitHub release. The lint step still exited 1 with Summary: 0 error(s), 5 warning(s), 0 info(s) even though no errors were emitted.

Locally with v6.66.0 (cargo install bashrs), the default exit is 0 on warnings — even without --fail-on error. So either:

  • the v6.66.1 release flipped the default exit behavior without flipping the --fail-on plumbing, or
  • the --fail-on flag never plumbs through to the exit code in the v6.66.1 prebuilt binary

Please confirm whether --fail-on is wired up in lint_cmds.rs and that the v6.66.1 release tarball ships the working flag. Locally I cannot reproduce because I'm on v6.66.0; the failing artifact is the v6.66.1 prebuilt linux x86_64 release tarball used by the CI workflow above.


2. MAKE003 false positive on awk's \$\$1 / \$\$2

Line 33 of the Makefile has the standard self-documenting help target:

help: ## Print available targets
	@awk 'BEGIN {FS = \":.*##\"; printf \"Targets:\n\"} /^[a-zA-Z_-]+:.*?##/ { printf \"  %-12s %s\n\", \$\$1, \$\$2 }' \"\$(MAKEFILE_LIST)\"

The linter flags \$\$1 and \$\$2 as MAKE003 "unquoted variable in command — may cause word splitting issues" with the suggested fix \"$\".

But \$\$ is the Make-escaped literal $ — the shell receives \$1 and \$2, which are awk field references (column 1 and column 2 of the parsed line), not Make variables. Quoting them would break awk parsing.

The MAKE003 check should recognize \$\$N (and \$\$<word>) as the Make-escaped literal-$ form, NOT as an unquoted Make variable. This pattern appears in essentially every "self-documenting Makefile" idiom on GitHub.

Repro: any Makefile containing awk … \$\$1 … or awk … \$\$NF ….


3. MAKE010 doesn't recognize compound || { ...; exit 1; } error handlers

Lines 53-54 (and similarly line 59):

\$(SAKILA_DIR)/sakila-data.sql:
	@echo \"→ downloading \$(SAKILA_URL)...\"
	curl -fsSL \"\$(SAKILA_URL)\" | tar xz || { echo \"✗ Sakila download failed\"; exit 1; }

The linter flags this as MAKE010 "Command 'curl' missing error handling — consider adding '|| exit 1'". The compound || { echo \"...\"; exit 1; } block IS error handling — it emits a diagnostic message, then exits with status 1. That's strictly better error handling than a bare || exit 1 (the user gets to know what failed).

The auto-fix output is particularly absurd:

Fix: curl -fsSL \"\$(SAKILA_URL)\" | tar xz || { echo \"✗ Sakila download failed\"; exit 1; } || exit 1

Suggesting we append another || exit 1 to a recipe that already has || { ...; exit 1; }. The second || exit 1 is dead code — the preceding compound already exits.

Suggested fix: the MAKE010 check should accept any ||-tail whose final command is exit (with any non-zero status), return, or another known-non-success terminal — not only the literal || exit 1 form. Probably want to also accept || die ... / || fail ... etc. for shells with helper functions.

Repro: any Makefile recipe of the form <cmd> || { echo ...; exit 1; }. Same false-positive on line 59 (rm -rf \"\$(SAKILA_DIR)\" || { echo \"✗ rm failed\"; exit 1; }).


4. MAKE016 wants quoted prerequisites that Make syntax doesn't support

Line 39 is a target with a parameterized prerequisite:

sakila: \$(SAKILA_DIR)/sakila-data.sql wait ## Download Sakila + load schema and data into the running container

The linter flags this as MAKE016 "Unquoted variable '$(SAKILA_DIR)' in prerequisites — may break with spaces in filenames" with this auto-fix:

Fix: sakila: \"\$(SAKILA_DIR)\"/sakila-data.sql wait ## Download Sakila + ...

But Make does not parse quoting in prerequisite lists. The literal \" characters become part of the prerequisite filename. Try it: a target line foo: \"bar\" looks for a file literally named \"bar\" (quotes included), not bar. The suggested fix would silently break the build by demanding a non-existent prerequisite.

GNU Make's prerequisite list is whitespace-separated and does no shell-style quoting. The conventional Make answer to filenames-with-spaces is don't, or use \$(subst ...) / backslash-escaping at the variable level — never quoting in the prereq list.

Suggested fix: MAKE016 should be silent on prerequisite lists entirely, OR only fire when the same path appears unquoted in the recipe body of the same target. Recipes use shell, prerequisites do not.

Repro: any target line foo: \$(VAR)/path other-target.


Net effect

After applying every safe fix the linter could suggest, this small Makefile still produces 5 warnings that have no real fix. The PR ended up dropping the bashrs lint Makefile step from CI entirely (commit 8be0cc0 on the linked PR) because we couldn't both (a) keep recipes correct and (b) get the linter to exit 0. The integration job (which actually exercises every recipe end-to-end against a MySQL service container) is still the gate that matters.

Happy to send a PR with regression tests for items 2-4 if you can confirm the desired behavior. Item 1 (the --fail-on plumbing) needs a maintainer to confirm whether the v6.66.1 prebuilt has a build-time issue or a code issue.

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions