From 192faa6f812ab9afe9ae866cdadf12310cd108b8 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 1 Aug 2025 15:34:35 -0700 Subject: [PATCH 1/3] Fix a bug where lfs gc never worked. --- modules/setting/cron_test.go | 53 +++++++++++++++++++++++++++++++++ services/cron/tasks_extended.go | 32 ++++++++++---------- 2 files changed, 69 insertions(+), 16 deletions(-) diff --git a/modules/setting/cron_test.go b/modules/setting/cron_test.go index 39a228068a042..53996b5de9b0f 100644 --- a/modules/setting/cron_test.go +++ b/modules/setting/cron_test.go @@ -41,3 +41,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) + assert.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) + assert.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) +} diff --git a/services/cron/tasks_extended.go b/services/cron/tasks_extended.go index 0018c5facc5d7..843bf4b5396da 100644 --- a/services/cron/tasks_extended.go +++ b/services/cron/tasks_extended.go @@ -176,29 +176,29 @@ func registerGCLFS() { return } type GCLFSConfig struct { - OlderThanConfig + BaseConfig + OlderThan time.Duration 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, From 17e00d68044cf79ea45994984b822ecb5bba0b7b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 1 Aug 2025 21:36:15 -0700 Subject: [PATCH 2/3] Add test for getting GC LFS Config --- services/cron/tasks_extended.go | 15 ++++---- services/cron/tasks_extended_test.go | 51 ++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 7 deletions(-) create mode 100644 services/cron/tasks_extended_test.go diff --git a/services/cron/tasks_extended.go b/services/cron/tasks_extended.go index 843bf4b5396da..37471119842ec 100644 --- a/services/cron/tasks_extended.go +++ b/services/cron/tasks_extended.go @@ -171,17 +171,18 @@ 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 { - BaseConfig - OlderThan time.Duration - LastUpdatedMoreThanAgo time.Duration - NumberToCheckPerRepo int64 - ProportionToCheckPerRepo float64 - } RegisterTaskFatal("gc_lfs", &GCLFSConfig{ BaseConfig: BaseConfig{ diff --git a/services/cron/tasks_extended_test.go b/services/cron/tasks_extended_test.go new file mode 100644 index 0000000000000..52ca43f187401 --- /dev/null +++ b/services/cron/tasks_extended_test.go @@ -0,0 +1,51 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package cron + +import ( + "testing" + "time" + + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/test" + + "github.com/stretchr/testify/assert" +) + +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 +`) + assert.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) + assert.NoError(t, err) + assert.Equal(t, true, config.Enabled) + assert.Equal(t, true, 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.Equal(t, 0.1, config.ProportionToCheckPerRepo) +} From 3655f8225ee5fd6a16c6ee43654b750ebf1ba5c6 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 1 Aug 2025 21:56:11 -0700 Subject: [PATCH 3/3] Fix lint --- services/cron/tasks_extended_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/cron/tasks_extended_test.go b/services/cron/tasks_extended_test.go index 52ca43f187401..a3d40630a3c89 100644 --- a/services/cron/tasks_extended_test.go +++ b/services/cron/tasks_extended_test.go @@ -41,11 +41,11 @@ PROPORTION_TO_CHECK_PER_REPO = 0.1 _, err = setting.GetCronSettings("gc_lfs", config) assert.NoError(t, err) - assert.Equal(t, true, config.Enabled) - assert.Equal(t, true, config.RunAtStart) + 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.Equal(t, 0.1, config.ProportionToCheckPerRepo) + assert.InDelta(t, 0.1, config.ProportionToCheckPerRepo, 0.001) }