Repository: columnflow/columnflow (language: python)

Issue:
Empty `cf` sandbox
After moving to a more recent version of Columnflow in our analysis setup, we began seeing an issue with the `cf` "production" sandbox, in that it does not seem to contain any of the packages specified in `requirements_prod.txt` (actually, it is completely empty aside from a minimal venv directory structure). This leads to failures of remote jobs, which rely on the production `cf` sandbox to run.

I suspect the reason for the empty sandbox has to do with the fact that the `requirements_prod.txt` is a subset of the packages installed in the `cf_dev` sandbox, which also contains development tools like IPython, and which is sourced by the analysis setup script. When triggering a build of `cf` from the sourced setup, nothing will be installed in `cf`, since all requirements are already found in the already-sourced `cf_dev` sandbox.

If that is indeed the case, an easy fix would be to run `pip --ignore-installed -r requirements_prod.txt` for `cf`, so the packages potentially found in `cf_dev` are not taken into account and therefore installed explicitly. However, this might also lead to packages being unnecessarily reinstalled. I'm wondering why we did not see this before, since in principle the problem should have been there from the beginning.


Discussion / hints:

Fix patch:
diff --git a/analysis_templates/cms_minimal/sandboxes/example_dev.sh b/analysis_templates/cms_minimal/sandboxes/example_dev.sh
index 5c62a3673..37ceb6556 100644
--- a/analysis_templates/cms_minimal/sandboxes/example_dev.sh
+++ b/analysis_templates/cms_minimal/sandboxes/example_dev.sh
@@ -11,7 +11,7 @@ action() {
     # set variables and source the generic venv setup
     export CF_SANDBOX_FILE="${CF_SANDBOX_FILE:-${this_file}}"
     export CF_VENV_NAME="$( basename "${this_file%.sh}" )"
-    export CF_VENV_REQUIREMENTS="${this_dir}/example.txt,${CF_BASE}/requirements_dev.txt"
+    export CF_VENV_REQUIREMENTS="${this_dir}/example.txt,${CF_BASE}/sandboxes/dev.txt"
 
     source "${CF_BASE}/sandboxes/_setup_venv.sh" "$@"
 }
diff --git a/bin/cf_create_venv b/bin/cf_create_venv
index 22be93584..73b197267 100755
--- a/bin/cf_create_venv
+++ b/bin/cf_create_venv
@@ -1,8 +1,7 @@
 #!/usr/bin/env bash
 
 cf_create_venv() {
-    # Creates a new virtual environment inside $CF_VENV_BASE, exchanges all absolute paths in the
-    # activation script with relative ones, and makes the environment relocatable.
+    # Creates a new virtual environment inside $CF_VENV_BASE and makes it relocatable.
     #
     # Arguments:
     #   1. name
@@ -11,11 +10,6 @@ cf_create_venv() {
     # Required environment variables:
     #   CF_VENV_BASE:
     #       The base path where CF virtual environments are stored.
-    #
-    # Optional environments variables:
-    #   CF_CI_JOB
-    #       When "1", the virtual environment is created without copying existing packages from the
-    #       encapsulating enviroment.
 
     # check arguments
     local name="$1"
@@ -30,22 +24,20 @@ cf_create_venv() {
         return "2"
     fi
 
-    # create the venv the usual way, not using copies for dev envs or in CI jobs
-    if [[ "${name}" == *_dev ]] || [ "${CF_CI_JOB}" = "1" ]; then
-        python3 -m venv "${CF_VENV_BASE}/${name}" || return "$?"
-    else
-        python3 -m venv --copies "${CF_VENV_BASE}/${name}" || return "$?"
-    fi
+    # create the venv the usual way, use symlinks with those pointing outside the venv being
+    # transformed into copies when making it relocatable
+    python3 -m venv --symlinks --upgrade-deps "${CF_VENV_BASE}/${name}" || return "$?"
 
     # remove csh and fish support
     rm -f "${CF_VENV_BASE}/${name}"/bin/activate{.csh,.fish}
 
-    # replace absolute paths in the activation file to make it relocateable for bash and zsh
+    # replace absolute paths in the activation file to make it relocatable for bash and zsh
     sed -i -r \
-        's/(VIRTUAL_ENV)=.+/\1="$( cd "$( dirname "$( [ ! -z "$ZSH_VERSION" ] \&\& echo "${(%):-%x}" || echo "${BASH_SOURCE[0]}" )" )" \&\& dirname "$( \/bin\/pwd )" )"/' \
+        's/(VIRTUAL_ENV)=.+/\1="$( cd "$( dirname "$( [ ! -z "${ZSH_VERSION}" ] \&\& echo "${(%):-%x}" || echo "${BASH_SOURCE[0]}" )" )" \&\& dirname "$( \/bin\/pwd )" )"/' \
         "${CF_VENV_BASE}/${name}/bin/activate"
 
-    cf_make_venv_relocateable "${name}"
+    # make it relocatable, loosing all relations to the outer env
+    cf_make_venv_relocatable "${name}"
 }
 
 cf_create_venv "$@"
diff --git a/bin/cf_make_venv_relocatable b/bin/cf_make_venv_relocatable
new file mode 100755
index 000000000..8938215d3
--- /dev/null
+++ b/bin/cf_make_venv_relocatable
@@ -0,0 +1,71 @@
+#!/usr/bin/env bash
+
+cf_make_venv_relocatable() {
+    # Traverses all executables in a virtual environment directory located in $CF_VENV_BASE and
+    # replaces absolute shebangs with relative ones using /usr/bin/env.
+    #
+    # Arguments:
+    #   1. The name of the virtual env inside $CF_VENV_BASE.
+    #
+    # Required environment variables:
+    #   CF_VENV_BASE
+    #       The base path where AP virtual environments are stored.
+
+    # check arguments
+    local name="$1"
+    if [ -z "${name}" ]; then
+        2>&1 echo "argument 0 (venv name) must not be empty"
+        return "1"
+    fi
+
+    # check environment variables
+    if [ -z "${CF_VENV_BASE}" ]; then
+        2>&1 echo "environment variable CF_VENV_BASE must not be empty"
+        return "2"
+    fi
+
+    # use /usr/bin/env in shebang's of bin scripts
+    local f
+    for f in $( find "${CF_VENV_BASE}/${name}/bin" -type f ); do
+        # must be readable and executable
+        if [ -r "${f}" ] && [ -x "${f}" ]; then
+            sed -i -r "s/#\!\/.+\/bin\/(python[\\\/]*)/#\!\/usr\/bin\/env \1/" "$f"
+            [ "$?" != "0" ] && return "3"
+        fi
+    done
+
+    # replace all symlinked files pointing outside of the venv with copies
+    for f in $( find "${CF_VENV_BASE}/${name}" -type l ); do
+        local real_dir
+        local link_f="$( readlink "${f}" )"
+        [ "$?" != "0" ] && return "4"
+        if [ "${link_f:0:1}" = "/" ]; then
+            # absolute link
+            real_dir="$( cd "$( dirname "${link_f}" )" && echo "${PWD}" )"
+            [ "$?" != "0" ] && return "5"
+        elif [ "${link_f:0:1}" = "." ]; then
+            # relative link
+            real_dir="$( cd "$( dirname "$( dirname "${f}" )/${link_f}" )" && echo "${PWD}" )"
+            [ "$?" != "0" ] && return "5"
+        else
+            # local link, always contained in venv
+            continue
+        fi
+        # do nothing when the directory is contained in the venv dir
+        if [[ "${real_dir}/" == "${CF_VENV_BASE}/${name}/"* ]]; then
+            continue
+        fi
+        # complain when a directory is found
+        if [ -d "${f}" ]; then
+            2>&1 echo "cannot handle symlinked directory ${f} in venv ${name} for relocation"
+            continue
+        fi
+        # copy
+        local real_f="${real_dir}/$( basename "${link_f}" )"
+        # echo "replace symlink ${f} with ${real_f}"
+        unlink "${f}" || return "6"
+        cp "${real_f}" "${f}" || return "7"
+    done
+}
+
+cf_make_venv_relocatable "$@"
diff --git a/bin/cf_make_venv_relocateable b/bin/cf_make_venv_relocateable
deleted file mode 100755
index e826f13f5..000000000
--- a/bin/cf_make_venv_relocateable
+++ /dev/null
@@ -1,36 +0,0 @@
-#!/usr/bin/env bash
-
-cf_make_venv_relocateable() {
-    # Traverses all executables in a virtual environment directory located in $CF_VENV_BASE and
-    # replaces absolute shebangs with relative ones using /usr/bin/env.
-    #
-    # Arguments:
-    #   1. The name of the virtual env inside $CF_VENV_BASE.
-    #
-    # Required environment variables:
-    #   CF_VENV_BASE
-    #       The base path where AP virtual environments are stored.
-
-    # check arguments
-    local name="$1"
-    if [ -z "${name}" ]; then
-        2>&1 echo "argument 0 (venv name) must not be empty"
-        return "1"
-    fi
-
-    # check environment variables
-    if [ -z "${CF_VENV_BASE}" ]; then
-        2>&1 echo "environment variable CF_VENV_BASE must not be empty"
-        return "2"
-    fi
-
-    # use /usr/bin/env in shebang's of bin scripts
-    for f in $( find "${CF_VENV_BASE}/${name}/bin" -type f ); do
-        # must be readable and executable
-        if [ -r "${f}" ] && [ -x "${f}" ]; then
-            sed -i -r "s/#\!\/.+\/bin\/(python[\\\/]*)/#\!\/usr\/bin\/env \1/" "$f"
-        fi
-    done
-}
-
-cf_make_venv_relocateable "$@"
diff --git a/sandboxes/_setup_venv.sh b/sandboxes/_setup_venv.sh
index ea81a2cde..6b378c31d 100644
--- a/sandboxes/_setup_venv.sh
+++ b/sandboxes/_setup_venv.sh
@@ -15,7 +15,7 @@
 #       The name of the virtual environment. It will be installed into $CF_VENV_BASE/$CF_VENV_NAME.
 #   CF_VENV_REQUIREMENTS
 #       The requirements file containing packages that are installed on top of
-#       $CF_BASE/requirements_prod.txt.
+#       $CF_BASE/sandboxes/cf.txt.
 #
 # Upon venv activation, two environment variables are set in addition to those exported by the venv:
 #   CF_DEV
@@ -113,7 +113,7 @@ setup_venv() {
 
     # split $CF_VENV_REQUIREMENTS into an array
     local requirement_files
-    local requirement_files_contains_prod="false"
+    local requirement_files_contains_cf="false"
     if ${shell_is_zsh}; then
         requirement_files=(${(@s:,:)CF_VENV_REQUIREMENTS})
     else
@@ -124,8 +124,8 @@ setup_venv() {
             >&2 echo "requirement file '${f}' does not exist"
             return "13"
         fi
-        if [ "${f}" = "${CF_BASE}/requirements_prod.txt" ]; then
-            requirement_files_contains_prod="true"
+        if [ "${f}" = "${CF_BASE}/sandboxes/cf.txt" ]; then
+            requirement_files_contains_cf="true"
         fi
     done
     local first_requirement_file="${requirement_files[0]}"
@@ -247,7 +247,7 @@ setup_venv() {
 
         # install if not existing
         if [ ! -f "${CF_SANDBOX_FLAG_FILE}" ]; then
-            cf_color cyan "installing venv ${sandbox_file} at ${install_path}"
+            cf_color cyan "installing venv ${CF_VENV_NAME} from ${sandbox_file} at ${install_path}"
 
             rm -rf "${install_path}"
             cf_create_venv "${venv_name_hashed}"
@@ -257,33 +257,32 @@ setup_venv() {
             source "${install_path}/bin/activate" ""
             [ "$?" != "0" ] && clear_pending && return "26"
 
-            # update pip
-            cf_color magenta "updating pip"
-            python -m pip install -U pip
-            [ "$?" != "0" ] && clear_pending && return "27"
-            echo
-
-            # install basic production requirements
-            if ! ${requirement_files_contains_prod}; then
-                cf_color magenta "installing requirement file ${CF_BASE}/requirements_prod.txt"
-                pip install -r "${CF_BASE}/requirements_prod.txt"
-                [ "$?" != "0" ] && clear_pending && return "28"
+            # compose a list of arguments containing dependencies to install
+            local install_reqs=""
+            add_requirements() {
+                local args="${@}"
+                echo "$( cf_color magenta "install" ) $( cf_color default_bright "${args}" )"
+                [ ! -z "${install_reqs}" ] && install_reqs="${install_reqs} "
+                install_reqs="${install_reqs}${args}"
+            }
+
+            # update packaging tools
+            add_requirements pip setuptools
+
+            # basic cf requirements
+            if ! ${requirement_files_contains_cf}; then
+                add_requirements -r "${CF_BASE}/sandboxes/cf.txt"
             fi
 
-            # install requirement files
+            # requirement files
             for f in ${requirement_files[@]}; do
-                cf_color magenta "installing requirement file ${f}"
-                pip install -r "${f}"
-                [ "$?" != "0" ] && clear_pending && return "29"
-                echo
+                add_requirements -r "${f}"
             done
 
-            # clear the pip cache
-            pip cache -q purge 2> /dev/null
-
-            # ensure that the venv is relocateable
-            cf_make_venv_relocateable "${venv_name_hashed}"
-            [ "$?" != "0" ] && clear_pending && return "30"
+            # actual installation
+            eval "python -m pip install -I -U --no-cache-dir ${install_reqs}"
+            [ "$?" != "0" ] && clear_pending && return "27"
+            echo
 
             # write the version and a timestamp into the flag file
             echo "version ${venv_version}" > "${CF_SANDBOX_FLAG_FILE}"
diff --git a/sandboxes/cf.sh b/sandboxes/cf.sh
index 993eb07a3..631d00d94 100644
--- a/sandboxes/cf.sh
+++ b/sandboxes/cf.sh
@@ -11,7 +11,7 @@ action() {
     # set variables and source the generic venv setup
     export CF_SANDBOX_FILE="${CF_SANDBOX_FILE:-${this_file}}"
     export CF_VENV_NAME="$( basename "${this_file%.sh}" )"
-    export CF_VENV_REQUIREMENTS="${CF_BASE}/requirements_prod.txt"
+    export CF_VENV_REQUIREMENTS="${this_dir}/cf.txt"
 
     source "${this_dir}/_setup_venv.sh" "$@"
 }
diff --git a/requirements_prod.txt b/sandboxes/cf.txt
similarity index 100%
rename from requirements_prod.txt
rename to sandboxes/cf.txt
diff --git a/sandboxes/cf_dev.sh b/sandboxes/cf_dev.sh
index d96c14117..b993d399d 100644
--- a/sandboxes/cf_dev.sh
+++ b/sandboxes/cf_dev.sh
@@ -11,7 +11,7 @@ action() {
     # set variables and source the generic venv setup
     export CF_SANDBOX_FILE="${CF_SANDBOX_FILE:-${this_file}}"
     export CF_VENV_NAME="$( basename "${this_file%.sh}" )"
-    export CF_VENV_REQUIREMENTS="${CF_BASE}/requirements_dev.txt,${CF_BASE}/requirements_prod.txt"
+    export CF_VENV_REQUIREMENTS="${this_dir}/dev.txt"
 
     source "${this_dir}/_setup_venv.sh" "$@"
 }
diff --git a/requirements_dev.txt b/sandboxes/dev.txt
similarity index 54%
rename from requirements_dev.txt
rename to sandboxes/dev.txt
index c6d230c05..56d89e667 100644
--- a/requirements_dev.txt
+++ b/sandboxes/dev.txt
@@ -1,5 +1,4 @@
-# version 6
-# packages might be installed in addition to other requirements
+# version 7
 
 ipython~=8.8
 pytest~=7.1
@@ -7,3 +6,4 @@ pytest-cov~=3.0
 flake8~=5.0
 flake8-commas~=2.1
 flake8-quotes~=3.3
+pipdeptree~=2.13
diff --git a/sandboxes/venv_columnar_dev.sh b/sandboxes/venv_columnar_dev.sh
index e479012ec..22e188091 100644
--- a/sandboxes/venv_columnar_dev.sh
+++ b/sandboxes/venv_columnar_dev.sh
@@ -11,7 +11,7 @@ action() {
     # set variables and source the generic venv setup
     export CF_SANDBOX_FILE="${CF_SANDBOX_FILE:-${this_file}}"
     export CF_VENV_NAME="$( basename "${this_file%.sh}" )"
-    export CF_VENV_REQUIREMENTS="${CF_BASE}/requirements_dev.txt,${this_dir}/columnar.txt"
+    export CF_VENV_REQUIREMENTS="${this_dir}/columnar.txt,${this_dir}/dev.txt"
 
     source "${this_dir}/_setup_venv.sh" "$@"
 }
diff --git a/sandboxes/venv_docs_dev.sh b/sandboxes/venv_docs_dev.sh
index c5aad23c5..2052833bc 100644
--- a/sandboxes/venv_docs_dev.sh
+++ b/sandboxes/venv_docs_dev.sh
@@ -11,7 +11,7 @@ action() {
     # set variables and source the generic venv setup
     export CF_SANDBOX_FILE="${CF_SANDBOX_FILE:-${this_file}}"
     export CF_VENV_NAME="$( basename "${this_file%.sh}" )"
-    export CF_VENV_REQUIREMENTS="${CF_BASE}/requirements_dev.txt,${CF_BASE}/docs/requirements.txt"
+    export CF_VENV_REQUIREMENTS="${CF_BASE}/docs/requirements.txt,${this_dir}/dev.txt"
 
     source "${this_dir}/_setup_venv.sh" "$@"
 }
diff --git a/sandboxes/venv_ml_tf_dev.sh b/sandboxes/venv_ml_tf_dev.sh
index 0c25bfde8..c5056339a 100644
--- a/sandboxes/venv_ml_tf_dev.sh
+++ b/sandboxes/venv_ml_tf_dev.sh
@@ -11,7 +11,7 @@ action() {
     # set variables and source the generic venv setup
     export CF_SANDBOX_FILE="${CF_SANDBOX_FILE:-${this_file}}"
     export CF_VENV_NAME="$( basename "${this_file%.sh}" )"
-    export CF_VENV_REQUIREMENTS="${CF_BASE}/requirements_dev.txt,${this_dir}/ml_tf.txt"
+    export CF_VENV_REQUIREMENTS="${this_dir}/ml_tf.txt,${this_dir}/dev.txt"
 
     source "${this_dir}/_setup_venv.sh" "$@"
 }
diff --git a/setup.py b/setup.py
index 528da111f..27ab10b9f 100644
--- a/setup.py
+++ b/setup.py
@@ -9,8 +9,8 @@
 
 
 keywords = [
-    "physics", "analysis", "experiment", "columnar", "vectoized",
-    "law", "order", "luigi", "lhc", "cms",
+    "physics", "analysis", "experiment", "columnar", "vectorized",
+    "law", "order", "luigi", "workflow", "automation", "lhc", "cern",
 ]
 
 
@@ -36,7 +36,7 @@
 
 
 # load installation requirements
-with open(os.path.join(this_dir, "requirements_prod.txt"), "r") as f:
+with open(os.path.join(this_dir, "sandboxes", "cf.txt"), "r") as f:
     install_requires = [line.strip() for line in f.readlines() if line.strip()]
 
 
@@ -59,7 +59,7 @@
     long_description=long_description,
     long_description_content_type="text/markdown",
     install_requires=install_requires,
-    python_requires=">=3.7, <4",
+    python_requires=">=3.7, <=3.9",
     zip_safe=False,
     packages=find_packages(exclude=["tests"]),
 )
diff --git a/setup.sh b/setup.sh
index 03e057c09..5e954550d 100644
--- a/setup.sh
+++ b/setup.sh
@@ -617,6 +617,17 @@ EOF
 cf_setup_git_hooks() {
     # Initializes lfs and custom githooks in the local checkout for both the columnflow
     # (sub)repository, as well as the analysis repository in case a directory bin/githooks is found.
+    #
+    # Optional environments variables:
+    #   CF_REMOTE_JOB
+    #       When "1", no hooks are setup.
+    #   CF_CI_JOB
+    #       When "1", no hooks are setup.
+
+    # do nothing when not local
+    if [ "${CF_REMOTE_JOB}" = "1" ] || [ "${CF_CI_JOB}" = "1" ]; then
+        return "0"
+    fi
 
     # helper to setup hooks
     setup_hooks() {


Test patch:
diff --git a/tests/run_linting b/tests/run_linting
index ff09013e1..564fbf887 100755
--- a/tests/run_linting
+++ b/tests/run_linting
@@ -10,7 +10,7 @@ action() {
 
     (
         cd "${cf_dir}" && \
-        flake8 columnflow tests docs setup.py
+        flake8 columnflow tests bin docs setup.py
     )
 }
 action "$@"