{"thread":{"id":"56098","subject":"[PATCH] ci: run `make sparse` as a GitHub workflow","startedAt":"2021-07-13T11:51:31Z","lastAt":"2021-07-26T22:20:22Z","messageCount":22,"participants":["Johannes Schindelin via GitGitGadget","Đoàn Trần Công Danh","Philippe Blain","Junio C Hamano","Johannes Schindelin","Ramsay Jones","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"429888","messageId":"pull.994.git.1626177086682.gitgitgadget@gmail.com","threadId":"56098","inReplyTo":null,"subject":"[PATCH] ci: run `make sparse` as a GitHub workflow","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-13T11:51:26Z","receivedAt":"2021-07-13T11:51:31Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nOccasionally we receive reviews after patches were integrated, where\n`sparse` identified problems such as file-local variables or functions\nbeing declared as global.\n\nBy running `sparse` as part of our Continuous Integration, we can catch\nsuch things much earlier. Even better: developers who activated GitHub\nActions on their forks can catch such issues before even sending their\npatches to the Git mailing list.\n\nThis addresses https://github.com/gitgitgadget/git/issues/345\n\nSigned-off-by: Johannes Schindelin \n---\n ci: run make sparse as a GitHub workflow\n \n One of the earliest open source static analyzers is called \"sparse\", and\n occasionally Ramsay Jones sends out mails on the Git mailing list that\n some function or other should be declared static because sparse found\n out that it is only used within the same file.\n \n Let's add a GitHub workflow running \"make sparse\".\n \n Example run: https://github.com/gitgitgadget/git/actions/runs/1026303823\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-994%2Fdscho%2Fci-enable-sparse-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-994/dscho/ci-enable-sparse-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/994\n\n .github/workflows/run-sparse.yml | 22 ++++++++++++++++++++++\n 1 file changed, 22 insertions(+)\n create mode 100644 .github/workflows/run-sparse.yml\n\ndiff --git a/.github/workflows/run-sparse.yml b/.github/workflows/run-sparse.yml\nnew file mode 100644\nindex 00000000000..25f6a6efb40\n--- /dev/null\n+++ b/.github/workflows/run-sparse.yml\n@@ -0,0 +1,22 @@\n+name: Run `sparse`\n+\n+on: [push, pull_request]\n+\n+jobs:\n+ sparse:\n+ runs-on: ubuntu-20.04\n+ steps:\n+ - name: Download the `sparse` package\n+ uses: git-for-windows/get-azure-pipelines-artifact@v0\n+ with:\n+ repository: git/git\n+ definitionId: 10\n+ artifact: sparse-20.04\n+ - name: Install the `sparse` package\n+ run: sudo dpkg -i sparse-20.04/sparse_*.deb\n+ - name: Install a couple of dependencies\n+ run: |\n+ sudo apt-get install -q -y libssl-dev libcurl4-openssl-dev libexpat-dev gettext zlib1g-dev\n+ - uses: actions/checkout@v2\n+ - name: make sparse\n+ run: make sparse\n\\ No newline at end of file\n\nbase-commit: ebf3c04b262aa27fbb97f8a0156c2347fecafafb\n-- \ngitgitgadget\n"},{"id":"429901","messageId":"YO3FelB47QrZwSa6@danh.dev","threadId":"56098","inReplyTo":"pull.994.git.1626177086682.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci: run `make sparse` as a GitHub workflow","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-07-13T16:55:22Z","receivedAt":"2021-07-13T16:55:28Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2021-07-13 11:51:26+0000, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> Occasionally we receive reviews after patches were integrated, where\n> `sparse` identified problems such as file-local variables or functions\n> being declared as global.\n> \n> By running `sparse` as part of our Continuous Integration, we can catch\n> such things much earlier. Even better: developers who activated GitHub\n> Actions on their forks can catch such issues before even sending their\n> patches to the Git mailing list.\n> \n> This addresses https://github.com/gitgitgadget/git/issues/345\n> \n> Signed-off-by: Johannes Schindelin \n\nThanks for doing this change.\n\nSome minor nits below.\n\n> diff --git a/.github/workflows/run-sparse.yml b/.github/workflows/run-sparse.yml\n> new file mode 100644\n> index 00000000000..25f6a6efb40\n> --- /dev/null\n> +++ b/.github/workflows/run-sparse.yml\n> @@ -0,0 +1,22 @@\n> +name: Run `sparse`\n\nMarkdown doesn't work with Workflow's name.\nPlease remove those backticks.\n\n> +on: [push, pull_request]\n> +\n> +jobs:\n> + sparse:\n> + runs-on: ubuntu-20.04\n> + steps:\n> + - name: Download the `sparse` package\n> + uses: git-for-windows/get-azure-pipelines-artifact@v0\n> + with:\n> + repository: git/git\n> + definitionId: 10\n> + artifact: sparse-20.04\n> + - name: Install the `sparse` package\n> + run: sudo dpkg -i sparse-20.04/sparse_*.deb\n> + - name: Install a couple of dependencies\n> + run: |\n> + sudo apt-get install -q -y libssl-dev libcurl4-openssl-dev libexpat-dev gettext zlib1g-dev\n> + - uses: actions/checkout@v2\n> + - name: make sparse\n> + run: make sparse\n> \\ No newline at end of file\n\nThe last step's name and run is the same. We can just drop name, it'll\nuse run as name. Anyway, remember the newline\n\nOtherwise, look good to me.\n\n-- \nDanh\n"},{"id":"429904","messageId":"f305f586-c3ac-a2ef-8769-5468a4c3e539@gmail.com","threadId":"56098","inReplyTo":"pull.994.git.1626177086682.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci: run `make sparse` as a GitHub workflow","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2021-07-13T17:34:26Z","receivedAt":"2021-07-13T17:34:30Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Dscho,\n\nLe 2021-07-13 à 07:51, Johannes Schindelin via GitGitGadget a écrit :\n> From: Johannes Schindelin \n> \n> Occasionally we receive reviews after patches were integrated, where\n> `sparse` identified problems such as file-local variables or functions\n> being declared as global.\n> \n> By running `sparse` \n\nmaybe here, we could add a link to https://sparse.docs.kernel.org/en/latest/,\nso interested readers who do not know about \"sparse\" can go and learn\nabout it ?\n\n> as part of our Continuous Integration, we can catch\n> such things much earlier. Even better: developers who activated GitHub\n> Actions on their forks can catch such issues before even sending their\n> patches to the Git mailing list.\n> \n> This addresses https://github.com/gitgitgadget/git/issues/345\n> \n> Signed-off-by: Johannes Schindelin \n\n> + - name: Download the `sparse` package\n> + uses: git-for-windows/get-azure-pipelines-artifact@v0\n> + with:\n> + repository: git/git\n> + definitionId: 10\n> + artifact: sparse-20.04\n> + - name: Install the `sparse` package\n> + run: sudo dpkg -i sparse-20.04/sparse_*.deb\n\nOut of curiosity, why is this necessary (as opposed to using\nthe Ubuntu package, i.e. 'sudo apt-get -q -y sparse') ?\n\nThanks,\nPhilippe.\n"},{"id":"429947","messageId":"xmqqbl7525w7.fsf@gitster.g","threadId":"56098","inReplyTo":"pull.994.git.1626177086682.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci: run `make sparse` as a GitHub workflow","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-13T22:41:44Z","receivedAt":"2021-07-13T22:41:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> Occasionally we receive reviews after patches were integrated, where\n> `sparse` identified problems such as file-local variables or functions\n> being declared as global.\n\nI really appreciate addition of this task, as I often notice sparse\nerrors _after_ queuing and refreshing the incoming patches for the\nday and soon after starting to run the day's test cycle.\n\n> By running `sparse` as part of our Continuous Integration, we can catch\n> such things much earlier. Even better: developers who activated GitHub\n> Actions on their forks can catch such issues before even sending their\n> patches to the Git mailing list.\n>\n> This addresses https://github.com/gitgitgadget/git/issues/345\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> ci: run make sparse as a GitHub workflow\n> \n> One of the earliest open source static analyzers is called \"sparse\", and\n> occasionally Ramsay Jones sends out mails on the Git mailing list that\n> some function or other should be declared static because sparse found\n> out that it is only used within the same file.\n> \n> Let's add a GitHub workflow running \"make sparse\".\n> \n> Example run: https://github.com/gitgitgadget/git/actions/runs/1026303823\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-994%2Fdscho%2Fci-enable-sparse-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-994/dscho/ci-enable-sparse-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/994\n>\n> .github/workflows/run-sparse.yml | 22 ++++++++++++++++++++++\n> 1 file changed, 22 insertions(+)\n> create mode 100644 .github/workflows/run-sparse.yml\n\nWe choose to do this as a separate new workflow not as part of the\nmain one because this is more like check-whitespace where there is\nno room for tests over the matrix of compilers and platforms play\nany useful role? Unlike check-whitespace one that has\n\n on:\n pull_request:\n types: [opened, synchronize]\n\nbut just like the primary one, this is triggered\n\n on: [push, pull_request]\n\nand before the patches even hit the list via GGG, which would be\nideal.\n\nAs has already been pointed out downthread, use of separate and\ndifferent ways to install the sparse itself and its dependencies\nlooks strange, so it would be appropriate to either explain why they\nhave to be that way in the proposed log message or to install them\nin the same way in the YAML file.\n\nAlso, \"\\ No newline at end of file\" would be a good thing to fix\nwhile we are at it.\n\n> diff --git a/.github/workflows/run-sparse.yml b/.github/workflows/run-sparse.yml\n> new file mode 100644\n> index 00000000000..25f6a6efb40\n> --- /dev/null\n> +++ b/.github/workflows/run-sparse.yml\n> @@ -0,0 +1,22 @@\n> +name: Run `sparse`\n> +\n> +on: [push, pull_request]\n> +\n> +jobs:\n> + sparse:\n> + runs-on: ubuntu-20.04\n> + steps:\n> + - name: Download the `sparse` package\n> + uses: git-for-windows/get-azure-pipelines-artifact@v0\n> + with:\n> + repository: git/git\n> + definitionId: 10\n> + artifact: sparse-20.04\n> + - name: Install the `sparse` package\n> + run: sudo dpkg -i sparse-20.04/sparse_*.deb\n> + - name: Install a couple of dependencies\n> + run: |\n> + sudo apt-get install -q -y libssl-dev libcurl4-openssl-dev libexpat-dev gettext zlib1g-dev\n> + - uses: actions/checkout@v2\n> + - name: make sparse\n> + run: make sparse\n> \\ No newline at end of file\n>\n> base-commit: ebf3c04b262aa27fbb97f8a0156c2347fecafafb\n"},{"id":"430007","messageId":"nycvar.QRO.7.76.6.2107141052350.76@tvgsbejvaqbjf.bet","threadId":"56098","inReplyTo":"f305f586-c3ac-a2ef-8769-5468a4c3e539@gmail.com","subject":"Re: [PATCH] ci: run `make sparse` as a GitHub workflow","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-07-14T09:09:32Z","receivedAt":"2021-07-14T09:09:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Philippe,\n\nOn Tue, 13 Jul 2021, Philippe Blain wrote:\n\n> Le 2021-07-13 à 07:51, Johannes Schindelin via GitGitGadget a écrit :\n> > From: Johannes Schindelin \n> >\n> > Occasionally we receive reviews after patches were integrated, where\n> > `sparse` identified problems such as file-local variables or functions\n> > being declared as global.\n> >\n> > By running `sparse`\n>\n> maybe here, we could add a link to https://sparse.docs.kernel.org/en/latest/,\n> so interested readers who do not know about \"sparse\" can go and learn\n> about it ?\n\nGood point.\n\n> > as part of our Continuous Integration, we can catch\n> > such things much earlier. Even better: developers who activated GitHub\n> > Actions on their forks can catch such issues before even sending their\n> > patches to the Git mailing list.\n> >\n> > This addresses https://github.com/gitgitgadget/git/issues/345\n> >\n> > Signed-off-by: Johannes Schindelin \n>\n> > + - name: Download the `sparse` package\n> > + uses: git-for-windows/get-azure-pipelines-artifact@v0\n> > + with:\n> > + repository: git/git\n> > + definitionId: 10\n> > + artifact: sparse-20.04\n> > + - name: Install the `sparse` package\n> > + run: sudo dpkg -i sparse-20.04/sparse_*.deb\n>\n> Out of curiosity, why is this necessary (as opposed to using\n> the Ubuntu package, i.e. 'sudo apt-get -q -y sparse') ?\n\nThis is actually a historical curiosity: years ago, I created an Azure\nPipeline that builds the `sparse` Debian package for the specific purpose\nof using it in our CI builds (if you care to look at the issue 345 I\nlinked above, you will see how long ago that idea was in the making). Now,\nthe historical curiosity is that back then, there was no current `sparse`\npackage available for Ubuntu, and Ramsay mentioned that a newer version\nwould be required to run `make sparse`.\n\nAnd when I implemented this patch yesterday, I did not even question this,\nI was just happy that I had come up with the GitHub Action\n`get-azure-pipelines-artifact` (to help with the `vcpkg` part of our CI\nbuilds).\n\nI was already writing a detailed paragraph in the commit message to\nexplain all that when it occurred to me that two years might make a big\ndifference and an up to date `sparse` might be available. And lo and\nbehold, this is the case!\n\nTherefore, v2 will no longer jump through that hoop.\n\nCiao,\nDscho\n"},{"id":"430008","messageId":"nycvar.QRO.7.76.6.2107141109420.76@tvgsbejvaqbjf.bet","threadId":"56098","inReplyTo":"YO3FelB47QrZwSa6@danh.dev","subject":"Re: [PATCH] ci: run `make sparse` as a GitHub workflow","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-07-14T09:12:52Z","receivedAt":"2021-07-14T09:13:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Danh,\n\nOn Tue, 13 Jul 2021, Đoàn Trần Công Danh wrote:\n\n> On 2021-07-13 11:51:26+0000, Johannes Schindelin via GitGitGadget wrote:\n> > From: Johannes Schindelin \n> >\n> > [...]\n> > diff --git a/.github/workflows/run-sparse.yml b/.github/workflows/run-sparse.yml\n> > new file mode 100644\n> > index 00000000000..25f6a6efb40\n> > --- /dev/null\n> > +++ b/.github/workflows/run-sparse.yml\n> > @@ -0,0 +1,22 @@\n> > +name: Run `sparse`\n>\n> Markdown doesn't work with Workflow's name.\n> Please remove those backticks.\n\nThe backticks are here not to render this as Markdown, but to make it\neasier to parse what is said. \"Run sparse\" is technically an English\nsentence, and a grammatically incorrect and confusing one. By enclosing\nthe word \"sparse\" in backticks, I make sure that the reader will\nunderstand that this refers to a programming term.\n\nFWIW I use backticks on this here mailing list all the time. And I am\nfairly certain that no reader renders my mails as Markdown before reading\nthem.\n\n> > +on: [push, pull_request]\n> > +\n> > +jobs:\n> > + sparse:\n> > + runs-on: ubuntu-20.04\n> > + steps:\n> > + - name: Download the `sparse` package\n> > + uses: git-for-windows/get-azure-pipelines-artifact@v0\n> > + with:\n> > + repository: git/git\n> > + definitionId: 10\n> > + artifact: sparse-20.04\n> > + - name: Install the `sparse` package\n> > + run: sudo dpkg -i sparse-20.04/sparse_*.deb\n> > + - name: Install a couple of dependencies\n> > + run: |\n> > + sudo apt-get install -q -y libssl-dev libcurl4-openssl-dev libexpat-dev gettext zlib1g-dev\n> > + - uses: actions/checkout@v2\n> > + - name: make sparse\n> > + run: make sparse\n> > \\ No newline at end of file\n>\n> The last step's name and run is the same. We can just drop name, it'll\n> use run as name.\n\nGood point.\n\n> Anyway, remember the newline\n\nRight. I edited this in VS Code, which does not care for that trailing\nnewline, and ran it through GitHub Actions, which also does not care for\nthat trailing newline, and the `check-whitespace` job also did not point\nout any issue, therefore I missed this.\n\nThanks,\nDscho\n"},{"id":"430009","messageId":"nycvar.QRO.7.76.6.2107141124530.76@tvgsbejvaqbjf.bet","threadId":"56098","inReplyTo":"xmqqbl7525w7.fsf@gitster.g","subject":"Re: [PATCH] ci: run `make sparse` as a GitHub workflow","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-07-14T10:09:18Z","receivedAt":"2021-07-14T10:09:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Tue, 13 Jul 2021, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n> > From: Johannes Schindelin \n> >\n> > .github/workflows/run-sparse.yml | 22 ++++++++++++++++++++++\n> > 1 file changed, 22 insertions(+)\n> > create mode 100644 .github/workflows/run-sparse.yml\n>\n> We choose to do this as a separate new workflow not as part of the\n> main one because this is more like check-whitespace where there is\n> no room for tests over the matrix of compilers and platforms play\n> any useful role?\n\nUbuntu's `sparse` package was historically not up to date, not enough at\nleast to support Git's `make sparse`. Hence I created an Azure Pipeline to\nbuild an up to date package, and since v1 used the GitHub Action\n`get-azure-pipelines-artifact`. As a consequence, I thought, that this was\ninappropriate for `main.yml` because we still try to _somewhat_ keep that\nin sync with `.travis.yml`.\n\nHowever, I realized that there are already too many differences (all the\nWindows builds for example, which our Travis CI definition did not follow\nsuite, even after Travis CI got support for Windows agents).\n\nSo I folded it into the regular GitHub workflow.\n\nThere is one really big downside to that, though: currently, there is no\nway to re-run only failed jobs in GitHub workflows (this is in contrast to\nAzure Pipelines). You can only re-run _all_ jobs.\n\nWhich means that the likelihood of a run to fail increases with the number\nof jobs in said run (even innocuous problems such as transient failures to\ndownload an Ubuntu package), and it also makes it much more painful to\nre-run the entire thing because you may well end up wasting a grand total\nof ~370 minutes even if only a 30-second-job would need to be re-run.\n\nHaving said that, I think you're right and the upside of keeping things\ntogether may outweigh that downside.\n\nCiao,\nDscho\n"},{"id":"430010","messageId":"nycvar.QRO.7.76.6.2107141212140.76@tvgsbejvaqbjf.bet","threadId":"56098","inReplyTo":"nycvar.QRO.7.76.6.2107141052350.76@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] ci: run `make sparse` as a GitHub workflow","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-07-14T10:13:36Z","receivedAt":"2021-07-14T10:13:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi again,\n\nOn Wed, 14 Jul 2021, Johannes Schindelin wrote:\n\n> On Tue, 13 Jul 2021, Philippe Blain wrote:\n>\n> > Le 2021-07-13 à 07:51, Johannes Schindelin via GitGitGadget a écrit :\n> > > From: Johannes Schindelin \n> > >\n> > > [...]\n> > > + - name: Download the `sparse` package\n> > > + uses: git-for-windows/get-azure-pipelines-artifact@v0\n> > > + with:\n> > > + repository: git/git\n> > > + definitionId: 10\n> > > + artifact: sparse-20.04\n> > > + - name: Install the `sparse` package\n> > > + run: sudo dpkg -i sparse-20.04/sparse_*.deb\n> >\n> > Out of curiosity, why is this necessary (as opposed to using\n> > the Ubuntu package, i.e. 'sudo apt-get -q -y sparse') ?\n>\n> This is actually a historical curiosity: years ago, I created an Azure\n> Pipeline that builds the `sparse` Debian package for the specific purpose\n> of using it in our CI builds (if you care to look at the issue 345 I\n> linked above, you will see how long ago that idea was in the making). Now,\n> the historical curiosity is that back then, there was no current `sparse`\n> package available for Ubuntu, and Ramsay mentioned that a newer version\n> would be required to run `make sparse`.\n>\n> And when I implemented this patch yesterday, I did not even question this,\n> I was just happy that I had come up with the GitHub Action\n> `get-azure-pipelines-artifact` (to help with the `vcpkg` part of our CI\n> builds).\n>\n> I was already writing a detailed paragraph in the commit message to\n> explain all that when it occurred to me that two years might make a big\n> difference and an up to date `sparse` might be available. And lo and\n> behold, this is the case!\n>\n> Therefore, v2 will no longer jump through that hoop.\n\n*Never mind*\n\nI made a mistake in testing, and Ubuntu-20.04's `sparse` package _is too\nold_. So I will reintroduce that hoop even before sending v2.\n\nCiao,\nDscho\n"},{"id":"430022","messageId":"pull.994.v2.git.1626263433838.gitgitgadget@gmail.com","threadId":"56098","inReplyTo":"pull.994.git.1626177086682.gitgitgadget@gmail.com","subject":"[PATCH v2] ci: run `make sparse` as part of the GitHub workflow","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-14T11:50:33Z","receivedAt":"2021-07-14T11:50:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nOccasionally we receive reviews after patches were integrated, where\n`sparse` (https://sparse.docs.kernel.org/en/latest/ has more information\non that project) identified problems such as file-local variables or\nfunctions being declared as global.\n\nBy running `sparse` as part of our Continuous Integration, we can catch\nsuch things much earlier. Even better: developers who activated GitHub\nActions on their forks can catch such issues before even sending their\npatches to the Git mailing list.\n\nThis addresses https://github.com/gitgitgadget/git/issues/345\n\nNote: Not even Ubuntu 20.04 ships with a new enough version of `sparse`\nto accommodate Git's needs. The symptom looks like this:\n\n add-interactive.c:537:51: error: Using plain integer as NULL pointer\n\nTo counter that, we download and install the custom-built `sparse`\npackage from the Azure Pipeline that we specifically created to address\nthis issue.\n\nSigned-off-by: Johannes Schindelin \n---\n ci: run make sparse as a GitHub workflow\n \n One of the earliest open source static analyzers is called \"sparse\", and\n occasionally Ramsay Jones sends out mails on the Git mailing list that\n some function or other should be declared static because sparse found\n out that it is only used within the same file.\n \n Let's add a GitHub workflow running \"make sparse\".\n \n Example run:\n https://github.com/gitgitgadget/git/pull/994/checks?check_run_id=3065255116\n \n Changes since v1:\n \n * The job was folded into main.yml\n * The commit message and a code comment now explain why we have to\n download & install a custom sparse package instead of using Ubuntu's\n default one\n * The commit message now contains a link to the documentation of the\n sparse tool\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-994%2Fdscho%2Fci-enable-sparse-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-994/dscho/ci-enable-sparse-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/994\n\nRange-diff vs v1:\n\n 1: d1af618dd73 ! 1: 8b631acfaa2 ci: run `make sparse` as a GitHub workflow\n @@ Metadata\n Author: Johannes Schindelin \n \n ## Commit message ##\n - ci: run `make sparse` as a GitHub workflow\n + ci: run `make sparse` as part of the GitHub workflow\n \n Occasionally we receive reviews after patches were integrated, where\n - `sparse` identified problems such as file-local variables or functions\n - being declared as global.\n + `sparse` (https://sparse.docs.kernel.org/en/latest/ has more information\n + on that project) identified problems such as file-local variables or\n + functions being declared as global.\n \n By running `sparse` as part of our Continuous Integration, we can catch\n such things much earlier. Even better: developers who activated GitHub\n @@ Commit message\n \n This addresses https://github.com/gitgitgadget/git/issues/345\n \n + Note: Not even Ubuntu 20.04 ships with a new enough version of `sparse`\n + to accommodate Git's needs. The symptom looks like this:\n +\n + add-interactive.c:537:51: error: Using plain integer as NULL pointer\n +\n + To counter that, we download and install the custom-built `sparse`\n + package from the Azure Pipeline that we specifically created to address\n + this issue.\n +\n Signed-off-by: Johannes Schindelin \n \n - ## .github/workflows/run-sparse.yml (new) ##\n -@@\n -+name: Run `sparse`\n -+\n -+on: [push, pull_request]\n -+\n -+jobs:\n + ## .github/workflows/main.yml ##\n +@@ .github/workflows/main.yml: jobs:\n + - uses: actions/checkout@v1\n + - run: ci/install-dependencies.sh\n + - run: ci/run-static-analysis.sh\n + sparse:\n ++ needs: ci-config\n ++ if: needs.ci-config.outputs.enabled == 'yes'\n ++ env:\n ++ jobname: sparse\n + runs-on: ubuntu-20.04\n + steps:\n -+ - name: Download the `sparse` package\n ++ - name: Download a current `sparse` package\n ++ # Ubuntu's `sparse` version is too old for us\n + uses: git-for-windows/get-azure-pipelines-artifact@v0\n + with:\n + repository: git/git\n + definitionId: 10\n + artifact: sparse-20.04\n -+ - name: Install the `sparse` package\n ++ - name: Install the current `sparse` package\n + run: sudo dpkg -i sparse-20.04/sparse_*.deb\n -+ - name: Install a couple of dependencies\n ++ - name: Install other dependencies\n + run: |\n + sudo apt-get install -q -y libssl-dev libcurl4-openssl-dev libexpat-dev gettext zlib1g-dev\n + - uses: actions/checkout@v2\n -+ - name: make sparse\n -+ run: make sparse\n - \\ No newline at end of file\n ++ - run: make sparse\n + documentation:\n + needs: ci-config\n + if: needs.ci-config.outputs.enabled == 'yes'\n\n\n .github/workflows/main.yml | 21 +++++++++++++++++++++\n 1 file changed, 21 insertions(+)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex 73856bafc9d..1b5c0392079 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -350,6 +350,27 @@ jobs:\n - uses: actions/checkout@v1\n - run: ci/install-dependencies.sh\n - run: ci/run-static-analysis.sh\n+ sparse:\n+ needs: ci-config\n+ if: needs.ci-config.outputs.enabled == 'yes'\n+ env:\n+ jobname: sparse\n+ runs-on: ubuntu-20.04\n+ steps:\n+ - name: Download a current `sparse` package\n+ # Ubuntu's `sparse` version is too old for us\n+ uses: git-for-windows/get-azure-pipelines-artifact@v0\n+ with:\n+ repository: git/git\n+ definitionId: 10\n+ artifact: sparse-20.04\n+ - name: Install the current `sparse` package\n+ run: sudo dpkg -i sparse-20.04/sparse_*.deb\n+ - name: Install other dependencies\n+ run: |\n+ sudo apt-get install -q -y libssl-dev libcurl4-openssl-dev libexpat-dev gettext zlib1g-dev\n+ - uses: actions/checkout@v2\n+ - run: make sparse\n documentation:\n needs: ci-config\n if: needs.ci-config.outputs.enabled == 'yes'\n\nbase-commit: 75ae10bc75336db031ee58d13c5037b929235912\n-- \ngitgitgadget\n"},{"id":"430063","messageId":"xmqq35sgzy0d.fsf@gitster.g","threadId":"56098","inReplyTo":"nycvar.QRO.7.76.6.2107141124530.76@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] ci: run `make sparse` as a GitHub workflow","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-14T16:00:18Z","receivedAt":"2021-07-14T16:00:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Which means that the likelihood of a run to fail increases with the number\n> of jobs in said run (even innocuous problems such as transient failures to\n> download an Ubuntu package), and it also makes it much more painful to\n> re-run the entire thing because you may well end up wasting a grand total\n> of ~370 minutes even if only a 30-second-job would need to be re-run.\n>\n> Having said that, I think you're right and the upside of keeping things\n> together may outweigh that downside.\n\nI wasn't make a request or a demand to change or not to change\nanything, so in this particular exchange there was no point where I\nwas right (or wrong, for that matter ;-). I was asking if there was\na solid reasoning behind the split, and if there is, I am perfectly\nhappy to see it done as a separate workflow with the log message\nthat explains why it is separate. I am also perfectly fine with\nthis rolled into the primary one, with clear reasoning behind the\nchoice recorded in the log message.\n\nThanks.\n\n\n\n"},{"id":"430128","messageId":"nycvar.QRO.7.76.6.2107142252060.59@tvgsbejvaqbjf.bet","threadId":"56098","inReplyTo":"xmqq35sgzy0d.fsf@gitster.g","subject":"Re: [PATCH] ci: run `make sparse` as a GitHub workflow","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-07-14T20:54:07Z","receivedAt":"2021-07-14T20:54:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 14 Jul 2021, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n>\n> > Which means that the likelihood of a run to fail increases with the number\n> > of jobs in said run (even innocuous problems such as transient failures to\n> > download an Ubuntu package), and it also makes it much more painful to\n> > re-run the entire thing because you may well end up wasting a grand total\n> > of ~370 minutes even if only a 30-second-job would need to be re-run.\n> >\n> > Having said that, I think you're right and the upside of keeping things\n> > together may outweigh that downside.\n>\n> I wasn't make a request or a demand to change or not to change\n> anything, so in this particular exchange there was no point where I\n> was right (or wrong, for that matter ;-). I was asking if there was\n> a solid reasoning behind the split, and if there is, I am perfectly\n> happy to see it done as a separate workflow with the log message\n> that explains why it is separate. I am also perfectly fine with\n> this rolled into the primary one, with clear reasoning behind the\n> choice recorded in the log message.\n\nI do not think that it would be an improvement to defend the default\nchoice (i.e. to add this new job to `.github/workflows/main.yml`) in the\ncommit message. It is the default for new CI stuff to go, after all, and\nwe do not need to clutter the message by stating the obvious.\n\nCiao,\nDscho\n"},{"id":"430129","messageId":"xmqqa6movcly.fsf@gitster.g","threadId":"56098","inReplyTo":"nycvar.QRO.7.76.6.2107142252060.59@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] ci: run `make sparse` as a GitHub workflow","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-14T20:56:09Z","receivedAt":"2021-07-14T20:56:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Hi Junio,\n>\n> On Wed, 14 Jul 2021, Junio C Hamano wrote:\n>\n>> Johannes Schindelin writes:\n>>\n>> > Which means that the likelihood of a run to fail increases with the number\n>> > of jobs in said run (even innocuous problems such as transient failures to\n>> > download an Ubuntu package), and it also makes it much more painful to\n>> > re-run the entire thing because you may well end up wasting a grand total\n>> > of ~370 minutes even if only a 30-second-job would need to be re-run.\n>> >\n>> > Having said that, I think you're right and the upside of keeping things\n>> > together may outweigh that downside.\n>>\n>> I wasn't make a request or a demand to change or not to change\n>> anything, so in this particular exchange there was no point where I\n>> was right (or wrong, for that matter ;-). I was asking if there was\n>> a solid reasoning behind the split, and if there is, I am perfectly\n>> happy to see it done as a separate workflow with the log message\n>> that explains why it is separate. I am also perfectly fine with\n>> this rolled into the primary one, with clear reasoning behind the\n>> choice recorded in the log message.\n>\n> I do not think that it would be an improvement to defend the default\n> choice (i.e. to add this new job to `.github/workflows/main.yml`) in the\n> commit message. It is the default for new CI stuff to go, after all, and\n> we do not need to clutter the message by stating the obvious.\n\nIt wasn't quite obvious why we justify spending 370 minutes one more\ntime only to rerun 30-second job, though.\n"},{"id":"430136","messageId":"nycvar.QRO.7.76.6.2107142350570.59@tvgsbejvaqbjf.bet","threadId":"56098","inReplyTo":"xmqqa6movcly.fsf@gitster.g","subject":"Re: [PATCH] ci: run `make sparse` as a GitHub workflow","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-07-14T22:03:52Z","receivedAt":"2021-07-14T22:04:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 14 Jul 2021, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n>\n> > On Wed, 14 Jul 2021, Junio C Hamano wrote:\n> >\n> >> Johannes Schindelin writes:\n> >>\n> >> > Which means that the likelihood of a run to fail increases with the number\n> >> > of jobs in said run (even innocuous problems such as transient failures to\n> >> > download an Ubuntu package), and it also makes it much more painful to\n> >> > re-run the entire thing because you may well end up wasting a grand total\n> >> > of ~370 minutes even if only a 30-second-job would need to be re-run.\n> >> >\n> >> > Having said that, I think you're right and the upside of keeping things\n> >> > together may outweigh that downside.\n> >>\n> >> I wasn't make a request or a demand to change or not to change\n> >> anything, so in this particular exchange there was no point where I\n> >> was right (or wrong, for that matter ;-). I was asking if there was\n> >> a solid reasoning behind the split, and if there is, I am perfectly\n> >> happy to see it done as a separate workflow with the log message\n> >> that explains why it is separate. I am also perfectly fine with\n> >> this rolled into the primary one, with clear reasoning behind the\n> >> choice recorded in the log message.\n> >\n> > I do not think that it would be an improvement to defend the default\n> > choice (i.e. to add this new job to `.github/workflows/main.yml`) in the\n> > commit message. It is the default for new CI stuff to go, after all, and\n> > we do not need to clutter the message by stating the obvious.\n>\n> It wasn't quite obvious why we justify spending 370 minutes one more\n> time only to rerun 30-second job, though.\n\nTrue.\n\nAnd this is not a new problem. Every time anything happens in those\n`osx-gcc` or `osx-clang` jobs (e.g. that transient problem with the broken\npipes in t5516 [*1*], that's a fun one), a full re-run is necessary, or\nelse the commit and/or PR will remain marked as broken.\n\nIn other words, while it is totally appropriate for me to explain this to\nyou in this here thread because it came up as a tangent, it would be\ninappropriate to stick that explanation into this patch's commit message.\nWe do not make a habit of adding tangents that came up during patch\nreviews into commit messages, and I do not intend to start such a habit\nhere, either.\n\nCiao,\nDscho\n\nFootnote *1*: See\nhttps://lore.kernel.org/git/20190829143805.GB1746@sigill.intra.peff.net/\n(I don't think this is fixed as of now)\n"},{"id":"430147","messageId":"xmqqsg0gttt5.fsf@gitster.g","threadId":"56098","inReplyTo":"nycvar.QRO.7.76.6.2107142350570.59@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] ci: run `make sparse` as a GitHub workflow","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-14T22:27:34Z","receivedAt":"2021-07-14T22:27:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> It wasn't quite obvious why we justify spending 370 minutes one more\n>> time only to rerun 30-second job, though.\n>\n> True.\n>\n> And this is not a new problem. Every time anything happens in those\n> `osx-gcc` or `osx-clang` jobs (e.g. that transient problem with the broken\n> pipes in t5516 [*1*], that's a fun one), a full re-run is necessary, or\n> else the commit and/or PR will remain marked as broken.\n>\n> In other words, while it is totally appropriate for me to explain this to\n> you in this here thread because it came up as a tangent, it would be\n> inappropriate to stick that explanation into this patch's commit message.\n> We do not make a habit of adding tangents that came up during patch\n> reviews into commit messages, and I do not intend to start such a habit\n> here, either.\n\nI do not agree; a brief mention \"even though piling more and more on\nthe primary workflow would make it even less convenient to re-run,\nit is already so bad that another one would not hurt too much more\"\nwould be a clue good enough to motivate others to do something about\nit if they feel like it.\n\n"},{"id":"430290","messageId":"aeecbcf6-eab8-54d4-9654-b5b2e1e1ea23@ramsayjones.plus.com","threadId":"56098","inReplyTo":"nycvar.QRO.7.76.6.2107141212140.76@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] ci: run `make sparse` as a GitHub workflow","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2021-07-16T01:37:31Z","receivedAt":"2021-07-16T01:45:06Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 14/07/2021 11:13, Johannes Schindelin wrote:\n> Hi again,\n> \n> On Wed, 14 Jul 2021, Johannes Schindelin wrote:\n> \n>> On Tue, 13 Jul 2021, Philippe Blain wrote:\n>>\n>>> Le 2021-07-13 à 07:51, Johannes Schindelin via GitGitGadget a écrit :\n>>>> From: Johannes Schindelin \n>>>>\n>>>> [...]\n>>>> + - name: Download the `sparse` package\n>>>> + uses: git-for-windows/get-azure-pipelines-artifact@v0\n>>>> + with:\n>>>> + repository: git/git\n>>>> + definitionId: 10\n>>>> + artifact: sparse-20.04\n>>>> + - name: Install the `sparse` package\n>>>> + run: sudo dpkg -i sparse-20.04/sparse_*.deb\n>>>\n>>> Out of curiosity, why is this necessary (as opposed to using\n>>> the Ubuntu package, i.e. 'sudo apt-get -q -y sparse') ?\n>>\n>> This is actually a historical curiosity: years ago, I created an Azure\n>> Pipeline that builds the `sparse` Debian package for the specific purpose\n>> of using it in our CI builds (if you care to look at the issue 345 I\n>> linked above, you will see how long ago that idea was in the making). Now,\n>> the historical curiosity is that back then, there was no current `sparse`\n>> package available for Ubuntu, and Ramsay mentioned that a newer version\n>> would be required to run `make sparse`.\n>>\n>> And when I implemented this patch yesterday, I did not even question this,\n>> I was just happy that I had come up with the GitHub Action\n>> `get-azure-pipelines-artifact` (to help with the `vcpkg` part of our CI\n>> builds).\n>>\n>> I was already writing a detailed paragraph in the commit message to\n>> explain all that when it occurred to me that two years might make a big\n>> difference and an up to date `sparse` might be available. And lo and\n>> behold, this is the case!\n>>\n>> Therefore, v2 will no longer jump through that hoop.\n> \n> *Never mind*\n> \n> I made a mistake in testing, and Ubuntu-20.04's `sparse` package _is too\n> old_. So I will reintroduce that hoop even before sending v2.\n\nYes, last time I looked, 20.04's version of sparse was 0.6.1-2build1,\n20.10 had 0.6.2-2 (both of which are too old), but 21.04, along with\nDebian Testing, had 0.6.3-1 which would work fine.\n\n[I am currently running v0.6.3-341-g8af24329]\n\nSo, maybe the next Ubuntu LTS, ... :-D\n\nATB,\nRamsay Jones\n\n"},{"id":"430344","messageId":"nycvar.QRO.7.76.6.2107161714290.59@tvgsbejvaqbjf.bet","threadId":"56098","inReplyTo":"xmqqsg0gttt5.fsf@gitster.g","subject":"Re: [PATCH] ci: run `make sparse` as a GitHub workflow","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-07-16T15:25:38Z","receivedAt":"2021-07-16T15:25:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 14 Jul 2021, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n>\n> >> It wasn't quite obvious why we justify spending 370 minutes one more\n> >> time only to rerun 30-second job, though.\n> >\n> > True.\n> >\n> > And this is not a new problem. Every time anything happens in those\n> > `osx-gcc` or `osx-clang` jobs (e.g. that transient problem with the broken\n> > pipes in t5516 [*1*], that's a fun one), a full re-run is necessary, or\n> > else the commit and/or PR will remain marked as broken.\n> >\n> > In other words, while it is totally appropriate for me to explain this to\n> > you in this here thread because it came up as a tangent, it would be\n> > inappropriate to stick that explanation into this patch's commit message.\n> > We do not make a habit of adding tangents that came up during patch\n> > reviews into commit messages, and I do not intend to start such a habit\n> > here, either.\n>\n> I do not agree; a brief mention \"even though piling more and more on\n> the primary workflow would make it even less convenient to re-run,\n> it is already so bad that another one would not hurt too much more\"\n> would be a clue good enough to motivate others to do something about\n> it if they feel like it.\n\nBy that rationale recent additions to `.github/workflows/`, which have a\nmuch much much much bigger impact on the runtime (such as the change that\nlets `linux-clang` run the test suite with SHA-1 and then again with\nSHA-256, almost doubling its runtime), should have added that apology to\nthe commit message.\n\nBut that did not happen.\n\nBesides, for all we know the problem might go away at any stage because\npretty much all other main CI systems have a way to re-run only failed\njobs. GitHub Actions will probably get it at some stage, too, I vaguely\nremember seeing it on some public roadmap somewhere.\n\nSo I really do not appreciate this pushing for including an explanation in\nthe commit message for something that is only relevant (if at all)\nin the context of an utter tangent (don't we have many of those over\nhere!) during the review of the patch. It's just some curiosity that will\neventually be an exclusively historical curiosity, of no interest except\nto a few CI nerds. The issue has everything to do with a currently\nmissing feature in GitHub Actions, and it has nothing to do at all with\nwhat the patch is about, which is: to add a `sparse` job to our CI runs.\n\nCiao,\nDscho\n"},{"id":"430351","messageId":"xmqq4kcurz0f.fsf@gitster.g","threadId":"56098","inReplyTo":"nycvar.QRO.7.76.6.2107161714290.59@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] ci: run `make sparse` as a GitHub workflow","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-16T16:42:40Z","receivedAt":"2021-07-16T16:42:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Besides, for all we know the problem might go away at any stage because\n> pretty much all other main CI systems have a way to re-run only failed\n> jobs.\n\nYes, that is something I find plausible. That's one more reason why\nwe currently feel it is OK to roll it into the primary job.\n\nNow we've left a handful of messages on the list, I think we are\nsafe against anybody who will soon complain that we are piling more\nand more on top of the primary job---instead of pointing at the log\nmessage of the change that did so, we can point at this discussion\nthread to make them understand why we decided that it is OK.\n\n\n"},{"id":"431204","messageId":"YP72o/1Yg02BcDXw@coredump.intra.peff.net","threadId":"56098","inReplyTo":"pull.994.v2.git.1626263433838.gitgitgadget@gmail.com","subject":"[PATCH] ci: run \"apt-get update\" before \"apt-get install\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-07-26T17:53:39Z","receivedAt":"2021-07-26T17:53:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 14, 2021 at 11:50:33AM +0000, Johannes Schindelin via GitGitGadget wrote:\n\n> + - name: Install other dependencies\n> + run: |\n> + sudo apt-get install -q -y libssl-dev libcurl4-openssl-dev libexpat-dev gettext zlib1g-dev\n\nMy CI runs based on 'next' started failing today. I think we'd want this\non top (of js/ci-make-sparse):\n\n-- >8 --\nSubject: [PATCH] ci: run \"apt-get update\" before \"apt-get install\"\n\nThe \"sparse\" workflow runs \"apt-get install\" to pick up a few necessary\npackages. But it needs to run \"apt-get update\" first, or it risks trying\nto download an old package version that no longer exists. And in fact\nthis happens now, with output like:\n\n 2021-07-26T17:40:51.2551880Z E: Failed to fetch http://security.ubuntu.com/ubuntu/pool/main/c/curl/libcurl4-openssl-dev_7.68.0-1ubuntu2.5_amd64.deb 404 Not Found [IP: 52.147.219.192 80]\n 2021-07-26T17:40:51.2554304Z E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing?\n\nOur other ci jobs don't suffer from this; they rely on scripts in ci/,\nand ci/install-dependencies does the appropriate \"apt-get update\".\n\nSigned-off-by: Jeff King \n---\nI guess this package setup could also be moved into ci/install-dependencies.sh,\nbut I don't think it really buys anything (the \"apt-get update\" line\nwould not even be shared, because the outermost layer is a big switch\nstatement on the jobname). OTOH, it looks like other one-off jobs are in\nthere (e.g., StaticAnalysis).\n\nAnyway, this is the minimal fixup.\n\n .github/workflows/main.yml | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex 1b5c039207..01878884ae 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -368,6 +368,7 @@ jobs:\n run: sudo dpkg -i sparse-20.04/sparse_*.deb\n - name: Install other dependencies\n run: |\n+ sudo apt-get update -q &&\n sudo apt-get install -q -y libssl-dev libcurl4-openssl-dev libexpat-dev gettext zlib1g-dev\n - uses: actions/checkout@v2\n - run: make sparse\n-- \n2.32.0.805.g1fa0022869\n\n"},{"id":"431211","messageId":"YP79Y/jESfCPBKM/@coredump.intra.peff.net","threadId":"56098","inReplyTo":"YP72o/1Yg02BcDXw@coredump.intra.peff.net","subject":"Re: [PATCH] ci: run \"apt-get update\" before \"apt-get install\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-07-26T18:22:27Z","receivedAt":"2021-07-26T18:22:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 26, 2021 at 01:53:40PM -0400, Jeff King wrote:\n\n> I guess this package setup could also be moved into ci/install-dependencies.sh,\n> but I don't think it really buys anything (the \"apt-get update\" line\n> would not even be shared, because the outermost layer is a big switch\n> statement on the jobname). OTOH, it looks like other one-off jobs are in\n> there (e.g., StaticAnalysis).\n> \n> Anyway, this is the minimal fixup.\n\nThat would look like this on top. I don't have a strong feeling either\nway.\n\n-- >8 --\nSubject: [PATCH] ci/install-dependencies: handle \"sparse\" job package installs\n\nThis just matches the style/location of the package installation for\nother jobs. There should be no functional change.\n\nI did flip the order of the options and command-name (\"-y update\"\ninstead of \"update -y\") for consistency with other lines in the same\nfile.\n\nNote also that we have to reorder the dependency install with the\n\"checkout\" action, so that we actually have the \"ci\" scripts available.\n\nSigned-off-by: Jeff King \n---\n .github/workflows/main.yml | 6 ++----\n ci/install-dependencies.sh | 5 +++++\n 2 files changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex 01878884ae..224c46b6d6 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -366,11 +366,9 @@ jobs:\n artifact: sparse-20.04\n - name: Install the current `sparse` package\n run: sudo dpkg -i sparse-20.04/sparse_*.deb\n- - name: Install other dependencies\n- run: |\n- sudo apt-get update -q &&\n- sudo apt-get install -q -y libssl-dev libcurl4-openssl-dev libexpat-dev gettext zlib1g-dev\n - uses: actions/checkout@v2\n+ - name: Install other dependencies\n+ run: ci/install-dependencies.sh\n - run: make sparse\n documentation:\n needs: ci-config\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex 67852d0d37..5772081b6e 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -65,6 +65,11 @@ StaticAnalysis)\n \tsudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \\\n \t\tlibexpat-dev gettext make\n \t;;\n+sparse)\n+\tsudo apt-get -q update -q\n+\tsudo apt-get -q -y install libssl-dev libcurl4-openssl-dev \\\n+\t\tlibexpat-dev gettext zlib1g-dev\n+\t;;\n Documentation)\n \tsudo apt-get -q update\n \tsudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns make\n-- \n2.32.0.806.g45f317a2a5\n\n"},{"id":"431234","messageId":"pull.994.v3.git.1627331236817.gitgitgadget@gmail.com","threadId":"56098","inReplyTo":"pull.994.v2.git.1626263433838.gitgitgadget@gmail.com","subject":"[PATCH v3] ci: run `make sparse` as part of the GitHub workflow","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-26T20:27:16Z","receivedAt":"2021-07-26T20:27:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nOccasionally we receive reviews after patches were integrated, where\n`sparse` (https://sparse.docs.kernel.org/en/latest/ has more information\non that project) identified problems such as file-local variables or\nfunctions being declared as global.\n\nBy running `sparse` as part of our Continuous Integration, we can catch\nsuch things much earlier. Even better: developers who activated GitHub\nActions on their forks can catch such issues before even sending their\npatches to the Git mailing list.\n\nThis addresses https://github.com/gitgitgadget/git/issues/345\n\nNote: Not even Ubuntu 20.04 ships with a new enough version of `sparse`\nto accommodate Git's needs. The symptom looks like this:\n\n add-interactive.c:537:51: error: Using plain integer as NULL pointer\n\nTo counter that, we download and install the custom-built `sparse`\npackage from the Azure Pipeline that we specifically created to address\nthis issue.\n\nHelped-by: Jeff King \nSigned-off-by: Johannes Schindelin \n---\n ci: run make sparse as a GitHub workflow\n \n One of the earliest open source static analyzers is called \"sparse\", and\n occasionally Ramsay Jones sends out mails on the Git mailing list that\n some function or other should be declared static because sparse found\n out that it is only used within the same file.\n \n Let's add a GitHub workflow running \"make sparse\".\n \n Example run:\n https://github.com/gitgitgadget/git/pull/994/checks?check_run_id=3065255116\n \n Changes since v2:\n \n * We're now reusing the ci/install-dependencies.sh script even in the\n sparse job.\n \n Changes since v1:\n \n * The job was folded into main.yml\n * The commit message and a code comment now explain why we have to\n download & install a custom sparse package instead of using Ubuntu's\n default one\n * The commit message now contains a link to the documentation of the\n sparse tool\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-994%2Fdscho%2Fci-enable-sparse-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-994/dscho/ci-enable-sparse-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/994\n\nRange-diff vs v2:\n\n 1: 8b631acfaa2 ! 1: 762c4cbb6e2 ci: run `make sparse` as part of the GitHub workflow\n @@ Commit message\n package from the Azure Pipeline that we specifically created to address\n this issue.\n \n + Helped-by: Jeff King \n Signed-off-by: Johannes Schindelin \n \n ## .github/workflows/main.yml ##\n @@ .github/workflows/main.yml: jobs:\n + artifact: sparse-20.04\n + - name: Install the current `sparse` package\n + run: sudo dpkg -i sparse-20.04/sparse_*.deb\n -+ - name: Install other dependencies\n -+ run: |\n -+ sudo apt-get install -q -y libssl-dev libcurl4-openssl-dev libexpat-dev gettext zlib1g-dev\n + - uses: actions/checkout@v2\n ++ - name: Install other dependencies\n ++ run: ci/install-dependencies.sh\n + - run: make sparse\n documentation:\n needs: ci-config\n if: needs.ci-config.outputs.enabled == 'yes'\n +\n + ## ci/install-dependencies.sh ##\n +@@ ci/install-dependencies.sh: StaticAnalysis)\n + \tsudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \\\n + \t\tlibexpat-dev gettext make\n + \t;;\n ++sparse)\n ++\tsudo apt-get -q update -q\n ++\tsudo apt-get -q -y install libssl-dev libcurl4-openssl-dev \\\n ++\t\tlibexpat-dev gettext zlib1g-dev\n ++\t;;\n + Documentation)\n + \tsudo apt-get -q update\n + \tsudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns make\n\n\n .github/workflows/main.yml | 20 ++++++++++++++++++++\n ci/install-dependencies.sh | 5 +++++\n 2 files changed, 25 insertions(+)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex 73856bafc9d..224c46b6d6a 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -350,6 +350,26 @@ jobs:\n - uses: actions/checkout@v1\n - run: ci/install-dependencies.sh\n - run: ci/run-static-analysis.sh\n+ sparse:\n+ needs: ci-config\n+ if: needs.ci-config.outputs.enabled == 'yes'\n+ env:\n+ jobname: sparse\n+ runs-on: ubuntu-20.04\n+ steps:\n+ - name: Download a current `sparse` package\n+ # Ubuntu's `sparse` version is too old for us\n+ uses: git-for-windows/get-azure-pipelines-artifact@v0\n+ with:\n+ repository: git/git\n+ definitionId: 10\n+ artifact: sparse-20.04\n+ - name: Install the current `sparse` package\n+ run: sudo dpkg -i sparse-20.04/sparse_*.deb\n+ - uses: actions/checkout@v2\n+ - name: Install other dependencies\n+ run: ci/install-dependencies.sh\n+ - run: make sparse\n documentation:\n needs: ci-config\n if: needs.ci-config.outputs.enabled == 'yes'\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex 67852d0d37f..5772081b6e5 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -65,6 +65,11 @@ StaticAnalysis)\n \tsudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \\\n \t\tlibexpat-dev gettext make\n \t;;\n+sparse)\n+\tsudo apt-get -q update -q\n+\tsudo apt-get -q -y install libssl-dev libcurl4-openssl-dev \\\n+\t\tlibexpat-dev gettext zlib1g-dev\n+\t;;\n Documentation)\n \tsudo apt-get -q update\n \tsudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns make\n\nbase-commit: 75ae10bc75336db031ee58d13c5037b929235912\n-- \ngitgitgadget\n"},{"id":"431241","messageId":"xmqqczr4hghl.fsf@gitster.g","threadId":"56098","inReplyTo":"YP72o/1Yg02BcDXw@coredump.intra.peff.net","subject":"Re: [PATCH] ci: run \"apt-get update\" before \"apt-get install\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-26T22:12:22Z","receivedAt":"2021-07-26T22:12:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Jul 14, 2021 at 11:50:33AM +0000, Johannes Schindelin via GitGitGadget wrote:\n>\n>> + - name: Install other dependencies\n>> + run: |\n>> + sudo apt-get install -q -y libssl-dev libcurl4-openssl-dev libexpat-dev gettext zlib1g-dev\n>\n> My CI runs based on 'next' started failing today. I think we'd want this\n> on top (of js/ci-make-sparse):\n\nMakes sense.\n\n> -- >8 --\n> Subject: [PATCH] ci: run \"apt-get update\" before \"apt-get install\"\n>\n> The \"sparse\" workflow runs \"apt-get install\" to pick up a few necessary\n> packages. But it needs to run \"apt-get update\" first, or it risks trying\n> to download an old package version that no longer exists. And in fact\n> this happens now, with output like:\n>\n> 2021-07-26T17:40:51.2551880Z E: Failed to fetch http://security.ubuntu.com/ubuntu/pool/main/c/curl/libcurl4-openssl-dev_7.68.0-1ubuntu2.5_amd64.deb 404 Not Found [IP: 52.147.219.192 80]\n> 2021-07-26T17:40:51.2554304Z E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing?\n>\n> Our other ci jobs don't suffer from this; they rely on scripts in ci/,\n> and ci/install-dependencies does the appropriate \"apt-get update\".\n>\n> Signed-off-by: Jeff King \n> ---\n> I guess this package setup could also be moved into ci/install-dependencies.sh,\n> but I don't think it really buys anything (the \"apt-get update\" line\n> would not even be shared, because the outermost layer is a big switch\n> statement on the jobname). OTOH, it looks like other one-off jobs are in\n> there (e.g., StaticAnalysis).\n>\n> Anyway, this is the minimal fixup.\n>\n> .github/workflows/main.yml | 1 +\n> 1 file changed, 1 insertion(+)\n>\n> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n> index 1b5c039207..01878884ae 100644\n> --- a/.github/workflows/main.yml\n> +++ b/.github/workflows/main.yml\n> @@ -368,6 +368,7 @@ jobs:\n> run: sudo dpkg -i sparse-20.04/sparse_*.deb\n> - name: Install other dependencies\n> run: |\n> + sudo apt-get update -q &&\n> sudo apt-get install -q -y libssl-dev libcurl4-openssl-dev libexpat-dev gettext zlib1g-dev\n> - uses: actions/checkout@v2\n> - run: make sparse\n"},{"id":"431243","messageId":"xmqq8s1shg4e.fsf@gitster.g","threadId":"56098","inReplyTo":"pull.994.v3.git.1627331236817.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] ci: run `make sparse` as part of the GitHub workflow","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-26T22:20:17Z","receivedAt":"2021-07-26T22:20:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> Occasionally we receive reviews after patches were integrated, where\n> `sparse` (https://sparse.docs.kernel.org/en/latest/ has more information\n> on that project) identified problems such as file-local variables or\n> functions being declared as global.\n>\n> By running `sparse` as part of our Continuous Integration, we can catch\n> such things much earlier. Even better: developers who activated GitHub\n> Actions on their forks can catch such issues before even sending their\n> patches to the Git mailing list.\n>\n> This addresses https://github.com/gitgitgadget/git/issues/345\n>\n> Note: Not even Ubuntu 20.04 ships with a new enough version of `sparse`\n> to accommodate Git's needs. The symptom looks like this:\n>\n> add-interactive.c:537:51: error: Using plain integer as NULL pointer\n>\n> To counter that, we download and install the custom-built `sparse`\n> package from the Azure Pipeline that we specifically created to address\n> this issue.\n>\n> Helped-by: Jeff King \n> Signed-off-by: Johannes Schindelin \n> ---\n\nOK, this is with Peff's \"oh, we need 'apt-get update' to avoid stale\npackage list\" fix already rolled in? It seems you took both of them,\nand they look good to me, too.\n\nUnfortunately, ci-make-sparse has already been in 'next' for the\npast week or so, so incrementals are vastly preferred.\n\nLet me squish in your \"Acked-by\" to both of Peff's patches and queue\nthem on top of js/ci-make-sparse topic.\n\nThanks.\n\n"}]}