git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
* [PATCH 0/1] Include hdr-check in the CI builds
@ 2019-10-01 11:16 Johannes Schindelin via GitGitGadget
  2019-10-01 11:16 ` [PATCH 1/1] ci: run `hdr-check` as part of the `Static Analysis` job Johannes Schindelin via GitGitGadget
  0 siblings, 1 reply; 7+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-10-01 11:16 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Junio C Hamano

Our hdr-check target is now functional in more ways than before. Let's run
it as part of the CI builds.

I offered this idea in a review of dl/honor-cflags-in-hdr-check
[https://public-inbox.org/git/nycvar.QRO.7.76.6.1909261452340.15067@tvgsbejvaqbjf.bet/] 
but it was not picked up. So I offer it as a stand-alone patch on top of
that series.

Johannes Schindelin (1):
  ci: run `hdr-check` as part of the `Static Analysis` job

 azure-pipelines.yml        | 2 +-
 ci/install-dependencies.sh | 3 ++-
 ci/run-static-analysis.sh  | 3 +++
 3 files changed, 6 insertions(+), 2 deletions(-)


base-commit: b503a2d515e6ac3d2cced7881791c12663c45d01
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-368%2Fdscho%2Fhdr-check-in-ci-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-368/dscho/hdr-check-in-ci-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/368
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/1] ci: run `hdr-check` as part of the `Static Analysis` job
  2019-10-01 11:16 [PATCH 0/1] Include hdr-check in the CI builds Johannes Schindelin via GitGitGadget
@ 2019-10-01 11:16 ` Johannes Schindelin via GitGitGadget
  2019-10-01 16:42   ` Denton Liu
                     ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2019-10-01 11:16 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Junio C Hamano, Johannes Schindelin

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 azure-pipelines.yml        | 2 +-
 ci/install-dependencies.sh | 3 ++-
 ci/run-static-analysis.sh  | 3 +++
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/azure-pipelines.yml b/azure-pipelines.yml
index c329b7218b..15831f6006 100644
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@@ -354,7 +354,7 @@ jobs:
        test "$GITFILESHAREPWD" = '$(gitfileshare.pwd)' || ci/mount-fileshare.sh //gitfileshare.file.core.windows.net/test-cache gitfileshare "$GITFILESHAREPWD" "$HOME/test-cache" || exit 1
 
        sudo apt-get update &&
-       sudo apt-get install -y coccinelle &&
+       sudo apt-get install -y coccinelle  coccinelle libcurl4-openssl-dev libssl-dev libexpat-dev gettext &&
 
        export jobname=StaticAnalysis &&
 
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 8cc72503cb..8ce9ce276e 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -49,7 +49,8 @@ osx-clang|osx-gcc)
 	;;
 StaticAnalysis)
 	sudo apt-get -q update
-	sudo apt-get -q -y install coccinelle
+	sudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \
+		libexpat-dev gettext
 	;;
 Documentation)
 	sudo apt-get -q update
diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh
index a19aa7ebbc..65bcebda41 100755
--- a/ci/run-static-analysis.sh
+++ b/ci/run-static-analysis.sh
@@ -26,4 +26,7 @@ then
 	exit 1
 fi
 
+make hdr-check ||
+exit 1
+
 save_good_tree
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/1] ci: run `hdr-check` as part of the `Static Analysis` job
  2019-10-01 11:16 ` [PATCH 1/1] ci: run `hdr-check` as part of the `Static Analysis` job Johannes Schindelin via GitGitGadget
@ 2019-10-01 16:42   ` Denton Liu
  2019-10-01 18:45     ` Johannes Schindelin
  2019-10-02  6:11   ` Junio C Hamano
  2019-10-07 18:51   ` SZEDER Gábor
  2 siblings, 1 reply; 7+ messages in thread
From: Denton Liu @ 2019-10-01 16:42 UTC (permalink / raw)
  To: Johannes Schindelin via GitGitGadget
  Cc: git, Johannes Schindelin, Junio C Hamano

Hi Johannes,

Sorry for not replying or rolling this in earlier, I was waiting for my
series to hit master before doing this change but I guess it doesn't
really hurt to do it any earlier.

On Tue, Oct 01, 2019 at 04:16:26AM -0700, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  azure-pipelines.yml        | 2 +-
>  ci/install-dependencies.sh | 3 ++-
>  ci/run-static-analysis.sh  | 3 +++
>  3 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/azure-pipelines.yml b/azure-pipelines.yml
> index c329b7218b..15831f6006 100644
> --- a/azure-pipelines.yml
> +++ b/azure-pipelines.yml
> @@ -354,7 +354,7 @@ jobs:
>         test "$GITFILESHAREPWD" = '$(gitfileshare.pwd)' || ci/mount-fileshare.sh //gitfileshare.file.core.windows.net/test-cache gitfileshare "$GITFILESHAREPWD" "$HOME/test-cache" || exit 1
>  
>         sudo apt-get update &&
> -       sudo apt-get install -y coccinelle &&
> +       sudo apt-get install -y coccinelle  coccinelle libcurl4-openssl-dev libssl-dev libexpat-dev gettext &&

Seems like you accidentally included coccinelle twice.

Anyway, thanks for picking up where I left off.

-Denton

>  
>         export jobname=StaticAnalysis &&
>  
> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index 8cc72503cb..8ce9ce276e 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -49,7 +49,8 @@ osx-clang|osx-gcc)
>  	;;
>  StaticAnalysis)
>  	sudo apt-get -q update
> -	sudo apt-get -q -y install coccinelle
> +	sudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \
> +		libexpat-dev gettext
>  	;;
>  Documentation)
>  	sudo apt-get -q update
> diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh
> index a19aa7ebbc..65bcebda41 100755
> --- a/ci/run-static-analysis.sh
> +++ b/ci/run-static-analysis.sh
> @@ -26,4 +26,7 @@ then
>  	exit 1
>  fi
>  
> +make hdr-check ||
> +exit 1
> +
>  save_good_tree
> -- 
> gitgitgadget

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/1] ci: run `hdr-check` as part of the `Static Analysis` job
  2019-10-01 16:42   ` Denton Liu
@ 2019-10-01 18:45     ` Johannes Schindelin
  0 siblings, 0 replies; 7+ messages in thread
From: Johannes Schindelin @ 2019-10-01 18:45 UTC (permalink / raw)
  To: Denton Liu; +Cc: Johannes Schindelin via GitGitGadget, git, Junio C Hamano

Hi Denton,

On Tue, 1 Oct 2019, Denton Liu wrote:

> Sorry for not replying or rolling this in earlier, I was waiting for
> my series to hit master before doing this change but I guess it
> doesn't really hurt to do it any earlier.

No worries, thank you for working on this!

I wonder whether it would make sense to add a `hdr-check` to at least
one macOS and on Windows job, too, so that we get some confidence in the
non-Linux contributions. But that I _will_ leave to others ;-)

> On Tue, Oct 01, 2019 at 04:16:26AM -0700, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  azure-pipelines.yml        | 2 +-
> >  ci/install-dependencies.sh | 3 ++-
> >  ci/run-static-analysis.sh  | 3 +++
> >  3 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/azure-pipelines.yml b/azure-pipelines.yml
> > index c329b7218b..15831f6006 100644
> > --- a/azure-pipelines.yml
> > +++ b/azure-pipelines.yml
> > @@ -354,7 +354,7 @@ jobs:
> >         test "$GITFILESHAREPWD" = '$(gitfileshare.pwd)' || ci/mount-fileshare.sh //gitfileshare.file.core.windows.net/test-cache gitfileshare "$GITFILESHAREPWD" "$HOME/test-cache" || exit 1
> >
> >         sudo apt-get update &&
> > -       sudo apt-get install -y coccinelle &&
> > +       sudo apt-get install -y coccinelle  coccinelle libcurl4-openssl-dev libssl-dev libexpat-dev gettext &&
>
> Seems like you accidentally included coccinelle twice.

Good point (copy-edit fail!). I force-pushed a fixed version, and will
probably send out a new version tomorrow or Friday.

Thanks,
Dscho

>
> Anyway, thanks for picking up where I left off.
>
> -Denton
>
> >
> >         export jobname=StaticAnalysis &&
> >
> > diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> > index 8cc72503cb..8ce9ce276e 100755
> > --- a/ci/install-dependencies.sh
> > +++ b/ci/install-dependencies.sh
> > @@ -49,7 +49,8 @@ osx-clang|osx-gcc)
> >  	;;
> >  StaticAnalysis)
> >  	sudo apt-get -q update
> > -	sudo apt-get -q -y install coccinelle
> > +	sudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \
> > +		libexpat-dev gettext
> >  	;;
> >  Documentation)
> >  	sudo apt-get -q update
> > diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh
> > index a19aa7ebbc..65bcebda41 100755
> > --- a/ci/run-static-analysis.sh
> > +++ b/ci/run-static-analysis.sh
> > @@ -26,4 +26,7 @@ then
> >  	exit 1
> >  fi
> >
> > +make hdr-check ||
> > +exit 1
> > +
> >  save_good_tree
> > --
> > gitgitgadget
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/1] ci: run `hdr-check` as part of the `Static Analysis` job
  2019-10-01 11:16 ` [PATCH 1/1] ci: run `hdr-check` as part of the `Static Analysis` job Johannes Schindelin via GitGitGadget
  2019-10-01 16:42   ` Denton Liu
@ 2019-10-02  6:11   ` Junio C Hamano
  2019-10-02  9:17     ` Johannes Schindelin
  2019-10-07 18:51   ` SZEDER Gábor
  2 siblings, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2019-10-02  6:11 UTC (permalink / raw)
  To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> -       sudo apt-get install -y coccinelle &&
> +       sudo apt-get install -y coccinelle  coccinelle libcurl4-openssl-dev libssl-dev libexpat-dev gettext &&

I think "s/coccinelle  //" is necessary here (assuming that "apt-get install"
is the same Debian thing I know about).

Will do so locally, so no need to resend, but "Yeah, that was a
typo" or "no stupid, what I wrote is right" would be a nice
response to see, especially if I have to undo the local fix.

Thanks.


> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index 8cc72503cb..8ce9ce276e 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -49,7 +49,8 @@ osx-clang|osx-gcc)
>  	;;
>  StaticAnalysis)
>  	sudo apt-get -q update
> -	sudo apt-get -q -y install coccinelle
> +	sudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \
> +		libexpat-dev gettext
>  	;;
>  Documentation)
>  	sudo apt-get -q update
> diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh
> index a19aa7ebbc..65bcebda41 100755
> --- a/ci/run-static-analysis.sh
> +++ b/ci/run-static-analysis.sh
> @@ -26,4 +26,7 @@ then
>  	exit 1
>  fi
>  
> +make hdr-check ||
> +exit 1
> +
>  save_good_tree

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/1] ci: run `hdr-check` as part of the `Static Analysis` job
  2019-10-02  6:11   ` Junio C Hamano
@ 2019-10-02  9:17     ` Johannes Schindelin
  0 siblings, 0 replies; 7+ messages in thread
From: Johannes Schindelin @ 2019-10-02  9:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin via GitGitGadget, git

Hi Junio,

On Wed, 2 Oct 2019, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > -       sudo apt-get install -y coccinelle &&
> > +       sudo apt-get install -y coccinelle  coccinelle libcurl4-openssl-dev libssl-dev libexpat-dev gettext &&
>
> I think "s/coccinelle  //" is necessary here (assuming that "apt-get install"
> is the same Debian thing I know about).
>
> Will do so locally, so no need to resend, but "Yeah, that was a
> typo" or "no stupid, what I wrote is right" would be a nice
> response to see, especially if I have to undo the local fix.

Yes, this was a copy-edit fail on my part.

Thanks,
Dscho

>
> Thanks.
>
>
> > diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> > index 8cc72503cb..8ce9ce276e 100755
> > --- a/ci/install-dependencies.sh
> > +++ b/ci/install-dependencies.sh
> > @@ -49,7 +49,8 @@ osx-clang|osx-gcc)
> >  	;;
> >  StaticAnalysis)
> >  	sudo apt-get -q update
> > -	sudo apt-get -q -y install coccinelle
> > +	sudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \
> > +		libexpat-dev gettext
> >  	;;
> >  Documentation)
> >  	sudo apt-get -q update
> > diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh
> > index a19aa7ebbc..65bcebda41 100755
> > --- a/ci/run-static-analysis.sh
> > +++ b/ci/run-static-analysis.sh
> > @@ -26,4 +26,7 @@ then
> >  	exit 1
> >  fi
> >
> > +make hdr-check ||
> > +exit 1
> > +
> >  save_good_tree
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/1] ci: run `hdr-check` as part of the `Static Analysis` job
  2019-10-01 11:16 ` [PATCH 1/1] ci: run `hdr-check` as part of the `Static Analysis` job Johannes Schindelin via GitGitGadget
  2019-10-01 16:42   ` Denton Liu
  2019-10-02  6:11   ` Junio C Hamano
@ 2019-10-07 18:51   ` SZEDER Gábor
  2 siblings, 0 replies; 7+ messages in thread
From: SZEDER Gábor @ 2019-10-07 18:51 UTC (permalink / raw)
  To: Johannes Schindelin via GitGitGadget
  Cc: git, Johannes Schindelin, Junio C Hamano

On Tue, Oct 01, 2019 at 04:16:26AM -0700, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>

Good idea, now that 'make hdr-check' runs clean this will help to keep
it that way.

I think adding it to the static analysis job makes sense, as opposed
to the additional VM startup, git clone, and dependency installation
overhead of a dedicated job.  However, this way we won't run 'make
hdr-check' if Coccinelle were to find something undesired, but I think
that's acceptable.

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  azure-pipelines.yml        | 2 +-
>  ci/install-dependencies.sh | 3 ++-
>  ci/run-static-analysis.sh  | 3 +++
>  3 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/azure-pipelines.yml b/azure-pipelines.yml
> index c329b7218b..15831f6006 100644
> --- a/azure-pipelines.yml
> +++ b/azure-pipelines.yml
> @@ -354,7 +354,7 @@ jobs:
>         test "$GITFILESHAREPWD" = '$(gitfileshare.pwd)' || ci/mount-fileshare.sh //gitfileshare.file.core.windows.net/test-cache gitfileshare "$GITFILESHAREPWD" "$HOME/test-cache" || exit 1
>  
>         sudo apt-get update &&
> -       sudo apt-get install -y coccinelle &&
> +       sudo apt-get install -y coccinelle  coccinelle libcurl4-openssl-dev libssl-dev libexpat-dev gettext &&

These are the same additional packages that the hunk below adds to
'ci/install-dependencies.sh'.

So... why not just run 'ci/install-dependencies.sh' in the first place?

(On a related note, I noticed that the regular Linux and OSX jobs on
Azure Pipelines do run 'ci/install-dependencies.sh', but even in those
jobs 'azure-pipelines.yml' runs its own 'apt-get update && install'
anyway.)

>         export jobname=StaticAnalysis &&
>  
> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index 8cc72503cb..8ce9ce276e 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -49,7 +49,8 @@ osx-clang|osx-gcc)
>  	;;
>  StaticAnalysis)
>  	sudo apt-get -q update
> -	sudo apt-get -q -y install coccinelle
> +	sudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \
> +		libexpat-dev gettext
>  	;;
>  Documentation)
>  	sudo apt-get -q update
> diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh
> index a19aa7ebbc..65bcebda41 100755
> --- a/ci/run-static-analysis.sh
> +++ b/ci/run-static-analysis.sh
> @@ -26,4 +26,7 @@ then
>  	exit 1
>  fi
>  
> +make hdr-check ||
> +exit 1

This '|| exit 1' is unnecessary: our CI scripts are run with 'set -e',
so the non-zero exit code from a failing 'make hdr-check' alone would
fail the build.

(Yeah, there is an other 'exit 1' in the patch context above, but
that's a whole different ballpark and we do need that one.  Unlike
'make hdr-check', 'make coccicheck' (or even COccinelle itself for
that matter) doesn't exit with error when it found something to
transform, and it doesn't print those transformations either.
Consequently, we had to take a few extra steps to fail the build if it
found something, and that 'exit 1' is part of that.  More details in
0860a7641b (travis-ci: fail if Coccinelle static analysis found
something to transform, 2018-07-23).)


> +
>  save_good_tree
> -- 
> gitgitgadget

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-01 11:16 [PATCH 0/1] Include hdr-check in the CI builds Johannes Schindelin via GitGitGadget
2019-10-01 11:16 ` [PATCH 1/1] ci: run `hdr-check` as part of the `Static Analysis` job Johannes Schindelin via GitGitGadget
2019-10-01 16:42   ` Denton Liu
2019-10-01 18:45     ` Johannes Schindelin
2019-10-02  6:11   ` Junio C Hamano
2019-10-02  9:17     ` Johannes Schindelin
2019-10-07 18:51   ` SZEDER Gábor

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox