build: move backend-checks CI checks to Makefile: make pr-go #11053

slink · 2026-01-26T13:48:26+01:00

slink commented

2026-01-26 13:48:26 +01:00

This is to have a simple and consistent make target for developers to locally run the checks which the CI will also run. The goal is to avoid wasting review cycles on CI failures.

To have a single source of truth, the CI is adjusted to call the same make target. Additional checks should no longer be added to the CI workflow, but rather to the makefile.

The pull request template is adjusted to remind of running this make target.

CI output is improved by using a simple bash script which uses sed to add ##[group] tags to "make --debug=b" output and filter out messages which usually do not contribute to understanding. While this approach has limits and depends on the specific debug output format of GNU make, it avoids adjusting the makefile itself for additional CI beautification, contributing to maintainability.

no tests needed (this is purely a build change)
docs: hint added to PR template
no release notes nedded

This is to have a simple and consistent make target for developers to locally run the checks which the CI will also run. The goal is to avoid wasting review cycles on CI failures. To have a single source of truth, the CI is adjusted to call the same make target. Additional checks should no longer be added to the CI workflow, but rather to the makefile. The pull request template is adjusted to remind of running this make target. CI output is improved by using a simple bash script which uses sed to add `##[group]` tags to "make --debug=b" output and filter out messages which usually do not contribute to understanding. While this approach has limits and depends on the specific debug output format of GNU make, it avoids adjusting the makefile itself for additional CI beautification, contributing to maintainability. - no tests needed (this is purely a build change) - docs: hint added to PR template - no release notes nedded

slink added the

test

needed

label

2026-01-26 13:48:27 +01:00

Gusted reviewed

2026-01-26 13:54:12 +01:00

Gusted left a comment

Otherwise sounds like a nice improvement. Maybe also should be mentioned on https://forgejo.org/docs/latest/contributor/testing/?

.forgejo/pull_request_template.md Outdated

					
				@ -13,3 +13,3 @@

				The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).

				### Tests

				### Tests for GO changes

S/GO/Go

👍 1

Gusted marked this conversation as resolved

slink force-pushed make-pr-go from 63b685a960

requirements / merge-conditions (pull_request) Waiting to run

Details

testing / backend-checks (pull_request) Waiting to run

Details

testing / frontend-checks (pull_request) Waiting to run

Details

testing / test-unit (pull_request) Blocked by required conditions

Details

testing / test-e2e (pull_request) Blocked by required conditions

Details

testing / test-remote-cacher (redis) (pull_request) Blocked by required conditions

Details

testing / test-remote-cacher (valkey) (pull_request) Blocked by required conditions

Details

testing / test-remote-cacher (garnet) (pull_request) Blocked by required conditions

Details

testing / test-remote-cacher (redict) (pull_request) Blocked by required conditions

Details

testing / test-mysql (pull_request) Blocked by required conditions

Details

testing / test-pgsql (pull_request) Blocked by required conditions

Details

testing / test-sqlite (pull_request) Blocked by required conditions

Details

testing / security-check (pull_request) Blocked by required conditions

Details

issue-labels / release-notes (pull_request_target) Waiting to run

Details

Integration tests for the release process / release-simulation (pull_request) Successful in 5m49s

Details

issue-labels / backporting (pull_request_target) Has been skipped

Details

issue-labels / cascade (pull_request_target) Has been skipped

Details

to 59e2a8874f

Integration tests for the release process / release-simulation (pull_request) Successful in 6m27s

Details

testing / frontend-checks (pull_request) Successful in 1m22s

Details

testing / backend-checks (pull_request) Successful in 6m1s

Details

testing / test-unit (pull_request) Successful in 4m18s

Details

testing / test-remote-cacher (redis) (pull_request) Successful in 1m14s

Details

testing / test-remote-cacher (valkey) (pull_request) Successful in 1m11s

Details

testing / test-remote-cacher (garnet) (pull_request) Successful in 1m1s

Details

testing / test-remote-cacher (redict) (pull_request) Successful in 1m4s

Details

testing / test-e2e (pull_request) Successful in 24m0s

Details

testing / test-mysql (pull_request) Successful in 27m28s

Details

testing / test-sqlite (pull_request) Successful in 35m6s

Details

testing / test-pgsql (pull_request) Successful in 39m40s

Details

testing / security-check (pull_request) Successful in 1m12s

Details

issue-labels / release-notes (pull_request_target) Has been skipped

Details

requirements / merge-conditions (pull_request) Failing after 3s

Details

2026-01-26 14:06:30 +01:00

Compare

slink referenced this pull request from a commit

2026-01-26 14:15:30 +01:00

docs: Suggest to run make pr-go

slink referenced this pull request from a commit

2026-01-26 14:16:59 +01:00

docs: Suggest to run make pr-go

slink referenced this pull request from forgejo/docs

2026-01-26 14:17:03 +01:00

docs: Suggest to run make pr-go #1714

slink commented

2026-01-26 14:17:45 +01:00

Changed GO into Go and docs pr opened: forgejo/docs#1714

Changed GO into Go and docs pr opened: https://codeberg.org/forgejo/docs/pulls/1714

viceice reviewed

2026-01-26 14:20:41 +01:00

.forgejo/workflows/testing.yml Outdated

					
				@ -26,3 +26,2 @@

				      - uses: ./.forgejo/workflows-composite/setup-env

				      - run: su forgejo -c 'make deps-backend deps-tools'

				      - run: su forgejo -c 'make --always-make -j$(nproc) lint-backend tidy-check swagger-check lint-swagger fmt-check swagger-validate' # ensure the "go-licenses" make target runs

				      - run: su forgejo -c 'make -j$(nproc) pr-go'

i would suggest to split it tioo multiple run commands for easier failure analyzation on failues. It's currently hard to find what fails

that would counter-act the goal of having a single source of truth.
If we have one thing in the CI and another in the Makefile, they will diverge again.

Should we maybe have a some magic string to make the CI format the output nicely?

that would counter-act the goal of having a single source of truth. If we have one thing in the CI and another in the `Makefile`, they will diverge again. Should we maybe have a some magic string to make the CI format the output nicely?

not sure, i don't know if log grouping works on forgejo actions

https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-commands#grouping-log-lines

then we could detect actions and emit grouping commands on ci

not sure, i don't know if log grouping works on forgejo actions https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-commands#grouping-log-lines then we could detect actions and emit grouping commands on ci

#3337, its supported.

https://codeberg.org/forgejo/forgejo/pulls/3337, its supported.

👀 1

@Gusted wrote in #11053 (comment):

#3337, its supported.

ha, found it too now😅

@Gusted wrote in https://codeberg.org/forgejo/forgejo/pulls/11053#issuecomment-10174722: > #3337, its supported. ha, found it too now😅

we can use GITHUB_ACTIONS or FORGEJO_ACTIONS to detect

we can use `GITHUB_ACTIONS` or `FORGEJO_ACTIONS` to detect

Just CI?
I am trying to come up with something manageable for the Makefile...

Just `CI`? I am trying to come up with something manageable for the Makefile...

sure
code.forgejo.org/forgejo/runner@6721a406e5/act/runner/run_context.go (L1360)

sure https://code.forgejo.org/forgejo/runner/src/commit/6721a406e5e6e5a07a9931d1d03fdcf4bbb0b48a/act/runner/run_context.go#L1360

slink changed title from ~~build: move backend-checks CI checks to Makefile~~ to WIP: build: move backend-checks CI checks to Makefile

2026-01-26 17:11:59 +01:00

slink force-pushed make-pr-go from 59e2a8874f

Integration tests for the release process / release-simulation (pull_request) Successful in 6m27s

Details

testing / frontend-checks (pull_request) Successful in 1m22s

Details

testing / backend-checks (pull_request) Successful in 6m1s

Details

testing / test-unit (pull_request) Successful in 4m18s

Details

testing / test-remote-cacher (redis) (pull_request) Successful in 1m14s

Details

testing / test-remote-cacher (valkey) (pull_request) Successful in 1m11s

Details

testing / test-remote-cacher (garnet) (pull_request) Successful in 1m1s

Details

testing / test-remote-cacher (redict) (pull_request) Successful in 1m4s

Details

testing / test-e2e (pull_request) Successful in 24m0s

Details

testing / test-mysql (pull_request) Successful in 27m28s

Details

testing / test-sqlite (pull_request) Successful in 35m6s

Details

testing / test-pgsql (pull_request) Successful in 39m40s

Details

testing / security-check (pull_request) Successful in 1m12s

Details

issue-labels / release-notes (pull_request_target) Has been skipped

Details

requirements / merge-conditions (pull_request) Failing after 3s

Details

to e3e271934c

testing.yml / test a CI grouping idea (pull_request) Failing after 0s

Details

issue-labels / release-notes (pull_request_target) Has been skipped

Details

requirements / merge-conditions (pull_request) Failing after 2s

Details

Integration tests for the release process / release-simulation (pull_request) Successful in 5m13s

Details

2026-01-26 17:15:31 +01:00

Compare

slink force-pushed make-pr-go from e3e271934c

testing.yml / test a CI grouping idea (pull_request) Failing after 0s

Details

issue-labels / release-notes (pull_request_target) Has been skipped

Details

requirements / merge-conditions (pull_request) Failing after 2s

Details

Integration tests for the release process / release-simulation (pull_request) Successful in 5m13s

Details

to 58fbedc17f

issue-labels / release-notes (pull_request_target) Has been skipped

Details

requirements / merge-conditions (pull_request) Failing after 1s

Details

testing / frontend-checks (pull_request) Successful in 55s

Details

testing / backend-checks (pull_request) Successful in 3m42s

Details

Integration tests for the release process / release-simulation (pull_request) Successful in 5m41s

Details

testing / test-unit (pull_request) Successful in 4m38s

Details

testing / test-remote-cacher (redis) (pull_request) Successful in 1m8s

Details

testing / test-remote-cacher (valkey) (pull_request) Successful in 1m13s

Details

testing / test-remote-cacher (redict) (pull_request) Successful in 1m6s

Details

testing / test-remote-cacher (garnet) (pull_request) Successful in 1m9s

Details

testing / test-e2e (pull_request) Successful in 19m30s

Details

testing / test-sqlite (pull_request) Has been cancelled

Details

testing / test-pgsql (pull_request) Has been cancelled

Details

testing / test-mysql (pull_request) Has been cancelled

Details

testing / security-check (pull_request) Has been cancelled

Details

2026-01-26 17:23:02 +01:00

Compare

slink commented

2026-01-26 17:40:21 +01:00

FTR: The following idea is nice, I think, because it does not require any changes to the makefile, but I found no way to replace the working directory output with the current target:

diff --git a/.forgejo/workflows/testing.yml b/.forgejo/workflows/testing.yml
index d40d000c4e..1e0c1bbc17 100644
--- a/.forgejo/workflows/testing.yml
+++ b/.forgejo/workflows/testing.yml
@@ -24,7 +24,7 @@ jobs:
           EOF
       - uses: https://data.forgejo.org/actions/checkout@v6
       - uses: ./.forgejo/workflows-composite/setup-env
-      - run: su forgejo -c 'make -j$(nproc) pr-go'
+      - run: su forgejo -c './tools/cimake.sh pr-go'
       # this will re-run the backend target also contained in pr-go, but
       # a re-build is insignificant
       - uses: ./.forgejo/workflows-composite/build-backend
diff --git a/tools/cimake.sh b/tools/cimake.sh
new file mode 100755
index 0000000000..1d935120e5
--- /dev/null
+++ b/tools/cimake.sh
@@ -0,0 +1,5 @@
+#!/bin/bash
+
+# GNU make wrapper for invocation from ci to generate grouping of output
+# without getting into the YAML madness of "what is a string"
+exec make -w -O -j$(nproc) "$@" 2>&1 | sed 's:^make\: Entering directory:\#\#[group]:;s:^make\: Leaving directory:\#\#[endgroup]:'

this produces:

But I am giving it up, because we should really have descriptive names for the groups

FTR: The following idea is nice, I think, because it does not require any changes to the makefile, but I found no way to replace the working directory output with the current target: ```diff diff --git a/.forgejo/workflows/testing.yml b/.forgejo/workflows/testing.yml index d40d000c4e..1e0c1bbc17 100644 --- a/.forgejo/workflows/testing.yml +++ b/.forgejo/workflows/testing.yml @@ -24,7 +24,7 @@ jobs: EOF - uses: https://data.forgejo.org/actions/checkout@v6 - uses: ./.forgejo/workflows-composite/setup-env - - run: su forgejo -c 'make -j$(nproc) pr-go' + - run: su forgejo -c './tools/cimake.sh pr-go' # this will re-run the backend target also contained in pr-go, but # a re-build is insignificant - uses: ./.forgejo/workflows-composite/build-backend diff --git a/tools/cimake.sh b/tools/cimake.sh new file mode 100755 index 0000000000..1d935120e5 --- /dev/null +++ b/tools/cimake.sh @@ -0,0 +1,5 @@ +#!/bin/bash + +# GNU make wrapper for invocation from ci to generate grouping of output +# without getting into the YAML madness of "what is a string" +exec make -w -O -j$(nproc) "$@" 2>&1 | sed 's:^make\: Entering directory:\#\#[group]:;s:^make\: Leaving directory:\#\#[endgroup]:' ``` this produces: ![image](/attachments/c6e2464d-4ce4-4e3f-9be1-3c8f5373cab7) But I am giving it up, because we should really have descriptive names for the groups

image.png

51 KiB

slink added 2 commits

2026-01-26 17:46:34 +01:00

try unbuffered sed 2456f82ab6

or does the idea actually work?

testing / frontend-checks (pull_request) Successful in 48s

Details

testing / backend-checks (pull_request) Successful in 2m16s

Details

Integration tests for the release process / release-simulation (pull_request) Successful in 5m49s

Details

testing / test-unit (pull_request) Successful in 4m14s

Details

testing / test-remote-cacher (redict) (pull_request) Successful in 1m18s

Details

testing / test-remote-cacher (valkey) (pull_request) Successful in 1m19s

Details

testing / test-remote-cacher (redis) (pull_request) Successful in 1m19s

Details

testing / test-remote-cacher (garnet) (pull_request) Successful in 1m19s

Details

testing / test-e2e (pull_request) Successful in 25m58s

Details

testing / test-mysql (pull_request) Successful in 26m36s

Details

testing / test-sqlite (pull_request) Successful in 32m41s

Details

testing / test-pgsql (pull_request) Successful in 35m41s

Details

testing / security-check (pull_request) Successful in 51s

Details

issue-labels / backporting (pull_request_target) Has been skipped

Details

issue-labels / cascade (pull_request_target) Has been skipped

Details

issue-labels / release-notes (pull_request_target) Has been skipped

Details

requirements / merge-conditions (pull_request) Successful in 1s

Details

140c9d8e8e

slink commented

2026-01-26 17:51:28 +01:00

Actually... There is a debug option which might give us what we want.
@viceice could you have a look at https://codeberg.org/forgejo/forgejo/actions/runs/134218/jobs/0/attempt/1 and tell me what you think?

As mentioned before, the compelling advantage of something like this is that it does not require changes to the makefile (which can be forgotten etc.).

The drawback (if it is any) is that we need to accept additional debug output which we are less interested in, or we'd need to hack more line replacements for those parts of the output which we do not want to see (with the danger of accidentally suppressing something which we are interested in)

Actually... There is a debug option which might give us what we want. @viceice could you have a look at https://codeberg.org/forgejo/forgejo/actions/runs/134218/jobs/0/attempt/1 and tell me what you think? As mentioned before, the compelling advantage of something like this is that it does not require changes to the makefile (which can be forgotten etc.). The drawback (if it is any) is that we need to accept additional debug output which we are less interested in, or we'd need to hack more line replacements for those parts of the output which we do not want to see (with the danger of accidentally suppressing something which we _are_ interested in) ![image](/attachments/19407ccb-9f31-4c24-91e5-0f8dc1c066b7)

image.png

164 KiB

slink changed title from ~~WIP: build: move backend-checks CI checks to Makefile~~ to WIP: build: move CI checks to Makefile

2026-01-26 17:54:25 +01:00

Gusted added

and removed

labels

2026-01-26 22:43:49 +01:00

viceice commented

2026-01-27 13:36:07 +01:00

looks promissing, maybe we should try to filter some more known verbose messages 🙃

Prerequisite 'tests/e2e/changes.go' is newer than target 'templates/swagger/v1_json.tmpl'.
File 'swagger-check' does not exist.

then we're porbably good to go

looks promissing, maybe we should try to filter some more known verbose messages 🙃 - `Prerequisite 'tests/e2e/changes.go' is newer than target 'templates/swagger/v1_json.tmpl'.` - `File 'swagger-check' does not exist.` then we're porbably good to go

👍 1

slink force-pushed make-pr-go from 140c9d8e8e

testing / frontend-checks (pull_request) Successful in 48s

Details

testing / backend-checks (pull_request) Successful in 2m16s

Details

Integration tests for the release process / release-simulation (pull_request) Successful in 5m49s

Details

testing / test-unit (pull_request) Successful in 4m14s

Details

testing / test-remote-cacher (redict) (pull_request) Successful in 1m18s

Details

testing / test-remote-cacher (valkey) (pull_request) Successful in 1m19s

Details

testing / test-remote-cacher (redis) (pull_request) Successful in 1m19s

Details

testing / test-remote-cacher (garnet) (pull_request) Successful in 1m19s

Details

testing / test-e2e (pull_request) Successful in 25m58s

Details

testing / test-mysql (pull_request) Successful in 26m36s

Details

testing / test-sqlite (pull_request) Successful in 32m41s

Details

testing / test-pgsql (pull_request) Successful in 35m41s

Details

testing / security-check (pull_request) Successful in 51s

Details

issue-labels / backporting (pull_request_target) Has been skipped

Details

issue-labels / cascade (pull_request_target) Has been skipped

Details

issue-labels / release-notes (pull_request_target) Has been skipped

Details

requirements / merge-conditions (pull_request) Successful in 1s

Details

to 6d13f7794e

issue-labels / release-notes (pull_request_target) Has been skipped

Details

requirements / merge-conditions (pull_request) Successful in 2s

Details

testing / frontend-checks (pull_request) Successful in 1m13s

Details

Integration tests for the release process / release-simulation (pull_request) Successful in 5m49s

Details

testing / backend-checks (pull_request) Successful in 4m34s

Details

testing / test-unit (pull_request) Successful in 4m35s

Details

testing / test-remote-cacher (redis) (pull_request) Successful in 1m2s

Details

testing / test-remote-cacher (valkey) (pull_request) Successful in 1m3s

Details

testing / test-remote-cacher (garnet) (pull_request) Successful in 1m1s

Details

testing / test-remote-cacher (redict) (pull_request) Successful in 1m3s

Details

testing / test-mysql (pull_request) Successful in 23m53s

Details

testing / test-sqlite (pull_request) Successful in 26m5s

Details

testing / test-pgsql (pull_request) Successful in 30m49s

Details

testing / security-check (pull_request) Successful in 40s

Details

testing / test-e2e (pull_request) Successful in 17m16s

Details

2026-01-27 14:39:25 +01:00

Compare

slink changed title from ~~WIP: build: move CI checks to Makefile~~ to build: move backend-checks CI checks to Makefile: make pr-go

2026-01-27 14:41:38 +01:00

slink commented

2026-01-27 14:44:11 +01:00

ready for merge:

tools/cimake.sh:

filtering of additional messages as suggested by @viceice
set pipefail mode to make sure a make failure is propagated
general polishing

CI:

added reference to makefile

ready for merge: tools/cimake.sh: * filtering of additional messages as suggested by @viceice * set pipefail mode to make sure a make failure is propagated * general polishing CI: * added reference to makefile

slink commented

2026-01-27 14:53:11 +01:00

This failure https://codeberg.org/forgejo/forgejo/actions/runs/134508/jobs/3/attempt/1 looks unrelated, I did not touch e2e and the error looks like ... something swallowed some characters...?

viceice approved these changes

2026-01-27 15:03:10 +01:00

Dismissed

slink force-pushed make-pr-go from 6d13f7794e

issue-labels / release-notes (pull_request_target) Has been skipped

Details

requirements / merge-conditions (pull_request) Successful in 2s

Details

testing / frontend-checks (pull_request) Successful in 1m13s

Details

Integration tests for the release process / release-simulation (pull_request) Successful in 5m49s

Details

testing / backend-checks (pull_request) Successful in 4m34s

Details

testing / test-unit (pull_request) Successful in 4m35s

Details

testing / test-remote-cacher (redis) (pull_request) Successful in 1m2s

Details

testing / test-remote-cacher (valkey) (pull_request) Successful in 1m3s

Details

testing / test-remote-cacher (garnet) (pull_request) Successful in 1m1s

Details

testing / test-remote-cacher (redict) (pull_request) Successful in 1m3s

Details

testing / test-mysql (pull_request) Successful in 23m53s

Details

testing / test-sqlite (pull_request) Successful in 26m5s

Details

testing / test-pgsql (pull_request) Successful in 30m49s

Details

testing / security-check (pull_request) Successful in 40s

Details

testing / test-e2e (pull_request) Successful in 17m16s

Details

to 0d8262e582

issue-labels / release-notes (pull_request_target) Has been skipped

Details

requirements / merge-conditions (pull_request) Successful in 2s

Details

testing / frontend-checks (pull_request) Successful in 57s

Details

testing / backend-checks (pull_request) Successful in 2m53s

Details

Integration tests for the release process / release-simulation (pull_request) Successful in 5m25s

Details

testing / test-unit (pull_request) Successful in 5m48s

Details

testing / test-remote-cacher (garnet) (pull_request) Successful in 1m48s

Details

testing / test-remote-cacher (redict) (pull_request) Successful in 1m51s

Details

testing / test-remote-cacher (redis) (pull_request) Successful in 1m54s

Details

testing / test-remote-cacher (valkey) (pull_request) Successful in 1m53s

Details

testing / test-e2e (pull_request) Successful in 24m2s

Details

testing / test-mysql (pull_request) Successful in 26m26s

Details

testing / test-sqlite (pull_request) Successful in 32m24s

Details

testing / test-pgsql (pull_request) Successful in 37m37s

Details

testing / security-check (pull_request) Successful in 1m10s

Details

issue-labels / backporting (pull_request_target) Has been skipped

Details

milestone / set (pull_request_target) Successful in 6s

Details

2026-02-05 11:07:12 +01:00

Compare

slink commented

2026-02-05 11:07:14 +01:00

rebased and force-pushed, no changes to the patch

viceice approved these changes

2026-02-05 11:18:59 +01:00

mfenniak commented

2026-02-07 03:51:06 +01:00

While this has been open, I've prepared a new backend check in #11142, using semgrep. I'm inclined to leave the semgrep change separate from this new pr-go make target because: (a) it already is, so neither of these changes will conflict on each other, (b) it requires a separate tool that can't be automatically installed by the go toolchain (semgrep) since it's not a go tool, (c) it's a relatively slow tool, (d) the slow perf is mitigated in CI because it uses patch-aware detection of what files to run against.

Just wanted to raise this note to see if there's an alternate opinion before either change moves forward. semgrep's applicability in #11142 will be so very small that it's unlikely to be an inconvenience, but that could change over time.

While this has been open, I've prepared a new backend check in #11142, using `semgrep`. I'm inclined to leave the `semgrep` change separate from this new `pr-go` make target because: (a) it already is, so neither of these changes will conflict on each other, (b) it requires a separate tool that can't be automatically installed by the go toolchain (`semgrep`) since it's not a go tool, (c) it's a relatively slow tool, (d) the slow perf is mitigated in CI because it uses patch-aware detection of what files to run against. Just wanted to raise this note to see if there's an alternate opinion before either change moves forward. `semgrep`'s applicability in #11142 will be so very small that it's unlikely to be an inconvenience, but that could change over time.

slink commented

2026-02-07 13:38:50 +01:00

Re @mfenniak

I am not competent regarding semgrep, but it seems to be a proprietary, closed source tool (I only checked briefly, the pip installer for the CE cli contains a big binary executable). As a matter of principle, I would not even install such a tool locally, and, consequently, I would vote against requiring it to pass for PRs to be merged. In my opinion, there is also a risk of relying too much on such a tool, and in my mind there is a broader conflict with forgejo's values.

I do not have any say in this project, so the maintainers are of course to make their decision independent of this opinion.

As a possible compromise, one option could be to just run the tool "asynchronously" against trunk.

Re @mfenniak I am not competent regarding semgrep, but it seems to be a proprietary, closed source tool (I only checked briefly, the pip installer for the CE cli contains a big binary executable). As a matter of principle, I would not even install such a tool locally, and, consequently, I would vote against requiring it to pass for PRs to be merged. In my opinion, there is also a risk of relying too much on such a tool, and in my mind there is a broader conflict with forgejo's values. I do not have any say in this project, so the maintainers are of course to make their decision independent of this opinion. As a possible compromise, one option could be to just run the tool "asynchronously" against trunk.

mfenniak commented

2026-02-07 15:03:30 +01:00

@slink wrote in #11053 (comment):

I am not competent regarding semgrep, but it seems to be a proprietary, closed source tool (I only checked briefly, the pip installer for the CE cli contains a big binary executable).

This is a misunderstanding of a weird packaging choice. semgrep is an LGPL-2.1 licensed open source tool. https://github.com/semgrep/semgrep It's written in OCaml, so I'm not sure why they use a pip package as a distribution tool, but that's the cause of the binary -- it's not a python tool.

Now, there is a fair criticism of the project in a more nuanced way -- the authors recently removed capabilities from the open source tool to package them as part of their commercial engine. (https://semgrep.dev/blog/2024/important-updates-to-semgrep-oss/#json-and-sarif-output-changes) This resulted in a fork, Opengrep. We could easily adopt Opengrep instead if it proves to be the healthier project in the long-term due to the limited usage proposed at this time.

With that information in mind, how is your opinion on make pr-go influenced?

@slink wrote in https://codeberg.org/forgejo/forgejo/pulls/11053#issuecomment-10426718: > I am not competent regarding semgrep, but it seems to be a proprietary, closed source tool (I only checked briefly, the pip installer for the CE cli contains a big binary executable). This is a misunderstanding of a weird packaging choice. semgrep is an LGPL-2.1 licensed open source tool. https://github.com/semgrep/semgrep It's written in OCaml, so I'm not sure why they use a pip package as a distribution tool, but that's the cause of the binary -- it's not a python tool. Now, there is a fair criticism of the project in a more nuanced way -- the authors recently removed capabilities from the open source tool to package them as part of their commercial engine. (https://semgrep.dev/blog/2024/important-updates-to-semgrep-oss/#json-and-sarif-output-changes) This resulted in a fork, Opengrep. We could easily adopt Opengrep instead if it proves to be the healthier project in the long-term due to the limited usage proposed at this time. With that information in mind, how is your opinion on `make pr-go` influenced?

👍 1

slink commented

2026-02-08 19:55:34 +01:00

@mfenniak wrote in #11053 (comment):

This is a misunderstanding of a weird packaging choice. semgrep is an LGPL-2.1 licensed open source tool.

Thank you so much for clearing up my misunderstanding. 🙏 FWIW, their corporate website does not exactly promote a link to their sources either, but I really should have looked more carefully. Thank you again for correcting my mistake.

As long as the toolchain can be built from sources, I am fine with requiring it.

@mfenniak wrote in https://codeberg.org/forgejo/forgejo/pulls/11053#issuecomment-10428132: > This is a misunderstanding of a weird packaging choice. semgrep is an LGPL-2.1 licensed open source tool. Thank you so much for clearing up my misunderstanding. 🙏 FWIW, their corporate website does not exactly promote a link to their sources either, but I really should have looked more carefully. Thank you again for correcting my mistake. As long as the toolchain can be built from sources, I am fine with requiring it.

slink commented

2026-02-08 20:02:44 +01:00

@mfenniak I just realized that I might not have answered your actual question: Being inexperienced with semgrep, I am not sure how relevant it would be to run it locally. As a newbie to this project, I wanted to avoid pushing PRs for which CI immediately fails, and just today I found issues which would only should up in e2e. So I guess in principle make pr-go could include all CI checks, but it also needs to remain manageable in terms of runtime.
So based on

it's a relatively slow tool, (d) the slow perf is mitigated in CI because it uses patch-aware detection of what files to run against.

it would seem sensible to not run it with make pr-go?

@mfenniak I just realized that I might not have answered your actual question: Being inexperienced with semgrep, I am not sure how relevant it would be to run it locally. As a newbie to this project, I wanted to avoid pushing PRs for which CI immediately fails, and just today I found issues which would only should up in e2e. So I guess in principle `make pr-go` could include _all_ CI checks, but it also needs to remain manageable in terms of runtime. So based on > it's a relatively slow tool, (d) the slow perf is mitigated in CI because it uses patch-aware detection of what files to run against. it would seem sensible to not run it with `make pr-go`?

slink referenced this pull request from a commit

2026-02-09 17:18:06 +01:00

docs: Suggest to run make pr-go

Gusted commented

2026-02-17 02:03:16 +01:00

@slink wrote in #11053 (comment):

it's a relatively slow tool, (d) the slow perf is mitigated in CI because it uses patch-aware detection of what files to run against.

it would seem sensible to not run it with make pr-go?

It runs in 6 seconds for me, but I'm not sure how well installing python packages is in the current Makefile, there's some as that was inherited from Gitea but I don't think we ever ran them.

@slink wrote in https://codeberg.org/forgejo/forgejo/pulls/11053#issuecomment-10449888: > > it's a relatively slow tool, (d) the slow perf is mitigated in CI because it uses patch-aware detection of what files to run against. > > it would seem sensible to not run it with `make pr-go`? It runs in 6 seconds for me, but I'm not sure how well installing python packages is in the current Makefile, there's some as that was inherited from Gitea but I don't think we ever ran them.

Gusted approved these changes

2026-02-17 02:11:18 +01:00

Gusted left a comment

Looks good, thanks!

Gusted merged commit a81fc2a290 into forgejo

2026-02-17 02:41:43 +01:00

Gusted referenced this pull request from a commit

2026-02-17 02:41:49 +01:00

chore: move backend-checks CI checks to Makefile: `make pr-go` (#11053)

forgejo-release-notes-assistant added this to the Forgejo v15.0.0 milestone

2026-02-17 02:41:52 +01:00

Gusted referenced this pull request from a commit

2026-02-17 02:51:52 +01:00

contributor: testing: suggest to run make pr-go (#1714)

ltdk commented

2026-02-17 18:36:15 +01:00

First-time contributor

Just want to say thank you for doing this & the docs update, since they would have helped with an earlier PR I submitted. Hopefully they help newer folks too.

❤️ 1

alberic89 referenced this pull request from a commit

2026-02-23 18:43:16 +01:00

chore: move backend-checks CI checks to Makefile: `make pr-go` (#11053)

mahlzahn referenced this pull request from forgejo/website

2026-03-01 07:54:05 +01:00

Monthly report for February 2026 #730

Sign in to join this conversation.

No reviewers