Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit e111761

Browse files
GiteaBotlunny
andauthored
Fix a bug where lfs gc never worked. (#35198) (#35255)
Backport #35198 by @lunny Fix #31113 After #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. Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
1 parent 4e0269e commit e111761

File tree

3 files changed

+126
-21
lines changed

3 files changed

+126
-21
lines changed

‎modules/setting/cron_test.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,3 +41,56 @@ EXTEND = true
4141
assert.Equal(t, "white rabbit", extended.Second)
4242
assert.True(t, extended.Extend)
4343
}
44+
45+
// Test_getCronSettings2 tests that getCronSettings can not handle two levels of embedding
46+
func Test_getCronSettings2(t *testing.T) {
47+
type BaseStruct struct {
48+
Enabled bool
49+
RunAtStart bool
50+
Schedule string
51+
}
52+
53+
type Extended struct {
54+
BaseStruct
55+
Extend bool
56+
}
57+
type Extended2 struct {
58+
Extended
59+
Third string
60+
}
61+
62+
iniStr := `
63+
[cron.test]
64+
ENABLED = TRUE
65+
RUN_AT_START = TRUE
66+
SCHEDULE = @every 1h
67+
EXTEND = true
68+
THIRD = white rabbit
69+
`
70+
cfg, err := NewConfigProviderFromData(iniStr)
71+
assert.NoError(t, err)
72+
73+
extended := &Extended2{
74+
Extended: Extended{
75+
BaseStruct: BaseStruct{
76+
Enabled: false,
77+
RunAtStart: false,
78+
Schedule: "@every 72h",
79+
},
80+
Extend: false,
81+
},
82+
Third: "black rabbit",
83+
}
84+
85+
_, err = getCronSettings(cfg, "test", extended)
86+
assert.NoError(t, err)
87+
88+
// This confirms the first level of embedding works
89+
assert.Equal(t, "white rabbit", extended.Third)
90+
assert.True(t, extended.Extend)
91+
92+
// This confirms 2 levels of embedding doesn't work
93+
assert.False(t, extended.Enabled)
94+
assert.False(t, extended.RunAtStart)
95+
assert.Equal(t, "@every 72h", extended.Schedule)
96+
}

‎services/cron/tasks_extended.go

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -171,34 +171,35 @@ func registerDeleteOldSystemNotices() {
171171
})
172172
}
173173

174+
type GCLFSConfig struct {
175+
BaseConfig
176+
OlderThan time.Duration
177+
LastUpdatedMoreThanAgo time.Duration
178+
NumberToCheckPerRepo int64
179+
ProportionToCheckPerRepo float64
180+
}
181+
174182
func registerGCLFS() {
175183
if !setting.LFS.StartServer {
176184
return
177185
}
178-
type GCLFSConfig struct {
179-
OlderThanConfig
180-
LastUpdatedMoreThanAgo time.Duration
181-
NumberToCheckPerRepo int64
182-
ProportionToCheckPerRepo float64
183-
}
184186

185187
RegisterTaskFatal("gc_lfs", &GCLFSConfig{
186-
OlderThanConfig: OlderThanConfig{
187-
BaseConfig: BaseConfig{
188-
Enabled: false,
189-
RunAtStart: false,
190-
Schedule: "@every 24h",
191-
},
192-
// Only attempt to garbage collect lfs meta objects older than a week as the order of git lfs upload
193-
// and git object upload is not necessarily guaranteed. It's possible to imagine a situation whereby
194-
// an LFS object is uploaded but the git branch is not uploaded immediately, or there are some rapid
195-
// changes in new branches that might lead to lfs objects becoming temporarily unassociated with git
196-
// objects.
197-
//
198-
// It is likely that a week is potentially excessive but it should definitely be enough that any
199-
// unassociated LFS object is genuinely unassociated.
200-
OlderThan: 24 * time.Hour * 7,
188+
BaseConfig: BaseConfig{
189+
Enabled: false,
190+
RunAtStart: false,
191+
Schedule: "@every 24h",
201192
},
193+
// Only attempt to garbage collect lfs meta objects older than a week as the order of git lfs upload
194+
// and git object upload is not necessarily guaranteed. It's possible to imagine a situation whereby
195+
// an LFS object is uploaded but the git branch is not uploaded immediately, or there are some rapid
196+
// changes in new branches that might lead to lfs objects becoming temporarily unassociated with git
197+
// objects.
198+
//
199+
// It is likely that a week is potentially excessive but it should definitely be enough that any
200+
// unassociated LFS object is genuinely unassociated.
201+
OlderThan: 24 * time.Hour * 7,
202+
202203
// Only GC things that haven't been looked at in the past 3 days
203204
LastUpdatedMoreThanAgo: 24 * time.Hour * 3,
204205
NumberToCheckPerRepo: 100,

‎services/cron/tasks_extended_test.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// Copyright 2025 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package cron
5+
6+
import (
7+
"testing"
8+
"time"
9+
10+
"code.gitea.io/gitea/modules/setting"
11+
"code.gitea.io/gitea/modules/test"
12+
13+
"github.com/stretchr/testify/assert"
14+
)
15+
16+
func Test_GCLFSConfig(t *testing.T) {
17+
cfg, err := setting.NewConfigProviderFromData(`
18+
[cron.gc_lfs]
19+
ENABLED = true
20+
RUN_AT_START = true
21+
SCHEDULE = "@every 2h"
22+
OLDER_THAN = "1h"
23+
LAST_UPDATED_MORE_THAN_AGO = "7h"
24+
NUMBER_TO_CHECK_PER_REPO = 10
25+
PROPORTION_TO_CHECK_PER_REPO = 0.1
26+
`)
27+
assert.NoError(t, err)
28+
defer test.MockVariableValue(&setting.CfgProvider, cfg)()
29+
30+
config := &GCLFSConfig{
31+
BaseConfig: BaseConfig{
32+
Enabled: false,
33+
RunAtStart: false,
34+
Schedule: "@every 24h",
35+
},
36+
OlderThan: 24 * time.Hour * 7,
37+
LastUpdatedMoreThanAgo: 24 * time.Hour * 3,
38+
NumberToCheckPerRepo: 100,
39+
ProportionToCheckPerRepo: 0.6,
40+
}
41+
42+
_, err = setting.GetCronSettings("gc_lfs", config)
43+
assert.NoError(t, err)
44+
assert.True(t, config.Enabled)
45+
assert.True(t, config.RunAtStart)
46+
assert.Equal(t, "@every 2h", config.Schedule)
47+
assert.Equal(t, 1*time.Hour, config.OlderThan)
48+
assert.Equal(t, 7*time.Hour, config.LastUpdatedMoreThanAgo)
49+
assert.Equal(t, int64(10), config.NumberToCheckPerRepo)
50+
assert.InDelta(t, 0.1, config.ProportionToCheckPerRepo, 0.001)
51+
}

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /