Skip to content

CI: Teach our Azure Pipeline to run sparse #345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
dscho opened this issue Sep 19, 2019 · 6 comments
Closed

CI: Teach our Azure Pipeline to run sparse #345

dscho opened this issue Sep 19, 2019 · 6 comments

Comments

@dscho
Copy link
Member

dscho commented Sep 19, 2019

One of the earliest open source static analyzers is 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 job to our Azure Pipeline to also run sparse. There is now a build offering an Ubuntu package of sparse as build artifact: https://dev.azure.com/git/git/_build?definitionId=10&_a=summary (which can be consumed in the same way the git-sdk-64-minimal build artifact is consumed, via a PowerShell step).

See the thread around https://public-inbox.org/git/[email protected]/ for more details.

Official documentation how to use sparse: https://kernelnewbies.org/Sparse

@sgn
Copy link

sgn commented Apr 29, 2020

Hi @dscho,

I made a draft version for this one in sgn@ae06de1

Would you mind take a look, if it's OK, I'll send to the list.

I would like to discuss off-list first, to reduce the noise.

@dscho
Copy link
Member Author

dscho commented Apr 29, 2020

I would like to discuss off-list first, to reduce the noise.

I added some comments.

Please note, however, that I am very uncomfortable with doing this "off-list". In the least, I would like to discuss this in a proper PR, so that others can see what I suggested later without having to jump through hoops.

@sgn
Copy link

sgn commented Apr 30, 2020

I created the PR here git#774

I replaced the call to json_pp with jq.
It's available already in both Travis and Github Action, and it's easier to read.

@dscho
Copy link
Member Author

dscho commented Nov 4, 2020

Since the subject came up on the Git mailing list again: the Bash step to download and install the sparse package would look like this:

urlbase=https://dev.azure.com/git/git/_apis/build/builds
id=$(curl "$urlbase?definitions=10&statusFilter=completed&resultFilter=succeeded&\$top=1" |
	json_pp |
	sed -n 's/^         "id" : \([1-9][0-9]*\).*/\1/p')
download_url="$(curl "$urlbase/$id/artifacts" |
	json_pp |
	sed -n '/^      {/{:1;N;/\n      }/b2;b1;:2;/"name" : "sparse"/{s/.*"downloadUrl" : "\([^"]*\).*/\1/p}}')"
curl -Lo sparse.zip "$download_url"
unzip sparse.zip
sudo dpkg -i sparse/*.deb

dscho added a commit to dscho/git that referenced this issue Jul 13, 2021
Occasionally we receive reviews after patches were integrated, where
`sparse` 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 gitgitgadget#345

Signed-off-by: Johannes Schindelin <[email protected]>
dscho added a commit to dscho/git that referenced this issue Jul 14, 2021
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 gitgitgadget#345

Note: since we cannot easily replicate the same on Travis (due to the
absence of the GitHub Action `get-azure-pipelines-artifact`), we keep
this separate from the standard CI/PR definition (which exists both as a
GitHub workflow as well as a Travis CI definition).

Signed-off-by: Johannes Schindelin <[email protected]>
dscho added a commit to dscho/git that referenced this issue Jul 14, 2021
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 gitgitgadget#345

Signed-off-by: Johannes Schindelin <[email protected]>
dscho added a commit to dscho/git that referenced this issue Jul 14, 2021
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 gitgitgadget#345

Signed-off-by: Johannes Schindelin <[email protected]>
dscho added a commit to dscho/git that referenced this issue Jul 14, 2021
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 gitgitgadget#345

Note: Not even Ubuntu 20.04 ships with a new enough version of `sparse`
to accommodate Git's needs. Therefore, we download and install the
custom-built `sparse` package from the Azure Pipeline we specifically
created to address this issue.

Signed-off-by: Johannes Schindelin <[email protected]>
dscho added a commit to dscho/git that referenced this issue Jul 14, 2021
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 gitgitgadget#345

Note: Not even Ubuntu 20.04 ships with a new enough version of `sparse`
to accommodate Git's needs. Therefore, we download and install the
custom-built `sparse` package from the Azure Pipeline we specifically
created to address this issue.

Signed-off-by: Johannes Schindelin <[email protected]>
dscho added a commit to dscho/git that referenced this issue Jul 14, 2021
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 gitgitgadget#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.

Signed-off-by: Johannes Schindelin <[email protected]>
gitster pushed a commit to git/git that referenced this issue Jul 14, 2021
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 gitgitgadget#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.

Signed-off-by: Johannes Schindelin <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
dscho added a commit to dscho/git that referenced this issue Jul 26, 2021
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 gitgitgadget#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 <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
@phil-blain
Copy link

This should be closed as e610596 was merged to master quite a while ago.

@dscho
Copy link
Member Author

dscho commented Jan 16, 2023

Yep, years ago 😀

@dscho dscho closed this as completed Jan 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants