Fix Actions being enabled accidentally

[wolfogre]

Regression of #24536. If the user doesn't explicitly disable Actions, it will be enabled.

1. Gitea will call loadRepositoryFrom before loadActionsFrom.
   

   gitea/modules/setting/setting.go

   Lines 234 to 237 in 25d4f95

   	loadRepositoryFrom(cfg)
   	loadPictureFrom(cfg)
   	loadPackagesFrom(cfg)
   	loadActionsFrom(cfg)

2. In loadRepositoryFrom, rootCfg.Section("actions").Key("ENABLED").MustBool(true) will set actions.ENABLED with true.
   

   gitea/modules/setting/repository.go

   Lines 313 to 315 in 25d4f95

   	if !rootCfg.Section("actions").Key("ENABLED").MustBool(true) {
   	Repository.DisabledRepoUnits = append(Repository.DisabledRepoUnits, "repo.actions")
   	}

3. In loadActionsFrom, rootCfg.Section("actions") will get a section with Actions enabled.
   

   gitea/modules/setting/actions.go

   Lines 23 to 26 in 25d4f95

   	sec := rootCfg.Section("actions")
   	if err := sec.MapTo(&Actions); err != nil {
   	log.Fatal("Failed to map Actions settings: %v", err)
   	}

Although the cause of the problem was using true by copy-paste mistake, it also surprised me that rootCfg.Section("actions").Key("ENABLED").MustBool(true) doesn't only read, but also write.

[silverwind]

I'm surprised there was no such line before #24536, so action.ENABLED previously defaulted to nil?

[wolfogre]

I'm surprised there was no such line before #24536, so action.ENABLED previously defaulted to nil?

Uh ... yes:

• actions.ENABLED defaulted to nil
• But Actions.Enabled defaulted to false

(Not sure if it's the right way)

gitea/modules/setting/actions.go

Lines 12 to 19 in c53ad05

	Actions = struct {
	Storage // how the created logs should be stored
	Enabled bool
	DefaultActionsURL string `ini:"DEFAULT_ACTIONS_URL"`
	}{
	Enabled: false,
	DefaultActionsURL: "https://gitea.com",
	}

[silverwind]

I see. I think this config system is in for an rewrite anyways, I don't like that there are two places to define a default, and the casing difference is also confusing and unnecessary.

I'd like us to use https://github.com/kelseyhightower/envconfig and map back the env vars to the config struct, but it will be a massive rewrite.

[lunny]

I see. I think this config system is in for an rewrite anyways, I don't like that there are two places to define a default, and the casing difference is also confusing and unnecessary.

I'd like us to use https://github.com/kelseyhightower/envconfig and map back the env vars to the config struct, but it will be a massive rewrite.

I think @silverwind is right. Maybe we should use the variable instead of the false here. @wolfogre

[silverwind]

See #24804 for some ideas regarding config system.

[wolfogre]

I see. I think this config system is in for an rewrite anyways, I don't like that there are two places to define a default, and the casing difference is also confusing and unnecessary.
I'd like us to use kelseyhightower/envconfig and map back the env vars to the config struct, but it will be a massive rewrite.

I think @silverwind is right. Maybe we should use the variable instead of the false here. @wolfogre

Done: 845f86e