From dd12384f224e98a130511ebc9ffcfb44ead0e736 Mon Sep 17 00:00:00 2001 From: zeripath Date: Fri, 30 Oct 2020 19:26:03 +0000 Subject: [PATCH] Fix --port setting (#13288) * Fix --port setting Unfortunately there was an error in #13195 which set the --port option before the settings were read. This PR fixes this by moving applying this option to after the the settings are read However, on looking further into this code I believe that the setPort code was slightly odd. Firstly, it may make sense to run the install page on a different temporary port to the full system and this should be possible with a --install-port option. Secondy, if the --port option is provided we should apply it to both otherwise there will be unusual behaviour on graceful restart Thirdly, the documentation for --port says that the setting is temporary - it should therefore not save its result to the configuration (This however, does mean that authorized_keys and internal links may not be correct. - I think we need to discuss this option further.) Fix #13277 Signed-off-by: Andrew Thornton * Update cmd/web.go * Apply suggestions from code review Co-authored-by: techknowlogick --- cmd/web.go | 30 ++++++++++++++------ docs/content/doc/usage/command-line.en-us.md | 1 + 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/cmd/web.go b/cmd/web.go index e16d1afb534..036f1a6d663 100644 --- a/cmd/web.go +++ b/cmd/web.go @@ -41,6 +41,11 @@ and it takes care of all the other things for you`, Value: "3000", Usage: "Temporary port number to prevent conflict", }, + cli.StringFlag{ + Name: "install-port", + Value: "3000", + Usage: "Temporary port number to run the install page on to prevent conflict", + }, cli.StringFlag{ Name: "pid, P", Value: setting.PIDFile, @@ -116,16 +121,20 @@ func runWeb(ctx *cli.Context) error { setting.WritePIDFile = true } - // Flag for port number in case first time run conflict. - if ctx.IsSet("port") { - if err := setPort(ctx.String("port")); err != nil { - return err - } - } - // Perform pre-initialization needsInstall := routers.PreInstallInit(graceful.GetManager().HammerContext()) if needsInstall { + // Flag for port number in case first time run conflict + if ctx.IsSet("port") { + if err := setPort(ctx.String("port")); err != nil { + return err + } + } + if ctx.IsSet("install-port") { + if err := setPort(ctx.String("install-port")); err != nil { + return err + } + } m := routes.NewMacaron() routes.RegisterInstallRoute(m) err := listen(m, false) @@ -152,6 +161,12 @@ func runWeb(ctx *cli.Context) error { // Perform global initialization routers.GlobalInit(graceful.GetManager().HammerContext()) + // Override the provided port number within the configuration + if ctx.IsSet("port") { + if err := setPort(ctx.String("port")); err != nil { + return err + } + } // Set up Macaron m := routes.NewMacaron() routes.RegisterRoutes(m) @@ -190,7 +205,6 @@ func setPort(port string) error { defaultLocalURL += ":" + setting.HTTPPort + "/" cfg.Section("server").Key("LOCAL_ROOT_URL").SetValue(defaultLocalURL) - if err := cfg.SaveTo(setting.CustomConf); err != nil { return fmt.Errorf("Error saving generated JWT Secret to custom config: %v", err) } diff --git a/docs/content/doc/usage/command-line.en-us.md b/docs/content/doc/usage/command-line.en-us.md index 6b78e8c025d..abb6b39fa04 100644 --- a/docs/content/doc/usage/command-line.en-us.md +++ b/docs/content/doc/usage/command-line.en-us.md @@ -40,6 +40,7 @@ Starts the server: - Options: - `--port number`, `-p number`: Port number. Optional. (default: 3000). Overrides configuration file. + - `--install-port number`: Port number to run the install page on. Optional. (default: 3000). Overrides configuration file. - `--pid path`, `-P path`: Pidfile path. Optional. - Examples: - `gitea web`