{"thread":{"id":"51947","subject":"[PATCH 0/1] Include hdr-check in the CI builds","startedAt":"2019-10-01T11:16:29Z","lastAt":"2019-10-07T18:51:50Z","messageCount":7,"participants":["Johannes Schindelin via GitGitGadget","Denton Liu","Johannes Schindelin","Junio C Hamano","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"383196","messageId":"pull.368.git.gitgitgadget@gmail.com","threadId":"51947","inReplyTo":null,"subject":"[PATCH 0/1] Include hdr-check in the CI builds","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-01T11:16:25Z","receivedAt":"2019-10-01T11:16:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Our hdr-check target is now functional in more ways than before. Let's run\nit as part of the CI builds.\n\nI offered this idea in a review of dl/honor-cflags-in-hdr-check\n[https://public-inbox.org/git/nycvar.QRO.7.76.6.1909261452340.15067@tvgsbejvaqbjf.bet/] \nbut it was not picked up. So I offer it as a stand-alone patch on top of\nthat series.\n\nJohannes Schindelin (1):\n ci: run `hdr-check` as part of the `Static Analysis` job\n\n azure-pipelines.yml | 2 +-\n ci/install-dependencies.sh | 3 ++-\n ci/run-static-analysis.sh | 3 +++\n 3 files changed, 6 insertions(+), 2 deletions(-)\n\n\nbase-commit: b503a2d515e6ac3d2cced7881791c12663c45d01\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-368%2Fdscho%2Fhdr-check-in-ci-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-368/dscho/hdr-check-in-ci-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/368\n-- \ngitgitgadget\n"},{"id":"383197","messageId":"4b3b58ca6ddff476acff6735129049588c0a1f13.1569928584.git.gitgitgadget@gmail.com","threadId":"51947","inReplyTo":"pull.368.git.gitgitgadget@gmail.com","subject":"[PATCH 1/1] ci: run `hdr-check` as part of the `Static Analysis` job","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-01T11:16:26Z","receivedAt":"2019-10-01T11:16:31Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nSigned-off-by: Johannes Schindelin \n---\n azure-pipelines.yml | 2 +-\n ci/install-dependencies.sh | 3 ++-\n ci/run-static-analysis.sh | 3 +++\n 3 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/azure-pipelines.yml b/azure-pipelines.yml\nindex c329b7218b..15831f6006 100644\n--- a/azure-pipelines.yml\n+++ b/azure-pipelines.yml\n@@ -354,7 +354,7 @@ jobs:\n test \"$GITFILESHAREPWD\" = '$(gitfileshare.pwd)' || ci/mount-fileshare.sh //gitfileshare.file.core.windows.net/test-cache gitfileshare \"$GITFILESHAREPWD\" \"$HOME/test-cache\" || exit 1\n \n sudo apt-get update &&\n- sudo apt-get install -y coccinelle &&\n+ sudo apt-get install -y coccinelle coccinelle libcurl4-openssl-dev libssl-dev libexpat-dev gettext &&\n \n export jobname=StaticAnalysis &&\n \ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex 8cc72503cb..8ce9ce276e 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -49,7 +49,8 @@ osx-clang|osx-gcc)\n \t;;\n StaticAnalysis)\n \tsudo apt-get -q update\n-\tsudo apt-get -q -y install coccinelle\n+\tsudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \\\n+\t\tlibexpat-dev gettext\n \t;;\n Documentation)\n \tsudo apt-get -q update\ndiff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh\nindex a19aa7ebbc..65bcebda41 100755\n--- a/ci/run-static-analysis.sh\n+++ b/ci/run-static-analysis.sh\n@@ -26,4 +26,7 @@ then\n \texit 1\n fi\n \n+make hdr-check ||\n+exit 1\n+\n save_good_tree\n-- \ngitgitgadget\n"},{"id":"383216","messageId":"20191001164258.GA1356@generichostname","threadId":"51947","inReplyTo":"4b3b58ca6ddff476acff6735129049588c0a1f13.1569928584.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] ci: run `hdr-check` as part of the `Static Analysis` job","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2019-10-01T16:42:58Z","receivedAt":"2019-10-01T16:43:03Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Hi Johannes,\n\nSorry for not replying or rolling this in earlier, I was waiting for my\nseries to hit master before doing this change but I guess it doesn't\nreally hurt to do it any earlier.\n\nOn Tue, Oct 01, 2019 at 04:16:26AM -0700, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> Signed-off-by: Johannes Schindelin \n> ---\n> azure-pipelines.yml | 2 +-\n> ci/install-dependencies.sh | 3 ++-\n> ci/run-static-analysis.sh | 3 +++\n> 3 files changed, 6 insertions(+), 2 deletions(-)\n> \n> diff --git a/azure-pipelines.yml b/azure-pipelines.yml\n> index c329b7218b..15831f6006 100644\n> --- a/azure-pipelines.yml\n> +++ b/azure-pipelines.yml\n> @@ -354,7 +354,7 @@ jobs:\n> test \"$GITFILESHAREPWD\" = '$(gitfileshare.pwd)' || ci/mount-fileshare.sh //gitfileshare.file.core.windows.net/test-cache gitfileshare \"$GITFILESHAREPWD\" \"$HOME/test-cache\" || exit 1\n> \n> sudo apt-get update &&\n> - sudo apt-get install -y coccinelle &&\n> + sudo apt-get install -y coccinelle coccinelle libcurl4-openssl-dev libssl-dev libexpat-dev gettext &&\n\nSeems like you accidentally included coccinelle twice.\n\nAnyway, thanks for picking up where I left off.\n\n-Denton\n\n> \n> export jobname=StaticAnalysis &&\n> \n> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\n> index 8cc72503cb..8ce9ce276e 100755\n> --- a/ci/install-dependencies.sh\n> +++ b/ci/install-dependencies.sh\n> @@ -49,7 +49,8 @@ osx-clang|osx-gcc)\n> \t;;\n> StaticAnalysis)\n> \tsudo apt-get -q update\n> -\tsudo apt-get -q -y install coccinelle\n> +\tsudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \\\n> +\t\tlibexpat-dev gettext\n> \t;;\n> Documentation)\n> \tsudo apt-get -q update\n> diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh\n> index a19aa7ebbc..65bcebda41 100755\n> --- a/ci/run-static-analysis.sh\n> +++ b/ci/run-static-analysis.sh\n> @@ -26,4 +26,7 @@ then\n> \texit 1\n> fi\n> \n> +make hdr-check ||\n> +exit 1\n> +\n> save_good_tree\n> -- \n> gitgitgadget\n"},{"id":"383228","messageId":"nycvar.QRO.7.76.6.1910012041550.46@tvgsbejvaqbjf.bet","threadId":"51947","inReplyTo":"20191001164258.GA1356@generichostname","subject":"Re: [PATCH 1/1] ci: run `hdr-check` as part of the `Static Analysis` job","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-01T18:45:24Z","receivedAt":"2019-10-01T18:45:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Denton,\n\nOn Tue, 1 Oct 2019, Denton Liu wrote:\n\n> Sorry for not replying or rolling this in earlier, I was waiting for\n> my series to hit master before doing this change but I guess it\n> doesn't really hurt to do it any earlier.\n\nNo worries, thank you for working on this!\n\nI wonder whether it would make sense to add a `hdr-check` to at least\none macOS and on Windows job, too, so that we get some confidence in the\nnon-Linux contributions. But that I _will_ leave to others ;-)\n\n> On Tue, Oct 01, 2019 at 04:16:26AM -0700, Johannes Schindelin via GitGitGadget wrote:\n> > From: Johannes Schindelin \n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > azure-pipelines.yml | 2 +-\n> > ci/install-dependencies.sh | 3 ++-\n> > ci/run-static-analysis.sh | 3 +++\n> > 3 files changed, 6 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/azure-pipelines.yml b/azure-pipelines.yml\n> > index c329b7218b..15831f6006 100644\n> > --- a/azure-pipelines.yml\n> > +++ b/azure-pipelines.yml\n> > @@ -354,7 +354,7 @@ jobs:\n> > test \"$GITFILESHAREPWD\" = '$(gitfileshare.pwd)' || ci/mount-fileshare.sh //gitfileshare.file.core.windows.net/test-cache gitfileshare \"$GITFILESHAREPWD\" \"$HOME/test-cache\" || exit 1\n> >\n> > sudo apt-get update &&\n> > - sudo apt-get install -y coccinelle &&\n> > + sudo apt-get install -y coccinelle coccinelle libcurl4-openssl-dev libssl-dev libexpat-dev gettext &&\n>\n> Seems like you accidentally included coccinelle twice.\n\nGood point (copy-edit fail!). I force-pushed a fixed version, and will\nprobably send out a new version tomorrow or Friday.\n\nThanks,\nDscho\n\n>\n> Anyway, thanks for picking up where I left off.\n>\n> -Denton\n>\n> >\n> > export jobname=StaticAnalysis &&\n> >\n> > diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\n> > index 8cc72503cb..8ce9ce276e 100755\n> > --- a/ci/install-dependencies.sh\n> > +++ b/ci/install-dependencies.sh\n> > @@ -49,7 +49,8 @@ osx-clang|osx-gcc)\n> > \t;;\n> > StaticAnalysis)\n> > \tsudo apt-get -q update\n> > -\tsudo apt-get -q -y install coccinelle\n> > +\tsudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \\\n> > +\t\tlibexpat-dev gettext\n> > \t;;\n> > Documentation)\n> > \tsudo apt-get -q update\n> > diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh\n> > index a19aa7ebbc..65bcebda41 100755\n> > --- a/ci/run-static-analysis.sh\n> > +++ b/ci/run-static-analysis.sh\n> > @@ -26,4 +26,7 @@ then\n> > \texit 1\n> > fi\n> >\n> > +make hdr-check ||\n> > +exit 1\n> > +\n> > save_good_tree\n> > --\n> > gitgitgadget\n>\n"},{"id":"383249","messageId":"xmqq7e5nfzgx.fsf@gitster-ct.c.googlers.com","threadId":"51947","inReplyTo":"4b3b58ca6ddff476acff6735129049588c0a1f13.1569928584.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] ci: run `hdr-check` as part of the `Static Analysis` job","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-02T06:11:58Z","receivedAt":"2019-10-02T06:12:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> - sudo apt-get install -y coccinelle &&\n> + sudo apt-get install -y coccinelle coccinelle libcurl4-openssl-dev libssl-dev libexpat-dev gettext &&\n\nI think \"s/coccinelle //\" is necessary here (assuming that \"apt-get install\"\nis the same Debian thing I know about).\n\nWill do so locally, so no need to resend, but \"Yeah, that was a\ntypo\" or \"no stupid, what I wrote is right\" would be a nice\nresponse to see, especially if I have to undo the local fix.\n\nThanks.\n\n\n> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\n> index 8cc72503cb..8ce9ce276e 100755\n> --- a/ci/install-dependencies.sh\n> +++ b/ci/install-dependencies.sh\n> @@ -49,7 +49,8 @@ osx-clang|osx-gcc)\n> \t;;\n> StaticAnalysis)\n> \tsudo apt-get -q update\n> -\tsudo apt-get -q -y install coccinelle\n> +\tsudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \\\n> +\t\tlibexpat-dev gettext\n> \t;;\n> Documentation)\n> \tsudo apt-get -q update\n> diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh\n> index a19aa7ebbc..65bcebda41 100755\n> --- a/ci/run-static-analysis.sh\n> +++ b/ci/run-static-analysis.sh\n> @@ -26,4 +26,7 @@ then\n> \texit 1\n> fi\n> \n> +make hdr-check ||\n> +exit 1\n> +\n> save_good_tree\n"},{"id":"383265","messageId":"nycvar.QRO.7.76.6.1910021116400.46@tvgsbejvaqbjf.bet","threadId":"51947","inReplyTo":"xmqq7e5nfzgx.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] ci: run `hdr-check` as part of the `Static Analysis` job","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-02T09:17:03Z","receivedAt":"2019-10-02T09:17:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 2 Oct 2019, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n> > - sudo apt-get install -y coccinelle &&\n> > + sudo apt-get install -y coccinelle coccinelle libcurl4-openssl-dev libssl-dev libexpat-dev gettext &&\n>\n> I think \"s/coccinelle //\" is necessary here (assuming that \"apt-get install\"\n> is the same Debian thing I know about).\n>\n> Will do so locally, so no need to resend, but \"Yeah, that was a\n> typo\" or \"no stupid, what I wrote is right\" would be a nice\n> response to see, especially if I have to undo the local fix.\n\nYes, this was a copy-edit fail on my part.\n\nThanks,\nDscho\n\n>\n> Thanks.\n>\n>\n> > diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\n> > index 8cc72503cb..8ce9ce276e 100755\n> > --- a/ci/install-dependencies.sh\n> > +++ b/ci/install-dependencies.sh\n> > @@ -49,7 +49,8 @@ osx-clang|osx-gcc)\n> > \t;;\n> > StaticAnalysis)\n> > \tsudo apt-get -q update\n> > -\tsudo apt-get -q -y install coccinelle\n> > +\tsudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \\\n> > +\t\tlibexpat-dev gettext\n> > \t;;\n> > Documentation)\n> > \tsudo apt-get -q update\n> > diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh\n> > index a19aa7ebbc..65bcebda41 100755\n> > --- a/ci/run-static-analysis.sh\n> > +++ b/ci/run-static-analysis.sh\n> > @@ -26,4 +26,7 @@ then\n> > \texit 1\n> > fi\n> >\n> > +make hdr-check ||\n> > +exit 1\n> > +\n> > save_good_tree\n>\n"},{"id":"383600","messageId":"20191007185144.GE11529@szeder.dev","threadId":"51947","inReplyTo":"4b3b58ca6ddff476acff6735129049588c0a1f13.1569928584.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] ci: run `hdr-check` as part of the `Static Analysis` job","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-10-07T18:51:44Z","receivedAt":"2019-10-07T18:51:50Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Tue, Oct 01, 2019 at 04:16:26AM -0700, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n\nGood idea, now that 'make hdr-check' runs clean this will help to keep\nit that way.\n\nI think adding it to the static analysis job makes sense, as opposed\nto the additional VM startup, git clone, and dependency installation\noverhead of a dedicated job. However, this way we won't run 'make\nhdr-check' if Coccinelle were to find something undesired, but I think\nthat's acceptable.\n\n> Signed-off-by: Johannes Schindelin \n> ---\n> azure-pipelines.yml | 2 +-\n> ci/install-dependencies.sh | 3 ++-\n> ci/run-static-analysis.sh | 3 +++\n> 3 files changed, 6 insertions(+), 2 deletions(-)\n> \n> diff --git a/azure-pipelines.yml b/azure-pipelines.yml\n> index c329b7218b..15831f6006 100644\n> --- a/azure-pipelines.yml\n> +++ b/azure-pipelines.yml\n> @@ -354,7 +354,7 @@ jobs:\n> test \"$GITFILESHAREPWD\" = '$(gitfileshare.pwd)' || ci/mount-fileshare.sh //gitfileshare.file.core.windows.net/test-cache gitfileshare \"$GITFILESHAREPWD\" \"$HOME/test-cache\" || exit 1\n> \n> sudo apt-get update &&\n> - sudo apt-get install -y coccinelle &&\n> + sudo apt-get install -y coccinelle coccinelle libcurl4-openssl-dev libssl-dev libexpat-dev gettext &&\n\nThese are the same additional packages that the hunk below adds to\n'ci/install-dependencies.sh'.\n\nSo... why not just run 'ci/install-dependencies.sh' in the first place?\n\n(On a related note, I noticed that the regular Linux and OSX jobs on\nAzure Pipelines do run 'ci/install-dependencies.sh', but even in those\njobs 'azure-pipelines.yml' runs its own 'apt-get update && install'\nanyway.)\n\n> export jobname=StaticAnalysis &&\n> \n> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\n> index 8cc72503cb..8ce9ce276e 100755\n> --- a/ci/install-dependencies.sh\n> +++ b/ci/install-dependencies.sh\n> @@ -49,7 +49,8 @@ osx-clang|osx-gcc)\n> \t;;\n> StaticAnalysis)\n> \tsudo apt-get -q update\n> -\tsudo apt-get -q -y install coccinelle\n> +\tsudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \\\n> +\t\tlibexpat-dev gettext\n> \t;;\n> Documentation)\n> \tsudo apt-get -q update\n> diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh\n> index a19aa7ebbc..65bcebda41 100755\n> --- a/ci/run-static-analysis.sh\n> +++ b/ci/run-static-analysis.sh\n> @@ -26,4 +26,7 @@ then\n> \texit 1\n> fi\n> \n> +make hdr-check ||\n> +exit 1\n\nThis '|| exit 1' is unnecessary: our CI scripts are run with 'set -e',\nso the non-zero exit code from a failing 'make hdr-check' alone would\nfail the build.\n\n(Yeah, there is an other 'exit 1' in the patch context above, but\nthat's a whole different ballpark and we do need that one. Unlike\n'make hdr-check', 'make coccicheck' (or even COccinelle itself for\nthat matter) doesn't exit with error when it found something to\ntransform, and it doesn't print those transformations either.\nConsequently, we had to take a few extra steps to fail the build if it\nfound something, and that 'exit 1' is part of that. More details in\n0860a7641b (travis-ci: fail if Coccinelle static analysis found\nsomething to transform, 2018-07-23).)\n\n\n> +\n> save_good_tree\n> -- \n> gitgitgadget\n"}]}