Close#31801. Follow #31761.
Since there are so many benefits of compression and there are no reports
of related issues after weeks, it should be fine to enable compression
by default.
Replace #32001.
To prevent the context cache from being misused for long-term work
(which would result in using invalid cache without awareness), the
context cache is designed to exist for a maximum of 10 seconds. This
leads to many false reports, especially in the case of slow SQL.
This PR increases it to 5 minutes to reduce false reports.
5 minutes is not a very safe value, as a lot of changes may have
occurred within that time frame. However, as far as I know, there has
not been a case of misuse of context cache discovered so far, so I think
5 minutes should be OK.
Please note that after this PR, if warning logs are found again, it
should get attention, at that time it can be almost 100% certain that it
is a misuse.
Follow #31908. The main refactor is that it has removed the returned
context of `Lock`.
The returned context of `Lock` in old code is to provide a way to let
callers know that they have lost the lock. But in most cases, callers
shouldn't cancel what they are doing even it has lost the lock. And the
design would confuse developers and make them use it incorrectly.
See the discussion history:
https://github.com/go-gitea/gitea/pull/31813#discussion_r1732041513 and
https://github.com/go-gitea/gitea/pull/31813#discussion_r1734078998
It's a breaking change, but since the new module hasn't been used yet, I
think it's OK to not add the `pr/breaking` label.
## Design principles
It's almost copied from #31908, but with some changes.
### Use spinlock even in memory implementation (unchanged)
In actual use cases, users may cancel requests. `sync.Mutex` will block
the goroutine until the lock is acquired even if the request is
canceled. And the spinlock is more suitable for this scenario since it's
possible to give up the lock acquisition.
Although the spinlock consumes more CPU resources, I think it's
acceptable in most cases.
### Do not expose the mutex to callers (unchanged)
If we expose the mutex to callers, it's possible for callers to reuse
the mutex, which causes more complexity.
For example:
```go
lock := GetLocker(key)
lock.Lock()
// ...
// even if the lock is unlocked, we cannot GC the lock,
// since the caller may still use it again.
lock.Unlock()
lock.Lock()
// ...
lock.Unlock()
// callers have to GC the lock manually.
RemoveLocker(key)
```
That's why
https://github.com/go-gitea/gitea/pull/31813#discussion_r1721200549
In this PR, we only expose `ReleaseFunc` to callers. So callers just
need to call `ReleaseFunc` to release the lock, and do not need to care
about the lock's lifecycle.
```go
release, err := locker.Lock(ctx, key)
if err != nil {
return err
}
// ...
release()
// if callers want to lock again, they have to re-acquire the lock.
release, err := locker.Lock(ctx, key)
// ...
```
In this way, it's also much easier for redis implementation to extend
the mutex automatically, so that callers do not need to care about the
lock's lifecycle. See also
https://github.com/go-gitea/gitea/pull/31813#discussion_r1722659743
### Use "release" instead of "unlock" (unchanged)
For "unlock", it has the meaning of "unlock an acquired lock". So it's
not acceptable to call "unlock" when failed to acquire the lock, or call
"unlock" multiple times. It causes more complexity for callers to decide
whether to call "unlock" or not.
So we use "release" instead of "unlock" to make it clear. Whether the
lock is acquired or not, callers can always call "release", and it's
also safe to call "release" multiple times.
But the code DO NOT expect callers to not call "release" after acquiring
the lock. If callers forget to call "release", it will cause resource
leak. That's why it's always safe to call "release" without extra
checks: to avoid callers to forget to call it.
### Acquired locks could be lost, but the callers shouldn't stop
Unlike `sync.Mutex` which will be locked forever once acquired until
calling `Unlock`, for distributed lock, the acquired lock could be lost.
For example, the caller has acquired the lock, and it holds the lock for
a long time since auto-extending is working for redis. However, it lost
the connection to the redis server, and it's impossible to extend the
lock anymore.
In #31908, it will cancel the context to make the operation stop, but
it's not safe. Many operations are not revert-able. If they have been
interrupted, then the instance goes corrupted. So `Lock` won't return
`ctx` anymore in this PR.
### Multiple ways to use the lock
1. Regular way
```go
release, err := Lock(ctx, key)
if err != nil {
return err
}
defer release()
// ...
```
2. Early release
```go
release, err := Lock(ctx, key)
if err != nil {
return err
}
defer release()
// ...
// release the lock earlier
release()
// continue to do something else
// ...
```
3. Functional way
```go
if err := LockAndDo(ctx, key, func(ctx context.Context) error {
// ...
return nil
}); err != nil {
return err
}
```
To help #31813, but do not replace it, since this PR just introduces the
new module but misses some work:
- New option in settings. `#31813` has done it.
- Use the locks in business logic. `#31813` has done it.
So I think the most efficient way is to merge this PR first (if it's
acceptable) and then finish #31813.
## Design principles
### Use spinlock even in memory implementation
In actual use cases, users may cancel requests. `sync.Mutex` will block
the goroutine until the lock is acquired even if the request is
canceled. And the spinlock is more suitable for this scenario since it's
possible to give up the lock acquisition.
Although the spinlock consumes more CPU resources, I think it's
acceptable in most cases.
### Do not expose the mutex to callers
If we expose the mutex to callers, it's possible for callers to reuse
the mutex, which causes more complexity.
For example:
```go
lock := GetLocker(key)
lock.Lock()
// ...
// even if the lock is unlocked, we cannot GC the lock,
// since the caller may still use it again.
lock.Unlock()
lock.Lock()
// ...
lock.Unlock()
// callers have to GC the lock manually.
RemoveLocker(key)
```
That's why
https://github.com/go-gitea/gitea/pull/31813#discussion_r1721200549
In this PR, we only expose `ReleaseFunc` to callers. So callers just
need to call `ReleaseFunc` to release the lock, and do not need to care
about the lock's lifecycle.
```go
_, release, err := locker.Lock(ctx, key)
if err != nil {
return err
}
// ...
release()
// if callers want to lock again, they have to re-acquire the lock.
_, release, err := locker.Lock(ctx, key)
// ...
```
In this way, it's also much easier for redis implementation to extend
the mutex automatically, so that callers do not need to care about the
lock's lifecycle. See also
https://github.com/go-gitea/gitea/pull/31813#discussion_r1722659743
### Use "release" instead of "unlock"
For "unlock", it has the meaning of "unlock an acquired lock". So it's
not acceptable to call "unlock" when failed to acquire the lock, or call
"unlock" multiple times. It causes more complexity for callers to decide
whether to call "unlock" or not.
So we use "release" instead of "unlock" to make it clear. Whether the
lock is acquired or not, callers can always call "release", and it's
also safe to call "release" multiple times.
But the code DO NOT expect callers to not call "release" after acquiring
the lock. If callers forget to call "release", it will cause resource
leak. That's why it's always safe to call "release" without extra
checks: to avoid callers to forget to call it.
### Acquired locks could be lost
Unlike `sync.Mutex` which will be locked forever once acquired until
calling `Unlock`, in the new module, the acquired lock could be lost.
For example, the caller has acquired the lock, and it holds the lock for
a long time since auto-extending is working for redis. However, it lost
the connection to the redis server, and it's impossible to extend the
lock anymore.
If the caller don't stop what it's doing, another instance which can
connect to the redis server could acquire the lock, and do the same
thing, which could cause data inconsistency.
So the caller should know what happened, the solution is to return a new
context which will be canceled if the lock is lost or released:
```go
ctx, release, err := locker.Lock(ctx, key)
if err != nil {
return err
}
defer release()
// ...
DoSomething(ctx)
// the lock is lost now, then ctx has been canceled.
// Failed, since ctx has been canceled.
DoSomethingElse(ctx)
```
### Multiple ways to use the lock
1. Regular way
```go
ctx, release, err := Lock(ctx, key)
if err != nil {
return err
}
defer release()
// ...
```
2. Early release
```go
ctx, release, err := Lock(ctx, key)
if err != nil {
return err
}
defer release()
// ...
// release the lock earlier and reset the context back
ctx = release()
// continue to do something else
// ...
```
3. Functional way
```go
if err := LockAndDo(ctx, key, func(ctx context.Context) error {
// ...
return nil
}); err != nil {
return err
}
```
Fix#31625.
If `pull_service.NewPullRequest` return an error which misses each `if`
check, `CompareAndPullRequestPost` will return immediately, since it
doesn't write the HTTP response, a 200 response with empty body will be
sent to clients.
```go
if err := pull_service.NewPullRequest(ctx, repo, pullIssue, labelIDs, attachments, pullRequest, assigneeIDs); err != nil {
if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) {
ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error())
} else if git.IsErrPushRejected(err) {
// ...
ctx.JSONError(flashError)
} else if errors.Is(err, user_model.ErrBlockedUser) {
// ...
ctx.JSONError(flashError)
} else if errors.Is(err, issues_model.ErrMustCollaborator) {
// ...
ctx.JSONError(flashError)
}
return
}
```
Not sure what kind of error can cause it to happen, so this PR just
expose it. And we can fix it when users report that creating PRs failed
with error responses.
It's all my guess since I cannot reproduce the problem, but even if it's
not related, the code here needs to be improved.
Fix#31271.
When gogit is enabled, `IsObjectExist` calls
`repo.gogitRepo.ResolveRevision`, which is not correct. It's for
checking references not objects, it could work with commit hash since
it's both a valid reference and a commit object, but it doesn't work
with blob objects.
So it causes #31271 because it reports that all blob objects do not
exist.
Support compression for Actions logs to save storage space and
bandwidth. Inspired by
https://github.com/go-gitea/gitea/issues/24256#issuecomment-1521153015
The biggest challenge is that the compression format should support
[seekable](https://github.com/facebook/zstd/blob/dev/contrib/seekable_format/zstd_seekable_compression_format.md).
So when users are viewing a part of the log lines, Gitea doesn't need to
download the whole compressed file and decompress it.
That means gzip cannot help here. And I did research, there aren't too
many choices, like bgzip and xz, but I think zstd is the most popular
one. It has an implementation in Golang with
[zstd](https://github.com/klauspost/compress/tree/master/zstd) and
[zstd-seekable-format-go](https://github.com/SaveTheRbtz/zstd-seekable-format-go),
and what is better is that it has good compatibility: a seekable format
zstd file can be read by a regular zstd reader.
This PR introduces a new package `zstd` to combine and wrap the two
packages, to provide a unified and easy-to-use API.
And a new setting `LOG_COMPRESSION` is added to the config, although I
don't see any reason why not to use compression, I think's it's a good
idea to keep the default with `none` to be consistent with old versions.
`LOG_COMPRESSION` takes effect for only new log files, it adds `.zst` as
an extension to the file name, so Gitea can determine if it needs
decompression according to the file name when reading. Old files will
keep the format since it's not worth converting them, as they will be
cleared after #31735.
<img width="541" alt="image"
src="https://github.com/user-attachments/assets/e9598764-a4e0-4b68-8c2b-f769265183c9">
Found at
https://github.com/go-gitea/gitea/pull/31790#issuecomment-2272898915
`unit-tests-gogit` never work since the workflow set `TAGS` with
`gogit`, but the Makefile use `TEST_TAGS`.
This PR adds the values of `TAGS` to `TEST_TAGS`, ensuring that setting
`TAGS` is always acceptable and avoiding confusion about which one
should be set.
Fix#31657.
According to the
[doc](https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#onschedule)
of GitHub Actions, The timezone for cron should be UTC, not the local
timezone. And Gitea Actions doesn't have any reasons to change this, so
I think it's a bug.
However, Gitea Actions has extended the syntax, as it supports
descriptors like `@weekly` and `@every 5m`, and supports specifying the
timezone like `TZ=UTC 0 10 * * *`. So we can make it use UTC only when
the timezone is not specified, to be compatible with GitHub Actions, and
also respect the user's specified.
It does break the feature because the times to run tasks would be
changed, and it may confuse users. So I don't think we should backport
this.
## ⚠️ BREAKING ⚠️
If the server's local time zone is not UTC, a scheduled task would run
at a different time after upgrading Gitea to this version.
Fix#31707.
Also related to #31715.
Some Actions resources could has different types of ownership. It could
be:
- global: all repos and orgs/users can use it.
- org/user level: only the org/user can use it.
- repo level: only the repo can use it.
There are two ways to distinguish org/user level from repo level:
1. `{owner_id: 1, repo_id: 2}` for repo level, and `{owner_id: 1,
repo_id: 0}` for org level.
2. `{owner_id: 0, repo_id: 2}` for repo level, and `{owner_id: 1,
repo_id: 0}` for org level.
The first way seems more reasonable, but it may not be true. The point
is that although a resource, like a runner, belongs to a repo (it can be
used by the repo), the runner doesn't belong to the repo's org (other
repos in the same org cannot use the runner). So, the second method
makes more sense.
And the first way is not user-friendly to query, we must set the repo id
to zero to avoid wrong results.
So, #31715 should be right. And the most simple way to fix#31707 is
just:
```diff
- shared.GetRegistrationToken(ctx, ctx.Repo.Repository.OwnerID, ctx.Repo.Repository.ID)
+ shared.GetRegistrationToken(ctx, 0, ctx.Repo.Repository.ID)
```
However, it is quite intuitive to set both owner id and repo id since
the repo belongs to the owner. So I prefer to be compatible with it. If
we get both owner id and repo id not zero when creating or finding, it's
very clear that the caller want one with repo level, but set owner id
accidentally. So it's OK to accept it but fix the owner id to zero.
Fix#31137.
Replace #31623#31697.
When migrating LFS objects, if there's any object that failed (like some
objects are losted, which is not really critical), Gitea will stop
migrating LFS immediately but treat the migration as successful.
This PR checks the error according to the [LFS api
doc](https://github.com/git-lfs/git-lfs/blob/main/docs/api/batch.md#successful-responses).
> LFS object error codes should match HTTP status codes where possible:
>
> - 404 - The object does not exist on the server.
> - 409 - The specified hash algorithm disagrees with the server's
acceptable options.
> - 410 - The object was removed by the owner.
> - 422 - Validation error.
If the error is `404`, it's safe to ignore it and continue migration.
Otherwise, stop the migration and mark it as failed to ensure data
integrity of LFS objects.
And maybe we should also ignore others errors (maybe `410`? I'm not sure
what's the difference between "does not exist" and "removed by the
owner".), we can add it later when some users report that they have
failed to migrate LFS because of an error which should be ignored.
There's already `initActionsTasks`; it will avoid additional check for
if Actions enabled to move `registerActionsCleanup` into it.
And we don't really need `OlderThanConfig`.
It's time (maybe somewhat late) to remove some deprecated stuff for the
runner.
- `x-runner-version`: runners needn't to report version in every
request, they will call `Declare`.
- `AgentLabels`: runners will report them as `Labels`.
Related to #30375.
It doesn't make sense to import `modules/web/middleware` and
`modules/setting` in `modules/web/session` since the last one is more
low-level.
And it looks like a workaround to call `DeleteLegacySiteCookie` in
`RegenerateSession`, so maybe we could reverse the importing by
registering hook functions.
Help #29999, or its tests cannot pass.
Also, add some comments to clarify the usage of `TxContext`.
I don't check all usages of `TxContext` because there are too many
(almost 140+). It's a better idea to replace them with `WithTx` instead
of checking them one by one. However, that may be another refactoring
PR.
Regression of #29493. If a branch has been deleted, repushing it won't
restore it.
Lunny may have noticed that, but I didn't delve into the comment then
overlooked it:
https://github.com/go-gitea/gitea/pull/29493#discussion_r1509046867
The additional comments added are to explain the issue I found during
testing, which are unrelated to the fixes.
It is convenient to skip by setting environment, since it's OK
to use root user in job containers.
It's not a bug, but I want to backport it to v1.21 since it doesn't
break anything.
---------
Co-authored-by: Giteabot <teabot@gitea.io>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Detect broken git hooks by checking if the commit id of branches in DB
is the same with the git repo.
It can help #29338#28277 and maybe more issues.
Users could complain about actions, webhooks, and activities not
working, but they were not aware that it is caused by broken git hooks
unless they could see a warning.
<img width="1348" alt="image"
src="https://github.com/go-gitea/gitea/assets/9418365/2b92a46d-7f1d-4115-bef4-9f970bd695da">
It should be merged after #29493. Otherwise, users could see a ephemeral
warning after committing and opening the repo home page immediately.
And it also waits for #29495, since the doc link (the anchor part) will
be updated.
---------
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: Giteabot <teabot@gitea.io>
Unlike other async processing in the queue, we should sync branches to
the DB immediately when handling git hook calling. If it fails, users
can see the error message in the output of the git command.
It can avoid potential inconsistency issues, and help #29494.
---------
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
It will fix#28268 .
<img width="1313" alt="image"
src="https://github.com/go-gitea/gitea/assets/9418365/cb1e07d5-7a12-4691-a054-8278ba255bfc">
<img width="1318" alt="image"
src="https://github.com/go-gitea/gitea/assets/9418365/4fd60820-97f1-4c2c-a233-d3671a5039e9">
## ⚠️ BREAKING ⚠️
But need to give up some features:
<img width="1312" alt="image"
src="https://github.com/go-gitea/gitea/assets/9418365/281c0d51-0e7d-473f-bbed-216e2f645610">
However, such abandonment may fix#28055 .
## Backgroud
When the user switches the dashboard context to an org, it means they
want to search issues in the repos that belong to the org. However, when
they switch to themselves, it means all repos they can access because
they may have created an issue in a public repo that they don't own.
<img width="286" alt="image"
src="https://github.com/go-gitea/gitea/assets/9418365/182dcd5b-1c20-4725-93af-96e8dfae5b97">
It's a confusing design. Think about this: What does "In your
repositories" mean when the user switches to an org? Repos belong to the
user or the org?
Whatever, it has been broken by #26012 and its following PRs. After the
PR, it searches for issues in repos that the dashboard context user owns
or has been explicitly granted access to, so it causes #28268.
## How to fix it
It's not really difficult to fix it. Just extend the repo scope to
search issues when the dashboard context user is the doer. Since the
user may create issues or be mentioned in any public repo, we can just
set `AllPublic` to true, which is already supported by indexers. The DB
condition will also support it in this PR.
But the real difficulty is how to count the search results grouped by
repos. It's something like "search issues with this keyword and those
filters, and return the total number and the top results. **Then, group
all of them by repo and return the counts of each group.**"
<img width="314" alt="image"
src="https://github.com/go-gitea/gitea/assets/9418365/5206eb20-f8f5-49b9-b45a-1be2fcf679f4">
Before #26012, it was being done in the DB, but it caused the results to
be incomplete (see the description of #26012).
And to keep this, #26012 implement it in an inefficient way, just count
the issues by repo one by one, so it cannot work when `AllPublic` is
true because it's almost impossible to do this for all public repos.
https://github.com/go-gitea/gitea/blob/1bfcdeef4cca0f5509476358e5931c13d37ed1ca/modules/indexer/issues/indexer.go#L318-L338
## Give up unnecessary features
We may can resovle `TODO: use "group by" of the indexer engines to
implement it`, I'm sure it can be done with Elasticsearch, but IIRC,
Bleve and Meilisearch don't support "group by".
And the real question is, does it worth it? Why should we need to know
the counts grouped by repos?
Let me show you my search dashboard on gitea.com.
<img width="1304" alt="image"
src="https://github.com/go-gitea/gitea/assets/9418365/2bca2d46-6c71-4de1-94cb-0c9af27c62ff">
I never think the long repo list helps anything.
And if we agree to abandon it, things will be much easier. That is this
PR.
## TODO
I know it's important to filter by repos when searching issues. However,
it shouldn't be the way we have it now. It could be implemented like
this.
<img width="1316" alt="image"
src="https://github.com/go-gitea/gitea/assets/9418365/99ee5f21-cbb5-4dfe-914d-cb796cb79fbe">
The indexers support it well now, but it requires some frontend work,
which I'm not good at. So, I think someone could help do that in another
PR and merge this one to fix the bug first.
Or please block this PR and help to complete it.
Finally, "Switch dashboard context" is also a design that needs
improvement. In my opinion, it can be accomplished by adding filtering
conditions instead of "switching".
When `webhook.PROXY_URL` has been set, the old code will check if the
proxy host is in `ALLOWED_HOST_LIST` or reject requests through the
proxy. It requires users to add the proxy host to `ALLOWED_HOST_LIST`.
However, it actually allows all requests to any port on the host, when
the proxy host is probably an internal address.
But things may be even worse. `ALLOWED_HOST_LIST` doesn't really work
when requests are sent to the allowed proxy, and the proxy could forward
them to any hosts.
This PR fixes it by:
- If the proxy has been set, always allow connectioins to the host and
port.
- Check `ALLOWED_HOST_LIST` before forwarding.
This PR has multiple parts, and I didn't split them because
it's not easy to test them separately since they are all about the
dashboard page for issues.
1. Support counting issues via indexer to fix#26361
2. Fix repo selection so it also fixes#26653
3. Keep keywords in filter links.
The first two are regressions of #26012.
After:
https://github.com/go-gitea/gitea/assets/9418365/71dfea7e-d9e2-42b6-851a-cc081435c946
Thanks to @CaiCandong for helping with some tests.
Fix#26536
Follow #26012
Whatever the comment type is, always update the issue indexer. So the
issue indexer will be updated when there is a change in Status,
Assignee, Label, and so on.
I added the logic for `NotifyUpdateComment`, but missed it for
`NotifyCreateIssueComment` and `NotifyDeleteComment`.
Fix#24662.
Replace #24822 and #25708 (although it has been merged)
## Background
In the past, Gitea supported issue searching with a keyword and
conditions in a less efficient way. It worked by searching for issues
with the keyword and obtaining limited IDs (as it is heavy to get all)
on the indexer (bleve/elasticsearch/meilisearch), and then querying with
conditions on the database to find a subset of the found IDs. This is
why the results could be incomplete.
To solve this issue, we need to store all fields that could be used as
conditions in the indexer and support both keyword and additional
conditions when searching with the indexer.
## Major changes
- Redefine `IndexerData` to include all fields that could be used as
filter conditions.
- Refactor `Search(ctx context.Context, kw string, repoIDs []int64,
limit, start int, state string)` to `Search(ctx context.Context, options
*SearchOptions)`, so it supports more conditions now.
- Change the data type stored in `issueIndexerQueue`. Use
`IndexerMetadata` instead of `IndexerData` in case the data has been
updated while it is in the queue. This also reduces the storage size of
the queue.
- Enhance searching with Bleve/Elasticsearch/Meilisearch, make them
fully support `SearchOptions`. Also, update the data versions.
- Keep most logic of database indexer, but remove
`issues.SearchIssueIDsByKeyword` in `models` to avoid confusion where is
the entry point to search issues.
- Start a Meilisearch instance to test it in unit tests.
- Add unit tests with almost full coverage to test
Bleve/Elasticsearch/Meilisearch indexer.
---------
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
The `FileBlame` function looks strange, it has `revision` as argument
but doesn't use it.
Since the function never be used, I think we could just remove it.
If anyone thinks it should be kept, please help fix `revision`.
Co-authored-by: Giteabot <teabot@gitea.io>
Set the correct permissions on the .ssh directory and authorized_keys
file, or sshd will refuse to use them and lead to clone/push/pull
failures.
It could happen when users have copied their data to a new volume and
changed the file permission by accident, and it would be very hard to
troubleshoot unless users know how to check the logs of sshd which is
started by s6.
Co-authored-by: Giteabot <teabot@gitea.io>
Fix regression of #5363 (so long ago).
The old code definded a document mapping for `issueIndexerDocType`, and
assigned it to `BleveIndexerData` as its type. (`BleveIndexerData` has
been renamed to `IndexerData` in #25174, but nothing more.) But the old
code never used `BleveIndexerData`, it wrote the index with an anonymous
struct type. Nonetheless, bleve would use the default auto-mapping for
struct it didn't know, so the indexer still worked. This means the
custom document mapping was always dead code.
The custom document mapping is not useless, it can reduce index storage,
this PR brings it back and disable default mapping to prevent it from
happening again. Since `IndexerData`(`BleveIndexerData`) has JSON tags,
and bleve uses them first, so we should use `repo_id` as the field name
instead of `RepoID`.
I did a test to compare the storage size before and after this, with
about 3k real comments that were migrated from some public repos.
Before:
```text
[ 160] .
├── [ 42] index_meta.json
├── [ 13] rupture_meta.json
└── [ 128] store
├── [6.9M] 00000000005d.zap
└── [256K] root.bolt
```
After:
```text
[ 160] .
├── [ 42] index_meta.json
├── [ 13] rupture_meta.json
└── [ 128] store
├── [3.5M] 000000000065.zap
└── [256K] root.bolt
```
It saves about half the storage space.
---------
Co-authored-by: Giteabot <teabot@gitea.io>
Resolve#24789
## ⚠️ BREAKING ⚠️
Before this, `DEFAULT_ACTIONS_URL` cound be set to any custom URLs like
`https://gitea.com` or `http://your-git-server,https://gitea.com`, and
the default value was `https://gitea.com`.
But now, `DEFAULT_ACTIONS_URL` supports only
`github`(`https://github.com`) or `self`(the root url of current Gitea
instance), and the default value is `github`.
If it has configured with a URL, an error log will be displayed and it
will fallback to `github`.
Actually, what we really want to do is always make it
`https://github.com`, however, this may not be acceptable for some
instances of internal use, so there's extra support for `self`, but no
more, even `https://gitea.com`.
Please note that `uses: https://xxx/yyy/zzz` always works and it does
exactly what it is supposed to do.
Although it's breaking, I belive it should be backported to `v1.20` due
to some security issues.
Follow-up on the runner side:
- https://gitea.com/gitea/act_runner/pulls/262
- https://gitea.com/gitea/act/pulls/70
Fix#25451.
Bugfixes:
- When stopping the zombie or endless tasks, set `LogInStorage` to true
after transferring the file to storage. It was missing, it could write
to a nonexistent file in DBFS because `LogInStorage` was false.
- Always update `ActionTask.Updated` when there's a new state reported
by the runner, even if there's no change. This is to avoid the task
being judged as a zombie task.
Enhancement:
- Support `Stat()` for DBFS file.
- `WriteLogs` refuses to write if it could result in content holes.
---------
Co-authored-by: Giteabot <teabot@gitea.io>
Fix regression of #25174.
The `Init` of the db indexer should return true to indicate that the
index was opened/existed, or the indexer will try to populate the index
(not really populate, just fill the queue, `Index` method of the db
indexer is a dummy).
Refactor `modules/indexer` to make it more maintainable. And it can be
easier to support more features. I'm trying to solve some of issue
searching, this is a precursor to making functional changes.
Current supported engines and the index versions:
| engines | issues | code |
| - | - | - |
| db | Just a wrapper for database queries, doesn't need version | - |
| bleve | The version of index is **2** | The version of index is **6**
|
| elasticsearch | The old index has no version, will be treated as
version **0** in this PR | The version of index is **1** |
| meilisearch | The old index has no version, will be treated as version
**0** in this PR | - |
## Changes
### Split
Splited it into mutiple packages
```text
indexer
├── internal
│ ├── bleve
│ ├── db
│ ├── elasticsearch
│ └── meilisearch
├── code
│ ├── bleve
│ ├── elasticsearch
│ └── internal
└── issues
├── bleve
├── db
├── elasticsearch
├── internal
└── meilisearch
```
- `indexer/interanal`: Internal shared package for indexer.
- `indexer/interanal/[engine]`: Internal shared package for each engine
(bleve/db/elasticsearch/meilisearch).
- `indexer/code`: Implementations for code indexer.
- `indexer/code/internal`: Internal shared package for code indexer.
- `indexer/code/[engine]`: Implementation via each engine for code
indexer.
- `indexer/issues`: Implementations for issues indexer.
### Deduplication
- Combine `Init/Ping/Close` for code indexer and issues indexer.
- ~Combine `issues.indexerHolder` and `code.wrappedIndexer` to
`internal.IndexHolder`.~ Remove it, use dummy indexer instead when the
indexer is not ready.
- Duplicate two copies of creating ES clients.
- Duplicate two copies of `indexerID()`.
### Enhancement
- [x] Support index version for elasticsearch issues indexer, the old
index without version will be treated as version 0.
- [x] Fix spell of `elastic_search/ElasticSearch`, it should be
`Elasticsearch`.
- [x] Improve versioning of ES index. We don't need `Aliases`:
- Gitea does't need aliases for "Zero Downtime" because it never delete
old indexes.
- The old code of issues indexer uses the orignal name to create issue
index, so it's tricky to convert it to an alias.
- [x] Support index version for meilisearch issues indexer, the old
index without version will be treated as version 0.
- [x] Do "ping" only when `Ping` has been called, don't ping
periodically and cache the status.
- [x] Support the context parameter whenever possible.
- [x] Fix outdated example config.
- [x] Give up the requeue logic of issues indexer: When indexing fails,
call Ping to check if it was caused by the engine being unavailable, and
only requeue the task if the engine is unavailable.
- It is fragile and tricky, could cause data losing (It did happen when
I was doing some tests for this PR). And it works for ES only.
- Just always requeue the failed task, if it caused by bad data, it's a
bug of Gitea which should be fixed.
---------
Co-authored-by: Giteabot <teabot@gitea.io>
The current command doesn't work on Fedora/RHEL/CentOS.
- `--gecos`: use `--comment` instead.
- `--group`: use `--gid` instead, `--group` means "A list of
supplementary groups" on F/R/C.
- `--disabled-password`: leave `--password` empty instead, "The default
is to disable the password".
- `--home`: use `--home-dir` and `--create-home` instead.
Replace #22117. Implement it in a more maintainable way.
Some licenses have placeholders e.g. the BSD licenses start with this
line:
```
Copyright (c) <year> <owner>.
```
This PR replaces the placeholders with the correct value when initialize
a new repo.
### FAQ
- Why not use a regex?
It will be a pretty complicated regex which could be hard to maintain.
- There're still missing placeholders.
There are over 500 licenses, it's impossible for anyone to inspect all
of them alone. Please help to add them if you find any, and it is also
OK to leave them for the future.
---------
Co-authored-by: Giteabot <teabot@gitea.io>
There is no fork concept in agit flow, anyone with read permission can
push `refs/for/<target-branch>/<topic-branch>` to the repo. So we should
treat it as a fork pull request because it may be from an untrusted
user.
The old code just parses an invalid key to `TypeInvalid` and uses it as
normal, and duplicate keys will be kept.
So this PR will ignore invalid key and log warning and also deduplicate
valid units.
I neglected that the `NameKey` of `Unit` is not only for translation,
but also configuration. So it should be `repo.actions` to maintain
consistency.
## ⚠️ BREAKING ⚠️
If users already use `actions.actions` in `DISABLED_REPO_UNITS` or
`DEFAULT_REPO_UNITS`, it will be treated as an invalid unit key.
Update replace:
```diff
- replace github.com/nektos/act => gitea.com/gitea/act v0.234.2-0.20230131074955-e46ede1b1744
+ replace github.com/nektos/act => gitea.com/gitea/act v0.243.1
```
Update require:
```diff
- github.com/nektos/act v0.0.0
+ github.com/nektos/act v0.2.43
```
Actually, `v0.2.43` doesn't work, it will be replaced by `gitea/act`, so
it's OK to put any version here. But `gitea/act` is based on
`nektos/act`, so keeping the right upstream version will make security
dependabot help.
BTW, the [security
report](https://github.com/go-gitea/gitea/security/dependabot/20) is
false positive, we don't use the artifact server in act, see #22738.
The Gitea Community Code of Conduct.
Authored by lunny. Slightly modified from [Go
CoC](https://go.dev/conduct)
---
CC all TOCs:
@go-gitea/technical-oversight-committee
And we welcome suggestions from everyone.
---------
Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: Nick <nick@kousu.ca>
Co-authored-by: Lauris BH <lauris@nix.lv>
Related to: #22294#23186#23054
Replace: #23218
Some discussion is in the comments of #23218.
Highlights:
- Add Expiration for cache context. If a cache context has been used for
more than 10s, the cache data will be ignored, and warning logs will be
printed.
- Add `discard` field to `cacheContext`, a `cacheContext` with `discard`
true will drop all cached data and won't store any new one.
- Introduce `WithNoCacheContext`, if one wants to run long-life tasks,
but the parent context is a cache context,
`WithNoCacheContext(perentCtx)` will discard the cache data, so it will
be safe to keep the context for a long time.
- It will be fine to treat an original context as a cache context, like
`GetContextData(context.Backgraud())`, no warning logs will be printed.
Some cases about nesting:
When:
- *A*, *B* or *C* means a cache context.
- ~*A*~, ~*B*~ or ~*C*~ means a discard cache context.
- `ctx` means `context.Backgrand()`
- *A(ctx)* means a cache context with `ctx` as the parent context.
- *B(A(ctx))* means a cache context with `A(ctx)` as the parent context.
- `With` means `WithCacheContext`
- `WithNo` means `WithNoCacheContext`
So:
- `With(ctx)` -> *A(ctx)*
- `With(With(ctx))` -> *A(ctx)*, not *B(A(ctx))*
- `With(With(With(ctx)))` -> *A(ctx)*, not *C(B(A(ctx)))*
- `WithNo(ctx)` -> *ctx*, not *~A~(ctx)*
- `WithNo(With(ctx))` -> *~A~(ctx)*
- `WithNo(WithNo(With(ctx)))` -> *~A~(ctx)*, not *~B~(~A~(ctx))*
- `With(WithNo(With(ctx)))` -> *B(~A~(ctx))*
- `WithNo(With(WithNo(With(ctx))))` -> *~B~(~A~(ctx))*
- `With(WithNo(With(WithNo(With(ctx)))))` -> *C(~B~(~A~(ctx)))*
Jason SongandGitHub
