main.yml

name: Flow-IPC pipeline 

on:
  push:
    branches:
      - '*'
  pull_request:
    branches:
      - '*'
  # To run a workflow manually, the workflow must be configured to run on the workflow_dispatch event.
  workflow_dispatch:
  
jobs:
  doc:
    strategy:
      fail-fast: false
      matrix:
        compiler:
          - id: clang-15
            name: clang
            version: 15
            c-path: /usr/bin/clang-15
            cpp-path: /usr/bin/clang++-15
        build-config:
          - id: release
            conan-profile-build-type: Release
            conan-preset: release

    runs-on: ubuntu-latest
        
    name: |
      doc-${{ matrix.compiler.id }}-${{ matrix.build-config.id }}
      
    steps: 
    - name: Update available software list for apt-get
      run: sudo apt-get update
      
    - name: Checkout `ipc` repository and submodules (`flow`, `ipc_*`)
      uses: actions/checkout@v4
      with:
        submodules: true

    - name: Install Flow-IPC dependencies (inc. Graphviz) with apt-get
      run: |
        sudo apt-get install -y graphviz

    - name: Install the latest version of Conan which is less than 2
      run: |
        pip install "conan<2"
    
    - name: Create Conan profile
      run: |
        cat <<EOF > conan_profile   
        [settings]
        compiler = ${{ matrix.compiler.name }}
        compiler.version = ${{ matrix.compiler.version }}
        compiler.cppstd = 17
        compiler.libcxx = libstdc++11
        arch = x86_64
        os = Linux
        build_type = ${{ matrix.build-config.conan-profile-build-type }}
          
        [conf]
        tools.build:compiler_executables = {"c": "${{ matrix.compiler.c-path }}", "cpp": "${{ matrix.compiler.cpp-path }}"}
  
        [buildenv]
        CC = ${{ matrix.compiler.c-path }}
        CXX = ${{ matrix.compiler.cpp-path }}

        [options]
        ipc:build = False
        ipc:doc = True
        EOF
    
    - name: Install Flow-IPC dependencies (inc. Doxygen) with Conan using the profile
      run: |
        conan install \
          . \
          --profile:build conan_profile \
          --profile:host conan_profile \
          --build missing

    - name: Prepare Makefile using CMake
      run: |
        cmake \
          --preset ${{ matrix.build-config.conan-preset }} \
          -DCFG_ENABLE_DOC_GEN=ON \
          -DCFG_SKIP_CODE_GEN=ON

    - name: Get the number of processor cores for parallelized work
      uses: SimenB/github-actions-cpu-cores@v1
      id: cpu-cores

    # Note that `flow_doc_public` and `flow_doc_full` targets are also available; and a user locally
    # may well want to use them.  We, however, have chosen to dedicate a Flow-only workflow
    # (in `flow` repo) to Flow; it will generate the Flow docs.  Meanwhile ipc_doc_* targets create
    # monolithic documentation from the ipc_* submodules and `ipc` itself -- but *not* from
    # `flow` submodule.
    - name: Doxygen-generate documentation sets using Makefile
      run: |
        VERBOSE=1 make \
          ipc_doc_public \
          ipc_doc_full \
          --directory $GITHUB_WORKSPACE/build/${{ matrix.build-config.conan-profile-build-type }} \
          -j${{ steps.cpu-cores.outputs.count }}

    - name: Package doc tarball [Doxygen documentation sets (full, API-only), landing page]
      run: |
        cd $GITHUB_WORKSPACE/doc/ipc_doc
        $GITHUB_WORKSPACE/tools/doc/stage_generated_docs.sh $GITHUB_WORKSPACE/build/${{ matrix.build-config.conan-profile-build-type }}
        rm -rf generated # Save runner space.

    - name: Upload documentation tarball
      uses: actions/upload-artifact@v3
      with:
        name: ipc-doc-${{ matrix.compiler.id }}-${{ matrix.build-config.id }}
        path: |
          ${{ github.workspace }}/doc/ipc_doc.tgz

  build:
    strategy:
      fail-fast: false
      matrix:
        compiler:
          - id: gcc-9
            name: gcc
            version: 9
            c-path: /usr/bin/gcc-9
            cpp-path: /usr/bin/g++-9  
          - id: gcc-10
            name: gcc
            version: 10
            c-path: /usr/bin/gcc-10
            cpp-path: /usr/bin/g++-10
          - id: gcc-11
            name: gcc
            version: 11
            c-path: /usr/bin/gcc-11
            cpp-path: /usr/bin/g++-11
          - id: gcc-13
            name: gcc
            version: 13
            c-path: /usr/bin/gcc-13
            cpp-path: /usr/bin/g++-13        
          - id: clang-13
            name: clang
            version: 13
            c-path: /usr/bin/clang-13
            cpp-path: /usr/bin/clang++-13  
          - id: clang-15
            name: clang
            version: 15
            c-path: /usr/bin/clang-15
            cpp-path: /usr/bin/clang++-15     
          - id: clang-16
            name: clang
            version: 16 
            c-path: /usr/bin/clang-16 
            cpp-path: /usr/bin/clang++-16
            install: True      
          - id: clang-17 
            name: clang
            version: 17
            c-path: /usr/bin/clang-17
            cpp-path: /usr/bin/clang++-17
            install: True     
        build-and-test-config:
          - id: debug
            conan-preset: debug  
            conan-profile-build-type: Debug
            conan-profile-jemalloc-build-type: Debug     
          - id: release
            conan-preset: release  
            conan-profile-build-type: Release
            conan-profile-jemalloc-build-type: Release  
          - id: relwithdebinfo-tsan 
            conan-preset: relwithdebinfo
            conan-profile-build-type: RelWithDebInfo
            conan-profile-jemalloc-build-type: Release
            conan-profile-custom-conf: |
              ipc:tools.build:cflags = [ "-fsanitize=thread" ]
              ipc:tools.build:cxxflags = [ "-fsanitize=thread" ]
              ipc:tools.build:sharedlinkflags = [ "-fsanitize=thread" ]
              ipc:tools.build:exelinkflags = [ "-fsanitize=thread" ]
            conan-profile-custom-settings: |
              compiler.sanitizer = thread   
            conan-profile-custom-buildenv: |
              ipc:LDFLAGS = -fsanitize=thread
              ipc:CXXFLAGS = -fsanitize=thread
              ipc:CFLAGS = -fsanitize=thread
            conan-custom-package-settings: |
              data['compiler']['gcc']['sanitizer'] = ['None', 'address', 'thread', 'memory', 'undefined-behavior']
              data['compiler']['clang']['sanitizer'] = ['None', 'address', 'thread', 'memory', 'undefined-behavior']
            unit-tests-params: |
              --gtest_filter=-Shm_session_test.In_process_array:\
              Shm_session_test.In_process_vector_offset_ptr:\
              Shm_session_test.In_process_string_offset_ptr:\
              Shm_session_test.In_process_list_offset_ptr:\
              Shm_session_test.Multisession_in_process:\
              Shm_session_test.Allocation_performance_five:\
              Jemalloc_shm_pool_collection_test.Interface:\
              Jemalloc_shm_pool_collection_test.Multiprocess:\
              Jemalloc_shm_pool_collection_test.Owner_shm_pool_removal:\
              Jemalloc_shm_pool_collection_test.Multithread_load:\
              Borrower_shm_pool_collection_test.Interface:\
              Borrower_shm_pool_collection_test.Multiprocess:\
              Shm_pool_collection_test.Interface:\
              Shm_pool_collection_test.Multiprocess:\
              Shm_session_data_test.Multithread_object_database
            skip_transport_tests: |
              true
          - id: relwithdebinfo
            conan-preset: relwithdebinfo 
            conan-profile-build-type: RelWithDebInfo
            conan-profile-jemalloc-build-type: Release     
          - id: minsizerel
            conan-preset: minsizerel
            conan-profile-build-type: MinSizeRel
            conan-profile-jemalloc-build-type: Release
            
        exclude:
          - compiler:  { id: gcc-9 }
            build-and-test-config: { id: relwithdebinfo-tsan }
          - compiler:  { id: gcc-10 }
            build-and-test-config: { id: relwithdebinfo-tsan }
          - compiler:  { id: gcc-11 }
            build-and-test-config: { id: relwithdebinfo-tsan }
          - compiler:  { id: gcc-13 }
            build-and-test-config: { id: relwithdebinfo-tsan }

    runs-on: ubuntu-latest
        
    name: |
      build-${{ matrix.compiler.id }}-${{ matrix.build-and-test-config.id }}

    env: 
      before_each_test: |
        export TSAN_OPTIONS="disable_coredump=0 second_deadlock_stack=1 suppressions=${{ github.workspace }}/tsan_suppressions_${{ matrix.compiler.name }}_${{ matrix.compiler.version }}.cfg"
    
    steps: 
    - name: Update available software list for apt-get
      run: sudo apt-get update
      
    - name: Checkout `ipc` repository and submodules (`flow`, `ipc_*`)
      uses: actions/checkout@v4
      with:
        submodules: true

    # Many compilers are pre-installed; if not then fetch it.
    - name: Install clang compiler
      if: |
        matrix.compiler.install && (matrix.compiler.name == 'clang')
      run: |
        wget https://apt.llvm.org/llvm.sh
        chmod u+x llvm.sh
        sudo ./llvm.sh ${{ matrix.compiler.version }}

    - name: Install gcc compiler
      if: |
        matrix.compiler.install && (matrix.compiler.name == 'gcc')
      run: |
        sudo apt-get install -y software-properties-common
        sudo add-apt-repository ppa:ubuntu-toolchain-r/test -y
        sudo apt-get update
        sudo apt-get install -y gcc-${{ matrix.compiler.version }} g++-${{ matrix.compiler.version }}

    - name: Install the latest version of Conan which is less than 2
      run: |
        pip install "conan<2"

    - name: Add custom settings for Conan packages
      if: |
        matrix.build-and-test-config.conan-custom-settings
      run: |
        conan config init
        
        pip install PyYAML

        CONAN_SETTINGS_PATH=$(conan config home)/settings.yml

        python -c "
        import yaml

        with open('$CONAN_SETTINGS_PATH', 'r') as file:
          data = yaml.safe_load(file)
        
        ${{ matrix.build-and-test-config.conan-custom-settings }}
      
        with open('$CONAN_SETTINGS_PATH', 'w') as file:
          yaml.dump(data, file)
        "
    
    - name: Create Conan profile
      run: |
        cat <<EOF > conan_profile   
        [settings]
        compiler = ${{ matrix.compiler.name }}
        compiler.version = ${{ matrix.compiler.version }}
        compiler.cppstd = 17
        compiler.libcxx = libstdc++11
        arch = x86_64
        os = Linux
        build_type = ${{ matrix.build-and-test-config.conan-profile-build-type }}
        jemalloc:build_type = ${{ matrix.build-and-test-config.conan-profile-jemalloc-build-type }}
        ${{ matrix.build-and-test-config.conan-profile-custom-settings }}
          
        [conf]
        tools.env.virtualenv:auto_use = True
        tools.build:compiler_executables = {"c": "${{ matrix.compiler.c-path }}", "cpp": "${{ matrix.compiler.cpp-path }}"}
        ${{ matrix.build-and-test-config.conan-profile-custom-conf }}
  
        [buildenv]
        CC = ${{ matrix.compiler.c-path }}
        CXX = ${{ matrix.compiler.cpp-path }}
        ${{ matrix.build-and-test-config.conan-profile-custom-buildenv }}

        [options]
        jemalloc:enable_cxx = False 
        jemalloc:prefix = je_
        ipc:build = True
        ipc:doc = False
        flow:build = True
        flow:doc = False
        EOF
    
    - name: Install Flow-IPC dependencies with Conan using the profile
      run: |
        conan editable add flow flow/1.0
        conan install \
          . \
          --profile:build conan_profile \
          --profile:host conan_profile \
          --build missing

    - name: Get the number of processor cores for parallelized work
      uses: SimenB/github-actions-cpu-cores@v1
      id: cpu-cores

    # TODO: Ideally let our CMake interrogate jemalloc-config to find jemalloc-prefix itself.
    # Less stuff for us to worry about that way here.  If it has not been tried, try it.
    # If it does not work, apply a small effort to see if it can be easily made to work.
    - name: Prepare Makefile using CMake
      run: |
        cmake \
          --preset ${{ matrix.build-and-test-config.conan-preset }} \
          -DCFG_ENABLE_TEST_SUITE=ON \
          -DJEMALLOC_PREFIX=je_ \
          -DCMAKE_INSTALL_PREFIX=$GITHUB_WORKSPACE/install/${{ matrix.build-and-test-config.conan-profile-build-type }}

    - name: Build targets (libraries, demos/tests) with Makefile
      run: |
        VERBOSE=1 make \
          --keep-going \
          --directory $GITHUB_WORKSPACE/build/${{ matrix.build-and-test-config.conan-profile-build-type }} \
          -j${{ steps.cpu-cores.outputs.count }}

    - name: Install targets with Makefile
      run: |
        VERBOSE=1 make install \
          --directory $GITHUB_WORKSPACE/build/${{ matrix.build-and-test-config.conan-profile-build-type }} \
          -j${{ steps.cpu-cores.outputs.count }}

    # From now on use !cancelled() to try to run any demo/test that exists regardless
    # of preceding failures if any.  Same-ish (always()) with the log-upload at the end.
    # Worst-case they'll all fail in super-basic ways and immediately; no extra harm done.

    - name: Run link test [`ipc_core` - Flow-IPC Core]
      if: |
        (!cancelled())
      run: |
        ${{ env.before_each_test }}
        cd $GITHUB_WORKSPACE/install/${{ matrix.build-and-test-config.conan-profile-build-type }}/bin
        ./ipc_core_link_test.exec

    - name: Run link test [`ipc_transport_structured` - Flow-IPC Structured Transport]
      if: |
        (!cancelled())
      run: |
        $before_each_test
        cd $GITHUB_WORKSPACE/install/${{ matrix.build-and-test-config.conan-profile-build-type }}/bin
        ./ipc_transport_structured_link_test.exec

    # Server will exit automatically (same below).
    - name: Run link test [`ipc_session` - Flow-IPC Sessions]
      if: |
        (!cancelled())
      run: |
        ${{ env.before_each_test }}
        cd $GITHUB_WORKSPACE/install/${{ matrix.build-and-test-config.conan-profile-build-type }}/bin
        ./ipc_session_link_test_srv.exec & 
        sleep 1
        ./ipc_session_link_test_cli.exec

    - name: Run link test [`ipc_shm` - Flow-IPC Shared Memory]
      if: |
        (!cancelled())
      run: |
        ${{ env.before_each_test }}
        cd $GITHUB_WORKSPACE/install/${{ matrix.build-and-test-config.conan-profile-build-type }}/bin
        ./ipc_shm_link_test_srv.exec & 
        sleep 1
        ./ipc_shm_link_test_cli.exec
        
    - name: Run link test [`ipc_shm_arena_lend` - Flow-IPC SHM-jemalloc]
      if: |
        (!cancelled())
      run: |
        ${{ env.before_each_test }}
        cd $GITHUB_WORKSPACE/install/${{ matrix.build-and-test-config.conan-profile-build-type }}/bin
        ./ipc_shm_arena_lend_link_test_srv.exec & 
        sleep 1
        ./ipc_shm_arena_lend_link_test_cli.exec

    # As of this writing this runs with high verbosity, so we do not re-run on failure
    # like the ones below.
    - name: Run unit tests
      if: |
        (!cancelled())
      run: |
        ${{ env.before_each_test }}
        echo $TSAN_OPTIONS
        cd $GITHUB_WORKSPACE/install/${{ matrix.build-and-test-config.conan-profile-build-type }}/bin
        ./libipc_unit_test.exec ${{ matrix.build-and-test-config.gtest-unit-tests-params }}

    # Runner can easily run out of space due to logs, so we tar-up log dir
    # at the end of each step.  We also blow away the build dir, as all
    # should be installed in install dir by then.

    # For tests below where on failure it can be very helpful to look at higher-verbosity logs,
    # any failing step is repeated with logging hiked up.  Note that formally (and practically)
    # higher-than-INFO verbosity is allowed to affect performance and thus is *not* right for
    # the main integration test run.  However on failure all bets are off, and we just want the
    # info we can get.  (Also higher-verbosity runs can take significantly longer; no one wants that.)

    # This follows the instructions in bin/transport_test/README.txt.
    - name: Prepare run script for [transport_test - Scripted mode] variations below
      if: |
        (!cancelled()) && !matrix.build-and-test-config.skip_transport_tests
      run: |
        ${{ env.before_each_test }}
        cat <<'EOF' > $GITHUB_WORKSPACE/install/${{ matrix.build-and-test-config.conan-profile-build-type }}/bin/run_transport_test_sc.sh
        # Script created by pipeline during job.
        echo "Log level: [$1]."
        OUT_DIR_NAME=log_level_$1
        OUT_DIR=runs/scripted/$OUT_DIR_NAME
        cd $GITHUB_WORKSPACE/install/${{ matrix.build-and-test-config.conan-profile-build-type }}/bin/transport_test
        mkdir -p $OUT_DIR
        ./transport_test.exec scripted $OUT_DIR/transport_test.srv.log info $1 \
          < srv-script.txt > $OUT_DIR/transport_test.srv.console.log 2>&1 &
        SRV_PID=$!
        sleep 1
        ./transport_test.exec scripted $OUT_DIR/transport_test.cli.log info $1 \
          < cli-script.txt > $OUT_DIR/transport_test.cli.console.log 2>&1 &
        CLI_PID=$!
        wait $SRV_PID
        echo "Server finished OK (logs will be uploaded as artifacts)."
        wait $CLI_PID
        echo "Client finished OK (logs will be uploaded as artifacts)."
        cd $OUT_DIR/..
        tar cvzf $OUT_DIR_NAME.tgz $OUT_DIR_NAME
        rm -rf $OUT_DIR_NAME

    - name: Run integration test [transport_test - Scripted mode]
      id: transport_test_scripted
      if: |
        (!cancelled()) && !matrix.build-and-test-config.skip_transport_tests
      run: |
        ${{ env.before_each_test }}
        /usr/bin/bash -e \
          $GITHUB_WORKSPACE/install/${{ matrix.build-and-test-config.conan-profile-build-type }}/bin/run_transport_test_sc.sh \
            info

    - name: Re-run with increased logging, on failure only
      if: |
        (!cancelled()) && (steps.transport_test_scripted.outcome == 'failure') && !matrix.build-and-test-config.skip_transport_tests
      run: |
        ${{ env.before_each_test }}
        /usr/bin/bash -e \
          $GITHUB_WORKSPACE/install/${{ matrix.build-and-test-config.conan-profile-build-type }}/bin/run_transport_test_sc.sh \
            data

    # The following [Exercise mode] tests follow the instructions in bin/transport_test/README.txt.
    # Note that the creation of ~/bin/ex_..._run and placement of executables there, plus
    # /tmp/var/run for run-time files (PID files and similar), is a necessary consequence of
    # the ipc::session safety model for estabshing IPC conversations (sessions).

    - name: Prepare IPC-session safety-friendly run-time environment for [transport_test - Exercise mode]
      if: |
        (!cancelled()) && !matrix.build-and-test-config.skip_transport_tests
      run: |
        rm -rf $GITHUB_WORKSPACE/build/${{ matrix.build-and-test-config.conan-profile-build-type }}
        mkdir -p ~/bin/ex_srv_run ~/bin/ex_cli_run
        mkdir -p /tmp/var/run
        cp -v $GITHUB_WORKSPACE/install/${{ matrix.build-and-test-config.conan-profile-build-type }}/bin/transport_test/transport_test.exec \
          ~/bin/ex_srv.exec
        cp -v ~/bin/ex_srv.exec ~/bin/ex_cli.exec

    - name: Prepare run script for [transport_test - Exercise mode] variations below
      if: |
        (!cancelled()) && !matrix.build-and-test-config.skip_transport_tests
      run: |
        cat <<'EOF' > $GITHUB_WORKSPACE/install/${{ matrix.build-and-test-config.conan-profile-build-type }}/bin/run_transport_test_ex.sh
        # Script created by pipeline during job.
        echo "Log level: [$1]."
        echo "Exercise sub-mode: [$2]."
        echo "Sub-mode snippet (none or 'shm-?'): [$3]."
        OUT_DIR=$GITHUB_WORKSPACE/install/${{ matrix.build-and-test-config.conan-profile-build-type }}/bin/transport_test/runs/exercise/$2
        mkdir -p $OUT_DIR
        cd ~/bin/ex_srv_run
        ~/bin/ex_srv.exec exercise-srv$3 $OUT_DIR/transport_test.srv.log info $1 \
          > $OUT_DIR/transport_test.srv.console.log 2>&1 &
        SRV_PID=$!
        sleep 5
        ~/bin/ex_cli.exec exercise-cli$3 $OUT_DIR/transport_test.cli.log info $1 \
          > $OUT_DIR/transport_test.cli.console.log 2>&1 &
        CLI_PID=$!
        wait $SRV_PID
        echo "Server finished OK (logs will be uploaded as artifacts)."
        wait $CLI_PID
        echo "Client finished OK (logs will be uploaded as artifacts)."
        cd $OUT_DIR/..
        tar cvzf $2.tgz $2
        rm -rf $2
        EOF

    - name: Run integration test [transport_test - Exercise mode - Heap sub-mode]
      id: transport_test_ex_heap
      if: |
        (!cancelled()) && !matrix.build-and-test-config.skip_transport_tests
      run: |
        ${{ env.before_each_test }}
        /usr/bin/bash -e \
          $GITHUB_WORKSPACE/install/${{ matrix.build-and-test-config.conan-profile-build-type }}/bin/run_transport_test_ex.sh \
            info heap

    - name: Re-run with increased logging, on failure only
      if: |
        (!cancelled()) && (steps.transport_test_ex_heap.outcome == 'failure') && !matrix.build-and-test-config.skip_transport_tests
      run: |
        ${{ env.before_each_test }}
        /usr/bin/bash -e \
          $GITHUB_WORKSPACE/install/${{ matrix.build-and-test-config.conan-profile-build-type }}/bin/run_transport_test_ex.sh \
            data heap_log_level_data

    - name: Run integration test [transport_test - Exercise mode - SHM-classic sub-mode]
      id: transport_test_ex_shm_c
      if: |
        (!cancelled()) && !matrix.build-and-test-config.skip_transport_tests
      run: |
        ${{ env.before_each_test }}
        /usr/bin/bash -e \
          $GITHUB_WORKSPACE/install/${{ matrix.build-and-test-config.conan-profile-build-type }}/bin/run_transport_test_ex.sh \
            info shm_classic -shm-c

    - name: Re-run with increased logging, on failure only
      if: |
        (!cancelled()) && (steps.transport_test_ex_shm_c.outcome == 'failure') && !matrix.build-and-test-config.skip_transport_tests
      run: |
        ${{ env.before_each_test }}
        /usr/bin/bash -e \
          $GITHUB_WORKSPACE/install/${{ matrix.build-and-test-config.conan-profile-build-type }}/bin/run_transport_test_ex.sh \
            data shm_classic_log_level_data -shm-c

    - name: Run integration test [transport_test - Exercise mode - SHM-jemalloc sub-mode]
      id: transport_test_ex_shm_j
      if: |
        (!cancelled()) && !matrix.build-and-test-config.skip_transport_tests
      run: |
        ${{ env.before_each_test }}
        /usr/bin/bash -e \
          $GITHUB_WORKSPACE/install/${{ matrix.build-and-test-config.conan-profile-build-type }}/bin/run_transport_test_ex.sh \
            info shm_jemalloc -shm-j

    - name: Re-run with increased logging, on failure only
      if: |
        (!cancelled()) && (steps.transport_test_ex_shm_j.outcome == 'failure') && !matrix.build-and-test-config.skip_transport_tests 
      run: |
        ${{ env.before_each_test }}
        /usr/bin/bash -e \
          $GITHUB_WORKSPACE/install/${{ matrix.build-and-test-config.conan-profile-build-type }}/bin/run_transport_test_ex.sh \
            data shm_jemalloc_log_level_data -shm-j

    - name: Upload logs for [transport_test - All modes]
      if: |
        always() && !matrix.build-and-test-config.skip_transport_tests
      uses: actions/upload-artifact@v3
      with:
        name: ipc-transport-test-run-${{ matrix.compiler.id }}-${{ matrix.build-and-test-config.id }}
        path: |
          ${{ github.workspace }}/install/${{ matrix.build-and-test-config.conan-profile-build-type }}/bin/transport_test/runs

# TODO: Look into the topic of debuggability in case of a crash.  Is a core generated?  Is it saved?
# Do we need to manually save it as an artifact?  For that matter we would then need the binary and
# ideally the source.  For now we save any logs that are not printed to console, and where possible
# our demos/tests keep it the console exclusively (but in some cases, such as transport_test, it
# is not practical due to parallel execution and other aspects).  In general it is always better that
# logs are sufficient; but look into situations where they are not.
#
# Possibly this is all handled beautifully automatically; then this should be deleted.