[v3] ci: run `make sparse` as part of the GitHub workflow

Commit Message

Johannes Schindelin July 26, 2021, 8:27 p.m. UTC

From: Johannes Schindelin <[e-mail]>

Occasionally we receive reviews after patches were integrated, where
`sparse` (https://sparse.docs.kernel.org/en/latest/ has more information
on that project) identified problems such as file-local variables or
functions being declared as global.

By running `sparse` as part of our Continuous Integration, we can catch
such things much earlier. Even better: developers who activated GitHub
Actions on their forks can catch such issues before even sending their
patches to the Git mailing list.

This addresses https://github.com/gitgitgadget/git/issues/345

Note: Not even Ubuntu 20.04 ships with a new enough version of `sparse`
to accommodate Git's needs. The symptom looks like this:

    add-interactive.c:537:51: error: Using plain integer as NULL pointer

To counter that, we download and install the custom-built `sparse`
package from the Azure Pipeline that we specifically created to address
this issue.

Helped-by: Jeff King <[e-mail]>
Signed-off-by: Johannes Schindelin <[e-mail]>
---
    ci: run make sparse as a GitHub workflow
    
    One of the earliest open source static analyzers is called "sparse", and
    occasionally Ramsay Jones sends out mails on the Git mailing list that
    some function or other should be declared static because sparse found
    out that it is only used within the same file.
    
    Let's add a GitHub workflow running "make sparse".
    
    Example run:
    https://github.com/gitgitgadget/git/pull/994/checks?check_run_id=3065255116
    
    Changes since v2:
    
     * We're now reusing the ci/install-dependencies.sh script even in the
       sparse job.
    
    Changes since v1:
    
     * The job was folded into main.yml
     * The commit message and a code comment now explain why we have to
       download & install a custom sparse package instead of using Ubuntu's
       default one
     * The commit message now contains a link to the documentation of the
       sparse tool

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-994%2Fdscho%2Fci-enable-sparse-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-994/dscho/ci-enable-sparse-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/994

Range-diff vs v2:

 1:  8b631acfaa2 ! 1:  762c4cbb6e2 ci: run `make sparse` as part of the GitHub workflow
     @@ Commit message
          package from the Azure Pipeline that we specifically created to address
          this issue.
      
     +    Helped-by: Jeff King <[e-mail]>
          Signed-off-by: Johannes Schindelin <[e-mail]>
      
       ## .github/workflows/main.yml ##
     @@ .github/workflows/main.yml: jobs:
      +        artifact: sparse-20.04
      +    - name: Install the current `sparse` package
      +      run: sudo dpkg -i sparse-20.04/sparse_*.deb
     -+    - name: Install other dependencies
     -+      run: |
     -+        sudo apt-get install -q -y libssl-dev libcurl4-openssl-dev libexpat-dev gettext zlib1g-dev
      +    - uses: actions/checkout@v2
     ++    - name: Install other dependencies
     ++      run: ci/install-dependencies.sh
      +    - run: make sparse
         documentation:
           needs: ci-config
           if: needs.ci-config.outputs.enabled == 'yes'
     +
     + ## ci/install-dependencies.sh ##
     +@@ ci/install-dependencies.sh: StaticAnalysis)
     + 	sudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \
     + 		libexpat-dev gettext make
     + 	;;
     ++sparse)
     ++	sudo apt-get -q update -q
     ++	sudo apt-get -q -y install libssl-dev libcurl4-openssl-dev \
     ++		libexpat-dev gettext zlib1g-dev
     ++	;;
     + Documentation)
     + 	sudo apt-get -q update
     + 	sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns make


 .github/workflows/main.yml | 20 ++++++++++++++++++++
 ci/install-dependencies.sh |  5 +++++
 2 files changed, 25 insertions(+)


base-commit: 75ae10bc75336db031ee58d13c5037b929235912

Comments

Junio C Hamano July 26, 2021, 10:20 p.m. UTC | #1

"Johannes Schindelin via GitGitGadget" <[e-mail]>
writes:

> From: Johannes Schindelin <[e-mail]>
>
> Occasionally we receive reviews after patches were integrated, where
> `sparse` (https://sparse.docs.kernel.org/en/latest/ has more information
> on that project) identified problems such as file-local variables or
> functions being declared as global.
>
> By running `sparse` as part of our Continuous Integration, we can catch
> such things much earlier. Even better: developers who activated GitHub
> Actions on their forks can catch such issues before even sending their
> patches to the Git mailing list.
>
> This addresses https://github.com/gitgitgadget/git/issues/345
>
> Note: Not even Ubuntu 20.04 ships with a new enough version of `sparse`
> to accommodate Git's needs. The symptom looks like this:
>
>     add-interactive.c:537:51: error: Using plain integer as NULL pointer
>
> To counter that, we download and install the custom-built `sparse`
> package from the Azure Pipeline that we specifically created to address
> this issue.
>
> Helped-by: Jeff King <[e-mail]>
> Signed-off-by: Johannes Schindelin <[e-mail]>
> ---

OK, this is with Peff's "oh, we need 'apt-get update' to avoid stale
package list" fix already rolled in?  It seems you took both of them,
and they look good to me, too.

Unfortunately, ci-make-sparse has already been in 'next' for the
past week or so, so incrementals are vastly preferred.

Let me squish in your "Acked-by" to both of Peff's patches and queue
them on top of js/ci-make-sparse topic.

Thanks.

Patch

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 73856bafc9d..224c46b6d6a 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -350,6 +350,26 @@  jobs:
     - uses: actions/checkout@v1
     - run: ci/install-dependencies.sh
     - run: ci/run-static-analysis.sh
+  sparse:
+    needs: ci-config
+    if: needs.ci-config.outputs.enabled == 'yes'
+    env:
+      jobname: sparse
+    runs-on: ubuntu-20.04
+    steps:
+    - name: Download a current `sparse` package
+      # Ubuntu's `sparse` version is too old for us
+      uses: git-for-windows/get-azure-pipelines-artifact@v0
+      with:
+        repository: git/git
+        definitionId: 10
+        artifact: sparse-20.04
+    - name: Install the current `sparse` package
+      run: sudo dpkg -i sparse-20.04/sparse_*.deb
+    - uses: actions/checkout@v2
+    - name: Install other dependencies
+      run: ci/install-dependencies.sh
+    - run: make sparse
   documentation:
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 67852d0d37f..5772081b6e5 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -65,6 +65,11 @@  StaticAnalysis)
 	sudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \
 		libexpat-dev gettext make
 	;;
+sparse)
+	sudo apt-get -q update -q
+	sudo apt-get -q -y install libssl-dev libcurl4-openssl-dev \
+		libexpat-dev gettext zlib1g-dev
+	;;
 Documentation)
 	sudo apt-get -q update
 	sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns make