Skip to content

Platform scripts audit #567

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mcottontensor merged 8 commits into from
Apr 8, 2025
Merged

Platform scripts audit #567

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
820ed74
Limited the bash script to build cjs
mcottontensor Apr 3, 2025
8ead9b3
Added a cjs build target
mcottontensor Apr 3, 2025
63c8326
Updating bash script to be a little more friendly with builds.
mcottontensor Apr 3, 2025
78953fa
Added rebuild options to bash script
mcottontensor Apr 3, 2025
a180038
Updating windows batch script to be inline with bash script.
mcottontensor Apr 3, 2025
9efc923
Cleaned up arguments further for the bash script
mcottontensor Apr 3, 2025
64df9ca
Tweaking build behaviour. Updating args
mcottontensor Apr 3, 2025
1bb21da
Sorting out webpack configs
mcottontensor Apr 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Frontend/implementations/typescript/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"build": "npx webpack --config webpack.prod.js",
"build:dev": "npx webpack --config webpack.dev.js",
"build:esm": "npx webpack --config webpack.esmodule.js",
"build:cjs": "npx webpack --config webpack.common.js",
Copy link
Copy Markdown
Contributor

@lukehb lukehb Apr 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This a funny one actually on closer inspection.

We have the following webpack build configs for the reference frontend:

webpack.common.js - This is the base config used by both webpack.dev.js and webpack.prod.js - but perhaps confusingly named is not actually configuring webpack to build commonjs, but rather umd (which is correct considering you can't run cjs in the browser and webpack transforms the cjs deps into browser compatible umd for us). One solution to avoid this confusion is to rename this file webpack.base.js. Of additional note, because the webpack.common.js configuration does not specify a mode (development or production) webpack emits some warnings saying falling back to development etc, so this config should probably never be called directly.

webpack.dev.js - The dev configuration of the umd build (with inline source maps). This is probably the one we want the start.bat script to use.

webpack.prod.js - The prod configuration of the umd build that does minification etc so you end up with a smaller bundle.

webpack.esmodule.js - A completely different configuration that bundles into an esm module. Which could also be interesting for some usecases (like if someone is an all ESM codebase), but probably umd is the one we and most users will want to use.

Suggested change
"build:cjs": "npx webpack --config webpack.common.js",
"build:prod": "npx webpack --config webpack.prod.js",

"rebuild": "npm run clean && npm run build",
"watch": "webpack --watch --config webpack.dev.js",
"serve": "webpack serve --config webpack.dev.js",
Expand Down
116 changes: 61 additions & 55 deletions SignallingWebServer/platform_scripts/bash/common.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,18 @@ function print_usage() {
--publicip Define public ip address (using default port) for turn server, syntax: --publicip ; it is used for
Default value: Retrieved from 'curl https://api.ipify.org' or if unsuccessful then set to 127.0.0.1. It is the IP address of the server and the default IP address of the TURN server
--turn TURN server to be used, syntax: --turn 127.0.0.1:19303
Default value: as above, IP address downloaded from https://api.ipify.org; in case if download failure it is set to 127.0.0.1
Default value: Retrieved from 'curl https://api.ipify.org' or if unsuccessful then set to 127.0.0.1.
--turn-user Sets the turn username when using a turn server.
--turn-pass Sets the turn password when using a turn server.
--start-turn Will launch the turnserver process.
--stun STUN server to be used, syntax: --stun stun.l.google.com:19302
Default value as above
--build Force a rebuild of the typescript frontend even if it already exists
Default value: stun.l.google.com:19302
--frontend-dir Sets the output path for the fontend build
--dev Dev mode. (forces build of wilbur and its dependencies)
--build Force a rebuild of the typescript frontend even if it already exists
--rebuild Force a rebuild of everything
--build-libraries Force a rebuild of shared libraries
--build-wilbur Force build of wilbur
--deps Force reinstall of dependencies

Other options: stored and passed to the server. All parameters printed once the script values are set.
Command line options might be omitted to run with defaults and it is a good practice to omit specific ones when just starting the TURN or the STUN server alone, not the whole set of scripts.
Expand All @@ -42,29 +45,27 @@ function parse_args() {
IS_DEBUG=0
NO_SUDO=0
VERBOSE=0
FORCE_BUILD=0
DEFAULT_STUN=0
DEFAULT_TURN=0
BUILD_LIBRARIES=0
BUILD_FRONTEND=0
BUILD_WILBUR=0
if [[ ! -d "${SCRIPT_DIR}/../../dist/" ]]; then
BUILD_WILBUR=1
fi
INSTALL_DEPS=0
while(($#)) ; do
case "$1" in
--debug ) IS_DEBUG=1; shift;;
--nosudo ) NO_SUDO=1; shift;;
--verbose ) VERBOSE=1; shift;;
--build ) FORCE_BUILD=1; shift;;
--default-stun ) DEFAULT_STUN=1; shift;;
--default-turn ) DEFAULT_TURN=1; shift;;
--build ) BUILD_FRONTEND=1; shift;;
--rebuild ) BUILD_LIBRARIES=1; BUILD_FRONTEND=1; BUILD_WILBUR=1; shift;;
--stun ) STUN_SERVER="$2"; shift 2;;
--turn ) TURN_SERVER="$2"; shift 2;;
--turn-user ) TURN_USER="$2"; shift 2;;
--turn-pass ) TURN_PASS="$2"; shift 2;;
--start-turn ) START_TURN=1; shift;;
--publicip ) PUBLIC_IP="$2"; shift 2;;
--frontend-dir ) FRONTEND_DIR="$(realpath "$2")"; shift 2;;
--dev ) BUILD_WILBUR=1; shift;;
--build-wilbur ) BUILD_WILBUR=1; shift;;
--build-libraries ) BUILD_LIBRARIES=1; shift;;
--deps ) INSTALL_DEPS=1; shift;;
--help ) print_usage;;
* ) SERVER_ARGS+=" $1"; shift;;
esac
Expand Down Expand Up @@ -130,6 +131,7 @@ function check_and_install() { #dep_name #get_version_string #version_min #insta
if [ "$?" -lt 2 ]; then
echo "$1 is installed."
is_installed=1
return 0
else
echo "Required install of $1 not found installing"
fi
Expand All @@ -142,8 +144,11 @@ function check_and_install() { #dep_name #get_version_string #version_min #insta

if [ $? -ge 1 ]; then
echo "Installation of $1 failed try running 'export VERBOSE=1' then run this script again for more details"
return -1
fi
fi

return 1
}

function setup_node() {
Expand All @@ -161,7 +166,7 @@ function setup_node() {
popd > /dev/null

# navigate to project root
pushd "${SCRIPT_DIR}/../.." > /dev/null
pushd "${SCRIPT_DIR}/../../.." > /dev/null

node_version=""
if [[ -f "${SCRIPT_DIR}/node/bin/node" ]]; then
Expand All @@ -187,11 +192,35 @@ function setup_node() {
&& rm node.tar.xz
&& mv node-v*-*-* \"${SCRIPT_DIR}/node\""

PATH="${SCRIPT_DIR}/node/bin:$PATH"
if [ $? -eq 1 ] || [ "$INSTALL_DEPS" == "1" ]; then
echo "Installing dependencies..."
PATH="${SCRIPT_DIR}/node/bin:$PATH"
"${NPM}" install
fi

popd > /dev/null
}

function setup_libraries() {
set -e

if [ ! -d "${SCRIPT_DIR}/../../../Common/dist/" ] || [ "$BUILD_LIBRARIES" == "1" ]; then
pushd "${SCRIPT_DIR}/../../../Common" > /dev/null
echo "Building common library."
"${NPM}" run build:cjs
popd > /dev/null
fi

if [ ! -d "${SCRIPT_DIR}/../../../Signalling/dist/" ] || [ "$BUILD_LIBRARIES" == "1" ]; then
pushd "${SCRIPT_DIR}/../../../Signalling" > /dev/null
echo "Building signalling library."
"${NPM}" run build:cjs
popd > /dev/null
fi

set +e
}

function setup_frontend() {
set -e

Expand All @@ -204,35 +233,25 @@ function setup_frontend() {

# navigate to root
pushd "${SCRIPT_DIR}/../../.." > /dev/null
OLDPATH=$PATH
export PATH="${SCRIPT_DIR}/node/bin:$PATH"

# If player.html doesn't exist, or --build passed as arg, rebuild the frontend
echo Testing ${WEBPACK_OUTPUT_PATH}/player.html
if [ ! -f "${WEBPACK_OUTPUT_PATH}/player.html" ] || [ "$FORCE_BUILD" == "1" ] ; then
echo Testing ${WEBPACK_OUTPUT_PATH}
if [ ! -d "${WEBPACK_OUTPUT_PATH}" ] || [ "$BUILD_FRONTEND" == "1" ] ; then
echo "Building Typescript Frontend."
# Using our bundled NodeJS, build the web frontend files
pushd "${SCRIPT_DIR}/../../../Common" > /dev/null
"${SCRIPT_DIR}/node/bin/npm" install
"${SCRIPT_DIR}/node/bin/npm" run build
popd > /dev/null
pushd "${SCRIPT_DIR}/../../../Frontend/library" > /dev/null
"${SCRIPT_DIR}/node/bin/npm" install
"${SCRIPT_DIR}/node/bin/npm" run build
"${NPM}" run build:cjs
popd > /dev/null
pushd "${SCRIPT_DIR}/../../../Frontend/ui-library" > /dev/null
"${SCRIPT_DIR}/node/bin/npm" install
"${SCRIPT_DIR}/node/bin/npm" run build
"${NPM}" run build:cjs
popd > /dev/null
pushd "${SCRIPT_DIR}/../../../Frontend/implementations/typescript" > /dev/null
"${SCRIPT_DIR}/node/bin/npm" install
"${SCRIPT_DIR}/node/bin/npm" run build
"${NPM}" run build:cjs
Copy link
Copy Markdown
Contributor

@lukehb lukehb Apr 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"${NPM}" run build:cjs
"${NPM}" run build:dev

popd > /dev/null
else
echo 'Skipping building Frontend because files already exist. Please run with "--build" to force a rebuild'
fi

export PATH=$OLDPATH
popd > /dev/null # root
set +e
}
Expand Down Expand Up @@ -278,6 +297,7 @@ function setup_coturn() {
function setup() {
echo "Checking Pixel Streaming Server dependencies."
setup_node
setup_libraries
setup_frontend
setup_coturn
}
Expand All @@ -295,13 +315,13 @@ function set_public_ip() {
# Sets up the turn variables and optionally launches the turn server
# Assumes PUBLIC_IP = public ip of this host
function setup_turn_stun() {
if [[ "$DEFAULT_TURN" == "1" ]]; then
if [[ -z "$TURN_SERVER" ]]; then
TURN_SERVER="${PUBLIC_IP}:19303"
TURN_USER="PixelStreamingUser"
TURN_PASS="AnotherTURNintheroad"
fi

if [[ "$DEFAULT_STUN" == "1" ]]; then
if [[ -z "$STUN_SERVER" ]]; then
STUN_SERVER="stun.l.google.com:19302"
fi

Expand Down Expand Up @@ -349,33 +369,19 @@ function start_process() {
}

# Assumes the following are set
# SCRIPT_DIR = The path to the root of the PixelStreamingInfrastructure repo.
# SCRIPT_DIR = The path to the platform_scripts
# NPM = The npm command path
function build_wilbur() {
pushd "${SCRIPT_DIR}/../../.." > /dev/null

pushd Common > /dev/null
echo Building common
"${NPM}" install
"${NPM}" run build
popd

pushd Signalling > /dev/null
echo Building signalling
"${NPM}" install
"${NPM}" run build
popd > /dev/null

pushd SignallingWebServer > /dev/null
echo Building wilbur
"${NPM}" install
"${NPM}" run build

popd > /dev/null
if [ ! -d "${SCRIPT_DIR}/../../dist" ] || [ "$BUILD_WILBUR" == "1" ] ; then
pushd "${SCRIPT_DIR}/../.." > /dev/null
echo Building wilbur
"${NPM}" run build
popd > /dev/null
fi
}

# Assumes the following are set
# SCRIPT_DIR = The path to the root of the PixelStreamingInfrastructure repo.
# SCRIPT_DIR = The path to the platform_scripts
# NPM = The npm command path
# SERVER_ARGS The arguments to be passed to the server
function start_wilbur() {
Expand Down
5 changes: 1 addition & 4 deletions SignallingWebServer/platform_scripts/bash/start.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,7 @@ if [[ ! -z "$FRONTEND_DIR" ]]; then
SERVER_ARGS+=" --http_root='$FRONTEND_DIR'"
fi

if [[ "$BUILD_WILBUR" == "1" ]]; then
build_wilbur
fi

build_wilbur
print_config
start_wilbur

Loading