[PATCH] scripts: Add a script to build and submit to coverity.

[PATCH] scripts: Add a script to build and submit to coverity.

[Ian Campbell]

From: Andrew Cooper <andrew.cooper3@citrix.com>

The submission requires a token and email address, which must be
registered as a project admin with the Coverity system. Nonetheless
this is a convenient place to keep it.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Took Andy script and:

- Wrote commit message.
- Generalised the settings handling.
- Added code to call the various steps based on the command line.
- Refactored construction of the curl command line (mainly in order to
  quote the email address).
- Clone mini-os before build.
- Make the upload stage unconditionally to the upload, but not that it
  still isn't in the default set of actions.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
ijc: Maybe we can think of a way to add this to osstest while still
keeping the coverity token private?
---
 scripts/coverity-build.sh | 108 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 108 insertions(+)
 create mode 100755 scripts/coverity-build.sh

diff --git a/scripts/coverity-build.sh b/scripts/coverity-build.sh
new file mode 100755
index 0000000..91defed
--- /dev/null
+++ b/scripts/coverity-build.sh
@@ -0,0 +1,108 @@
+#!/bin/bash -e
+#
+# Copyright 2015 Andrew Cooper <andrew.cooper3@citrix.com>
+#
+# WARNING: This script will blow away any changes in your git working
+# directory. It should probably be run in a dedicated checkout.
+#
+# Requires the coverity tools (e.g. cov-build) to be in $PATH.
+#
+# Set $COV_EMAIL and $COV_TOKEN to credentials, otherwise it will
+# prompt for them.
+#
+# ./scripts/coverity-build.sh <steps>
+#
+# By default <steps> is to do all the prep and build, but not to upload.
+#
+
+function checktools ()
+{
+    if ! command -v cov-build ; then
+        echo "Coverity tools (cov-build, ...) must be in \$PATH"
+        exit 1
+    fi
+}
+function hardclean ()
+{
+    echo "Cleaning working tree"
+    git reset --hard
+    make distclean -j8
+}
+
+function update ()
+{
+    echo "Pulling latest staging"
+    git fetch --all
+    git checkout master
+    git pull --ff
+    git checkout staging
+    git pull --ff
+}
+
+function softclean ()
+{
+    echo "Removing any remaining junk"
+    git clean -dxf
+}
+
+function vars ()
+{
+    export COV_HEAD=$(git rev-parse HEAD)
+    export COV_TARBALL="xen-coverity-$COV_HEAD.tgz"
+    export COV_VERSION="Xen-$(make -C xen xenversion --no-print-directory)"
+}
+
+function prebuild ()
+{
+    echo "Running the pre-build"
+
+    ./configure
+    make -C tools/firmware/etherboot all -j4
+    make mini-os-dir
+}
+
+function build ()
+{
+    echo "Starting Coverity build from $(pwd)"
+
+    cov-build --dir cov-int make -C extras/mini-os/
+    cov-build --dir cov-int make xen tools -j4
+    tar czvf $COV_TARBALL cov-int
+}
+
+function upload ()
+{
+    if [ -z "$COV_EMAIL" ] ; then
+	read -p "Email: " COV_EMAIL
+    fi
+    if [ -z "$COV_TOKEN" ] ; then
+	read -p "Form token: " COV_TOKEN
+    fi
+
+    declare -a curl_args
+    curl_args+=("--form" "token=$COV_TOKEN")
+    curl_args+=("--form" "email=$COV_EMAIL")
+    curl_args+=("--form" "file=@$COV_TARBALL")
+    curl_args+=("--form" "version=$COV_VERSION")
+    curl_args+=("--form" "description=$COV_HEAD")
+    curl_args+=("https://scan.coverity.com/builds?project=XenProject")
+
+    echo "Uploading... curl ${curl_args[@]}"
+    echo curl "${curl_args[@]}" | tee cov-upload.log
+}
+
+function all ()
+{
+    checktools
+    hardclean
+    softclean
+    update
+    softclean
+    vars
+    prebuild
+    build
+}
+
+for cmd in ${@:-all} ; do
+    eval $cmd
+done
-- 
2.1.4

Re: [PATCH] scripts: Add a script to build and submit to coverity.

[Jan Beulich]

>>> On 10.12.15 at 17:53, <ian.campbell@citrix.com> wrote:
> From: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> The submission requires a token and email address, which must be
> registered as a project admin with the Coverity system. Nonetheless
> this is a convenient place to keep it.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Took Andy script and:
> 
> - Wrote commit message.
> - Generalised the settings handling.
> - Added code to call the various steps based on the command line.
> - Refactored construction of the curl command line (mainly in order to
>   quote the email address).
> - Clone mini-os before build.
> - Make the upload stage unconditionally to the upload, but not that it
>   still isn't in the default set of actions.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH] scripts: Add a script to build and submit to coverity.

[Andrew Cooper]

On 10/12/15 16:53, Ian Campbell wrote:
> From: Andrew Cooper <andrew.cooper3@citrix.com>
>
> The submission requires a token and email address, which must be
> registered as a project admin with the Coverity system. Nonetheless
> this is a convenient place to keep it.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> Took Andy script and:

's

>
> - Wrote commit message.
> - Generalised the settings handling.
> - Added code to call the various steps based on the command line.
> - Refactored construction of the curl command line (mainly in order to
>   quote the email address).
> - Clone mini-os before build.
> - Make the upload stage unconditionally to the upload, but not that it
>   still isn't in the default set of actions.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

Looks good to me.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, perhaps with a
few clarifications to comments for other benifits.  See below.

> ---
> ijc: Maybe we can think of a way to add this to osstest while still
> keeping the coverity token private?

This would be ideal.

> ---
>  scripts/coverity-build.sh | 108 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 108 insertions(+)
>  create mode 100755 scripts/coverity-build.sh
>
> diff --git a/scripts/coverity-build.sh b/scripts/coverity-build.sh
> new file mode 100755
> index 0000000..91defed
> --- /dev/null
> +++ b/scripts/coverity-build.sh
> @@ -0,0 +1,108 @@
> +#!/bin/bash -e
> +#
> +# Copyright 2015 Andrew Cooper <andrew.cooper3@citrix.com>
> +#
> +# WARNING: This script will blow away any changes in your git working
> +# directory. It should probably be run in a dedicated checkout.
> +#
> +# Requires the coverity tools (e.g. cov-build) to be in $PATH.
> +#
> +# Set $COV_EMAIL and $COV_TOKEN to credentials, otherwise it will
> +# prompt for them.
> +#
> +# ./scripts/coverity-build.sh <steps>
> +#
> +# By default <steps> is to do all the prep and build, but not to upload.
> +#
> +
> +function checktools ()
> +{
> +    if ! command -v cov-build ; then
> +        echo "Coverity tools (cov-build, ...) must be in \$PATH"
> +        exit 1
> +    fi
> +}
> +function hardclean ()
> +{
> +    echo "Cleaning working tree"
> +    git reset --hard
> +    make distclean -j8
> +}
> +
> +function update ()
> +{
> +    echo "Pulling latest staging"
> +    git fetch --all
> +    git checkout master
> +    git pull --ff
> +    git checkout staging
> +    git pull --ff
> +}
> +
> +function softclean ()
> +{
> +    echo "Removing any remaining junk"
> +    git clean -dxf
> +}
> +
> +function vars ()
> +{
> +    export COV_HEAD=$(git rev-parse HEAD)
> +    export COV_TARBALL="xen-coverity-$COV_HEAD.tgz"
> +    export COV_VERSION="Xen-$(make -C xen xenversion --no-print-directory)"
> +}
> +
> +function prebuild ()
> +{
> +    echo "Running the pre-build"

This is "all the things which might end up calling $(CC) which we don't
care about analysing"

> +
> +    ./configure
> +    make -C tools/firmware/etherboot all -j4
> +    make mini-os-dir
> +}
> +
> +function build ()
> +{
> +    echo "Starting Coverity build from $(pwd)"

And "all the things we care about analysing".

~Andrew

> +
> +    cov-build --dir cov-int make -C extras/mini-os/
> +    cov-build --dir cov-int make xen tools -j4
> +    tar czvf $COV_TARBALL cov-int
> +}
> +
> +function upload ()
> +{
> +    if [ -z "$COV_EMAIL" ] ; then
> +	read -p "Email: " COV_EMAIL
> +    fi
> +    if [ -z "$COV_TOKEN" ] ; then
> +	read -p "Form token: " COV_TOKEN
> +    fi
> +
> +    declare -a curl_args
> +    curl_args+=("--form" "token=$COV_TOKEN")
> +    curl_args+=("--form" "email=$COV_EMAIL")
> +    curl_args+=("--form" "file=@$COV_TARBALL")
> +    curl_args+=("--form" "version=$COV_VERSION")
> +    curl_args+=("--form" "description=$COV_HEAD")
> +    curl_args+=("https://scan.coverity.com/builds?project=XenProject")
> +
> +    echo "Uploading... curl ${curl_args[@]}"
> +    echo curl "${curl_args[@]}" | tee cov-upload.log
> +}
> +
> +function all ()
> +{
> +    checktools
> +    hardclean
> +    softclean
> +    update
> +    softclean
> +    vars
> +    prebuild
> +    build
> +}
> +
> +for cmd in ${@:-all} ; do
> +    eval $cmd
> +done

Re: [PATCH] scripts: Add a script to build and submit to coverity.

[Ian Jackson]

Andrew Cooper writes ("Re: [PATCH] scripts: Add a script to build and submit to coverity."):
> On 10/12/15 16:53, Ian Campbell wrote:
> > ijc: Maybe we can think of a way to add this to osstest while still
> > keeping the coverity token private?
> 
> This would be ideal.

It doesn't sound particularly hard, apart from this wrinkle.

> > +# Set $COV_EMAIL and $COV_TOKEN to credentials, otherwise it will
...
> > +    declare -a curl_args
> > +    curl_args+=("--form" "token=$COV_TOKEN")
> > +    curl_args+=("--form" "email=$COV_EMAIL")
> > +    curl_args+=("--form" "file=@$COV_TARBALL")
> > +    curl_args+=("--form" "version=$COV_VERSION")
> > +    curl_args+=("--form" "description=$COV_HEAD")
> > +    curl_args+=("https://scan.coverity.com/builds?project=XenProject")

Is there a way to do this that does not involve the authentication
token being passed on curl's command line ?

Ideally there would be a way to get it to read the token from a file
in $HOME.  We could then provide the token in ~osstest in the
production colo.

Ian.

Re: [PATCH] scripts: Add a script to build and submit to coverity.

[Andrew Cooper]

On 10/12/15 17:48, Ian Jackson wrote:
> Andrew Cooper writes ("Re: [PATCH] scripts: Add a script to build and submit to coverity."):
>> On 10/12/15 16:53, Ian Campbell wrote:
>>> ijc: Maybe we can think of a way to add this to osstest while still
>>> keeping the coverity token private?
>> This would be ideal.
> It doesn't sound particularly hard, apart from this wrinkle.
>
>>> +# Set $COV_EMAIL and $COV_TOKEN to credentials, otherwise it will
> ...
>>> +    declare -a curl_args
>>> +    curl_args+=("--form" "token=$COV_TOKEN")
>>> +    curl_args+=("--form" "email=$COV_EMAIL")
>>> +    curl_args+=("--form" "file=@$COV_TARBALL")
>>> +    curl_args+=("--form" "version=$COV_VERSION")
>>> +    curl_args+=("--form" "description=$COV_HEAD")
>>> +    curl_args+=("https://scan.coverity.com/builds?project=XenProject")
> Is there a way to do this that does not involve the authentication
> token being passed on curl's command line ?
>
> Ideally there would be a way to get it to read the token from a file
> in $HOME.  We could then provide the token in ~osstest in the
> production colo.

One way or another, the authentication token needs to be in a post
header.  How this script and `curl` make it happen is open to improvement.

Perhaps something like this? (Completely untested)

--form token=@~osstest/coverity-secret-token

~Andrew

Re: [PATCH] scripts: Add a script to build and submit to coverity.

[Ian Jackson]

Andrew Cooper writes ("Re: [PATCH] scripts: Add a script to build and submit to coverity."):
> On 10/12/15 17:48, Ian Jackson wrote:
> > Ideally there would be a way to get it to read the token from a file
> > in $HOME.  We could then provide the token in ~osstest in the
> > production colo.
> 
> One way or another, the authentication token needs to be in a post
> header.  How this script and `curl` make it happen is open to improvement.
> 
> Perhaps something like this? (Completely untested)
> 
> --form token=@~osstest/coverity-secret-token

If curl can do that then fine.  Given

> >>> +    declare -a curl_args
> >>> +    curl_args+=("--form" "token=$COV_TOKEN")
> >>> +    curl_args+=("--form" "email=$COV_EMAIL")

this could be achieved by having ts-do-coverity-thing set COV_TOKEN to
$HOME/.xen-osstest/coverity-secret or whatever.  ts-do-coverity-thing
would need to set a bunch of other COV_SOMETHING anyay.

Ian.

Re: [PATCH] scripts: Add a script to build and submit to coverity.

[Ian Jackson]

Ian Jackson writes ("Re: [PATCH] scripts: Add a script to build and submit to coverity."):
> If curl can do that then fine.  Given
> 
> > >>> +    declare -a curl_args
> > >>> +    curl_args+=("--form" "token=$COV_TOKEN")
> > >>> +    curl_args+=("--form" "email=$COV_EMAIL")
> 
> this could be achieved by having ts-do-coverity-thing set COV_TOKEN to
> $HOME/.xen-osstest/coverity-secret or whatever.  ts-do-coverity-thing
> would need to set a bunch of other COV_SOMETHING anyay.

It occurs to me that it would be better if
 - the Coverity token did not have to be sent to the build host,
    but could remain on the controller
 - the Coverity log file thing could be left in the build logs

But I don't think this means that your script ought not to have an
`upload' function.  It just means that maybe osstest will need what
amounts to a copy of it.

Ian.

Re: [PATCH] scripts: Add a script to build and submit to coverity.

[Andrew Cooper]

On 10/12/15 18:06, Ian Jackson wrote:
> Ian Jackson writes ("Re: [PATCH] scripts: Add a script to build and submit to coverity."):
>> If curl can do that then fine.  Given
>>
>>>>>> +    declare -a curl_args
>>>>>> +    curl_args+=("--form" "token=$COV_TOKEN")
>>>>>> +    curl_args+=("--form" "email=$COV_EMAIL")
>> this could be achieved by having ts-do-coverity-thing set COV_TOKEN to
>> $HOME/.xen-osstest/coverity-secret or whatever.  ts-do-coverity-thing
>> would need to set a bunch of other COV_SOMETHING anyay.
> It occurs to me that it would be better if
>  - the Coverity token did not have to be sent to the build host,
>     but could remain on the controller
>  - the Coverity log file thing could be left in the build logs
>
> But I don't think this means that your script ought not to have an
> `upload' function.  It just means that maybe osstest will need what
> amounts to a copy of it.

The build() part of this script creates xen-coverity-$HEAD-SHA.tgz which
contains everything wanted by Coverity, other than the metadata used to
correctly submit it to their cloud.

OSSTest could definitely scoop that file (~140mb iirc) and submit it
from the controller instead of the build host.

~Andrew

Re: [PATCH] scripts: Add a script to build and submit to coverity.

[Ian Campbell]

On Thu, 2015-12-10 at 18:06 +0000, Ian Jackson wrote:
> Ian Jackson writes ("Re: [PATCH] scripts: Add a script to build and
> submit to coverity."):
> > If curl can do that then fine.  Given
> > 
> > > > > > +    declare -a curl_args
> > > > > > +    curl_args+=("--form" "token=$COV_TOKEN")
> > > > > > +    curl_args+=("--form" "email=$COV_EMAIL")
> > 
> > this could be achieved by having ts-do-coverity-thing set COV_TOKEN to
> > $HOME/.xen-osstest/coverity-secret or whatever.  ts-do-coverity-thing
> > would need to set a bunch of other COV_SOMETHING anyay.
> 
> It occurs to me that it would be better if
>  - the Coverity token did not have to be sent to the build host,
>     but could remain on the controller
>  - the Coverity log file thing could be left in the build logs
> 
> But I don't think this means that your script ought not to have an
> `upload' function.  It just means that maybe osstest will need what
> amounts to a copy of it.

Having implemented the bulk of a new ts-coverity-scan this morning I'm
basically concluding that all going via xen.git/scripts/coverity-build.sh
is doing is making things more opaque and more difficult to work with from
the test system, without really adding much value (the commands are not all
that complex after all).

Therefore I'm considering (actually, I've pretty much decided) that the ts
script should probably just do things itself.

And given a regular automated scan run that I'm not sure what value the in
tree helper script is, so I'd likely propose to drop this patch too, unless
other folks think it would be useful.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel