Improve dev tools in the Nix environment (#1666)

* Improve dev tools in the nix environment

Added postgrest-build, postgrest-run, postgrest-clean, postgrest-check
and postgrest-watch tools. Changed order of postgrest-test-spec-all to
run backwards from pg13 to pg9.5. Added shellcheck to lint shell scripts
in nix environment.

Co-authored-by: monacoremo <monacoremo>

.circleci/config.yml

@@ -10,7 +10,7 @@ jobs:
       - checkout
       - run:
           name: Install linting and styling scripts
-          command: nix-env -f default.nix -iA devtools
+          command: nix-env -f default.nix -iA style
       - run:
           name: Run linter
           command: |

default.nix

@@ -31,9 +31,10 @@ let
 
   overlays =
     [
-      allOverlays.postgresql-default
+      allOverlays.checked-shell-script
-      allOverlays.gitignore
       allOverlays.ghr
+      allOverlays.gitignore
+      allOverlays.postgresql-default
       (allOverlays.haskell-packages { inherit compiler; })
     ];
 
@@ -42,14 +43,14 @@ let
     import nixpkgs { inherit overlays; };
 
   postgresqlVersions =
-    {
+    [
-      postgresql-13 = pkgs.postgresql_13;
+      { name = "postgresql-13"; postgresql = pkgs.postgresql_13; }
-      postgresql-12 = pkgs.postgresql_12;
+      { name = "postgresql-12"; postgresql = pkgs.postgresql_12; }
-      postgresql-11 = pkgs.postgresql_11;
+      { name = "postgresql-11"; postgresql = pkgs.postgresql_11; }
-      postgresql-10 = pkgs.postgresql_10;
+      { name = "postgresql-10"; postgresql = pkgs.postgresql_10; }
-      "postgresql-9.6" = pkgs.postgresql_9_6;
+      { name = "postgresql-9.6"; postgresql = pkgs.postgresql_9_6; }
-      "postgresql-9.5" = pkgs.postgresql_9_5;
+      { name = "postgresql-9.5"; postgresql = pkgs.postgresql_9_5; }
-    };
+    ];
 
   patches =
     pkgs.callPackage nix/patches { };
@@ -63,6 +64,10 @@ let
   staticHaskellPackage =
     import nix/static-haskell-package.nix { inherit nixpkgs compiler patches allOverlays; };
 
+  # Options passed to cabal in dev tools and tests
+  devCabalOptions =
+    "-f FailOnWarn --test-show-detail=direct";
+
   profiledHaskellPackages =
     pkgs.haskell.packages."${compiler}".extend (self: super:
       {
@@ -109,11 +114,15 @@ rec {
 
   # Scripts for running tests.
   tests =
-    pkgs.callPackage nix/tests.nix { inherit postgrest postgrestStatic postgrestProfiled postgresqlVersions; };
+    pkgs.callPackage nix/tests.nix { inherit postgrest postgrestStatic postgrestProfiled postgresqlVersions devCabalOptions; };
+
+  # Linting and styling scripts.
+  style =
+    pkgs.callPackage nix/style.nix { };
 
   # Development tools, including linting and styling scripts.
   devtools =
-    pkgs.callPackage nix/devtools.nix { };
+    pkgs.callPackage nix/devtools.nix { inherit tests style devCabalOptions; };
 
   # Scripts for publishing new releases.
   release =

nix/README.md

@@ -59,7 +59,7 @@ Within `nix-shell`, you can run Cabal commands as usual. You can also run
 stack with the `--nix` option, which causes stack to pick up the non-Haskell
 dependencies from the same pinned Nixpkgs version that the Nix builds use.
 
-## Aside: Working with `nix-shell` and the PostgREST utility scripts
+## Working with `nix-shell` and the PostgREST utility scripts
 
 The PostgREST utilities available in `nix-shell` all have names that begin with
 `postgrest-`, so you can use tab completion (typing `postgrest-` and pressing
@@ -74,6 +74,7 @@ postgrest-style-check               postgrest-test-spec-postgresql-13
 postgrest-test-spec                 postgrest-test-spec-postgresql-9.5
 postgrest-test-spec-all             postgrest-test-spec-postgresql-9.6
 postgrest-test-spec-postgresql-10
+...
 
 [nix-shell]$
 
@@ -94,6 +95,7 @@ postgrest-style-check               postgrest-test-spec-postgresql-12
 postgrest-test-io                   postgrest-test-spec-postgresql-13
 postgrest-test-spec                 postgrest-test-spec-postgresql-9.5
 postgrest-test-spec-all             postgrest-test-spec-postgresql-9.6
+...
 
 ```
 
@@ -161,6 +163,20 @@ $ nix-shell --run postgrest-style
 
 ```
 
+There is also `postgrest-style-check` that exits with a non-zero exit code if
+the check resulted in any uncommited changes. It's mostly useful for CI.
+
+## General development tools
+
+Tools like `postgrest-build`, `postgrest-run` etc. are simple wrappers around
+`cabal` and should do what you expect. `postgrest-check` runs most checks that will
+also run in CI, with the exception of the IO and Memory checks that need to be run
+separately.
+
+`postgrest-watch` takes a command as an argument that it will re-run if any source
+file is changed. For example, `postgrest-watch postgrest-test-spec-all` will re-run
+the full spec test suite against all PostgreSQL versions on every change.
+
 ## Tour
 
 The following is not required for working on PostgREST with Nix, but it will

nix/devtools.nix

@@ -1,49 +1,59 @@
-{ writeShellScriptBin
+{ buildEnv
-, buildEnv
+, cabal-install
+, checkedShellScript
+, devCabalOptions
+, entr
 , git
-, hlint
-, nixpkgs-fmt
 , silver-searcher
-, stylish-haskell
+, style
+, tests
 }:
 let
-  style =
+  watch =
-    writeShellScriptBin "postgrest-style"
+    checkedShellScript "postgrest-watch"
       ''
-        set -euo pipefail
-
         rootdir="$(${git}/bin/git rev-parse --show-toplevel)"
 
-        # Format Nix files
+        ${silver-searcher}/bin/ag -l . "$rootdir" | ${entr}/bin/entr -r "$@"
-        ${nixpkgs-fmt}/bin/nixpkgs-fmt "$rootdir" > /dev/null 2> /dev/null
+      '';
 
-        # Format Haskell files
+  pushCachix =
-        ${silver-searcher}/bin/ag -l -g '\.l?hs$' "$rootdir" \
+    checkedShellScript "postgrest-push-cachix"
-          | xargs ${stylish-haskell}/bin/stylish-haskell -i
+      ''
+        nix-store -qR --include-outputs "$(nix-instantiate)" \
+          | cachix push postgrest
+      '';
+
+  build =
+    checkedShellScript "postgrest-build"
+      ''exec ${cabal-install}/bin/cabal v2-build ${devCabalOptions} "$@"'';
+
+  run =
+    checkedShellScript "postgrest-run"
+      ''exec ${cabal-install}/bin/cabal v2-run postgrest ${devCabalOptions} -- "$@"'';
+
+  clean =
+    checkedShellScript "postgrest-clean"
+      ''
+        ${cabal-install}/bin/cabal v2-clean
       '';
 
   check =
-    writeShellScriptBin "postgrest-style-check"
+    checkedShellScript "postgrest-check"
       ''
-        set -euo pipefail
+        ${tests}/bin/postgrest-test-spec-all
-
+        ${style}/bin/postgrest-lint
-        ${style}/bin/${style.name}
+        ${style}/bin/postgrest-style-check
-
-        ${git}/bin/git diff-index --exit-code HEAD -- '*.hs' '*.lhs' '*.nix'
-      '';
-  lint =
-    writeShellScriptBin "postgrest-lint"
-      ''
-        set -euo pipefail
-
-        rootdir="$(${git}/bin/git rev-parse --show-toplevel)"
-
-        # Lint Haskell files
-        ${silver-searcher}/bin/ag -l -g '\.l?hs$' "$rootdir" \
-          | xargs ${hlint}/bin/hlint -X QuasiQuotes -X NoPatternSynonyms
       '';
 in
 buildEnv {
   name = "postgrest-devtools";
-  paths = [ style check lint ];
+  paths = [
+    watch.bin
+    pushCachix.bin
+    build.bin
+    run.bin
+    clean.bin
+    check.bin
+  ];
 }

nix/docker/default.nix

@@ -1,4 +1,8 @@
-{ buildEnv, postgrest, dockerTools, writeShellScriptBin }:
+{ buildEnv
+, postgrest
+, dockerTools
+, checkedShellScript
+}:
 let
   config =
     ./postgrest.conf;
@@ -52,7 +56,7 @@ let
 
   # Helper script for loading the image.
   load =
-    writeShellScriptBin "postgrest-docker-load"
+    checkedShellScript "postgrest-docker-load"
       ''
         docker load -i ${image}
       '';
@@ -60,5 +64,5 @@ in
 buildEnv
   {
     name = "postgrest-docker";
-    paths = [ load ];
+    paths = [ load.bin ];
   } // { inherit image config; }

nix/nixpkgs-upgrade.nix

@@ -1,6 +1,6 @@
-{ curl
+{ checkedShellScript
+, curl
 , jq
-, writeShellScriptBin
 , nix
 }:
 # Utility script for pinning the latest unstable version of Nixpkgs.
@@ -20,23 +20,24 @@ let
 
   tarballUrlBase =
     https://github.com/nixos/nixpkgs/archive/;
+
+  script =
+    checkedShellScript
+      name
+      ''
+        commitHash="$(${curl}/bin/curl "${refUrl}" -H "${githubV3Header}" | ${jq}/bin/jq -r .object.sha)"
+        tarballUrl="${tarballUrlBase}$commitHash.tar.gz"
+        tarballHash="$(${nix}/bin/nix-prefetch-url --unpack "$tarballUrl")"
+        currentDate="$(date --iso)"
+
+        cat << EOF
+        # Pinned version of Nixpkgs, generated with ${name}.
+        {
+          date = "$currentDate";
+          rev = "$commitHash";
+          tarballHash = "$tarballHash";
+        }
+        EOF
+      '';
 in
-writeShellScriptBin
+script.bin
-  name
-  ''
-    set -euo pipefail
-
-    commitHash="$(${curl}/bin/curl "${refUrl}" -H "${githubV3Header}" | ${jq}/bin/jq -r .object.sha)"
-    tarballUrl="${tarballUrlBase}$commitHash.tar.gz"
-    tarballHash="$(${nix}/bin/nix-prefetch-url --unpack "$tarballUrl")"
-    currentDate="$(date --iso)"
-
-    cat << EOF
-    # Pinned version of Nixpkgs, generated with ${name}.
-    {
-      date = "$currentDate";
-      rev = "$commitHash";
-      tarballHash = "$tarballHash";
-    }
-    EOF
-  ''

nix/overlays/checked-shell-script/checked-shell-script.nix

@@ -0,0 +1,43 @@
+# Create a bash script that is checked with shellcheck. You can either use it
+# directly, or use the .bin attribute to get the script in a bin/ directory,
+# to be used in a path for example.
+{ writeTextFile
+, runtimeShell
+, runCommand
+, stdenv
+, shellcheck
+}:
+name: text:
+let
+  writeBin =
+    name: text:
+    writeTextFile {
+      inherit name;
+      executable = true;
+      destination = "/bin/${name}";
+
+      text =
+        ''
+          #!${runtimeShell}
+          set -euo pipefail
+
+          ${text}
+        '';
+
+      checkPhase =
+        ''
+          # check syntax
+          ${stdenv.shell} -n $out/bin/${name}
+
+          # check for shellcheck recommendations
+          ${shellcheck}/bin/shellcheck $out/bin/${name}
+        '';
+    };
+
+  bin =
+    writeBin name text;
+
+  script =
+    runCommand name { inherit bin name; } "ln -s $bin/bin/$name $out";
+in
+script // { inherit bin; }

nix/overlays/checked-shell-script/default.nix

@@ -0,0 +1,6 @@
+self: super:
+# Overlay that adds `checkedShellScript`, an enhanced version of
+# writeShellScript and writeShellScriptBin
+{
+  checkedShellScript = super.callPackage ./checked-shell-script.nix { };
+}

nix/overlays/default.nix

@@ -1,6 +1,7 @@
 {
-  gitignore = import ./gitignore.nix;
+  checked-shell-script = import ./checked-shell-script;
   ghr = import ./ghr;
+  gitignore = import ./gitignore.nix;
   haskell-packages = import ./haskell-packages.nix;
   postgresql-default = import ./postgresql-default.nix;
 }

nix/release/default.nix

@@ -6,21 +6,19 @@
 , postgrest
 , docker
 , runCommand
-, writeShellScriptBin
+, checkedShellScript
 }:
 let
   # Script for publishing a new release on GitHub.
   github =
-    writeShellScriptBin "postgrest-release-github"
+    checkedShellScript "postgrest-release-github"
       ''
-        set -euo pipefail
-
         version=$1
 
-        if test $version = "nightly"
+        if test "$version" = "nightly"
         then
           suffix=$(git show -s --format="%cd-%h" --date="format:%Y-%m-%d-%H-%M")
-          tar cvJf postgrest-nightly-$suffix-linux-x64-static.tar.xz \
+          tar cvJf "postgrest-nightly-$suffix-linux-x64-static.tar.xz" \
             -C ${postgrest}/bin postgrest
 
           ${ghr}/bin/ghr \
@@ -28,11 +26,11 @@ let
             -u "$GITHUB_USERNAME" \
             -r "$GITHUB_REPONAME" \
             --replace nightly \
-            postgrest-nightly-$suffix-linux-x64-static.tar.xz
+            "postgrest-nightly-$suffix-linux-x64-static.tar.xz"
         else
           changes="$(sed -n "1,/$version/d;/## \[/q;p" ${../../CHANGELOG.md})"
 
-          tar cvJf postgrest-$version-linux-x64-static.tar.xz \
+          tar cvJf "postgrest-$version-linux-x64-static.tar.xz" \
             -C ${postgrest}/bin postgrest
 
           ${ghr}/bin/ghr \
@@ -40,43 +38,39 @@ let
             -u "$GITHUB_USERNAME" \
             -r "$GITHUB_REPONAME" \
             -b "$changes" \
-            --replace $version \
+            --replace "$version" \
-            postgrest-$version-linux-x64-static.tar.xz
+            "postgrest-$version-linux-x64-static.tar.xz"
         fi
       '';
 
   # Wrapper for login with docker. $DOCKER_USER/$DOCKER_PASS vars come from CircleCI.
   # The DOCKER_USER is not the same as DOCKER_REPO because we use the https://hub.docker.com/u/postgrestbot account for uploading to dockerhub.
   dockerLogin =
-    writeShellScriptBin "postgrest-docker-login"
+    checkedShellScript "postgrest-docker-login"
       ''
-        set -euo pipefail
+        docker login -u "$DOCKER_USER" -p "$DOCKER_PASS"
-
-        docker login -u $DOCKER_USER -p $DOCKER_PASS
       '';
 
   # Script for publishing a new release on Docker Hub.
   dockerHub =
-    writeShellScriptBin "postgrest-release-dockerhub"
+    checkedShellScript "postgrest-release-dockerhub"
       ''
-        set -euo pipefail
-
         version=$1
 
         docker load -i ${docker.image}
 
-        if test $version = "nightly"
+        if test "$version" = "nightly"
         then
           suffix=$(git show -s --format="%cd-%h" --date="format:%Y-%m-%d-%H-%M")
 
-          docker tag postgrest:latest "$DOCKER_REPO"/postgrest:nightly-$suffix
+          docker tag postgrest:latest "$DOCKER_REPO/postgrest:nightly-$suffix"
-          docker push "$DOCKER_REPO"/postgrest:nightly-$suffix
+          docker push "$DOCKER_REPO/postgrest:nightly-$suffix"
         else
           docker tag postgrest:latest "$DOCKER_REPO"/postgrest:latest
-          docker tag postgrest:latest "$DOCKER_REPO"/postgrest:$version
+          docker tag postgrest:latest "$DOCKER_REPO/postgrest:$version"
 
           docker push "$DOCKER_REPO"/postgrest:latest
-          docker push "$DOCKER_REPO"/postgrest:$version
+          docker push "$DOCKER_REPO/postgrest:$version"
         fi
       '';
 
@@ -89,10 +83,8 @@ let
       fullDescription =
         ./docker-hub-full-description.md;
     in
-    writeShellScriptBin "postgrest-release-dockerhubdescription"
+    checkedShellScript "postgrest-release-dockerhubdescription"
       ''
-        set -euo pipefail
-
         # Login to Docker Hub and get a token.
         token="$(
           ${curl}/bin/curl -s \
@@ -103,13 +95,14 @@ let
         )"
 
         # Plug the default config file into the full description.
-        export DEFAULT_CONFIG="$(cat ${docker.config})"
+        defaultConfig="$(cat ${docker.config})"
+        export DEFAULT_CONFIG="$defaultConfig"
         fullDescription="$(${envsubst}/bin/envsubst < ${fullDescription})"
 
         # Patch the full description.
         responseCode="$(
-          ${curl}/bin/curl -s --write-out %{response_code} --output /dev/null \
+          ${curl}/bin/curl -s --write-out "%{response_code}" \
-            -H "Authorization: JWT $token" -X PATCH \
+            --output /dev/null -H "Authorization: JWT $token" -X PATCH \
             --data-urlencode description@${description} \
             --data-urlencode "full_description=$fullDescription" \
             "https://hub.docker.com/v2/repositories/$DOCKER_REPO/postgrest/"
@@ -120,5 +113,5 @@ let
 in
 buildEnv {
   name = "postgrest-release";
-  paths = [ github dockerLogin dockerHub dockerHubDescription ];
+  paths = [ github.bin dockerLogin.bin dockerHub.bin dockerHubDescription.bin ];
 }

nix/style.nix

@@ -0,0 +1,49 @@
+{ buildEnv
+, checkedShellScript
+, git
+, hlint
+, nixpkgs-fmt
+, silver-searcher
+, stylish-haskell
+}:
+let
+  style =
+    checkedShellScript "postgrest-style"
+      ''
+        rootdir="$(${git}/bin/git rev-parse --show-toplevel)"
+
+        # Format Nix files
+        ${nixpkgs-fmt}/bin/nixpkgs-fmt "$rootdir" > /dev/null 2> /dev/null
+
+        # Format Haskell files
+        ${silver-searcher}/bin/ag -l -g '\.l?hs$' . "$rootdir" \
+          | xargs ${stylish-haskell}/bin/stylish-haskell -i
+      '';
+
+  # Script to check whether any uncommited changes result from postgrest-style
+  styleCheck =
+    checkedShellScript "postgrest-style-check"
+      ''
+        ${style}
+
+        ${git}/bin/git diff-index --exit-code HEAD -- '*.hs' '*.lhs' '*.nix'
+      '';
+
+  lint =
+    checkedShellScript "postgrest-lint"
+      ''
+        rootdir="$(${git}/bin/git rev-parse --show-toplevel)"
+
+        # Lint Haskell files
+        ${silver-searcher}/bin/ag -l -g '\.l?hs$' "$rootdir" \
+          | xargs ${hlint}/bin/hlint -X QuasiQuotes -X NoPatternSynonyms
+      '';
+in
+buildEnv {
+  name = "postgrest-devtools";
+  paths = [
+    style.bin
+    styleCheck.bin
+    lint.bin
+  ];
+}

nix/tests.nix

@@ -2,7 +2,9 @@
 
 { buildEnv
 , cabal-install
+, checkedShellScript
 , curl
+, devCabalOptions
 , git
 , haskell
 , lib
@@ -13,17 +15,13 @@
 , postgrestStatic
 , postgrestProfiled
 , runtimeShell
-, writeShellScript
-, writeShellScriptBin
 }:
 let
   # Wrap the `test/with_tmp_db` script with the required dependencies from Nix.
   withTmpDb =
     postgresql:
-    writeShellScript "postgrest-test-${postgresql.name}"
+    checkedShellScript "postgrest-test-${postgresql.name}"
       ''
-        set -euo pipefail
-
         export PATH=${postgresql}/bin:${git}/bin:${runtimeShell}/bin:"$PATH"
 
         exec ${../test/with_tmp_db} "$@"
@@ -33,12 +31,11 @@ let
   # PostgreSQL.
   testSpec =
     name: postgresql:
-    writeShellScriptBin
+    checkedShellScript
       name
       ''
-        set -euo pipefail
+        env="$(cat ${postgrest.env})"
-
+        export PATH="$env/bin:$PATH"
-        export PATH="$(cat ${postgrest.env})"/bin:"$PATH"
 
         cat << EOF
 
@@ -46,8 +43,11 @@ let
 
         EOF
 
-        ${withTmpDb postgresql} ${cabal-install}/bin/cabal v2-test -f FailOnWarn \
+        trap 'echo "Failed on ${postgresql.name}"' exit
-          --test-show-detail=direct
+
+        ${withTmpDb postgresql} ${cabal-install}/bin/cabal v2-test ${devCabalOptions}
+
+        trap "" exit
 
         cat << EOF
 
@@ -59,8 +59,9 @@ let
   # Create a `testSpec` for each PostgreSQL version that we want to test
   # against.
   testSpecVersions =
-    lib.mapAttrsToList
+    builtins.map
-      (name: postgresql: testSpec "postgrest-test-spec-${name}" postgresql)
+      ({ name, postgresql }:
+        (testSpec "postgrest-test-spec-${name}" postgresql).bin)
       postgresqlVersions;
 
   # Helper script for running the tests against all PostgreSQL versions.
@@ -69,20 +70,14 @@ let
       testRunners =
         map (test: "${test}/bin/${test.name}") testSpecVersions;
     in
-    writeShellScriptBin "postgrest-test-spec-all"
+    checkedShellScript "postgrest-test-spec-all"
-      ''
+      (lib.concatStringsSep "\n" testRunners);
-        set -euo pipefail
-
-        ${lib.concatStringsSep "\n" testRunners}
-      '';
 
   testIO =
     name: postgresql:
-    writeShellScriptBin
+    checkedShellScript
       name
       ''
-        set -euo pipefail
-
         rootdir="$(${git}/bin/git rev-parse --show-toplevel)"
         cd "$rootdir"
 
@@ -93,11 +88,9 @@ let
 
   testMemory =
     name: postgresql:
-    writeShellScriptBin
+    checkedShellScript
       name
       ''
-        set -euo pipefail
-
         rootdir="$(${git}/bin/git rev-parse --show-toplevel)"
         cd "$rootdir"
 
@@ -115,8 +108,8 @@ buildEnv
 
     paths =
       [
-        (testSpec "postgrest-test-spec" postgresql)
+        (testSpec "postgrest-test-spec" postgresql).bin
-        testSpecAllVersions
+        testSpecAllVersions.bin
       ] ++ testSpecVersions;
   }
   # The IO an memory tests have large dependencies (a static and a profiled
@@ -125,8 +118,8 @@ buildEnv
   # them available through separate attributes:
   // {
   ioTests =
-    (testIO "postgrest-test-io" postgresql);
+    (testIO "postgrest-test-io" postgresql).bin;
 
   memoryTests =
-    (testMemory "postgrest-test-memory" postgresql);
+    (testMemory "postgrest-test-memory" postgresql).bin;
 }

shell.nix

@@ -30,6 +30,7 @@ lib.overrideDerivation postgrest.env (
         postgrest.nixpkgsUpgrade
         postgrest.devtools
         postgrest.tests
+        postgrest.style
       ]
       ++ lib.optional ioTests postgrest.tests.ioTests
       ++ lib.optional memoryTests postgrest.tests.memoryTests