Skip to content

perf: replace goheader linter with custom check#37599

Merged
silverwind merged 31 commits into
go-gitea:mainfrom
silverwind:lint-go-replace-goheader
May 8, 2026
Merged

perf: replace goheader linter with custom check#37599
silverwind merged 31 commits into
go-gitea:mainfrom
silverwind:lint-go-replace-goheader

Conversation

@silverwind
Copy link
Copy Markdown
Member

@silverwind silverwind commented May 7, 2026

Replace the slow goheader linter with a custom check.

Local go lint time is down from 247s to 32s. 6 new files that were previously undetected because of //go:build ignore are fixed. The exit code of the make target preserves the golangci-lint exit code, if present.


This PR was written with the help of Claude Opus 4.7

The goheader linter spawns `git diff` and `git log` for every .go file
to populate `mod-year` template variables that are not used in the gitea
config. With ~2900 files, that's ~5800 forks per cold lint run, which
dominates wall time on macOS.

Replace it with `tools/lint-go-header.go`, a 60-line filesystem-walk +
RE2 regex check that runs in ~0.2s on the full repo.

Cold lint time on a fully cleared cache: 247s -> 32s (7.6x faster).

Six files had the copyright header below a build directive or generator
marker; they passed previously only because golangci-lint never loaded
them under default build tags or detected them as generated. The custom
check walks the filesystem, so all of them are checked. Reorder them
to put the copyright header first, and update the charset generator
templates so re-generation preserves the order.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 7, 2026
@silverwind silverwind added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label May 7, 2026
@silverwind silverwind changed the title perf: replace goheader linter with custom check chore: replace goheader linter with custom check May 8, 2026
@silverwind silverwind changed the title chore: replace goheader linter with custom check perf: replace goheader linter with custom check May 8, 2026
silverwind and others added 3 commits May 8, 2026 02:28
Previously the two checks ran as separate make recipe lines, so a
header failure short-circuited golangci-lint. Run both, OR their
exit codes, and exit accordingly so a single make invocation reports
all issues.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
When both checks fail, golangci-lint's exit code is more informative
than the header check's generic 1, so propagate lint's code first.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR replaces the goheader golangci-lint linter with a custom Go header-checker to significantly reduce Go lint runtime, while updating a few files/templates to match the required header layout.

Changes:

  • Added a standalone header checker (tools/lint-go-header.go) that scans .go files for the expected copyright + SPDX header.
  • Updated make lint-go / make lint-go-fix to run the new header checker alongside golangci-lint and preserve golangci-lint’s exit code when it fails.
  • Adjusted several Go files and generator templates to ensure the header appears before other leading comments/build tags; removed goheader from enabled linters.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tools/lint-go-header.go New custom header linter (filesystem walk + regex match).
Makefile Runs the custom header linter in lint-go / lint-go-fix and combines exit codes.
.golangci.yml Disables the goheader linter.
modules/charset/generate/generate.go Updates generator templates so generated files place the license header first.
modules/charset/ambiguous_gen.go Reorders generated-file header to match new expectations.
modules/charset/invisible_gen.go Reorders generated-file header to match new expectations.
modules/auth/pam/pam_test.go Moves build tag below the license header.
models/db/driver_sqlite_modernc.go Moves build tag below the license header.
models/db/driver_sqlite_mattn.go Moves build tag below the license header.
build/generate-gitignores.go Adds missing license header to an ignore build-tagged tool.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tools/lint-go-header.go Outdated
Comment thread tools/lint-go-header.go Outdated
Comment thread Makefile Outdated
silverwind and others added 6 commits May 8, 2026 03:34
- Handle Read error and use defer for close in lint-go-header
- Skip vendor directory
- Run header check in lint-go-windows too

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
Drops two imports and the manual buffer + EOF handling. Same runtime
(~0.2s on the full repo); gitea source files are small enough that
reading the whole file vs the first 1KB is a wash.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
The lint-go-windows CI job sets GOOS=windows GOARCH=amd64; without
forcing the host platform, `go run` cross-compiles a Windows binary
and fails with `exec format error` when trying to run it on Linux.
Match the pattern used for the golangci-lint install above.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
The combined header check + golangci-lint exit-code propagation was
duplicated across lint-go, lint-go-fix, and lint-go-windows. Move it
to a shared script that takes the golangci-lint command as args.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
All three lint-go targets now go through the script. The script reads
GOLANGCI_LINT_PACKAGE from the env (passed by make) and accepts the
LINT_GO_INSTALL=1 toggle for cross-compile envs like lint-go-windows
where 'go run' would build a target-platform binary that can't exec
on the host.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
silverwind and others added 3 commits May 8, 2026 05:36
Bash arrays for the env prefix and linter command were a clever way to
deduplicate the two branches, but the explicit duplication reads better.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
Restore the GOOS= GOARCH= prefix on the header `go run` in the install
branch so cross-compile envs (lint-go-windows) build the header binary
for the host.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

tools/lint-go.sh:19

  • lint-go.sh hard-codes invoking go/go run instead of respecting the Makefile’s $(GO) override (used throughout the build for toolchain pinning/wrappers). This change makes make GO=... lint-go behave differently from other targets; consider passing GO through from the Makefile and using it consistently inside the script.
if [ -n "$LINT_GO_INSTALL" ]; then
	GOOS= GOARCH= go install "$GOLANGCI_LINT_PACKAGE"
	GOOS= GOARCH= go run tools/lint-go-header.go; header=$?
	golangci-lint run "$@"; lint=$?
else
	go run tools/lint-go-header.go; header=$?
	go run "$GOLANGCI_LINT_PACKAGE" run "$@"; lint=$?
fi

exit $((lint ? lint : header))

Comment thread tools/lint-go-header.go Outdated
Comment thread .golangci.yml
silverwind and others added 3 commits May 8, 2026 05:43
The linter was already removed from the enabled list; clean up the
matching settings to avoid stale config drift.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
Comment thread tools/lint-go-header.go Outdated
Comment thread tools/lint-go-header.go Outdated
Comment thread modules/charset/invisible_gen.go Outdated
@silverwind
Copy link
Copy Markdown
Member Author

At minimum make it silent, output on success should be

0 issues.
0 issues.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 8, 2026
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 8, 2026
@silverwind silverwind removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 8, 2026
@silverwind silverwind marked this pull request as draft May 8, 2026 17:22
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented May 8, 2026

Planning to implement the header check as minimal fast golangci-lint plugin. Per golangci-lint's design bug, it will not work on files with //go:build ignore, but that's fine. Having the plugin infrastructure now will enable us to easily write more custom linters later.

@wxiaoguang wxiaoguang dismissed their stale review May 8, 2026 20:17

dismiss

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels May 8, 2026
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented May 8, 2026

Given that the slow lint issue is very cumbersome for current development, let's merge this now. I'm not happy with the verbosity and local windows run but the speed gain is more important. I will follow up with the plugin rewrite.

@silverwind silverwind marked this pull request as ready for review May 8, 2026 20:17
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 8, 2026
@silverwind silverwind enabled auto-merge (squash) May 8, 2026 20:18
Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, my philosophy is: as long as nothing blocks, the design doesn't introduce new problems, nothing worse, then good to merge.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 8, 2026
@silverwind
Copy link
Copy Markdown
Member Author

my philosophy is: as long as nothing blocks, the design doesn't introduce new problems, nothing worse, then good to merge.

No, your philosophy is as long as it's not absolutely perfect, you block it 😆.

@silverwind
Copy link
Copy Markdown
Member Author

Admins need to remove lint-go-gogit and lint-go-windows from required check before this can merge.

@silverwind silverwind merged commit a5d81d9 into go-gitea:main May 8, 2026
21 checks passed
@silverwind silverwind deleted the lint-go-replace-goheader branch May 8, 2026 21:39
@GiteaBot GiteaBot added this to the 1.27.0 milestone May 8, 2026
silverwind added a commit to mohammad-rj/gitea that referenced this pull request May 8, 2026
* origin/main:
  perf: replace `goheader` linter with custom check (go-gitea#37599)
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 8, 2026
zjjhot added a commit to zjjhot/gitea that referenced this pull request May 9, 2026
* main:
  perf: replace `goheader` linter with custom check (go-gitea#37599)
  build(deps): bump fast-uri from 3.1.0 to 3.1.2 (go-gitea#37616)
  fix: make clone URL respect public URL detection setting (go-gitea#37615)
  chore(deps): bump go-git/go-git/v5 to 5.19.0 (go-gitea#37608)
  chore(deps): update action dependencies (go-gitea#37603)
  fix(actions): fix blank lines after `::endgroup::` (go-gitea#37597)
  fix: treat email addresses case-insensitively (go-gitea#37600)
  fix(git): Fix smart http request scope bug (go-gitea#37583)
  chore(deps): update dependency go to v1.26.3 (go-gitea#37601)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. topic/code-linting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants