Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for build testing

[Wei Liu]

On Mon, Oct 23, 2017 at 04:09:58PM +0100, Ian Jackson wrote:
> Wei Liu writes ("Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for 
> build testing"):
> > On Mon, Oct 23, 2017 at 03:50:31PM +0100, Anthony PERARD wrote:
> > > FIY, I do like to put script and other files in my checkouts, the git
> > > clean will remove them.
> > 
> > I changed that to make distclean this morning.
> 
> Urgh.  This script depends on git, so please continue to use git to
> check if the tree is clean.
> 
> The right answer would be to *check that the tree is clean* before
> starting.

Sure, that's also something I just did.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for build testing

[Ian Jackson]

Wei Liu writes ("Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for 
build testing"):
> On Mon, Oct 23, 2017 at 03:50:31PM +0100, Anthony PERARD wrote:
> > FIY, I do like to put script and other files in my checkouts, the git
> > clean will remove them.
> 
> I changed that to make distclean this morning.

Urgh.  This script depends on git, so please continue to use git to
check if the tree is clean.

The right answer would be to *check that the tree is clean* before
starting.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for build testing

[Wei Liu]

On Mon, Oct 23, 2017 at 03:50:31PM +0100, Anthony PERARD wrote:
> On Mon, Oct 23, 2017 at 02:02:53PM +0100, George Dunlap wrote:
> > On 10/20/2017 06:32 PM, Wei Liu wrote:
> > > Signed-off-by: Wei Liu 
> > > ---
> > > Cc: Andrew Cooper 
> > > Cc: George Dunlap 
> > > Cc: Ian Jackson 
> > > Cc: Jan Beulich 
> > > Cc: Konrad Rzeszutek Wilk 
> > > Cc: Stefano Stabellini 
> > > Cc: Tim Deegan 
> > > Cc: Wei Liu 
> > > Cc: Julien Grall 
> > > 
> > > The risk for this is zero, hence the for-4.10 tag.
> > 
> > I'm not necessarily arguing against this, but in my estimation this
> > isn't zero risk.  It's a new feature (even if one only for developers).
> > It's not *intended* to destroy anything, but a bug in it well could
> > destroy data.
> 
> There is a `git clean -dxf` in the script, this is destructif! I'm sure
> it's going to take some people by surprise.
> 
> FIY, I do like to put script and other files in my checkouts, the git
> clean will remove them.

I changed that to make distclean this morning.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for build testing

[Anthony PERARD]

On Mon, Oct 23, 2017 at 02:02:53PM +0100, George Dunlap wrote:
> On 10/20/2017 06:32 PM, Wei Liu wrote:
> > Signed-off-by: Wei Liu 
> > ---
> > Cc: Andrew Cooper 
> > Cc: George Dunlap 
> > Cc: Ian Jackson 
> > Cc: Jan Beulich 
> > Cc: Konrad Rzeszutek Wilk 
> > Cc: Stefano Stabellini 
> > Cc: Tim Deegan 
> > Cc: Wei Liu 
> > Cc: Julien Grall 
> > 
> > The risk for this is zero, hence the for-4.10 tag.
> 
> I'm not necessarily arguing against this, but in my estimation this
> isn't zero risk.  It's a new feature (even if one only for developers).
> It's not *intended* to destroy anything, but a bug in it well could
> destroy data.

There is a `git clean -dxf` in the script, this is destructif! I'm sure
it's going to take some people by surprise.

FIY, I do like to put script and other files in my checkouts, the git
clean will remove them.

-- 
Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for build testing

[George Dunlap]

On 10/20/2017 06:32 PM, Wei Liu wrote:
> Signed-off-by: Wei Liu 
> ---
> Cc: Andrew Cooper 
> Cc: George Dunlap 
> Cc: Ian Jackson 
> Cc: Jan Beulich 
> Cc: Konrad Rzeszutek Wilk 
> Cc: Stefano Stabellini 
> Cc: Tim Deegan 
> Cc: Wei Liu 
> Cc: Julien Grall 
> 
> The risk for this is zero, hence the for-4.10 tag.

I'm not necessarily arguing against this, but in my estimation this
isn't zero risk.  It's a new feature (even if one only for developers).
It's not *intended* to destroy anything, but a bug in it well could
destroy data.

 -George

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for build testing

[Wei Liu]

On Mon, Oct 23, 2017 at 01:07:36PM +0100, Ian Jackson wrote:
> Wei Liu writes ("Re: [PATCH for-4.10] scripts: add a script for build 
> testing"):
> > On Mon, Oct 23, 2017 at 01:02:00PM +0100, Ian Jackson wrote:
> > > In particular, if you:
> > >  * check that the tree is not dirty
> > >  * detach HEAD
> > 
> > I think these two checks are good.
> > 
> > >  * reattach HEAD afterwards at least on success
> > 
> > This is already the case for git-rebase on success.
> 
> No.  git-rebase _rewrites_ HEAD.
> 

I see. I will steal bits from your snippet where appropriate.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for build testing

[Ian Jackson]

Wei Liu writes ("Re: [PATCH for-4.10] scripts: add a script for build testing"):
> On Mon, Oct 23, 2017 at 02:24:40AM -0600, Jan Beulich wrote:
> > On 20.10.17 at 19:32,  wrote:
> > > +git rebase $BASE $TIP -x "$CMD"
> > 
> > Is this quoting on $CMD really going to work right no matter what
> > the variable actually expands to? I.e. don't you either want to use
> > "eval" or adjust script arguments such that you can use "$@" with
> > its special quoting rules?

Yes.  Jan is completely right.

> What sort of use cases you have in mind that involve complex quoting and
> expansion?

There is really no excuse at all, in a script like this, for not using
`shift' to eat the main positional parameters, and then executing
"$@", faithfully reproducing the incoming parameters.

Of course there is a problem with getting this through git-rebase but
as I have just pointed out, git-rev-list and git-checkout are much
more suitable building blocks than git-rebase (which does a lot of
undesirable stuff that has to be suppressed, etc.)

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for build testing

[Jan Beulich]

>>> On 23.10.17 at 13:41,  wrote:
> On Mon, Oct 23, 2017 at 02:24:40AM -0600, Jan Beulich wrote:
>> >>> On 20.10.17 at 19:32,  wrote:
>> > --- /dev/null
>> > +++ b/scripts/build-test.sh
>> > @@ -0,0 +1,40 @@
>> > +#!/bin/sh
>> > +
>> > +# WARNING: Always backup the branch by creating another reference to it if
>> > +# you're not familiar with git-rebase(1).
>> > +#
>> > +# Use `git rebase` to run command or script on every commit within the 
>> > range
>> > +# specified. If no command or script is provided, use the default one to 
>> > clean
>> > +# and build the whole tree.
>> > +#
>> > +# If something goes wrong, the script will stop at the commit that fails. 
>> >  Fix
>> > +# the failure and run `git rebase --continue`.
>> > +#
>> > +# If for any reason the tree is screwed, use `git rebase --abort` to 
>> > restore to
>> > +# original state.
>> > +
>> > +if ! test -f xen/Kconfig; then
>> > +echo "Please run this script from top-level directory"
>> 
>> Wouldn't running this in one of the top-level sub-trees also be useful?
>> E.g. why would one want a hypervisor only series not touching the
>> public interface to have the tools tree rebuilt all the time?
>> 
> 
> You can do that by supplying your custom command.

Oh, of course - silly me.

>> > +echo
>> > +
>> > +git rebase $BASE $TIP -x "$CMD"
>> 
>> Is this quoting on $CMD really going to work right no matter what
>> the variable actually expands to? I.e. don't you either want to use
>> "eval" or adjust script arguments such that you can use "$@" with
>> its special quoting rules?
> 
> What sort of use cases you have in mind that involve complex quoting and
> expansion?

A typical cross build command line of mine looks like

make -sC build/xen/$v {XEN_TARGET_ARCH,t}=x86_64 CC=gccx LD=ldx 
OBJCOPY=objcopyx NM=nmx -j32 xen

which you can see leverages the fact that make allows variable
settings on the command line. For other utilities this would require
e.g. "CC=gccx my-script", and I'm not sure whether quoting you
apply would work right (largely depends on what git does with the
argument).

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for build testing

[Ian Jackson]

Wei Liu writes ("Re: [PATCH for-4.10] scripts: add a script for build testing"):
> On Mon, Oct 23, 2017 at 01:02:00PM +0100, Ian Jackson wrote:
> > In particular, if you:
> >  * check that the tree is not dirty
> >  * detach HEAD
> 
> I think these two checks are good.
> 
> >  * reattach HEAD afterwards at least on success
> 
> This is already the case for git-rebase on success.

No.  git-rebase _rewrites_ HEAD.

Your script should just check out the intermediate commits.  You
probably don't in fact want git-rebase.  In particular, you don't want
to risk merge conflicts.

I have a script I use for dgit testing that looks like this:

  #!/bin/bash
  #
  # run  git fetch main
  # and then this

  set -e
  set -o pipefail

  revspec=main/${STTM_TESTED-tested}..main/pretest

  echo "testing $revspec ..."

  git-rev-list $revspec | nl -ba | tac | \
  while read num rev; do
  echo >&2 ""
  echo >&2 "testing $num $rev"
  git checkout $rev
  ${0%/*}/sometest-to-tested
  done

FAOD,

Signed-off-by: Ian Jackson 

for inclusion of parts of this in the Xen build system.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for build testing

[Wei Liu]

On Mon, Oct 23, 2017 at 01:02:00PM +0100, Ian Jackson wrote:
> Wei Liu writes ("Re: [PATCH for-4.10] scripts: add a script for build 
> testing"):
> > On Mon, Oct 23, 2017 at 02:24:40AM -0600, Jan Beulich wrote:
> > > What is this startup delay intended for?
> > 
> > To give user a chance to check the command -- git-rebase can be
> > destructive after all.
> 
> I can't resist this bikeshed.  This kind of thing is quite annoying.
> If your command might be destructive, why not fix it so that it's not
> destructive.
> 
> In particular, if you:
>  * check that the tree is not dirty
>  * detach HEAD

I think these two checks are good.

>  * reattach HEAD afterwards at least on success

This is already the case for git-rebase on success.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for build testing

[Ian Jackson]

Wei Liu writes ("Re: [PATCH for-4.10] scripts: add a script for build testing"):
> On Mon, Oct 23, 2017 at 02:24:40AM -0600, Jan Beulich wrote:
> > What is this startup delay intended for?
> 
> To give user a chance to check the command -- git-rebase can be
> destructive after all.

I can't resist this bikeshed.  This kind of thing is quite annoying.
If your command might be destructive, why not fix it so that it's not
destructive.

In particular, if you:
 * check that the tree is not dirty
 * detach HEAD
 * reattach HEAD afterwards at least on success
then the risk of lossage is low and you can safely just go ahead.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for build testing

[Wei Liu]

On Mon, Oct 23, 2017 at 12:30:33PM +0100, Anthony PERARD wrote:
> On Fri, Oct 20, 2017 at 06:32:55PM +0100, Wei Liu wrote:
> > +CMD=${3:-git clean -fdx && ./configure && make -j4}
> > +
> > +echo "Running command \"$CMD\" on every commit from $BASE to $TIP"
> > +echo -n "Starting in "
> > +
> > +for i in `seq 5 -1 1`; do
> > +echo -n "$i ... "
> > +sleep 1
> > +done
> > +
> 
> Instead of the count down, I would do:
> echo -n 'Continue ? (^C to quit) '
> read
> 
> 
> OR something like:
> echo -n 'Continue ? [Yn] '
> read answer
> [[ "$answer" =~ ^(|Y|y|yes)$ ]] || exit
> 

No objection from me. The latter is better.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for build testing

[Wei Liu]

On Mon, Oct 23, 2017 at 02:24:40AM -0600, Jan Beulich wrote:
> >>> On 20.10.17 at 19:32,  wrote:
> > --- /dev/null
> > +++ b/scripts/build-test.sh
> > @@ -0,0 +1,40 @@
> > +#!/bin/sh
> > +
> > +# WARNING: Always backup the branch by creating another reference to it if
> > +# you're not familiar with git-rebase(1).
> > +#
> > +# Use `git rebase` to run command or script on every commit within the 
> > range
> > +# specified. If no command or script is provided, use the default one to 
> > clean
> > +# and build the whole tree.
> > +#
> > +# If something goes wrong, the script will stop at the commit that fails.  
> > Fix
> > +# the failure and run `git rebase --continue`.
> > +#
> > +# If for any reason the tree is screwed, use `git rebase --abort` to 
> > restore to
> > +# original state.
> > +
> > +if ! test -f xen/Kconfig; then
> > +echo "Please run this script from top-level directory"
> 
> Wouldn't running this in one of the top-level sub-trees also be useful?
> E.g. why would one want a hypervisor only series not touching the
> public interface to have the tools tree rebuilt all the time?
> 

You can do that by supplying your custom command.

The script really aims to be an easy to use thing to point contributors
to hence the checks, warning and restrictions, while at the same time
allows some flexibility.

For example, if you want to build hypervisor only:

$ ./scripts/build-test.sh $BASE $TIP "make -C xen clean && make -C xen"

> > +exit 1
> > +fi
> > +
> > +if test $# -lt 2 ; then
> > +echo "Usage: $0   [CMD|SCRIPT]"
> 
> Perhaps
> 
> echo "Usage: $0   

Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for build testing

[Anthony PERARD]

On Mon, Oct 23, 2017 at 12:30:33PM +0100, Anthony PERARD wrote:
> On Fri, Oct 20, 2017 at 06:32:55PM +0100, Wei Liu wrote:
> > +CMD=${3:-git clean -fdx && ./configure && make -j4}
> > +
> > +echo "Running command \"$CMD\" on every commit from $BASE to $TIP"
> > +echo -n "Starting in "
> > +
> > +for i in `seq 5 -1 1`; do
> > +echo -n "$i ... "
> > +sleep 1
> > +done
> > +
> 
> Instead of the count down, I would do:
> echo -n 'Continue ? (^C to quit) '
> read
> 
> 
> OR something like:
> echo -n 'Continue ? [Yn] '
> read answer
> [[ "$answer" =~ ^(|Y|y|yes)$ ]] || exit
> 
> 
> I don't like to wait.

And I don't like the presure to have to decide if I want to ^C within a
limited time. I would probably kill the script if I did not know what
it was going to do as soon as I see the count down.

-- 
Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for build testing

[Anthony PERARD]

On Fri, Oct 20, 2017 at 06:32:55PM +0100, Wei Liu wrote:
> +CMD=${3:-git clean -fdx && ./configure && make -j4}
> +
> +echo "Running command \"$CMD\" on every commit from $BASE to $TIP"
> +echo -n "Starting in "
> +
> +for i in `seq 5 -1 1`; do
> +echo -n "$i ... "
> +sleep 1
> +done
> +

Instead of the count down, I would do:
echo -n 'Continue ? (^C to quit) '
read


OR something like:
echo -n 'Continue ? [Yn] '
read answer
[[ "$answer" =~ ^(|Y|y|yes)$ ]] || exit


I don't like to wait.

-- 
Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for build testing

[Julien Grall]

Hi Wei,

On 20/10/17 18:32, Wei Liu wrote:

Signed-off-by: Wei Liu 
---
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Wei Liu 
Cc: Julien Grall 

The risk for this is zero, hence the for-4.10 tag.


Agree.


---
  scripts/build-test.sh | 40 
  1 file changed, 40 insertions(+)
  create mode 100755 scripts/build-test.sh

diff --git a/scripts/build-test.sh b/scripts/build-test.sh
new file mode 100755
index 00..a08468e83b
--- /dev/null
+++ b/scripts/build-test.sh
@@ -0,0 +1,40 @@
+#!/bin/sh
+
+# WARNING: Always backup the branch by creating another reference to it if
+# you're not familiar with git-rebase(1).
+#
+# Use `git rebase` to run command or script on every commit within the range
+# specified. If no command or script is provided, use the default one to clean
+# and build the whole tree.
+#
+# If something goes wrong, the script will stop at the commit that fails.  Fix
+# the failure and run `git rebase --continue`.
+#
+# If for any reason the tree is screwed, use `git rebase --abort` to restore to
+# original state.
+
+if ! test -f xen/Kconfig; then
+echo "Please run this script from top-level directory"
+exit 1
+fi
+
+if test $# -lt 2 ; then
+echo "Usage: $0   [CMD|SCRIPT]"
+exit 1
+fi
+
+BASE=$1
+TIP=$2
+CMD=${3:-git clean -fdx && ./configure && make -j4}


Can you document somewhere that there are no cross-compilation supported?


+
+echo "Running command \"$CMD\" on every commit from $BASE to $TIP"
+echo -n "Starting in "
+
+for i in `seq 5 -1 1`; do
+echo -n "$i ... "
+sleep 1
+done
+
+echo
+
+git rebase $BASE $TIP -x "$CMD"



Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH for-4.10] scripts: add a script for build testing

[Jan Beulich]

>>> On 20.10.17 at 19:32,  wrote:
> --- /dev/null
> +++ b/scripts/build-test.sh
> @@ -0,0 +1,40 @@
> +#!/bin/sh
> +
> +# WARNING: Always backup the branch by creating another reference to it if
> +# you're not familiar with git-rebase(1).
> +#
> +# Use `git rebase` to run command or script on every commit within the range
> +# specified. If no command or script is provided, use the default one to 
> clean
> +# and build the whole tree.
> +#
> +# If something goes wrong, the script will stop at the commit that fails.  
> Fix
> +# the failure and run `git rebase --continue`.
> +#
> +# If for any reason the tree is screwed, use `git rebase --abort` to restore 
> to
> +# original state.
> +
> +if ! test -f xen/Kconfig; then
> +echo "Please run this script from top-level directory"

Wouldn't running this in one of the top-level sub-trees also be useful?
E.g. why would one want a hypervisor only series not touching the
public interface to have the tools tree rebuilt all the time?

> +exit 1
> +fi
> +
> +if test $# -lt 2 ; then
> +echo "Usage: $0   [CMD|SCRIPT]"

Perhaps

echo "Usage: $0   

[Xen-devel] [PATCH for-4.10] scripts: add a script for build testing

[Wei Liu]

Signed-off-by: Wei Liu 
---
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Wei Liu 
Cc: Julien Grall 

The risk for this is zero, hence the for-4.10 tag.
---
 scripts/build-test.sh | 40 
 1 file changed, 40 insertions(+)
 create mode 100755 scripts/build-test.sh

diff --git a/scripts/build-test.sh b/scripts/build-test.sh
new file mode 100755
index 00..a08468e83b
--- /dev/null
+++ b/scripts/build-test.sh
@@ -0,0 +1,40 @@
+#!/bin/sh
+
+# WARNING: Always backup the branch by creating another reference to it if
+# you're not familiar with git-rebase(1).
+#
+# Use `git rebase` to run command or script on every commit within the range
+# specified. If no command or script is provided, use the default one to clean
+# and build the whole tree.
+#
+# If something goes wrong, the script will stop at the commit that fails.  Fix
+# the failure and run `git rebase --continue`.
+#
+# If for any reason the tree is screwed, use `git rebase --abort` to restore to
+# original state.
+
+if ! test -f xen/Kconfig; then
+echo "Please run this script from top-level directory"
+exit 1
+fi
+
+if test $# -lt 2 ; then
+echo "Usage: $0   [CMD|SCRIPT]"
+exit 1
+fi
+
+BASE=$1
+TIP=$2
+CMD=${3:-git clean -fdx && ./configure && make -j4}
+
+echo "Running command \"$CMD\" on every commit from $BASE to $TIP"
+echo -n "Starting in "
+
+for i in `seq 5 -1 1`; do
+echo -n "$i ... "
+sleep 1
+done
+
+echo
+
+git rebase $BASE $TIP -x "$CMD"
-- 
2.11.0


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel