This change is very similar to what was done in forgejo/forgejo#7727. When a PR is updated, `checkIfPRContentChanged` is called to check if the diff changed - this is done on a temporary repository. This change improves this checking by doing this operation on the bare repository. The change is split into several commits.
The following changes were made (in this exact order)
1. Update the `getTestPatchCtx` function that was introduced in forgejo/forgejo#7727 so it can be used outside the context of conflict checking. This is a simple change by making the caller determine if it can use a bare repository or not.
2. Do a small refactor of `ValidatePullRequest` to avoid indentation hell in this function, this is purely a refactor but necessary to not blow my brain while working on this function.
3. The first enhancement, introduce `testPatchCtx` in `ValidatePullRequest` to get diverging commits via the bare repository.
4. The main enhancement, do a refactor to move the function to be part of the repository struct and do a rename as this fits as a general purpose function. This refactoring includes it no longer being specific to a temporary repository and works on a bare repository.
5. Add extensive units tests, integration tests are added in forgejo/forgejo#8450 it checks that both calls to `CheckIfDiffDiffers` work. Because it also fixes a bug I sent it as a different PR.
6. Extend the integration test to check for diffs with commits from different repositories, to demonstrate that `getTestPatchCtx` works (specifically the `env` variable).
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8451
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>
Fixes#8488.
The issue was caused by having exactly 20 lines of text between two diff sections, which is `BlobExcerptChunkSize`. The previous diff section ends at 530, the next diff section starts at 551, leaving 20 hidden lines. This triggered an off-by-one error in determining whether a synthetic section should be added to the render to include a new expander; it was falsely being triggered when it shouldn't have been.
Mostly correct logic was already present in `GetExpandDirection`, so I changed the `ExcerptBlob` web rendering to use `GetExpandDirection` on whether to include the synthetic section. Then I covered `GetExpandDirection` with unit tests covering all boundary conditions, which discovered one other minor bug -- the case where exactly `BlobExcerptChunkSize` is hidden between two diff sections should never have displayed an "Up" and "Down" expansion, but just a single expander (as below).
Note that *technically* the `ExcerptBlob` change is not covered by any new tests... but the relevant logic has been moved to somewhere more easily testable and fully covered. If this isn't covered enough for tests, let me know.
## Checklist
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
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
- [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
- [ ] in `web_src/js/*.test.js` if it can be unit tested.
- [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [ ] I do not want this change to show in the release notes.
- [x] I want the title to show in the release notes with a link to this pull request.
- [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8519
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
Reimplement the submodules parser to not depend on the go-git dependency.
See #8222 for the full refactor context.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8438
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: oliverpool <git@olivier.pfad.fr>
Co-committed-by: oliverpool <git@olivier.pfad.fr>
Allow for a directory `default_merge_message` in the `CustomPath` to be used as a fallback if no merge template is specified by the repository. Effectively being a global merge message template feature.
Resolves#6648
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8347
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: kochklops <kochklops@noreply.codeberg.org>
Co-committed-by: kochklops <kochklops@noreply.codeberg.org>
Adds the ability to selectively choose which branches are pushed to a mirror.
This change adds an additional text box on the repository settings for each push mirror. Existing behavior is preserved when the field is left blank.
When the repository is being pushed, only branches matching the comma separated branch filter are pushed.
Resolvesforgejo/forgejo#7242
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7823
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Paul Campbell <pcampbell@kemitix.net>
Co-committed-by: Paul Campbell <pcampbell@kemitix.net>
closes#7946
- The `rpmsRepoPattern` regex has been fixed to handle releases with dots correctly. For example, the version `0.9.0-alt1.git.17.g2ba905d` is valid, just like `0.1.0-1.n1` mentioned in the issue (https://codeberg.org/forgejo/forgejo/issues/7946#issue-1628991)
- getEntries now returns entry names. In the integration tests, there were lines like:
```go
assert.Equal(t, []string{"", ""}, result.ProvideNames)
```
and it’s unclear how such test logic could have ever worked correctly (fixes problems with deps https://codeberg.org/forgejo/forgejo/issues/7946#issuecomment-5109795)
- ALT is an acronym for ALT Linux Team, so `Alt` was replaced with `ALT`. Strictly speaking, it should probably be `ALT Linux`, but since we use `Arch` instead of `Arch Linux`, this seems fine. Also, Distrowatch shows `Arch`/`ALT` in its dropdown, so it’s consistent.
- The strings `"Alt Linux Team"` and `"Sisyphus"` in the `Origin` and `Suite` fields have been replaced with `setting.AppName` and `"Unknown"`. `Unknown` is a valid value and is set by default, so this won’t cause any issues.
- The documentation link has been fixed: (404 docs.gitea.com/usage/packages/alt/ -> 200 forgejo.org/docs/latest/user/packages/alt/)
---
## Checklist
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
- I added test coverage for Go changes...
- [ ] in their respective `*_test.go` for unit tests.
- [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
- [ ] in `web_src/js/*.test.js` if it can be unit tested.
- [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [ ] I do not want this change to show in the release notes.
- [x] I want the title to show in the release notes with a link to this pull request.
- [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8475
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Maxim Slipenko <maks1ms@altlinux.org>
Co-committed-by: Maxim Slipenko <maks1ms@altlinux.org>
When a user is assigning an issue or PR to another user, an entry is posted in the issue comment list. This PR adds a link to the one assigning the issue. Additionally, tests for preventing XSS vulnerabilities in label rendering were added.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8264
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
When a mail notification is sent because of a failed pull request run,
the body will show:
Branch: #661 (f57df45)
where #661 is the number of the pull request and not the branch. This
is because run.PrettyRef() is used and has a misleading special case
returning a PR number instead of a ref.
Remove the textual description as it can easily be discovered from the
run web page linked in the mail.
## Checklist
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
- I added test coverage for Go changes...
- [ ] in their respective `*_test.go` for unit tests.
- [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
- [ ] in `web_src/js/*.test.js` if it can be unit tested.
- [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [ ] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [ ] I do not want this change to show in the release notes.
- [ ] I want the title to show in the release notes with a link to this pull request.
- [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8448
Reviewed-by: Christopher Besch <mail@chris-besch.com>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>
The argument order for `ValidatePullRequest` is to first give the new commitID and then the old commit ID.
This results in reviews not being marked as stale when they are not stale and reviews as not stale when they are stale. The test will fail if the fix is not present.
Add testing for the following three scenarios:
1. A review is made, the PR is updated and as a consequence the PR's diff is changed. The review is now marked as stale.
2. A review is made but in the meantime the PR is updated and the review is submitted on a older commit ID. If the diff changed the review is marked as stale.
3. A review that was made against a older commit ID is no longer marked as stale if the PR is force-pushed to that older commit ID.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8450
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>
- Regression of forgejo/forgejo#8425 (I was fooled by the github flow being `headGitRepo`).
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8431
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>
- Instead of invoking a new git command to check if a reference exists, use the already opened git repository that uses a batch check to check if the reference exists.
- Instead of invoking a new git command to check if a branch exists, use the already opened git repository that uses a batch check to check if the branch exists.
- Do not depend on `IsReferenceExist` as its no longer used by any other function.
- Specify `--quiet` to not waste Git's time on printing something we do not use.
- Run it via `Run` so no buffers are created for stdout and stderr.
- Add a test that verifies `IsBranchExist` works and does not work for other types of references.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8425
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>
- Refactor arguments of the function to make more sense.
- `path` can be inferred from `repo` receiver.
- `line` can be `uint64`.
- The two calls to this function check for specific errors, do this error checking in the function.
- The ID of a object format is not 40 in the case of SHA256, get the object format and use the correct length.
- Add test coverage for `LineBlame`, notably it checks for the errors that can legitimately happen.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8419
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>
From the point of view of a notification, the only status which must be considered a fail is `StatusFailure`. All others are either success `StatusSuccess` or undetermined `StatusCancelled` and `StatusSkipped`.
Those are the only four status in which a run can be when it reaches the `ActionRunNowDone` function because it is filtered on `IsDone` which is only true for those.
## Checklist
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
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
### Documentation
- [x] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [x] I do not want this change to show in the release notes.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8366
Reviewed-by: Antonin Delpeuch <wetneb@noreply.codeberg.org>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>
- Make the migration to add an index a noop - do not remove it as it would break v12.next & v13.next
- Keep the logic that relies on finding the last run, only always fail to find which is the same as assuming each run is one of a kind
Refs forgejo/forgejo#8373
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8374
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>
This PR is part of https://codeberg.org/forgejo/forgejo/pulls/4767
It improves the signature handling:
1. move logic to a service (might be used from other services as well)
2. make a clear difference between ` ReqHTTPUserSignature` and `ReqHTTPUserOrInstanceSignature`
3. improve test ability (activitypub/client & distant_federation_server_mock
Adjust instance actor
1. name &
2. webfinger
## Strategy for next PRs is
Integration tests are in the driving seat.
I will step by step add integration tests form original PR and add code required by the integration test changes.
## Meta
Proposal howto process large PRs can be discussed here: https://codeberg.org/forgejo-contrib/federation/pulls/37
Current state with rendered diagrams can be found here: https://codeberg.org/meissa/federation/src/branch/merge-large-pr/doc/merge-large-pr.md
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8275
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Michael Jerger <michael.jerger@meissa-gmbh.de>
Co-committed-by: Michael Jerger <michael.jerger@meissa-gmbh.de>
This adds pasted images to the dropzone to provide the same experience
as when using the dropzone. This gives the possibility to preview and
delete the image. Additionally it provides a copy button to copy the
markdown code for inserting the image.
Removed the old implementation in `repo-legacy.js` and generalized
everything in `common-global.js` as common implementation.
Replaced old jquery code with plain JS.
Fixes#4588
Followup to https://codeberg.org/forgejo/forgejo/pulls/6053
Adds the repository description to the "alt" tag of the OpenGraph summary card, improving accessibility when these images are displayed. Fixes#8192.
Other summary cards, for issues and releases, are not modified as they already contain the issue title or release title, which seems reasonable.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8325
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
- Resolvesforgejo/discussions#324
- Remove all checks of `CheckGitVersionAtLeast` that checked for a version below 2.34.1
- The version was chosen because Debian stable supports 2.39.5 and Ubuntu 22.04 LTS supports 2.34.1
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8328
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>
See #8222 for context.
This PR removes a call to `Blob.GetBlobContent` and `Blob.DataAsync` and unifies symlink resolution:
- length was unlimited in one case
- length was truncated to 1024 chars in the other case
Now it is hard-limited to 4096 chars (ie error if larger), which is a length which seems appropriate according to https://stackoverflow.com/a/22575737.
### Tests
- Tests are already present in `tests/integration/repo_test.go:972`: `TestRepoFollowSymlink` (it caught a cap/len stupid mistake).
- [x] I did not document these changes and I do not expect someone else to do it.
- [x] I do not want this change to show in the release notes.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8323
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: oliverpool <git@olivier.pfad.fr>
Co-committed-by: oliverpool <git@olivier.pfad.fr>
- When doing CRUD actions, the commiter and author are reconstructed and
do not contain the doer's ID. Make sure to pass this ID along so it can
be used to verify the rules of instance signing for CRUD actions.
- Regression of forgejo/forgejo#7693. It seems that previously this
didn't work correctly as it would not care about a empty ID.
- Resolvesforgejo/forgejo#8278
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8304
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>
Reviewed-by: Beowulf <beowulf@beocode.eu>
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>
See #8222 for context.i
`GetBlobContentBase64` was using a pipe and a goroutine to read the blob content as base64. This can be replace by a pre-allocated buffer and a direct copy.
Note that although similar to `GetBlobContent`, it does not truncate the content if the blob size is over the limit (but returns an error). I think that `GetBlobContent` should adopt the same behavior at some point (error instead of truncating).
### Tests
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
- [x] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [x] I do not want this change to show in the release notes.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8297
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: oliverpool <git@olivier.pfad.fr>
Co-committed-by: oliverpool <git@olivier.pfad.fr>
Two pull requests were merged at the same time
- https://codeberg.org/forgejo/forgejo/pulls/7699
- https://codeberg.org/forgejo/forgejo/pulls/7508
And added conflicting structs ActionRun modules/structs. That broke
the forgejo development branch and a quick fix was made to resolve
the name conflict.
- https://codeberg.org/forgejo/forgejo/pulls/8066
However that creates an undesirable duplication of two structures that
serve the same purpose but are different.
- Remove RepoActionRun and replace it with ActionRun
- convert.ToActionRun has one more argument, the doer, because it
is determined differently in the context of webhooks or API
### Tests
- No need because the two pull requests involved already have good coverage.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8250
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>
Reviewed-by: klausfyhn <klausfyhn@noreply.codeberg.org>
Reviewed-by: Christopher Besch <mail@chris-besch.com>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>
* Add the `notify-email` column / NotifyEmail to ActionRun and set it:
* services/actions/workflows.go `Dispatch`
* services/actions/schedule_tasks.go `CreateScheduleTask`
* services/actions/notifier_helper.go `handleWorkflows`
* Only send an email if the workflow has `enable-email-notifications: true` by having `MailActionRun` return immediately if `NotifyEmail` is false.
* Ignore or silently fail on `enable-email-notifications: true` parsing errors. Reporting such errors belongs in workflow validation, not when it is evaluated for the notifications.
* Add unit and integration tests.
Refs: https://codeberg.org/forgejo/forgejo/issues/8187
## Checklist
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
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
- [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
### Documentation
- [x] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [x] I do not want this change to show in the release notes.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8242
Reviewed-by: Christopher Besch <mail@chris-besch.com>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>
- If the run was attributed to a system user (which is the case for scheduled runs for instance), ignore it and fallback to the mail of the owner. System users may have email addresses, but they are not to be used.
- If the owner is a system user or an organization with no email associated with it, do nothing.
- If a user with an email exists, check if they did not disable notifications and send the email.
Refs: https://codeberg.org/forgejo/forgejo/issues/8187
Refs: https://codeberg.org/forgejo/forgejo/issues/8233
## Checklist
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
- I added test coverage for Go changes...
- [x] in their respective `*_test.go` for unit tests.
### Documentation
- [x] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [x] I do not want this change to show in the release notes.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8227
Reviewed-by: Christopher Besch <mail@chris-besch.com>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>
This PR is part of https://codeberg.org/forgejo/forgejo/pulls/4767
This should not have an outside impact but bring all model changes needed & bring migrations.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8078
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Michael Jerger <michael.jerger@meissa-gmbh.de>
Co-committed-by: Michael Jerger <michael.jerger@meissa-gmbh.de>
Automerge can be ignored when the following race happens:
* Conflict check is happening on a repository and
`pr.Status = issues_model.PullRequestStatusChecking` for all open pull
requests (this happens every time a pull request is merged).
* While the conflict check is ongoing, an event (Forgejo Actions being
successful for instance) happens and and `StartPRCheckAndAutoMerge*` is called.
* Because `pr.CanAutoMerge()` is false, the pull request is not
selected and not added to the automerge queue.
* When the conflict check completes and `pr.CanAutoMerge()` becomes
true, there no longer is a task in the auto merge queue and the
auto merge does not happen.
This is fixed by adding a task to the auto merge queue when the conflict check for a pull request completes. This is done when the mutx protecting the conflict check task is released to prevent a deadlock when a synchronous queues are used in the following situation:
* the conflict check task finds the pull request is mergeable
* it schedules the auto merge tasks that finds it must be merged
* merging concludes with scheduling a conflict check task
Avoid an extra loop where a conflict check task queues an auto merge task that will schedule a conflict check task if the pull request can be merged. The auto merge row is removed from the database before merging. It would otherwise be removed after the merge commit is received via the git hook which happens asynchronously and can lead to a race.
StartPRCheckAndAutoMerge is modified to re-use HeadCommitID when available, such as when called after a pull request conflict check.
---
A note on tests: they cover the new behavior, i.e. automerge being triggered by a successful conflict check. This is also on the critical paths for every test that involve creating, merging or updating a pull request.
- `tests/integration/git_test.go`
- `tests/integration/actions_commit_status_test.go`
- `tests/integration/api_helper_for_declarative_test.go`
- `tests/integration/patch_status_test.go`
- `tests/integration/pull_merge_test.go`
The [missing fixture file](https://codeberg.org/forgejo/forgejo/pulls/8189/files#diff-b86fdd79108b3ba3cb2e56ffcfd1be2a7b32f46c) for the auto merge table can be verified to be necessary simply by removing it an observing that the integration tests fail.
The [scheduling of the auto merge task](https://codeberg.org/forgejo/forgejo/pulls/8189/files#diff-9489262e93967f6bb2db41837f37c06f4e70d978) in `testPR` can be verified to be required by moving it in the `testPRProtected` function and observing that the tests hang forever because of the deadlock.
## Checklist
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
- I added test coverage for Go changes...
- [ ] in their respective `*_test.go` for unit tests.
- [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
- [ ] in `web_src/js/*.test.js` if it can be unit tested.
- [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).
### Documentation
- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.
### Release notes
- [ ] I do not want this change to show in the release notes.
- [x] I want the title to show in the release notes with a link to this pull request.
- [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.
<!--start release-notes-assistant-->
## Release notes
<!--URL:https://codeberg.org/forgejo/forgejo-->
- Bug fixes
- [PR](https://codeberg.org/forgejo/forgejo/pulls/8189): <!--number 8189 --><!--line 0 --><!--description ZG8gbm90IGlnbm9yZSBhdXRvbWVyZ2Ugd2hpbGUgYSBQUiBpcyBjaGVja2luZyBmb3IgY29uZmxpY3Rz-->do not ignore automerge while a PR is checking for conflicts<!--description-->
<!--end release-notes-assistant-->
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8189
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>
Reviewed-by: Lucas <sclu1034@noreply.codeberg.org>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>
This PR contains the following updates:
| Package | Type | Update | Change |
|---|---|---|---|
| [gitlab.com/gitlab-org/api/client-go](https://gitlab.com/gitlab-org/api/client-go) | require | minor | `v0.129.0` -> `v0.130.1` |
---
### Release Notes
<details>
<summary>gitlab-org/api/client-go (gitlab.com/gitlab-org/api/client-go)</summary>
### [`v0.130.1`](https://gitlab.com/gitlab-org/api/client-go/tags/v0.130.1)
[Compare Source](https://gitlab.com/gitlab-org/api/client-go/compare/v0.130.0...v0.130.1)
#### [0.130.1](https://gitlab.com/gitlab-org/api/client-go/compare/v0.130.0...v0.130.1) (2025-06-11)
##### Bug Fixes
* add missing nil check on create group with avatar ([3298a05](3298a058f3))
### [`v0.130.0`](https://gitlab.com/gitlab-org/api/client-go/tags/v0.130.0)
[Compare Source](https://gitlab.com/gitlab-org/api/client-go/compare/v0.129.0...v0.130.0)
### [0.130.0](https://gitlab.com/gitlab-org/api/client-go/compare/v0.129.0...v0.130.0) (2025-06-11)
##### Bug Fixes
* **workflow:** the `release.config.mjs` file mustn't be hidden ([5d423a5](5d423a55d5))
##### Features
* add "emoji_events" support to group hooks ([c6b770f](c6b770f350))
* Add `active` to ListProjects ([7818155](78181558db))
* add generated_file field for MergeRequestDiff ([4b95dac](4b95dac3ef))
* add support for `administrator` to Group `project_creation_level` enums ([664bbd7](664bbd7e3c))
* add the `WithTokenSource` client option ([6ccfcf8](6ccfcf857a))
* add url field to MergeCommentEvent.merge_request ([bd639d8](bd639d811c))
* implement a specialized `TokenSource` interface ([83c2e06](83c2e06cbe))
* **projects:** add ci_push_repository_for_job_token_allowed parameter ([3d539f6](3d539f66fd))
* **terraform-states:** add Terraform States API ([082b81c](082b81cd45))
</details>
---
### Configuration
📅 **Schedule**: Branch creation - Between 12:00 AM and 03:59 AM ( * 0-3 * * * ) (UTC), Automerge - Between 12:00 AM and 03:59 AM ( * 0-3 * * * ) (UTC).
🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.
♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
🔕 **Ignore**: Close this PR and you won't be reminded about this update again.
---
- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box
---
This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MC40OC40IiwidXBkYXRlZEluVmVyIjoiNDAuNDguNCIsInRhcmdldEJyYW5jaCI6ImZvcmdlam8iLCJsYWJlbHMiOlsiZGVwZW5kZW5jeS11cGdyYWRlIiwidGVzdC9ub3QtbmVlZGVkIl19-->
Co-authored-by: Earl Warren <contact@earl-warren.org>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8163
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Renovate Bot <forgejo-renovate-action@forgejo.org>
Co-committed-by: Renovate Bot <forgejo-renovate-action@forgejo.org>
Currently the `POST /repos/{owner}/{repo}/contents` API endpoint accepts request without any `ChangeFileOperation.SHA`, unlike stated by the doc:
33eee199cf/modules/structs/repo_file.go (L80-L81)
This PR adds:
- some more (already passing) tests around this function
- a new (failing) test to show this wrong behavior
- a fix (note that this is a breaking change for clients exploiting this bug)
- an update for all the existing tests
<!--start release-notes-assistant-->
## Release notes
<!--URL:https://codeberg.org/forgejo/forgejo-->
- Breaking bug fixes
- [PR](https://codeberg.org/forgejo/forgejo/pulls/8139): <!--number 8139 --><!--line 0 --><!--description QVBJOiBlbmZvcmNlIHNoYSByZXF1aXJlbWVudCBvbiBgUE9TVCAvcmVwb3Mve293bmVyfS97cmVwb30vY29udGVudHNg-->API: enforce sha requirement on `POST /repos/{owner}/{repo}/contents`<!--description-->
<!--end release-notes-assistant-->
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8139
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: oliverpool <git@olivier.pfad.fr>
Co-committed-by: oliverpool <git@olivier.pfad.fr>
