Skip to content

Commit

Permalink
Delegate to unified strip-marker-sections script (#1722)
Browse files Browse the repository at this point in the history
Resolves #1710.
Stacked on #1720.

Based on the [simplifications in
`unset-static-ip`](#1719),
this PR removes the code redundancies in the `unset-static-ip` and
`change-hostname` privileged scripts, and calls the new, unified
`strip-marker-sections` script instead.

## Notes

- `change-hostname`:
- Naively speaking, it would seem simpler to *append* the new entry to
the file. I remembered, however, that the prepending was done on
purpose, because in `/etc/hosts` the first matching entry takes
precedence. I captured that as a comment therefore.
- I slightly reordered the code so that all `/etc/hosts`-related logic
is close together now (i.e., clearing the markers, then rewriting the
file).
- `unset-static-ip`
- ~I couldn’t figure out why we have been using `tee` previously to
write to the config file, because we discarded `tee`’s stdout output by
redirecting to `/dev/null`. I’m also not sure why we used `sudo` for
invoking `tee` – [we already execute the script with `root`
privileges](https://github.com/tiny-pilot/tinypilot-pro/blob/cfb99f88a3039d587a9108d427e7ecfa81b7d190/app/static_ip.py#L128).
It’s a bit odd, because I cannot imagine we did that without good
reason~ 🤔 (We [had been using `sudo tee` to avoid permission
issues](#1722 (comment))
when writing the file.)

<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1722"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>

---------

Co-authored-by: Jan Heuermann <[email protected]>
  • Loading branch information
jotaen4tinypilot and jotaen authored Jan 17, 2024
1 parent a141353 commit 88cf332
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 55 deletions.
37 changes: 10 additions & 27 deletions debian-pkg/opt/tinypilot-privileged/scripts/change-hostname
Original file line number Diff line number Diff line change
Expand Up @@ -56,36 +56,19 @@ if ! [[ "${NEW_HOSTNAME}" =~ $VALID_HOSTNAME_PATTERN ]] \
exit 1
fi

# Retrieve original `/etc/hosts`.
# - Read file line by line, make sure to preserve all whitespace.
# - Remove all marker sections.
etc_hosts_original=()
is_in_marker_section='false'
while IFS= read -r line; do
if "${is_in_marker_section}" && [[ "${line}" == "${MARKER_END}" ]]; then
is_in_marker_section='false'
continue
fi
if "${is_in_marker_section}" || [[ "${line}" == "${MARKER_START}" ]]; then
is_in_marker_section='true'
continue
fi
etc_hosts_original+=("${line}")
done < /etc/hosts
# Remove any existing marker sections from the `/etc/hosts` file.
"${SCRIPT_DIR}/strip-marker-sections" /etc/hosts

if "${is_in_marker_section}"; then
echo 'Unclosed marker section' >&2
exit 1
fi
# Populate new entry to `/etc/hosts`.
# Note that the first matching entry takes precedence, which is why we prepend
# our new entry.
OLD_ETC_HOSTS="$(</etc/hosts)"
readonly OLD_ETC_HOSTS
printf "%s\n127.0.1.1 %s\n%s\n%s\n" \
"${MARKER_START}" "${NEW_HOSTNAME}" "${MARKER_END}" "${OLD_ETC_HOSTS}" \
> /etc/hosts

# Populate new hostname to `/etc/hostname`.
# Note that the value must be newline-terminated, see:
# https://manpages.debian.org/stretch/systemd/hostname.5.en.html
printf "%s\n" "${NEW_HOSTNAME}" > /etc/hostname

# Populate corresponding entry to `etc/hosts`.
OLD_ETC_HOSTS=$(printf "%s\n" "${etc_hosts_original[@]}")
readonly OLD_ETC_HOSTS
printf "%s\n127.0.1.1 %s\n%s\n%s\n" \
"${MARKER_START}" "${NEW_HOSTNAME}" "${MARKER_END}" "${OLD_ETC_HOSTS}" \
> /etc/hosts
30 changes: 2 additions & 28 deletions debian-pkg/opt/tinypilot-privileged/scripts/unset-static-ip
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ EOF

SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"
readonly SCRIPT_DIR
readonly CONFIG_FILE="/etc/dhcpcd.conf"
# shellcheck source=lib/markers.sh
. "${SCRIPT_DIR}/lib/markers.sh"

Expand Down Expand Up @@ -59,33 +58,8 @@ fi
# Exit on unset variable
set -u

# Remove any existing auto-generated configuration as populated by the
# set-static-ip script.
dhcpcd_original=()
is_in_marker_section='false'
while IFS= read -r line; do
if "${is_in_marker_section}" && [[ "${line}" == "${MARKER_END}" ]]; then
is_in_marker_section='false'
continue
fi
if "${is_in_marker_section}" || [[ "${line}" == "${MARKER_START}" ]]; then
is_in_marker_section='true'
continue
fi
dhcpcd_original+=("${line}")
done < "${CONFIG_FILE}"

if "${is_in_marker_section}"; then
echo 'Unclosed marker section' >&2
exit 1
fi

# Convert array of lines to a single string.
OLD_CONFIG_FILE=$(printf "%s\n" "${dhcpcd_original[@]}")
readonly OLD_CONFIG_FILE

# Write original file contents back to the file.
echo "${OLD_CONFIG_FILE}" | sudo tee "${CONFIG_FILE}" > /dev/null
# Remove any existing marker sections from the config file.
"${SCRIPT_DIR}/strip-marker-sections" /etc/dhcpcd.conf

# Changes require a reboot so prompt the user.
if [[ "${QUIET}" == "false" ]] ; then
Expand Down

0 comments on commit 88cf332

Please sign in to comment.