Handle config path args in server startup
Updated server launch behavior to reliably treat JSON file arguments as config paths, including file-manager “Open with” cases. The CLI `serve` command now accepts an optional positional config path (equivalent to `--config`) and errors on conflicting values. `start.sh` and `start.bat` now parse config-related arguments more explicitly, validate required values, and always pass `--config` to avoid accidental positional forwarding. Added tests covering both positional and `--config` forms, plus README documentation for the new startup behavior.
This commit is contained in:
@@ -49,6 +49,8 @@ Update dependencies explicitly when needed:
|
||||
|
||||
`--update-dependencies` is consumed by the start script and is **not** forwarded to the server.
|
||||
|
||||
If a file manager “Open with” passes `commonwealth-server.json` as an argument, `start.sh` treats that path as the config file and still launches with `--config` (it is not forwarded as a bare positional argument).
|
||||
|
||||
---
|
||||
|
||||
## Linux Installation
|
||||
|
||||
@@ -272,6 +272,10 @@ def _print_help_table(*, interactive: bool = False) -> None:
|
||||
|
||||
@app.command()
|
||||
def serve(
|
||||
config_path: Optional[str] = typer.Argument(
|
||||
None,
|
||||
help="Optional path to config.json (same as --config)",
|
||||
),
|
||||
config: Optional[str] = typer.Option(
|
||||
None,
|
||||
"--config",
|
||||
@@ -302,6 +306,14 @@ def serve(
|
||||
service: Optional[ServerService] = None
|
||||
_shutdown_requested.clear()
|
||||
|
||||
# Accept either `serve --config FILE` or `serve FILE` (file managers may pass FILE).
|
||||
if config and config_path and Path(config).resolve() != Path(config_path).resolve():
|
||||
console.print(
|
||||
"[red]Error: Conflicting config paths from positional argument and --config.[/red]"
|
||||
)
|
||||
raise typer.Exit(code=1)
|
||||
config = config or config_path
|
||||
|
||||
previous_sigint = signal.getsignal(signal.SIGINT)
|
||||
previous_sigterm = signal.getsignal(signal.SIGTERM) if hasattr(signal, "SIGTERM") else None
|
||||
try:
|
||||
|
||||
+35
-5
@@ -24,14 +24,44 @@ if not exist "requirements-server.txt" (
|
||||
)
|
||||
|
||||
set "UPDATE_DEPENDENCIES=0"
|
||||
set "CONFIG_FILE=%cd%\commonwealth-server.json"
|
||||
set "SERVER_ARGS="
|
||||
:parse_args
|
||||
if "%~1"=="" goto args_done
|
||||
if /I "%~1"=="--update-dependencies" (
|
||||
set "UPDATE_DEPENDENCIES=1"
|
||||
) else (
|
||||
set "SERVER_ARGS=!SERVER_ARGS! %1"
|
||||
shift
|
||||
goto parse_args
|
||||
)
|
||||
if /I "%~1"=="--config" (
|
||||
if "%~2"=="" (
|
||||
echo ERROR: --config requires a config file path.
|
||||
pause
|
||||
exit /b 1
|
||||
)
|
||||
set "CONFIG_FILE=%~f2"
|
||||
shift
|
||||
shift
|
||||
goto parse_args
|
||||
)
|
||||
if /I "%~1"=="-c" (
|
||||
if "%~2"=="" (
|
||||
echo ERROR: -c requires a config file path.
|
||||
pause
|
||||
exit /b 1
|
||||
)
|
||||
set "CONFIG_FILE=%~f2"
|
||||
shift
|
||||
shift
|
||||
goto parse_args
|
||||
)
|
||||
echo %~1| findstr /I /R "\.json$" >nul
|
||||
if not errorlevel 1 (
|
||||
set "CONFIG_FILE=%~f1"
|
||||
shift
|
||||
goto parse_args
|
||||
)
|
||||
set "SERVER_ARGS=!SERVER_ARGS! %1"
|
||||
shift
|
||||
goto parse_args
|
||||
:args_done
|
||||
@@ -78,9 +108,9 @@ if "%NEED_INSTALL%"=="1" (
|
||||
)
|
||||
echo.
|
||||
|
||||
if not exist "commonwealth-server.json" (
|
||||
if not exist "!CONFIG_FILE!" (
|
||||
echo Generating default configuration file...
|
||||
"%VENV_PYTHON%" -u consumer_server_cli.py config init commonwealth-server.json
|
||||
"%VENV_PYTHON%" -u consumer_server_cli.py config init "!CONFIG_FILE!"
|
||||
if errorlevel 1 (
|
||||
echo ERROR: Failed to generate config file.
|
||||
pause
|
||||
@@ -93,7 +123,7 @@ echo Starting Commonwealth Online Server...
|
||||
echo Type help for commands. Type quit or press Ctrl+C to stop.
|
||||
echo.
|
||||
|
||||
"%VENV_PYTHON%" -u consumer_server_cli.py serve --config commonwealth-server.json --interactive !SERVER_ARGS!
|
||||
"%VENV_PYTHON%" -u consumer_server_cli.py serve --config "!CONFIG_FILE!" --interactive !SERVER_ARGS!
|
||||
set "EXIT_CODE=!ERRORLEVEL!"
|
||||
|
||||
if not "!EXIT_CODE!"=="0" (
|
||||
|
||||
+60
-4
@@ -18,16 +18,58 @@ ENTRY_POINT="${SERVER_DIR}/consumer_server_cli.py"
|
||||
|
||||
UPDATE_DEPENDENCIES=0
|
||||
SERVER_ARGS=()
|
||||
|
||||
for arg in "$@"; do
|
||||
ARGS=("$@")
|
||||
ARG_INDEX=0
|
||||
while [[ ${ARG_INDEX} -lt ${#ARGS[@]} ]]; do
|
||||
arg="${ARGS[${ARG_INDEX}]}"
|
||||
case "${arg}" in
|
||||
--update-dependencies)
|
||||
UPDATE_DEPENDENCIES=1
|
||||
;;
|
||||
*)
|
||||
--config|-c)
|
||||
ARG_INDEX=$((ARG_INDEX + 1))
|
||||
if [[ ${ARG_INDEX} -ge ${#ARGS[@]} ]]; then
|
||||
echo "ERROR: ${arg} requires a config file path." >&2
|
||||
exit 1
|
||||
fi
|
||||
CONFIG_FILE="${ARGS[${ARG_INDEX}]}"
|
||||
;;
|
||||
--config=*)
|
||||
CONFIG_FILE="${arg#--config=}"
|
||||
;;
|
||||
--host|--port|-H|-p|--interactive|-i)
|
||||
SERVER_ARGS+=("${arg}")
|
||||
if [[ "${arg}" == "--host" || "${arg}" == "-H" || "${arg}" == "--port" || "${arg}" == "-p" ]]; then
|
||||
ARG_INDEX=$((ARG_INDEX + 1))
|
||||
if [[ ${ARG_INDEX} -ge ${#ARGS[@]} ]]; then
|
||||
echo "ERROR: ${arg} requires a value." >&2
|
||||
exit 1
|
||||
fi
|
||||
SERVER_ARGS+=("${ARGS[${ARG_INDEX}]}")
|
||||
fi
|
||||
;;
|
||||
--host=*|--port=*)
|
||||
SERVER_ARGS+=("${arg}")
|
||||
;;
|
||||
*.json)
|
||||
# File managers / "Open with" often pass the config path as $1.
|
||||
CONFIG_FILE="${arg}"
|
||||
;;
|
||||
-*)
|
||||
echo "ERROR: Unknown option: ${arg}" >&2
|
||||
echo "Supported: --update-dependencies, --config PATH, --host HOST, --port PORT, --interactive" >&2
|
||||
exit 1
|
||||
;;
|
||||
*)
|
||||
if [[ -f "${arg}" ]]; then
|
||||
CONFIG_FILE="${arg}"
|
||||
else
|
||||
echo "ERROR: Unexpected argument: ${arg}" >&2
|
||||
exit 1
|
||||
fi
|
||||
;;
|
||||
esac
|
||||
ARG_INDEX=$((ARG_INDEX + 1))
|
||||
done
|
||||
|
||||
echo
|
||||
@@ -145,8 +187,22 @@ else
|
||||
fi
|
||||
echo
|
||||
|
||||
# Resolve config to an absolute path after cd'ing into the server directory.
|
||||
if [[ "${CONFIG_FILE}" != /* ]]; then
|
||||
CONFIG_FILE="${SERVER_DIR}/${CONFIG_FILE}"
|
||||
fi
|
||||
CONFIG_FILE="$(cd -- "$(dirname -- "${CONFIG_FILE}")" && pwd)/$(basename -- "${CONFIG_FILE}")"
|
||||
|
||||
# Do not source .venv/bin/activate — invoke the venv interpreter directly.
|
||||
CMD=("${VENV_PYTHON}" -u "${ENTRY_POINT}" serve --config "${CONFIG_FILE}")
|
||||
# Always pass --config explicitly so a bare path is never a positional serve arg.
|
||||
CMD=(
|
||||
"${VENV_PYTHON}"
|
||||
-u
|
||||
"${ENTRY_POINT}"
|
||||
serve
|
||||
--config
|
||||
"${CONFIG_FILE}"
|
||||
)
|
||||
if [[ ${#INTERACTIVE_ARGS[@]} -gt 0 ]]; then
|
||||
CMD+=("${INTERACTIVE_ARGS[@]}")
|
||||
fi
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from pathlib import Path
|
||||
|
||||
from typer.testing import CliRunner
|
||||
|
||||
import consumer_server_cli
|
||||
|
||||
|
||||
runner = CliRunner()
|
||||
|
||||
|
||||
def test_serve_accepts_positional_config_path(tmp_path: Path, monkeypatch) -> None:
|
||||
config_path = tmp_path / "commonwealth-server.json"
|
||||
config_path.write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"host": "127.0.0.1",
|
||||
"port": 1,
|
||||
"server_name": "Arg Test",
|
||||
"max_players": 2,
|
||||
"log_verbosity": "info",
|
||||
"admin_port": 2,
|
||||
}
|
||||
)
|
||||
+ "\n",
|
||||
encoding="utf-8",
|
||||
newline="\n",
|
||||
)
|
||||
|
||||
# Avoid binding real sockets; validate CLI parsing only.
|
||||
called: dict[str, object] = {}
|
||||
|
||||
class FakeService:
|
||||
def __init__(self) -> None:
|
||||
self.config = None
|
||||
|
||||
def add_log_listener(self, _callback) -> None:
|
||||
return None
|
||||
|
||||
def serve_forever(self) -> None:
|
||||
called["served"] = True
|
||||
|
||||
def stop(self) -> None:
|
||||
return None
|
||||
|
||||
monkeypatch.setattr(consumer_server_cli, "get_service", FakeService)
|
||||
monkeypatch.setattr(consumer_server_cli, "print_startup_banner", lambda _cfg: None)
|
||||
monkeypatch.setattr(consumer_server_cli, "ensure_writable_directory", lambda _path: None)
|
||||
monkeypatch.setattr(
|
||||
consumer_server_cli,
|
||||
"validate_config",
|
||||
lambda _cfg: (True, []),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
consumer_server_cli.signal,
|
||||
"signal",
|
||||
lambda *_args, **_kwargs: None,
|
||||
)
|
||||
|
||||
result = runner.invoke(
|
||||
consumer_server_cli.app,
|
||||
["serve", str(config_path)],
|
||||
)
|
||||
assert result.exit_code == 0, result.output
|
||||
assert called.get("served") is True
|
||||
|
||||
|
||||
def test_serve_accepts_config_option(tmp_path: Path, monkeypatch) -> None:
|
||||
config_path = tmp_path / "commonwealth-server.json"
|
||||
config_path.write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"host": "127.0.0.1",
|
||||
"port": 1,
|
||||
"server_name": "Arg Test",
|
||||
"max_players": 2,
|
||||
"log_verbosity": "info",
|
||||
"admin_port": 2,
|
||||
}
|
||||
)
|
||||
+ "\n",
|
||||
encoding="utf-8",
|
||||
newline="\n",
|
||||
)
|
||||
|
||||
called: dict[str, object] = {}
|
||||
|
||||
class FakeService:
|
||||
def __init__(self) -> None:
|
||||
self.config = None
|
||||
|
||||
def add_log_listener(self, _callback) -> None:
|
||||
return None
|
||||
|
||||
def serve_forever(self) -> None:
|
||||
called["served"] = True
|
||||
|
||||
def stop(self) -> None:
|
||||
return None
|
||||
|
||||
monkeypatch.setattr(consumer_server_cli, "get_service", FakeService)
|
||||
monkeypatch.setattr(consumer_server_cli, "print_startup_banner", lambda _cfg: None)
|
||||
monkeypatch.setattr(consumer_server_cli, "ensure_writable_directory", lambda _path: None)
|
||||
monkeypatch.setattr(consumer_server_cli, "validate_config", lambda _cfg: (True, []))
|
||||
monkeypatch.setattr(consumer_server_cli.signal, "signal", lambda *_args, **_kwargs: None)
|
||||
|
||||
result = runner.invoke(
|
||||
consumer_server_cli.app,
|
||||
["serve", "--config", str(config_path)],
|
||||
)
|
||||
assert result.exit_code == 0, result.output
|
||||
assert called.get("served") is True
|
||||
Reference in New Issue
Block a user