${var} in nix will substitute the value of var.

''${var} will escape the $, so that it renders as ${var}

Essentially, the mutliline string in nix was constructed to look like shell variable usage when referencing nix variables. But if you want to use the bash ${var} paradigm, then you need to escape a lot.

The intent was that redis would be started in a later diff. And eventually improve on the status-quo by running the keyserver, web, and landing dev workflows in their own processes as well. Other than "Start services required for development" which is just rewording the comment, I'm not sure what you would like to see.

So the & guarantees we will wait for the execution of the mariadb-up command to finish

no, & starts it as a background process, so it forks the process and then immediately resumes with the rest of the script. The wait forces the shell to pause until it's ready.

storing the pid and the $! stores the process ID of the last background process

$! stores the PID of the last background process, but it's subject to change with subsequent calls, and redis is the next revision.

I agree that for a single service, this is overkill. But redis was going to be the nix diff, and I would need to implement either there or now. Chose two smaller diffs, rather than one really small diff and one slightly larger.

So I'm understanding correctly, this exit command will exit the subshell

There's no subshell here. This is just a script. However, the behavior of executing a script is creating a child process. However, since I also spawn a mariadbd child process, this won't immediately exit unless I call exit.

and then return control to the parent shell, which is the one entered by nix develop

Since nix develop calls this as a background process, control flow for nix develop resumes immediately. This is to satisfy that the process passed to wait will exit.

I'm having trouble understanding your comment, why do we need to explicitly exit instead of letting the script finish?

Even if the script finishes, the process will just idle because we also do:

# Launch in subshell so if the original terminal is closed, the process
# will be inherited instead of also being forced closed
("${mariadb-entrypoint}/bin/mariadb-init" &
  echo "$!" > "$MARIADB_PIDFILE")

Since this launches another child process, the default behavior is that the calling script will just idle as long as the child is still active. Without exit here, then nix develop will also idle.

Is there a way to pass this through Shellcheck?

There is, but I would prefer not to, as the amount of bash + glue code I think would add more complexity than the benefit it's adding.

Setting the env var defaults could probably be moved to another file though, and we would just have

# Service creation
... # mariadb, redis

# defaults
source "${./set-defaults}"

# shell
source "${better-prompt}/bin/better-prompt"

Created: https://linear.app/comm/issue/ENG-1681/move-nix-develop-env-var-defaults-to-separate-file

Since the yarn dev commands will likely be opt-in now. I will just explicitly name the services for now.

the amount of bash + glue code I think would add more complexity than the benefit it's adding.

I understand that the glue code would maybe make the nix files harder to review, but wouldn't moving the bash into a separate file make it easier to review, especially with the added benefit of allowing ShellCheck to take a look at it?

From my perspective, even if the paths become more messy that's okay. Testing the paths is part of the Test Plan, which the reviewers and reviewee should both do. But reviewing the bash is much harder when you have to read it through a string in a nix file. Plus, I've started running a lot of the code in the strings that get passed into writeShellApplication through ShellCheck and found errors, anyways?

Examples:

• https://phab.comm.dev/D4880#inline-31405
• https://phab.comm.dev/D4880#inline-31401

Reviewing bash in a bash file makes the most sense to me from a CI and reviewer perspective. It's not great having to review bash in strings in nix files.

wouldn't moving the bash into a separate file make it easier to review, especially with the added benefit of allowing ShellCheck to take a look at it?

I would agree with this if there's a fair amount of logic, like the start_comm_daemon.sh but if nix is just interpolating store paths and running a few commands then I don't think there's much value. Just calling a few commands aren't going to be super prone to bash's "surprising behaviors".

From my perspective, even if the paths become more messy that's okay. Testing the paths is part of the Test Plan, which the reviewers and reviewee should both do.

Well, it's more than just messy, the factored out scripts are contextually dependent and broken when ran independently.

I've started running a lot of the code in the strings that get passed into writeShellApplication through ShellCheck and found errors, anyways?

If you run the code post nix substitution, there aren't errors. ${gnused} in a nix string will be substituted for the nix store path. They do look similar, but they are very different.

Examples:

Shellcheck will receive the script with all of the "nix variables" being replaced by the store paths, so shellcheck just sees a bunch of absolute paths, which is why it's not complaining. The nix code is more of a "pre-script" rather than an actual script. It would be similar to asking JSX to be linted with an HTML linter. You could do it after rendering the JSX into HTML, but JSX isn't HTML thus you would get a lot of errors doing so

For example:

$ nix build .#mariadb-up
$ shellcheck ./result/bin/mariadb-up 
$ cat ./result/bin/mariadb-up 
#!/nix/store/wns6kkf93d55703pssdwz2dj0ws5pfvf-bash-5.1-p16/bin/bash
set -o errexit
set -o nounset
set -o pipefail

export PATH=":$PATH"

# "$HOME/Library/Application Support/<app>" is the canonical path to use
# on darwin for storing user data for installed applications.
# However, mysql and mariadb don't quote paths in the mariadbd script,
# so use XDG conventions and hope $HOME doesn't have a space.
MARIADB_DATA_HOME="${XDG_DATA_HOME:-$HOME/.local/share}/MariaDB"
MARIADB_PIDFILE="$MARIADB_DATA_HOME"/mariadb.pid
export MYSQL_UNIX_PORT="$MARIADB_DATA_HOME"/mysql.sock

if [[ ! -d "$MARIADB_DATA_HOME"/mysql ]]; then
  # mysql directory should exist if MariaDB has been initialized
  echo "Initializing MariaDB database at $MARIADB_DATA_HOME" >&2
  "/nix/store/l6i5592cw9b5yk76a2kfdg6h5699jfcw-mariadb-server-10.8.3/bin/mariadb-install-db" \
    --datadir="$MARIADB_DATA_HOME" \
    --auth-root-authentication-method=socket
fi

# Check if MariaDB server was already started
set +e # allow for pgrep to not find matches
# BSD pgrep doesn't have a "count" feature, use wc then trim whitespace
mariadb_count=$(pgrep mariadbd | wc -l | xargs echo)
set -e

if [[ "$mariadb_count" -eq "0" ]]; then
  echo "Starting MariaDB server"
  # No MariaDB present, start our own
  # Launch in subshell so if the original terminal is closed, the process
  # will be inherited instead of also being forced closed
  ("/nix/store/ybnx1j7287s7njhv0b6xshldrj3kc24n-mariadb-init/bin/mariadb-init" &
    echo "$!" > "$MARIADB_PIDFILE")

  echo "Waiting for MariaDB to come up"
  while [[ ! -S "$MYSQL_UNIX_PORT" ]]; do sleep 1; done

elif [[ "$mariadb_count" -eq "1" ]]; then

  # Check if it was started by this script
  running_pid="$(pgrep mariadbd)"
  if [[ ! -f "$MARIADB_PIDFILE" ]] || \
      [[ "$(cat "$MARIADB_PIDFILE")" != "$running_pid" ]]; then
    echo "Existing MariaDB instance found outside of nix environment" >&2
    echo "Please stop existing services and attempt 'mariadb-up' again" >&2
    exit 1
  fi

else
  echo "Many MariaDB instances found outside of nix environment" >&2
  echo "Please stop existing services and attempt 'mariadb-up' again" >&2
  exit 1

fi

# Initialize comm user, database, and secrets file for MariaDB
# Connecting through socket doesn't require a password
userCount=$("/nix/store/l6i5592cw9b5yk76a2kfdg6h5699jfcw-mariadb-server-10.8.3/bin/mariadb" -u "$USER" \
  -Bse "SELECT COUNT(1) FROM mysql.user WHERE user = 'comm';"
)
if [[ "$userCount" -eq 0 ]]; then
  PASS=$("/nix/store/x5x316n5blm7y94a5wvqcnfr90k47q5v-openssl-1.1.1q-bin/bin/openssl" rand -hex 6)

  echo "Creating comm user and comm database" >&2
  "/nix/store/l6i5592cw9b5yk76a2kfdg6h5699jfcw-mariadb-server-10.8.3/bin/mariadb" -u "$USER" \
    -Bse "CREATE DATABASE comm;
          CREATE USER comm@localhost IDENTIFIED BY '$PASS';
          GRANT ALL ON "'comm.*'" TO comm@localhost;"
  echo "Comm user and database has been created!" >&2

  # Assume this was ran from git repository
  PRJ_ROOT=$(git rev-parse --show-toplevel)
  KEYSERVER_DB_CONFIG="$PRJ_ROOT"/keyserver/secrets/db_config.json

  echo "Writing connection information to $KEYSERVER_DB_CONFIG" >&2
  mkdir -p "$(dirname "$KEYSERVER_DB_CONFIG")"

  # It's very difficult to write json from bash, just copy a nix
  # file then use sed to subsitute
  cp "/nix/store/30yyh4z4sp7v6j3ra350gj487h2kmag2-db-config" "$KEYSERVER_DB_CONFIG"
  chmod +w "$KEYSERVER_DB_CONFIG" # Nix files are read-only
  "/nix/store/69mrs7lcp77cln6d02gfw5hxiq3g8mkp-gnused-4.8/bin/sed" -i -e "s|PASS|$PASS|g" "$KEYSERVER_DB_CONFIG"
fi

echo "View MariaDB Logs: tail -f $MARIADB_DATA_HOME/logs" >&2
echo "Kill MariaDB server: pkill mariadbd" >&2

# Explicitly exit this script so the parent shell can determine
# when it's safe to return control of terminal to user
exit 0

Reviewing bash in a bash file makes the most sense to me from a CI and reviewer perspective. It's not great having to review bash in strings in nix files.

I agree it's not great. But it's hard to review bash even in a bash script. Having bash-like chunks in nix will never be a great experience as the best it can do is be as good as bash, and that's already pretty bad.