Split extra args into launch and exec (#976)

Author: Nuru

ReleaseNotes-v4.md

@@ -722,3 +722,11 @@ V4 changes:
   a "monorepo" containing multiple projects. It must be an absolute path either equal to or a subdirectory of `WORKSPACE_FOLDER_HOST_DIR`.
 - `GEODESIC_TERM_THEME` is normally unset. Set it to "light" or "dark" _before launching Geodesic_ to disable the initial attempt at automatic terminal light/dark theme detection, and use the set value instead.
 - `GEODESIC_TERM_THEME_AUTO` is normally unset. Set it to "enabled" to enable automatic attempts to detect changes in terminal light/dark theme. This feature should be considered experimental and may not work as expected.
+
+Starting with Geodesic version 4.4.0:
+
+- `GEODESIC_DOCKER_EXTRA_ARGS` is deprecated. Geodesic distinguishes between options for launching a container
+  and options for exec’ing into a running one. Use `GEODESIC_DOCKER_EXTRA_LAUNCH_ARGS` for container launch and
+  `GEODESIC_DOCKER_EXTRA_EXEC_ARGS` for exec’ing into a running container. As with `GEODESIC_DOCKER_EXTRA_ARGS`,
+  the new variables are placed unquoted on the command line, so they cannot contain arguments with
+  spaces or other special characters, and quoting will not help.

docs/environment.md

@@ -40,6 +40,14 @@ Geodesic version 4 additions and changes:
 - `MAP_FILE_OWNERSHIP` replaces `GEODESIC_HOST_BINDFS_ENABLED`. If set to true, Geodesic will use `bindfs` to map file ownership
   between the host and container. This not normally needed, as it should be handled automatically by Docker.
 
+Starting with Geodesic version 4.4.0:
+
+- `GEODESIC_DOCKER_EXTRA_ARGS` is deprecated. Geodesic distinguishes between options for launching a container
+  and options for exec’ing into a running one. Use `GEODESIC_DOCKER_EXTRA_LAUNCH_ARGS` for container launch and
+  `GEODESIC_DOCKER_EXTRA_EXEC_ARGS` for exec’ing into a running container. As with `GEODESIC_DOCKER_EXTRA_ARGS`,
+  the new variables are placed unquoted on the command line, so they cannot contain arguments with
+  spaces or other special characters, and quoting will not help.
+
 ### Geodesic Version 3 Environment Variables
 
 Below is a list of environment variables that may be visible in the shell and were present in Geodesic v3.

rootfs/etc/profile.d/_01-launch-warning.sh

@@ -11,6 +11,11 @@
 # Specifically, this guards against:
 #   docker run -it cloudposse/geodesic:latest-debian  | bash
 
+# It works by outputting a command that will print a warning message to stderr and then exit,
+# which will be evaluated by the shell receiving the output, which will print the message.
+# It also outputs backspaces to erase the message, so the message does not appear
+# on the screen during the normal startup processing if the output is a terminal.
+
 # If the warning message is longer than the terminal width,
 # on some terminals the message may be truncated. In that case,
 # all the erasure characters will not be printed, and the message will not be erased.
@@ -59,5 +64,5 @@ function warn_if_piped() {
 	trap - EXIT
 }
 
-warn_if_piped
+[[ -t 0 ]] && warn_if_piped
 unset -f warn_if_piped

rootfs/etc/profile.d/_60-register-stray-shells.sh

@@ -19,5 +19,13 @@
 
 if [[ $$ != 1 ]] && [[ $PPID == 0 ]] && [[ -z $G_HOST_PID ]]; then
 	export G_HOST_PID=0
-	[[ -t 0 ]] && yellow '# Detected shell launched by `docker exec` without wrapper info, tracking as stray shell.'
+	[[ -t 0 ]] && yellow '# Detected shell launched by `docker exec` without wrapper info, tracking as stray shell.' >&2
+fi
+
+if [[ $$ == 1 ]] && ! [[ -t 0 ]] && [[ $# -eq 0 ]]; then
+	echo >&2
+	echo "#  You have launched a login shell without a terminal and without a command." >&2
+	echo "#  This is not supported. Please run a command or attach a terminal." >&2
+	echo >&2
+	exit 1
 fi

rootfs/templates/wrapper-body.sh

@@ -10,7 +10,7 @@ function require_installed() {
 
 ## Verify we have the foundations in place
 
-if [ "${GEODESIC_SHELL}" == "true" ]; then
+if [ "${GEODESIC_SHELL}" = "true" ]; then
 	echo "Cannot run while in a geodesic shell"
 	exit 1
 fi
@@ -239,17 +239,33 @@ options_to_env
 [ "$VERBOSE" = "true" ] && [ -n "$verbose_buffer" ] && printf "%s\n" "${verbose_buffer[@]}"
 
 function debug() {
-	if [ "${VERBOSE}" == "true" ]; then
-		echo "[DEBUG] $*"
+	if [ "${VERBOSE}" = "true" ]; then
+		printf "[DEBUG] %s\n" "$*" >&2
 	fi
 }
 
+function debug_and_run() {
+	local noerr
+	[ "$1" = "--noerr" ] && noerr=true && shift
+	debug '>>>'
+	debug "Running: $*"
+	if [ "$noerr" = true ]; then
+		"$@" 2>/dev/null
+	else
+		"$@"
+	fi
+  local status=$?
+  debug "Exit status: $status"
+  debug '<<<'
+  return $status
+}
+
 function _running_shell_pids() {
-	docker exec "${DOCKER_NAME}" list-wrapper-shells 2>/dev/null
+	debug_and_run --noerr docker exec "${DOCKER_NAME}" list-wrapper-shells
 }
 
 function _our_shell_pid() {
-	docker exec "${DOCKER_NAME}" list-wrapper-shells "$WRAPPER_PID" 2>/dev/null || true
+	debug_and_run --noerr docker exec "${DOCKER_NAME}" list-wrapper-shells "$WRAPPER_PID" || true
 }
 
 function _running_shell_count() {
@@ -258,14 +274,14 @@ function _running_shell_count() {
 }
 
 function _on_shell_exit() {
-	command -v "${ON_SHELL_EXIT:=geodesic_on_exit}" >/dev/null && "${ON_SHELL_EXIT}"
+	command -v "${ON_SHELL_EXIT:=geodesic_on_exit}" >/dev/null && debug_and_run "${ON_SHELL_EXIT}"
 }
 
 function _on_container_exit() {
 	export GEODESIC_EXITING_CONTAINER_ID="${CONTAINER_ID:0:12}"
 	export GEODESIC_EXITING_CONTAINER_NAME="${DOCKER_NAME}"
 	_on_shell_exit
-	[ -n "${ON_CONTAINER_EXIT}" ] && command -v "${ON_CONTAINER_EXIT}" >/dev/null && "${ON_CONTAINER_EXIT}"
+	[ -n "${ON_CONTAINER_EXIT}" ] && command -v "${ON_CONTAINER_EXIT}" >/dev/null && debug_and_run "${ON_CONTAINER_EXIT}"
 }
 
 # Call this function to wait for the container to exit, after all other shells have exited.
@@ -309,7 +325,7 @@ function run_exit_hooks() {
 	# There can then be a further delay before the container exits.
 	# So we need to build in some delays to allow for these events to occur.
 
-	if [[ ${ONE_SHELL} == "true" ]]; then
+	if [[ ${ONE_SHELL} = "true" ]]; then
 		# We can expect the Docker container to exit quickly, and do not need to report on it.
 		_on_container_exit
 		return 0
@@ -320,7 +336,7 @@ function run_exit_hooks() {
 
 	# Best case scenario: no shells running
 	if [ "${#shell_pids[@]}" -eq 0 ]; then
-		wait_for_container_exit
+		debug_and_run wait_for_container_exit
 		return $?
 	fi
 
@@ -355,16 +371,42 @@ function run_exit_hooks() {
 		[ $i -lt $n ] && echo " Finished." >&2 || printf "\nTimeout waiting for container shell to exit.\n" >&2
 	fi
 
-	wait_for_container_exit
+	debug_and_run wait_for_container_exit
 	return $?
 }
 
+function _exec_existing {
+	if [ $# -eq 0 ]; then
+		set -- "/bin/bash" "-l"
+	fi
+	[ -t 0 ] && DOCKER_EXEC_ARGS+=(-it)
+	[ -z "${GEODESIC_DOCKER_EXTRA_EXEC_ARGS}" ] || echo "# Exec'ing shell with extra Docker args: ${GEODESIC_DOCKER_EXTRA_EXEC_ARGS}" >&2
+	# GEODESIC_DOCKER_EXTRA_EXEC_ARGS is not quoted because it is expected to be a list of arguments
+
+	# We set unusual detach keys because (a) the default first char is ctrl-p, which is used for command history,
+	# and (b) if you detach from the shell, there is no way to reattach to it, so we want to effectively disable detach.
+	debug_and_run docker exec --env G_HOST_PID="${WRAPPER_PID}" --detach-keys "ctrl-^,ctrl-[,ctrl-@" "${DOCKER_EXEC_ARGS[@]}" ${GEODESIC_DOCKER_EXTRA_EXEC_ARGS} "${DOCKER_NAME}" "$@"
+}
+
 function use() {
 	[ "$1" = "use" ] && shift
 	trap run_exit_hooks EXIT
 
 	export WRAPPER_PID=$$
 
+	if [ -n "${GEODESIC_DOCKER_EXTRA_ARGS+x}" ]; then
+		echo '# WARNING: $GEODESIC_DOCKER_EXTRA_ARGS is deprecated. ' >&2
+		echo '#   Use GEODESIC_DOCKER_EXTRA_LAUNCH_ARGS to configure container launch (`docker run`)' >&2
+		echo '#   and GEODESIC_DOCKER_EXTRA_EXEC_ARGS to configure starting a new shell' >&2
+		echo '#          in a running container (`docker exec`).' >&2
+			if [ -n "${GEODESIC_DOCKER_EXTRA_LAUNCH_ARGS+x}" ]; then
+				echo '# WARNING: Both $GEODESIC_DOCKER_EXTRA_ARGS and $GEODESIC_DOCKER_EXTRA_LAUNCH_ARGS are set. ' >&2
+				echo '#   $GEODESIC_DOCKER_EXTRA_ARGS will be ignored.' >&2
+			else
+				export GEODESIC_DOCKER_EXTRA_LAUNCH_ARGS="${GEODESIC_DOCKER_EXTRA_ARGS}"
+			fi
+	fi
+
 	DOCKER_EXEC_ARGS=(--env LS_COLORS --env TERM --env TERM_COLOR --env TERM_PROGRAM)
 	# Some settings from the host environment need to propagate into the container
 	# Set them explicitly so they do not have to be exported in `launch-options.sh`
@@ -375,7 +417,7 @@ function use() {
 		fi
 	done
 
-	if [[ ${GEODESIC_CUSTOMIZATION_DISABLED-false} == false ]]; then
+	if [[ ${GEODESIC_CUSTOMIZATION_DISABLED-false} = false ]]; then
 		if [ -n "${GEODESIC_TRACE}" ]; then
 			DOCKER_EXEC_ARGS+=(--env GEODESIC_TRACE)
 		fi
@@ -394,13 +436,7 @@ function use() {
 		CONTAINER_ID=$(docker ps --filter name="^/${DOCKER_NAME}\$" --format '{{ .ID }}')
 		if [ -n "$CONTAINER_ID" ]; then
 			echo "# Starting shell in already running ${DOCKER_NAME} container ($CONTAINER_ID)"
-			if [ $# -eq 0 ]; then
-				set -- "/bin/bash" "-l"
-			fi
-			[ -t 0 ] && DOCKER_EXEC_ARGS+=(-it)
-			# We set unusual detach keys because (a) the default first char is ctrl-p, which is used for command history,
-			# and (b) if you detach from the shell, there is no way to reattach to it, so we want to effectively disable detach.
-			docker exec --env G_HOST_PID=$WRAPPER_PID --detach-keys "ctrl-^,ctrl-[,ctrl-@" "${DOCKER_EXEC_ARGS[@]}" "${DOCKER_NAME}" "$@"
+			_exec_existing "$@"
 			return 0
 		fi
 	fi
@@ -412,7 +448,7 @@ function use() {
 			-e SSH_AUTH_SOCK=/run/host-services/ssh-auth.sock)
 	fi
 
-	if [ "${WITH_DOCKER}" == "true" ]; then
+	if [ "${WITH_DOCKER}" = "true" ]; then
 		# Bind-mount docker socket into container
 		# Should work on Linux and Mac.
 		# Note that the mounted /var/run/docker.sock is not a file or
@@ -590,26 +626,19 @@ function use() {
 	if [ "$ONE_SHELL" = "true" ]; then
 		[ -t 0 ] && DOCKER_EXEC_ARGS+=(-it)
 		DOCKER_NAME="${DOCKER_NAME}-$(date +%d%H%M%S)"
-		echo "# Starting single shell ${DOCKER_NAME} session from ${DOCKER_IMAGE}"
-		echo "# Exposing port ${GEODESIC_PORT}"
-		[ -z "${GEODESIC_DOCKER_EXTRA_ARGS}" ] || echo "# Launching with extra Docker args: ${GEODESIC_DOCKER_EXTRA_ARGS}"
-		# GEODESIC_DOCKER_EXTRA_ARGS is not quoted because it is expected to be a list of arguments
-		docker run --name "${DOCKER_NAME}" "${DOCKER_LAUNCH_ARGS[@]}" "${DOCKER_EXEC_ARGS[@]}" ${GEODESIC_DOCKER_EXTRA_ARGS} "${DOCKER_IMAGE}" -l "$@"
+		echo "# Starting single shell ${DOCKER_NAME} session from ${DOCKER_IMAGE}" >&2
+		echo "# Exposing port ${GEODESIC_PORT}" >&2
+		[ -z "${GEODESIC_DOCKER_EXTRA_LAUNCH_ARGS}" ] || echo "# Launching with extra Docker args: ${GEODESIC_DOCKER_EXTRA_LAUNCH_ARGS}" >&2
+		# GEODESIC_DOCKER_EXTRA_LAUNCH_ARGS is not quoted because it is expected to be a list of arguments
+		debug_and_run docker run --name "${DOCKER_NAME}" "${DOCKER_LAUNCH_ARGS[@]}" "${DOCKER_EXEC_ARGS[@]}" ${GEODESIC_DOCKER_EXTRA_LAUNCH_ARGS} "${DOCKER_IMAGE}" -l "$@"
 	else
-		echo "# Running new ${DOCKER_NAME} container from ${DOCKER_IMAGE}"
-		echo "# Exposing port ${GEODESIC_PORT}"
-		[ -z "${GEODESIC_DOCKER_EXTRA_ARGS}" ] || echo "# Launching with extra Docker args: ${GEODESIC_DOCKER_EXTRA_ARGS}"
-		# GEODESIC_DOCKER_EXTRA_ARGS is not quoted because it is expected to be a list of arguments
-		CONTAINER_ID=$(docker run --detach --init --name "${DOCKER_NAME}" "${DOCKER_LAUNCH_ARGS[@]}" "${DOCKER_EXEC_ARGS[@]}" ${GEODESIC_DOCKER_EXTRA_ARGS} "${DOCKER_IMAGE}" /usr/local/sbin/shell-monitor)
-		echo "# Started session ${CONTAINER_ID:0:12}. Starting shell via \`docker exec\`..."
-		if [ $# -eq 0 ]; then
-			set -- "/bin/bash" "-l"
-		fi
-
-		[ -t 0 ] && DOCKER_EXEC_ARGS+=(-it)
-		# We set unusual detach keys because (a) the default first char is ctrl-p, which is used for command history,
-		# and (b) if you detach from the shell, there is no way to reattach to it, so we want to effectively disable detach.
-		docker exec --env G_HOST_PID=$$ --detach-keys "ctrl-^,ctrl-[,ctrl-@" "${DOCKER_EXEC_ARGS[@]}" "${DOCKER_NAME}" "$@"
+		echo "# Running new ${DOCKER_NAME} container from ${DOCKER_IMAGE}"  >&2
+		echo "# Exposing port ${GEODESIC_PORT}"  >&2
+		[ -z "${GEODESIC_DOCKER_EXTRA_LAUNCH_ARGS}" ] || echo "# Launching with extra Docker args: ${GEODESIC_DOCKER_EXTRA_LAUNCH_ARGS}" >&2
+		# GEODESIC_DOCKER_EXTRA_LAUNCH_ARGS is not quoted because it is expected to be a list of arguments
+		CONTAINER_ID=$(debug_and_run docker run --detach --init --name "${DOCKER_NAME}" "${DOCKER_LAUNCH_ARGS[@]}" "${DOCKER_EXEC_ARGS[@]}" ${GEODESIC_DOCKER_EXTRA_LAUNCH_ARGS} "${DOCKER_IMAGE}" /usr/local/sbin/shell-monitor)
+		echo "# Started session ${CONTAINER_ID:0:12}. Starting shell via \`docker exec\`..." >&2
+		_exec_existing "$@"
 	fi
 	true
 }

rootfs/usr/local/bin/boot

@@ -19,7 +19,8 @@ else
   function color() { echo "$*"  >&2; }
   color "########################################################################################"
   color "# Attach a terminal (docker run --rm --it ...) if you want to run a shell."
-  color "# Run the following to install the script that runs ${APP_NAME:-$(basename ${DOCKER_IMAGE:-Geodesic})} "
+  color "#"
+  color "# Run the following to install the script that runs ${APP_NAME:-$(basename ${DOCKER_IMAGE:-Geodesic})}"
   color "# with all its features (the recommended way to use Geodesic):"
 fi