Skip to content

Commit dd12384

Browse files
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 <[email protected]> * Update cmd/web.go * Apply suggestions from code review Co-authored-by: techknowlogick <[email protected]>
1 parent 20a5eff commit dd12384

File tree

2 files changed

+23
-8
lines changed

2 files changed

+23
-8
lines changed

cmd/web.go

+22-8
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ and it takes care of all the other things for you`,
4141
Value: "3000",
4242
Usage: "Temporary port number to prevent conflict",
4343
},
44+
cli.StringFlag{
45+
Name: "install-port",
46+
Value: "3000",
47+
Usage: "Temporary port number to run the install page on to prevent conflict",
48+
},
4449
cli.StringFlag{
4550
Name: "pid, P",
4651
Value: setting.PIDFile,
@@ -116,16 +121,20 @@ func runWeb(ctx *cli.Context) error {
116121
setting.WritePIDFile = true
117122
}
118123

119-
// Flag for port number in case first time run conflict.
120-
if ctx.IsSet("port") {
121-
if err := setPort(ctx.String("port")); err != nil {
122-
return err
123-
}
124-
}
125-
126124
// Perform pre-initialization
127125
needsInstall := routers.PreInstallInit(graceful.GetManager().HammerContext())
128126
if needsInstall {
127+
// Flag for port number in case first time run conflict
128+
if ctx.IsSet("port") {
129+
if err := setPort(ctx.String("port")); err != nil {
130+
return err
131+
}
132+
}
133+
if ctx.IsSet("install-port") {
134+
if err := setPort(ctx.String("install-port")); err != nil {
135+
return err
136+
}
137+
}
129138
m := routes.NewMacaron()
130139
routes.RegisterInstallRoute(m)
131140
err := listen(m, false)
@@ -152,6 +161,12 @@ func runWeb(ctx *cli.Context) error {
152161
// Perform global initialization
153162
routers.GlobalInit(graceful.GetManager().HammerContext())
154163

164+
// Override the provided port number within the configuration
165+
if ctx.IsSet("port") {
166+
if err := setPort(ctx.String("port")); err != nil {
167+
return err
168+
}
169+
}
155170
// Set up Macaron
156171
m := routes.NewMacaron()
157172
routes.RegisterRoutes(m)
@@ -190,7 +205,6 @@ func setPort(port string) error {
190205
defaultLocalURL += ":" + setting.HTTPPort + "/"
191206

192207
cfg.Section("server").Key("LOCAL_ROOT_URL").SetValue(defaultLocalURL)
193-
194208
if err := cfg.SaveTo(setting.CustomConf); err != nil {
195209
return fmt.Errorf("Error saving generated JWT Secret to custom config: %v", err)
196210
}

docs/content/doc/usage/command-line.en-us.md

+1
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ Starts the server:
4040

4141
- Options:
4242
- `--port number`, `-p number`: Port number. Optional. (default: 3000). Overrides configuration file.
43+
- `--install-port number`: Port number to run the install page on. Optional. (default: 3000). Overrides configuration file.
4344
- `--pid path`, `-P path`: Pidfile path. Optional.
4445
- Examples:
4546
- `gitea web`

0 commit comments

Comments
 (0)