Notify about outdated settings #23203

delvh · 2023-03-01T00:24:29Z

At the moment, all notifications about setting deprecations are either maintained by hand, or are not really connected to the original version they were deprecated in anymore.
This PR cleans that up by doing the following things:

move ConfigProvider to its own package
store the history of breaking changes
- what the old value was
- what the replacement is
- where the setting originated and was moved to (i.e. ini→db)
- if I haven't missed anything, all breaking changes since the beginning of Gitea are included
- everything is checked in one method, where you can simply add new calls to it
Print any setting that is now outdated
Remove the previously scattered deprecation notices
It was developed with maintainability in mind:
- new setting sources can be added fairly easy: Simply define a setting type in modules/setting/history/setting_types.go and define a convenience method in modules/setting/history/exposed_api.go. No further changes are needed.
- new breaking configuration changes can be added by adding one call to the corresponding convenience method in modules/setting/history/breaking_changes.go

Appearance

or as text:

2023/02/28 19:42:12 ...story/exposed_api.go:96:func1() [E] [63fe4b04-14] The setting [server].LFS_CONTENT_PATH in your ini config file is no longer used starting with Gitea 1.18.0. Please use the new value [lfs].PATH in the ini config file instead.
2023/02/28 19:42:12 ...story/exposed_api.go:96:func1() [E] [63fe4b04-14] The setting [ui].ONLY_SHOW_RELEVANT_REPOS in your ini config file is no longer used starting with Gitea 1.19.0. It has no documented replacement.
2023/02/28 19:42:12 ...story/exposed_api.go:96:func1() [E] [63fe4b04-14] The setting [indexer].ISSUE_INDEXER_QUEUE_TYPE in your ini config file is no longer used starting with Gitea 1.15.0. Please use the new value [queue.issue_indexer].TYPE in the ini config file instead.
2023/02/28 19:42:12 ...story/exposed_api.go:96:func1() [E] [63fe4b04-14] The setting [indexer].ISSUE_INDEXER_QUEUE_DIR in your ini config file is no longer used starting with Gitea 1.15.0. Please use the new value [queue.issue_indexer].DATADIR in the ini config file instead.
2023/02/28 19:42:12 ...story/exposed_api.go:96:func1() [E] [63fe4b04-14] The setting [indexer].ISSUE_INDEXER_QUEUE_CONN_STR in your ini config file is no longer used starting with Gitea 1.15.0. Please use the new value [queue.issue_indexer].CONN_STR in the ini config file instead.
2023/02/28 19:42:12 ...story/exposed_api.go:96:func1() [E] [63fe4b04-14] The setting [indexer].ISSUE_INDEXER_QUEUE_BATCH_NUMBER in your ini config file is no longer used starting with Gitea 1.15.0. Please use the new value [queue.issue_indexer].BATCH_LENGTH in the ini config file instead.
2023/02/28 19:42:12 ...story/exposed_api.go:96:func1() [E] [63fe4b04-14] The setting [indexer].UPDATE_BUFFER_LEN in your ini config file is no longer used starting with Gitea 1.15.0. Please use the new value [queue.issue_indexer].LENGTH in the ini config file instead.
2023/02/28 19:42:12 ...story/exposed_api.go:96:func1() [E] [63fe4b04-14] The setting [picture].ENABLE_FEDERATED_AVATAR in your ini config file is no longer used starting with Gitea 1.18.0. Please use the key picture.ENABLE_FEDERATED_AVATAR in the database table 'system setting' instead. The current value will be/has been copied to it.
2023/02/28 19:42:12 ...story/exposed_api.go:96:func1() [E] [63fe4b04-14] The setting [picture].DISABLE_GRAVATAR in your ini config file is no longer used starting with Gitea 1.18.0. Please use the key picture.DISABLE_GRAVATAR in the database table 'system setting' instead. The current value will be/has been copied to it.

…removed-settings

Additionally, fix some unexpected bugs as `mailer` didn't set a default value for some properties by accident.

cmd/web.go

lunny · 2023-03-01T00:57:03Z

modules/setting/history/breaking_changes.go

+
+// This file is the only file that should be changed frequently in this package
+
+var currentGiteaVersion = getVersion("1.19")


Wow, I don't think it's a good idea to ask maintainers to remember this.

In my mind, maybe:

Prod mode: parse version string

Dev mode: run git to describe the base tag to get the working version?

Could you please elaborate on how I get the information on what our current version is?
If I know how to do it, I can implement it.

Released binary: there is a var containing the build version (set by Makefile from git describe)

Dev mode: git describe:

~/work/gitea$ git describe v1.20.0-dev-52-g151d3e7832

From #26094

The main problem of that PR is that I still haven't found out how I have to set LD_FLAGS so that the version will be updated automatically.

The version is currently set in main

gitea/main.go

Line 28 in 6aa30af

Version = "development" // program version for this build

gitea/Makefile

Line 108 in 6aa30af

LDFLAGS := $(LDFLAGS) -X "main.MakeVersion=$(MAKE_VERSION)" -X "main.Version=$(GITEA_VERSION)" -X "main.Tags=$(TAGS)"

To use it elsewhere you'll need to move it to a non-main package.

The problem is especially that it isn't in the main package, and my attempts to convert it to modules.setting.history.currentGiteaVersion were all fruitless endeavors thus far.
That's why this PR has moved to the back of my TODO list as I haven't found out yet what to use.

-X code.gitea.io/gitea/modules/setting/history.currentGiteaVersion=$(GITEA_VERSION) should do the trick, although I'd recommend exporting the variable so we can use the same one in both places.

lunny · 2023-03-01T00:57:49Z

modules/setting/history/breaking_changes.go

+
+// Adds all previously removed settings
+// It should declare all breaking configuration changes in chronological order to ensure a monotone increasing error log
+func init() {


So these information will be displayed before setting.Init ?

Hmm, init seems uncontrollable. Is the logging system ready at the moment?

Is the logging system ready at the moment?

Yes, that's one of the reasons it is in its own package. (The other is a clear separation of what belongs to what)
The package is only loaded when PrintRemovedSettings is called, which is exactly after the logging has been set up, and before any other processing has happened.
If we decide to drop these two trace calls, it doesn't even matter for the init method if the other package has been loaded already or not, the log is then only needed for PrintRemovedSettings.
So, what is the preferred course of action here?
Dropping the trace calls, or keeping them in and relying on the logger being initialized?
Or replacing the init with an Init we call ourselves?

So these information will be displayed before setting.Init ?

No, it is called as soon as setting.CfgProvider has been set, but before any other settings have been initialized.
Since there is no setting.Init (anymore?), that's the best course of action.

Or replacing the init with an Init we call ourselves?

I think we should make the Init called explicitly, it makes the calling order controllable.

lunny · 2023-03-01T00:59:19Z

modules/setting/history/exposed_api.go

+	"code.gitea.io/gitea/modules/setting/base"
+)
+
+var removedSettings map[SettingsSource]map[string][]*historyEntry = make(map[SettingsSource]map[string][]*historyEntry) // ordered by old source and then by old section (for performance)


Should we really need to store this in the memory, once we release Gitea v100, it will become very big. :) I think we just need to print them in console or log?

Or we can destroy it after printing.

I would have guessed that the Go GC takes care of it once PrintRemovedSettings has been called, as that package isn't referenced from anything anymore then.

Even if the Go GC fails to do its duty, I doubt we will ever reach noticeable sizes: Let's assume one entry uses about 200 Bytes (which is already an overestimation), then we still need 5 settings for 1KB, or 5 000 settings for 1MB.

But yes, we can of course clear the map afterward manually.

If a package is referenced once, it's always referenced, Go never de-references / unloads packages.

GC doesn't GC global variables.

I would have guessed that the Go GC takes care of it once PrintRemovedSettings has been called, as that package isn't referenced from anything anymore then.

Even if the Go GC fails to do its duty, I doubt we will ever reach noticeable sizes: Let's assume one entry uses about 200 Bytes (which is already an overestimation), then we still need 5 settings for 1KB, or 5 000 settings for 1MB.

But yes, we can of course clear the map afterward manually.

Of course, I know it's small but I want to save every byte when possible.

lunny · 2023-03-01T01:00:29Z

modules/setting/history/history_entry.go

@@ -0,0 +1,53 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.


The filename should not have the prefix of the package name.

wxiaoguang · 2023-03-01T07:36:16Z

If I understand correctly, this frameworks handles 1v1 mapping.

Is it possible (necessary) to have these mappings?

OLD_KEY_1 & OLD_KEY_2 => NEW_KEY_X
OLD_KEY_X => NEW_KEY_1 & NEW_KEY_2

delvh · 2023-03-01T11:01:29Z

OLD_KEY_1 & OLD_KEY_2 => NEW_KEY_X: Yes, that's absolutely possible. Simply deprecate both settings with the new replacement. (So, simply call move(old1, new) and move(old2, new))

OLD_KEY => NEW_KEY_1 & NEW_KEY_2: No, that's not possible*. That case also arose once with mailer.HOST => mailer.SMTP_ADDR+mailer.SMTP_PORT. I recommend the following procedure in this case: Call move(old, new1/new2) (whichever is more dominant), and document in the new value (i.e. config-cheat-sheet and app.example.ini) that it should be called in combination with the other parameter. I decided against supporting that case separately as that would make the logic much more complicated, and it is fairly uncommon.

* One possible workaround would be to call move(old, "$(section new1)$(key new1)+$(key new2)"), so that we would get move("mailer.HOST", "mailer.SMTP_ADDR+SMTP_PORT") in the case above.
This will work with the current implementation.
However, I decided against using it, as that would violate the assumption that the new key can be found like this in the new source, which future developers might rely on. (At the moment, this assumption is simply ignored)
I'm fine with both approaches, as long as we can agree on one.

Regarding the other comments: I'll probably be able to take care of them later today.

modules/setting/history/exposed_api.go

delvh · 2023-10-07T23:57:21Z

Closing as it is far too outdated by now.

delvh added 10 commits February 6, 2023 00:30

Add advanced mechanism to notify about removed/deprecated settings

d07784e

Rename methods, add previously removed settings, better tense detection

93bbad1

Refactor setting history to use composition instead

008f77d

Refactor mechanism completely once more

aeb965d

Extract the changes into separate files

c45ab0c

rename functions and store which types of source an event belongs to

7f5503b

Merge remote-tracking branch 'origin/main' into feature/notify-about-…

f3ddbd1

…removed-settings

Remove previous deprecatedSetting methods with the new mechanism

61e8af2

Additionally, fix some unexpected bugs as `mailer` didn't set a default value for some properties by accident.

Fix final bugs, move config provider into its own package

9f24391

Fix bug printing unset outdated settings as set and use maps as provider

b25df8b

delvh added type/enhancement An improvement of existing functionality topic/deployment outdated/backport/v1.19 This PR should be backported to Gitea 1.19 labels Mar 1, 2023

delvh added this to the 1.20.0 milestone Mar 1, 2023

delvh self-assigned this Mar 1, 2023

delvh added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Mar 1, 2023

lunny reviewed Mar 1, 2023

View reviewed changes

cmd/web.go Outdated Show resolved Hide resolved

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 1, 2023

lunny reviewed Mar 1, 2023

View reviewed changes

Run make fmt

78dc2d0

delvh mentioned this pull request Mar 2, 2023

Plans for Gitea 1.20 #23072

Closed

jolheiser reviewed Mar 3, 2023

View reviewed changes

modules/setting/history/exposed_api.go Show resolved Hide resolved

delvh removed the outdated/backport/v1.19 This PR should be backported to Gitea 1.19 label Mar 21, 2023

delvh removed this from the 1.20.0 milestone Jun 4, 2023

delvh mentioned this pull request Jun 7, 2023

Plans for Gitea 1.21 #25123

Closed

delvh mentioned this pull request Jul 24, 2023

Display deprecated warning in admin panel pages as well as in the log file #26094

Merged

delvh closed this Oct 7, 2023

delvh deleted the feature/notify-about-removed-settings branch October 7, 2023 23:57

go-gitea locked as resolved and limited conversation to collaborators Jan 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Notify about outdated settings #23203

Notify about outdated settings #23203

delvh commented Mar 1, 2023

lunny Mar 1, 2023

wxiaoguang Mar 1, 2023

delvh Mar 1, 2023

wxiaoguang Mar 2, 2023

jolheiser Jul 24, 2023

delvh Jul 24, 2023

jolheiser Jul 24, 2023

lunny Mar 1, 2023

wxiaoguang Mar 1, 2023

delvh Mar 1, 2023 •

edited

Loading

wxiaoguang Mar 2, 2023

lunny Mar 1, 2023

lunny Mar 1, 2023

delvh Mar 1, 2023

wxiaoguang Mar 2, 2023

lunny Mar 2, 2023

lunny Mar 1, 2023

wxiaoguang commented Mar 1, 2023 •

edited

Loading

delvh commented Mar 1, 2023 •

edited

Loading

delvh commented Oct 7, 2023


		// This file is the only file that should be changed frequently in this package

		var currentGiteaVersion = getVersion("1.19")

		@@ -0,0 +1,53 @@
		// Copyright 2023 The Gitea Authors. All rights reserved.

Notify about outdated settings #23203

Notify about outdated settings #23203

Conversation

delvh commented Mar 1, 2023

Appearance

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

delvh Mar 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wxiaoguang commented Mar 1, 2023 • edited Loading

delvh commented Mar 1, 2023 • edited Loading

delvh commented Oct 7, 2023

delvh Mar 1, 2023 •

edited

Loading

wxiaoguang commented Mar 1, 2023 •

edited

Loading

delvh commented Mar 1, 2023 •

edited

Loading