Merge branch 'review-nov22' of github.com:DiamondLightSource/python3-pip-skeleton into review-nov22

Author: gilesknap

.devcontainer.json

@@ -5,7 +5,9 @@
         "dockerfile": "Dockerfile",
         "target": "build",
         "context": ".",
-        "args": {}
+        "args": {
+            "PIP_OPTIONS": "-e .[dev]"
+        }
     },
     "remoteEnv": {
         "DISPLAY": "${localEnv:DISPLAY}"
@@ -24,6 +26,7 @@
     "initializeCommand": "bash -c 'for i in $HOME/.inputrc; do [ -f $i ] || touch $i; done'",
     "runArgs": [
         "--net=host",
+        "--security-opt=label=type:container_runtime_t",
         "-v=${localEnv:HOME}/.ssh:/root/.ssh",
         "-v=${localEnv:HOME}/.inputrc:/root/.inputrc"
     ],
@@ -33,7 +36,5 @@
     ],
     // make the workspace folder the same inside and outside of the container
     "workspaceMount": "source=${localWorkspaceFolder},target=${localWorkspaceFolder},type=bind",
-    "workspaceFolder": "${localWorkspaceFolder}",
-    // After the container is created, install the python project in editable form
-    "postCreateCommand": "pip install $([ -f requirements_dev.txt ] && echo -r requirements_dev.txt ) -e .[dev]"
+    "workspaceFolder": "${localWorkspaceFolder}"
 }

.dockerignore

@@ -1,3 +1,5 @@
+# Ignore folders that can be large.
+# This saves time copying them into the context at the start of a build.
 build/
 .mypy_cache
 .tox

.github/actions/install_requirements/action.yml

@@ -0,0 +1,58 @@
+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
+
+    - name: Upload lockfiles
+      uses: actions/upload-artifact@v3
+      with:
+        name: lockfiles
+        path: lockfiles
+
+    # 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: If requirements file exists, check it matches pip installed packages
+      run: |
+        if [ -s ${{ inputs.requirements_file }} ]; then
+          if ! diff -u ${{ inputs.requirements_file }} lockfiles/${{ inputs.requirements_file }}; then
+            echo "Error: ${{ inputs.requirements_file }} need the above changes to be exhaustive"
+            exit 1
+          fi
+        fi
+      shell: bash
+

.github/workflows/code.yml

@@ -6,6 +6,9 @@ on:
   schedule:
     # Run every Monday at 8am to check latest versions of dependencies
     - cron: "0 8 * * WED"
+env:
+  # The target python version, which must match the Dockerfile version
+  CONTAINER_PYTHON: "3.11"
 
 jobs:
   lint:
@@ -17,29 +20,28 @@ jobs:
       - name: Checkout
         uses: actions/checkout@v3
 
-      - name: Setup python
-        uses: actions/setup-python@v4
+      - name: Install python packages
+        uses: ./.github/actions/install_requirements
         with:
-          python-version: "3.x"
+          requirements_file: requirements-dev-3.x.txt
+          install_options: -e .[dev]
 
       - name: Lint
-        run: |
-          .github/workflows/pip_install.sh lint -e .[dev]
-          tox -e pre-commit,mypy
-
-      - name: Upload lockfiles
-        uses: actions/upload-artifact@v3
-        with:
-          name: lockfiles
-          path: lockfiles
+        run: tox -e pre-commit,mypy
 
   test:
     if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.repository
     strategy:
       fail-fast: false
       matrix:
         os: ["ubuntu-latest"] # can add windows-latest, macos-latest
-        python: ["3.8", "3.9", "3.10", "3.11"]
+        python: ["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:
@@ -55,62 +57,61 @@ jobs:
       - name: Checkout
         uses: actions/checkout@v3
         with:
+          # Need this to get version number from last tag
           fetch-depth: 0
 
-      - name: Setup python ${{ matrix.python }}
-        uses: actions/setup-python@v4
+      - name: Install python packages
+        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
+        run: pytest
 
       - name: Upload coverage to Codecov
         uses: codecov/codecov-action@v3
         with:
           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"
 
     steps:
-      - name: Checkout Source
+      - name: Checkout
         uses: actions/checkout@v3
         with:
+          # Need this to get version number from last tag
           fetch-depth: 0
 
-      - name: Build Sdist and wheel
+      - name: Build sdist and wheel
         run: |
           export SOURCE_DATE_EPOCH=$(git log -1 --pretty=%ct) && \
           pipx run build
 
-      - name: Test module --version works using the installed wheel
-        run: |
-          touch requirements.txt
-          pip install -r requirements.txt dist/*.whl
-          # If more than one module in src/ replace with module name to test
-          python -m $(ls src | head -1) --version
-
-      - name: Check for packaging errors
-        run: pipx run twine check dist/*
-
       - name: Upload sdist and wheel as artifacts
         uses: actions/upload-artifact@v3
         with:
           name: dist
           path: dist
 
+      - name: Check for packaging errors
+        run: pipx run twine check dist/*
+
+      - name: Install python packages
+        uses: ./.github/actions/install_requirements
+        with:
+          python_version: ${{env.CONTAINER_PYTHON}}
+          requirements_file: requirements.txt
+          install_options: dist/*.whl
+
+      - name: Test module --version works using the installed wheel
+        # If more than one module in src/ replace with module name to test
+        run: python -m $(ls src | head -1) --version
+
   container:
     needs: [lint, dist, test]
     runs-on: ubuntu-latest
@@ -124,11 +125,11 @@ 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
+      - name: Download wheel and lockfiles
+        uses: actions/download-artifact@v3
 
       - name: Log in to GitHub Docker Registry
         if: github.event_name != 'pull_request'
@@ -154,6 +155,8 @@ jobs:
       - name: Build runtime image
         uses: docker/build-push-action@v3
         with:
+          build-args: |
+            PIP_OPTIONS=-r lockfiles/requirements.txt dist/*.whl
           push: ${{ github.event_name == 'push' && startsWith(github.ref, 'refs/tags') }}
           load: ${{ ! (github.event_name == 'push' && startsWith(github.ref, 'refs/tags')) }}
           tags: ${{ steps.meta.outputs.tags }}
@@ -162,17 +165,7 @@ 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
-
-      - name: Upload lockfiles
-        uses: actions/upload-artifact@v3
-        with:
-          name: lockfiles
-          path: lockfiles
+        run: docker run ${{ env.IMAGE_REPOSITORY }} --version
 
   release:
     # upload to PyPI and make a release on every tag
@@ -183,6 +176,10 @@ jobs:
     steps:
       - uses: actions/download-artifact@v3
 
+      - name: Fixup blank lockfiles
+        # Github release artifacts can't be blank
+        run: for f in lockfiles/*; [ -s $f ] || echo '# No requirements' >> $f; done
+
       - 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/docs.yml

@@ -7,36 +7,28 @@ on:
 jobs:
   docs:
     if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.repository
-    strategy:
-      fail-fast: false
-      matrix:
-        python: ["3.10"]
-
     runs-on: ubuntu-latest
 
     steps:
       - name: Avoid git conflicts when tag and branch pushed at same time
         if: startsWith(github.ref, 'refs/tags')
         run: sleep 60
 
-      - name: Install python version
-        uses: actions/setup-python@v4
+      - name: Checkout
+        uses: actions/checkout@v3
         with:
-          python-version: ${{ matrix.python }}
+          # Need this to get version number from last tag
+          fetch-depth: 0
 
-      - name: Install Packages
+      - name: Install system packages
         # Can delete this if you don't use graphviz in your docs
         run: sudo apt-get install graphviz
 
-      - name: checkout
-        uses: actions/checkout@v3
+      - name: Install python packages
+        uses: ./.github/actions/install_requirements
         with:
-          fetch-depth: 0
-
-      - name: Install dependencies
-        run: |
-          touch requirements_dev.txt
-          pip install -r requirements_dev.txt -e .[dev]
+          requirements_file: requirements-dev-3.x.txt
+          install_options: -e .[dev]
 
       - name: Build docs
         run: tox -e docs

.github/workflows/docs_clean.yml

@@ -17,7 +17,7 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-      - name: checkout
+      - name: Checkout
         uses: actions/checkout@v3
         with:
           ref: gh-pages
@@ -35,7 +35,7 @@ jobs:
 
       - name: update index and push changes
         run: |
-          rm -r ${{ env.DOCS_VERSION }}
+          rm -r $DOCS_VERSION
           python make_switcher.py --remove $DOCS_VERSION ${{ github.repository }} switcher.json
           git config --global user.name 'GitHub Actions Docs Cleanup CI'
           git config --global user.email '[email protected]'