mirrors/forgejo
1
0
Fork 0
mirror of https://codeberg.org/forgejo/forgejo.git synced 2025-09-19 17:25:55 +00:00
Code Issues Projects Releases Packages Wiki Activity

fix: LFS GC is never running because of a bug in the parsing of the INI file (#9202)

Browse source 
After https://github.com/go-gitea/gitea/pull/22385 introduced LFS GC, it never worked due to a bug in the INI library: fields in structs embedded more than one level deep are not populated from the INI file.

This PR fixes the issue by replacing the multi-level embedded struct with a single-level struct for parsing the cron.gc_lfs configuration.

Added a new test for retrieving cron settings to demonstrate the bug in the INI package.

---

Fix #9048 by cherrypicking the fix from Gitea

Gitea PR: https://github.com/go-gitea/gitea/pull/35198

Confirmed to work on my own instance, I now see the cron schedule for gc_lfs listed in the site admin menu where it was empty before

<!--start release-notes-assistant-->

## Release notes
<!--URL:https://codeberg.org/forgejo/forgejo-->
- Bug fixes
  - [PR](https://codeberg.org/forgejo/forgejo/pulls/9202): <!--number 9202 --><!--line 0 --><!--description TEZTIEdDIGlzIG5ldmVyIHJ1bm5pbmcgYmVjYXVzZSBvZiBhIGJ1ZyBpbiB0aGUgcGFyc2luZyBvZiB0aGUgSU5JIGZpbGU=-->LFS GC is never running because of a bug in the parsing of the INI file<!--description-->
<!--end release-notes-assistant-->

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9202
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Andrew Cassidy <drewcassidy@me.com>
Co-committed-by: Andrew Cassidy <drewcassidy@me.com>
This commit is contained in:
Andrew Cassidy 2025-09-09 22:32:49 +02:00 • committed by Earl Warren
commit 252efbda5c
3 changed files with 127 additions and 21 deletions
Download patch file Download diff file Expand all files Collapse all files

53
modules/setting/cron_test.go
View file

@ -42,3 +42,56 @@ EXTEND = true
assert.Equal(t, "white rabbit", extended.Second)
assert.True(t, extended.Extend)
}
// Test_getCronSettings2 tests that getCronSettings can not handle two levels of embedding
func Test_getCronSettings2(t *testing.T) {
type BaseStruct struct {
Enabled bool
RunAtStart bool
Schedule string
}
type Extended struct {
BaseStruct
Extend bool
}
type Extended2 struct {
Extended
Third string
}
iniStr := `
[cron.test]
ENABLED = TRUE
RUN_AT_START = TRUE
SCHEDULE = @every 1h
EXTEND = true
THIRD = white rabbit
`
cfg, err := NewConfigProviderFromData(iniStr)
require.NoError(t, err)
extended := &Extended2{
Extended: Extended{
BaseStruct: BaseStruct{
Enabled: false,
RunAtStart: false,
Schedule: "@every 72h",
},
Extend: false,
},
Third: "black rabbit",
}
_, err = getCronSettings(cfg, "test", extended)
require.NoError(t, err)
// This confirms the first level of embedding works
assert.Equal(t, "white rabbit", extended.Third)
assert.True(t, extended.Extend)
// This confirms 2 levels of embedding doesn't work
assert.False(t, extended.Enabled)
assert.False(t, extended.RunAtStart)
assert.Equal(t, "@every 72h", extended.Schedule)
}

43
services/cron/tasks_extended.go
View file

@ -174,34 +174,35 @@ func registerDeleteOldSystemNotices() {
})
}
type GCLFSConfig struct {
BaseConfig
OlderThan time.Duration
LastUpdatedMoreThanAgo time.Duration
NumberToCheckPerRepo int64
ProportionToCheckPerRepo float64
}
func registerGCLFS() {
if !setting.LFS.StartServer {
return
}
type GCLFSConfig struct {
OlderThanConfig
LastUpdatedMoreThanAgo time.Duration
NumberToCheckPerRepo int64
ProportionToCheckPerRepo float64
}
RegisterTaskFatal("gc_lfs", &GCLFSConfig{
OlderThanConfig: OlderThanConfig{
BaseConfig: BaseConfig{
Enabled: false,
RunAtStart: false,
Schedule: "@every 24h",
},
// Only attempt to garbage collect lfs meta objects older than a week as the order of git lfs upload
// and git object upload is not necessarily guaranteed. It's possible to imagine a situation whereby
// an LFS object is uploaded but the git branch is not uploaded immediately, or there are some rapid
// changes in new branches that might lead to lfs objects becoming temporarily unassociated with git
// objects.
//
// It is likely that a week is potentially excessive but it should definitely be enough that any
// unassociated LFS object is genuinely unassociated.
OlderThan: 24 * time.Hour * 7,
BaseConfig: BaseConfig{
Enabled: false,
RunAtStart: false,
Schedule: "@every 24h",
},
// Only attempt to garbage collect lfs meta objects older than a week as the order of git lfs upload
// and git object upload is not necessarily guaranteed. It's possible to imagine a situation whereby
// an LFS object is uploaded but the git branch is not uploaded immediately, or there are some rapid
// changes in new branches that might lead to lfs objects becoming temporarily unassociated with git
// objects.
//
// It is likely that a week is potentially excessive but it should definitely be enough that any
// unassociated LFS object is genuinely unassociated.
OlderThan: 24 * time.Hour * 7,
// Only GC things that haven't been looked at in the past 3 days
LastUpdatedMoreThanAgo: 24 * time.Hour * 3,
NumberToCheckPerRepo: 100,

52
services/cron/tasks_extended_test.go Normal file
View file

@ -0,0 +1,52 @@
// Copyright 2025 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package cron
import (
"testing"
"time"
"forgejo.org/modules/setting"
"forgejo.org/modules/test"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func Test_GCLFSConfig(t *testing.T) {
cfg, err := setting.NewConfigProviderFromData(`
[cron.gc_lfs]
ENABLED = true
RUN_AT_START = true
SCHEDULE = "@every 2h"
OLDER_THAN = "1h"
LAST_UPDATED_MORE_THAN_AGO = "7h"
NUMBER_TO_CHECK_PER_REPO = 10
PROPORTION_TO_CHECK_PER_REPO = 0.1
`)
require.NoError(t, err)
defer test.MockVariableValue(&setting.CfgProvider, cfg)()
config := &GCLFSConfig{
BaseConfig: BaseConfig{
Enabled: false,
RunAtStart: false,
Schedule: "@every 24h",
},
OlderThan: 24 * time.Hour * 7,
LastUpdatedMoreThanAgo: 24 * time.Hour * 3,
NumberToCheckPerRepo: 100,
ProportionToCheckPerRepo: 0.6,
}
_, err = setting.GetCronSettings("gc_lfs", config)
require.NoError(t, err)
assert.True(t, config.Enabled)
assert.True(t, config.RunAtStart)
assert.Equal(t, "@every 2h", config.Schedule)
assert.Equal(t, 1*time.Hour, config.OlderThan)
assert.Equal(t, 7*time.Hour, config.LastUpdatedMoreThanAgo)
assert.Equal(t, int64(10), config.NumberToCheckPerRepo)
assert.InDelta(t, 0.1, config.ProportionToCheckPerRepo, 0.001)
}