changes from PR review

Author: gilesknap

.github/actions/install_requirements/action.yml

@@ -0,0 +1,54 @@
+name: "install requirements"
+description: "run pip install with requirements and upload resulting requirements"
+inputs:
+  requirements_file:
+    description: "name of requirements file to use and upload"
+    required: true
+  install_options:
+    description: "parameters to pass to pip install"
+    required: true
+  python_version:
+    description: "python version to install"
+    default: "3.x"
+
+runs:
+  using: "composite"
+
+  steps:
+    - name: Setup python
+      uses: actions/setup-python@v4
+      with:
+        python-version: ${{ inputs.python_version }}
+
+    - name: Pip Install
+      run: |
+        touch ${{ inputs.requirements_file }}
+        # -c uses requirements.txt as constraints, see 'Validate requirements file'
+        pip install -c ${{ inputs.requirements_file }} ${{ inputs.install_options }}
+      shell: bash
+
+    - name: Create Lockfile
+      run: |
+        mkdir -p lockfiles
+        pip freeze --exclude-editable > lockfiles/${{ inputs.requirements_file }}
+        # delete the self referencing line
+        sed -i  '/file:/d' lockfiles/${{ inputs.requirements_file }}
+      shell: bash
+
+    # This eliminates the class of problems where the requirements being given no
+    # longer match what the packages themselves dictate. E.g. In the rare instance
+    # where I install some-package which used to depend on vulnerable-dependency
+    # but now uses good-dependency (despite being nominally the same version)
+    # pip will install both if given a requirements file with -r
+    - name: Validate requirements file
+      run: if [ -s ${{ inputs.requirements_file }} ] ; then
+        diff ${{ inputs.requirements_file }} lockfiles/${{ inputs.requirements_file }};
+        echo "requirements files match";
+        fi
+      shell: bash
+
+    - name: Upload lockfiles
+      uses: actions/upload-artifact@v3
+      with:
+        name: lockfiles
+        path: lockfiles

.github/workflows/code.yml

@@ -17,15 +17,14 @@ jobs:
       - name: Checkout
         uses: actions/checkout@v3
 
-      - name: Setup python
-        uses: actions/setup-python@v4
+      - name: Install
+        uses: ./.github/actions/install_requirements
         with:
-          python-version: "3.x"
+          requirements_file: requirements-lint.txt
+          install_options: -e .[dev]
 
       - name: Lint
-        run: |
-          .github/workflows/pip_install.sh lint -e .[dev]
-          tox -e pre-commit,mypy
+        run: tox -e pre-commit,mypy
 
       - name: Upload lockfiles
         uses: actions/upload-artifact@v3
@@ -40,6 +39,12 @@ jobs:
       matrix:
         os: ["ubuntu-latest"] # can add windows-latest, macos-latest
         python: ["3.8", "3.9", "3.10", "3.11"]
+        install: ["-e .[dev]"]
+        # Make one version be non-editable to test both paths of version code
+        include:
+          - os: "ubuntu-latest"
+            python: "3.8"
+            install: ".[dev]"
 
     runs-on: ${{ matrix.os }}
     env:
@@ -52,13 +57,12 @@ jobs:
         with:
           fetch-depth: 0
 
-      - name: Setup python ${{ matrix.python }}
-        uses: actions/setup-python@v4
+      - name: Install
+        uses: ./.github/actions/install_requirements
         with:
-          python-version: ${{ matrix.python }}
-
-      - name: Install with latest dependencies
-        run: .github/workflows/pip_install.sh test-${{ matrix.python }}-${{ matrix.os }} .[dev]
+          python_version: ${{ matrix.python }}
+          requirements_file: requirements-test-${{ matrix.os }}-${{ matrix.python }}.txt
+          install_options: ${{ matrix.install }}
 
       - name: Run tests
         run: pytest tests
@@ -69,12 +73,6 @@ jobs:
           name: ${{ matrix.python }}/${{ matrix.os }}
           files: cov.xml
 
-      - name: Upload lockfiles
-        uses: actions/upload-artifact@v3
-        with:
-          name: lockfiles
-          path: lockfiles
-
   dist:
     if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.repository
     runs-on: "ubuntu-latest"
@@ -119,8 +117,8 @@ jobs:
         uses: actions/checkout@v3
 
       # image names must be all lower case
-      - run: |
-          echo IMAGE_REPOSITORY=ghcr.io/$(tr '[:upper:]' '[:lower:]' <<< "${{ github.repository }}") >> $GITHUB_ENV
+      - name: Generate image repo name
+        run: echo IMAGE_REPOSITORY=ghcr.io/$(tr '[:upper:]' '[:lower:]' <<< "${{ github.repository }}") >> $GITHUB_ENV
 
       # obtain the python wheel from the dist step
       - uses: actions/download-artifact@v3
@@ -157,11 +155,11 @@ jobs:
           cache-to: type=gha,mode=max
 
       - name: Test cli works in runtime image
-        # check that the latest tag can run with --version parameter
         run: |
           docker run ${{ env.IMAGE_REPOSITORY }} --version
           mkdir -p lockfiles
-          docker run --entrypoint pip ${{ env.IMAGE_REPOSITORY }} freeze > lockfiles/requirements.txt
+          docker run --entrypoint pip ${{ env.IMAGE_REPOSITORY }} freeze | \
+            sed '/file:/s/^/# Requirements for /' > lockfiles/requirements.txt
 
       - name: Upload lockfiles
         uses: actions/upload-artifact@v3
@@ -178,10 +176,6 @@ jobs:
     steps:
       - uses: actions/download-artifact@v3
 
-      - name: fixup requirements files
-        # use sed to comment out the self references in requirements files
-        run: sed -i '/file:/s/^/# Requirements for /' lockfiles/requirements*.txt
-
       - name: Github Release
         # We pin to the SHA, not the tag, for security reasons.
         # https://docs.github.com/en/actions/learn-github-actions/security-hardening-for-github-actions#using-third-party-actions

.github/workflows/pip_install.sh

@@ -6,10 +6,11 @@
 
 # usage: install_with_requirements.sh suffix [remaining parameters to pip install]
 
-suffix=$1
+requirements_file=$1
 shift
 
-touch requirements-${suffix}.txt
-pip install -r requirements-${suffix}.txt "${@}"
+touch ${requirements_file}
+pip install -r ${requirements_file} "${@}"
 mkdir -p lockfiles
-pip freeze --exclude-editable > lockfiles/requirements-${suffix}.txt
+# get a freeze of the installed packages but delete the self referencing line
+pip freeze --exclude-editable | sed '/file:/d' > lockfiles/${requirements_file}

Dockerfile

@@ -8,9 +8,8 @@ FROM python:3.11 as build
 # Add any system dependencies for the developer/build environment here e.g.
 # RUN apt-get update && apt-get upgrade -y && \
 #     apt-get install -y --no-install-recommends \
-#     busybox \
-#     && rm -rf /var/lib/apt/lists/* \
-#     && busybox --install
+#     desired-packages \
+#     && rm -rf /var/lib/apt/lists/*
 
 COPY . /project
 WORKDIR /project