uenv start should exit with error if in a non-interactive shell

.github/workflows/build_and_test.yml

@@ -61,4 +61,4 @@ jobs:
       - name: test cli
         run: |
           sudo mkdir /user-tools /user-environment
-          ./meson/meson test -Cbuild --verbose cli
+          UENV_BATS_SKIP_START=on ./meson/meson test -Cbuild --verbose cli

VERSION

@@ -1 +1 @@
-6.0.0-dev
+7.0.0

src/cli/start.cpp

@@ -1,4 +1,5 @@
 // vim: ts=4 sts=4 sw=4 et
+#include <unistd.h>
 
 #include <string>
 
@@ -31,14 +32,60 @@ void start_args::add_cli(CLI::App& cli,
         ->add_option("uenv", uenv_description,
                      "comma separated list of uenv to mount")
         ->required();
+    start_cli->add_flag("--ignore-tty", ignore_tty,
+                        "don't check for non-interactive shells");
     start_cli->callback(
         [&settings]() { settings.mode = uenv::cli_mode::start; });
     start_cli->footer(start_footer);
 }
 
+// check whether running in a tty session.
+// uenv start doesn't make sense when not tty - it is designed for running
+// an interactive session.
+std::optional<std::string> detect_non_interactive() {
+    if (!isatty(fileno(stdout))) {
+        return "stdout is redirected";
+    }
+    if (!isatty(fileno(stdin))) {
+        return "stdin is redirected";
+    }
+    if (std::getenv("BASH_EXECUTION_STRING")) {
+        return "BASH_EXECUTION_STRING is set";
+    }
+    if (!std::getenv("PS1")) {
+        return "PS1 is not set";
+    }
+    return std::nullopt;
+}
+
 int start(const start_args& args,
           [[maybe_unused]] const global_settings& globals) {
     spdlog::info("start with options {}", args);
+
+    if (auto reason = detect_non_interactive(); !args.ignore_tty && reason) {
+        term::error(
+            "{}", fmt::format(
+                      R"('uenv start' must be run in an interactive shell ({}).
+
+Use the flag --ignore-tty to skip this check.
+
+This error will occur if uenv start is called within contexts like the following:
+
+- inside .bashrc
+- in a slurm batch script
+- in a bash script
+
+If your intention is to execute a command in a uenv, use run.
+See '{}' for more information
+
+If your intention is to initialize an environment (like module load), uenv start
+will not work, because it starts a new interactive shell.)",
+                      reason.value(), "uenv run --help"));
+        return 1;
+    } else {
+        fmt::println("uenv start worked!");
+    }
+
     const auto env = concretise_env(args.uenv_description,
                                     args.view_description, globals.repo);
 

src/cli/start.h

@@ -16,6 +16,7 @@ void start_help();
 
 struct start_args {
     std::string uenv_description;
+    bool ignore_tty = false;
     std::optional<std::string> view_description;
     void add_cli(CLI::App&, global_settings& settings);
 };

test/integration/cli.bats

@@ -211,6 +211,31 @@ function teardown() {
     assert_output "hello tool"
 }
 
+@test "start" {
+    export UENV_REPO_PATH=$REPOS/apptool
+    export CLUSTER_NAME=arapiles
+
+    # set UENV_BATS_SKIP_START to skip the start tests.
+    # used when running tests in a no-tty env (e.g. in GitHub action runner).
+    [[ ! -z "$UENV_BATS_SKIP_START" ]] && skip
+
+    #
+    # check that run looks up images in the repo and mounts at the correct location
+    #
+    run uenv start tool <<EOF
+echo 'hello world'
+exit
+EOF
+    assert_failure
+    assert_output --partial "must be run in an interactive shell"
+
+    run uenv start --ignore-tty tool <<EOF
+echo 'hello world'
+exit
+EOF
+    assert_success
+}
+
 @test "image add" {
     # using UENV_REPO_PATH env variable
     RP=$(mktemp -d $TMP/create-XXXXXX)

test/integration/slurm.bats

@@ -1,15 +1,27 @@
 function setup() {
-    bats_install_path=$(realpath ./install)
-    export BATS_LIB_PATH=$bats_install_path/bats-helpers
+    # set the cluster name to be arapiles
+    # this is required for tests to work when run on a vCluster
+    # that sets this variable
+    export CLUSTER_NAME=arapiles
 
     bats_load_library bats-support
     bats_load_library bats-assert
     load ./common
 
-    export REPOS=$(realpath ../scratch/repos)
-    export SQFS_LIB=$(realpath ../scratch/sqfs)
+    unset UENV_MOUNT_LIST
+
+    export PATH="$BUILD_PATH:$PATH"
 
     unset UENV_MOUNT_LIST
+
+    # set up location for creation of working repos
+    export TMP=$DATA/scratch
+    rm -rf $TMP
+    mkdir -p $TMP
+
+    # remove the bash function uenv, if an older version of uenv is installed on
+    # the system
+    unset -f uenv
 }
 
 function teardown() {
@@ -165,6 +177,20 @@ srun --uenv=tool bash -c '! findmnt /user-environment'
 EOF
 }
 
+@test "uenv start in sbatch should fail" {
+    export UENV_REPO_PATH=$REPOS/apptool
+    # check that images mounted via sbatch --uenv are overriden when `--uenv` flag is given to srun
+    run run_sbatch <<EOF
+#!/bin/bash
+
+set -e
+
+uenv start app/42.0
+EOF
+    [ "${status}" -eq "1" ]
+    assert_output --partial "'uenv start' must be run in an interactive shell"
+}
+
 @test "sbatch UENV_MOUNT_LIST with no --uenv" {
     # this should be independent of the repo
     unset UENV_REPO_PATH

test/meson.build

@@ -26,6 +26,7 @@ test_generated_data = custom_target (
 
 fs = import('fs')
 fs.copyfile('integration/cli.bats', 'cli.bats')
+fs.copyfile('integration/slurm.bats', 'slurm.bats')
 fs.copyfile('integration/common.bash', 'common.bash')
 
 suite_input = integration_path + '/setup_suite.bash.in'