[Fuego] [RFC] kernel_build: rebuild vs pull patch for discussion

[Fuego] [RFC] kernel_build: rebuild vs pull patch for discussion

[Daniel Sangorrin]

Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
---
 engine/scripts/functions.sh                        | 16 ++++++++++
 engine/tests/Functional.kernel_build/fuego_test.sh | 35 ++++++++++------------
 engine/tests/Functional.kernel_build/spec.json     |  4 ++-
 3 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/engine/scripts/functions.sh b/engine/scripts/functions.sh
index 41dbb25..6c3dd41 100755
--- a/engine/scripts/functions.sh
+++ b/engine/scripts/functions.sh
@@ -341,8 +341,24 @@ function pre_build {
 
     lock_build_dir
 
+    # Rebuild needs to be 'true' when the source code needs to be unpacked
+    # or cloned from scratch on each run. For example, git repositories that
+    # may be rebased. If you are always using the same tarball or a git
+    # repository that is not supposed to be rebased then Rebuild false
+    # is the fastest method. Note that if git pull is able to retrieve changes
+    # a (possibly partial) rebuild will happen. If you want to stick to
+    # a specific commit id or tag, you can specify it on your spec with gitref
     if [ "$Rebuild" = "false" ] && [ -e fuego_test_successfully_built ]; then
         echo "The test is already built"
+        if [ -d ".git" ]; then
+            echo "Trying to update the source code with git pull"
+            if git pull | grep "Already up-to-date"; then
+                echo "No source code updates"
+            else
+                echo "The source code was updated, rebuilding"
+                rm -f fuego_test_successfully_built
+            fi
+        fi
     else
         find . -maxdepth 1 -mindepth 1 ! -name "fuego.build.lock" -print0 | xargs -0 rm -rf
         unpack || abort_job "Error while unpacking"
diff --git a/engine/tests/Functional.kernel_build/fuego_test.sh b/engine/tests/Functional.kernel_build/fuego_test.sh
index 0b4ac3c..687a6f4 100755
--- a/engine/tests/Functional.kernel_build/fuego_test.sh
+++ b/engine/tests/Functional.kernel_build/fuego_test.sh
@@ -15,24 +15,16 @@ function test_pre_check {
         assert_define PROGRAM_FLEX
     fi
 
+    if [ -z "$FUNCTIONAL_KERNEL_BUILD_TARGET" ]; then
+        FUNCTIONAL_KERNEL_BUILD_TARGET="bzImage"
+    fi
+
     if [[ "$FUNCTIONAL_KERNEL_BUILD_TARGET" == *"uImage"* ]]; then
         is_on_sdk mkimage
     fi
-
-    # Force the kernel to be rebuilt
-    # That's the whole point of the test - don't let Fuego skip the build.
-    export Rebuild=True
 }
 
 function test_build {
-    # make sure we have the latest source code.
-    cd "$WORKSPACE/$JOB_BUILD_DIR"
-    if [ -d ".git" ]; then
-        git pull || echo "WARNING: git pull failed"
-    else
-        echo "WARNING: no git repository, assuming you used a tarball"
-    fi
-
     # Config
     if [ -z "$FUNCTIONAL_KERNEL_BUILD_CONFIG" ]; then
         FUNCTIONAL_KERNEL_BUILD_CONFIG="defconfig"
@@ -48,10 +40,6 @@ function test_build {
 
     # Building
     echo "Building Kernel"
-    if [ -z "$FUNCTIONAL_KERNEL_BUILD_TARGET" ]; then
-        FUNCTIONAL_KERNEL_BUILD_TARGET="bzImage"
-    fi
-
     if [ -z "$FUNCTIONAL_KERNEL_BUILD_PARAMS" ]; then
         FUNCTIONAL_KERNEL_BUILD_PARAMS="-j$(nproc)"
     fi
@@ -70,6 +58,7 @@ function test_build {
         cd ..
         rm -rf built_modules
     fi
+    BUILD_EXECUTED=1
 }
 
 function test_deploy {
@@ -106,11 +95,17 @@ function test_deploy {
 }
 
 function test_processing {
-    echo "Processing kernel build log"
-    if [ -z "$FUNCTIONAL_KERNEL_BUILD_REGEX_P" ]; then
-        log_compare "$TESTDIR" "1" "[ \t]*Kernel: arch/.* is ready" "p"
+    if [ -z "$BUILD_EXECUTED" ]; then
+        echo "test_build was not executed, so there is not log to parse"
+        RETURN_VALUE=0
+        return $RETURN_VALUE
     else
-        log_compare "$TESTDIR" "1" "$FUNCTIONAL_KERNEL_BUILD_REGEX_P" "p"
+        echo "Processing kernel build log"
+        if [ -z "$FUNCTIONAL_KERNEL_BUILD_REGEX_P" ]; then
+            log_compare "$TESTDIR" "1" "[ \t]*Kernel: arch/.* is ready" "p"
+        else
+            log_compare "$TESTDIR" "1" "$FUNCTIONAL_KERNEL_BUILD_REGEX_P" "p"
+        fi
     fi
 }
 
diff --git a/engine/tests/Functional.kernel_build/spec.json b/engine/tests/Functional.kernel_build/spec.json
index 37d2aea..3ad51d5 100644
--- a/engine/tests/Functional.kernel_build/spec.json
+++ b/engine/tests/Functional.kernel_build/spec.json
@@ -3,7 +3,9 @@
     "specs": {
         "default": {
             "gitrepo": "https://github.com/torvalds/linux.git",
-            "config": "tinyconfig"
+            "config": "tinyconfig",
+            "deploy_method": "local_dir",
+            "deploy_path": "/fuego-rw/"
         },
         "lts-414y": {
             "gitrepo": "https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git",
-- 
2.7.4

Re: [Fuego] [RFC] kernel_build: rebuild vs pull patch for discussion

[Tim.Bird]

> -----Original Message-----
> From: Daniel Sangorrin
> 
> Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> ---
>  engine/scripts/functions.sh                        | 16 ++++++++++
>  engine/tests/Functional.kernel_build/fuego_test.sh | 35 ++++++++++-------
> -----
>  engine/tests/Functional.kernel_build/spec.json     |  4 ++-
>  3 files changed, 34 insertions(+), 21 deletions(-)
> 
> diff --git a/engine/scripts/functions.sh b/engine/scripts/functions.sh
> index 41dbb25..6c3dd41 100755
> --- a/engine/scripts/functions.sh
> +++ b/engine/scripts/functions.sh
> @@ -341,8 +341,24 @@ function pre_build {
> 
>      lock_build_dir
> 
> +    # Rebuild needs to be 'true' when the source code needs to be unpacked
> +    # or cloned from scratch on each run. For example, git repositories that
> +    # may be rebased. If you are always using the same tarball or a git
> +    # repository that is not supposed to be rebased then Rebuild false
> +    # is the fastest method. Note that if git pull is able to retrieve changes
> +    # a (possibly partial) rebuild will happen. If you want to stick to
> +    # a specific commit id or tag, you can specify it on your spec with gitref
>      if [ "$Rebuild" = "false" ] && [ -e fuego_test_successfully_built ]; then
>          echo "The test is already built"
> +        if [ -d ".git" ]; then
> +            echo "Trying to update the source code with git pull"
> +            if git pull | grep "Already up-to-date"; then
> +                echo "No source code updates"
> +            else
> +                echo "The source code was updated, rebuilding"
> +                rm -f fuego_test_successfully_built
> +            fi
> +        fi
I don't like this.  If the user says "Rebuild=false", then Fuego shouldn't rebuild
the source.  It's possible (maybe even likely) that the user wants to execute the
test again with exactly the same test program that was used in the previous
test.

Maybe we need some other flag value, like: Rebuild=if_source_changed
to cover this case.  Having the test source code change out-from-under Fuego is
not something we've handled before.  In order to avoid confusion, we likely
need to record the version of the test source in the run logs somehow
(probably in run.json).

If we introduce a new value for Rebuild, then we may want to make the new
value the default when jobs are created, rather than "false'.

>      else
>          find . -maxdepth 1 -mindepth 1 ! -name "fuego.build.lock" -print0 | xargs
> -0 rm -rf
>          unpack || abort_job "Error while unpacking"
> diff --git a/engine/tests/Functional.kernel_build/fuego_test.sh
> b/engine/tests/Functional.kernel_build/fuego_test.sh
> index 0b4ac3c..687a6f4 100755
> --- a/engine/tests/Functional.kernel_build/fuego_test.sh
> +++ b/engine/tests/Functional.kernel_build/fuego_test.sh
> @@ -15,24 +15,16 @@ function test_pre_check {
>          assert_define PROGRAM_FLEX
>      fi
> 
> +    if [ -z "$FUNCTIONAL_KERNEL_BUILD_TARGET" ]; then
> +        FUNCTIONAL_KERNEL_BUILD_TARGET="bzImage"
> +    fi
> +
>      if [[ "$FUNCTIONAL_KERNEL_BUILD_TARGET" == *"uImage"* ]]; then
>          is_on_sdk mkimage
>      fi
> -
> -    # Force the kernel to be rebuilt
> -    # That's the whole point of the test - don't let Fuego skip the build.
> -    export Rebuild=True
>  }
> 
>  function test_build {
> -    # make sure we have the latest source code.
> -    cd "$WORKSPACE/$JOB_BUILD_DIR"
> -    if [ -d ".git" ]; then
> -        git pull || echo "WARNING: git pull failed"
> -    else
> -        echo "WARNING: no git repository, assuming you used a tarball"
> -    fi
> -
>      # Config
>      if [ -z "$FUNCTIONAL_KERNEL_BUILD_CONFIG" ]; then
>          FUNCTIONAL_KERNEL_BUILD_CONFIG="defconfig"
> @@ -48,10 +40,6 @@ function test_build {
> 
>      # Building
>      echo "Building Kernel"
> -    if [ -z "$FUNCTIONAL_KERNEL_BUILD_TARGET" ]; then
> -        FUNCTIONAL_KERNEL_BUILD_TARGET="bzImage"
> -    fi
> -
>      if [ -z "$FUNCTIONAL_KERNEL_BUILD_PARAMS" ]; then
>          FUNCTIONAL_KERNEL_BUILD_PARAMS="-j$(nproc)"
>      fi
> @@ -70,6 +58,7 @@ function test_build {
>          cd ..
>          rm -rf built_modules
>      fi
> +    BUILD_EXECUTED=1
>  }
> 
>  function test_deploy {
> @@ -106,11 +95,17 @@ function test_deploy {
>  }
> 
>  function test_processing {
> -    echo "Processing kernel build log"
> -    if [ -z "$FUNCTIONAL_KERNEL_BUILD_REGEX_P" ]; then
> -        log_compare "$TESTDIR" "1" "[ \t]*Kernel: arch/.* is ready" "p"
> +    if [ -z "$BUILD_EXECUTED" ]; then
> +        echo "test_build was not executed, so there is not log to parse"
not->no

Hitting test_processing() with no log is what caused me to find the issue
with test_build not getting run on subsequent runs.

Detecting this situation is good.  I had coded up a different solution, which was to use
the build log from the previous run, as this run's build log.  That required adding
back in the 'tee -a build_log' stuff we had building steps before, which I found annoying.
However, having the previous run's build log solves the problem of possibly
missing useful information.  Not for this test, which is basically a no-op, but for
when this test is used in a bigger context.

That is...
If Functional.kernel_build is used as part of a sequence of actions during a test, then
it may be useful for the build_log associated with the artifacts (ie kernel) used
for the run sequence to be present in this run's log directory.

For example, if there were a testplan (say: testplan_install_everything_and_run_some_test) that did:
 Functional.kernel_build
 Functional.kernel_install
 Functional.distro_build
 Functional.distro_install
 Functional.some_test

Then, if the kernel didn't build (due to already being built), but the distro did, then in the
overall results it might still be useful to save the build_log for the kernel in this overall test's
log directory.
-----
Looking at the bigger picture,
I think the issue here is that for this particular test we're conflating the
software under test with the test program.

Normally, Fuego builds the test program, which is built, deployed to the board and executed.
Most Fuego tests ignore the issue of how the software under test was built and installed.
But the kernel is not the test program, it is the software under test (SUT).

From Fuego's perspective, the test program for a kernel build is the compiler (and all other elements
of the toolchain).  That's the thing that gives the pass/fail result in this case.
Functional.kernel_build is sort-of the first time Fuego has built the software under
test (that's not strictly true, but it's close enough).  We've left SUT building and
provisioning as an exercise or the user.  But I'd like to move past that.

I would like to Fuego to be able to manage the software under test (e.g. the kernel, the distro,
and individual programs and libraries).  But I don't want to turn Fuego into Yocto, where it's a full
blown build system and distro creator.

> +        RETURN_VALUE=0
> +        return $RETURN_VALUE
>      else
> -        log_compare "$TESTDIR" "1" "$FUNCTIONAL_KERNEL_BUILD_REGEX_P"
> "p"
> +        echo "Processing kernel build log"
> +        if [ -z "$FUNCTIONAL_KERNEL_BUILD_REGEX_P" ]; then
> +            log_compare "$TESTDIR" "1" "[ \t]*Kernel: arch/.* is ready" "p"
> +        else
> +            log_compare "$TESTDIR" "1"
> "$FUNCTIONAL_KERNEL_BUILD_REGEX_P" "p"
> +        fi
>      fi
>  }
> 
> diff --git a/engine/tests/Functional.kernel_build/spec.json
> b/engine/tests/Functional.kernel_build/spec.json
> index 37d2aea..3ad51d5 100644
> --- a/engine/tests/Functional.kernel_build/spec.json
> +++ b/engine/tests/Functional.kernel_build/spec.json
> @@ -3,7 +3,9 @@
>      "specs": {
>          "default": {
>              "gitrepo": "https://github.com/torvalds/linux.git",
> -            "config": "tinyconfig"
> +            "config": "tinyconfig",
> +            "deploy_method": "local_dir",
> +            "deploy_path": "/fuego-rw/"
This doesn't look right.
Won't this put the kernel for every board in the same directory?
potentially with the same name?

>          },
>          "lts-414y": {
>              "gitrepo":
> "https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git",
> --
> 2.7.4

The bottom line for this commit is that I'd like to see some other value for
the Rebuild flag, that is potentially special for this test, to avoid 
confusing behavior for users.  I think if the user specifies "Rebuild=true"
the software should be built, and if the user specifies "Rebuild=false"
the software should not be built.
 -- Tim

Re: [Fuego] [RFC] kernel_build: rebuild vs pull patch for discussion

[Daniel Sangorrin]

> -----Original Message-----
> From: Tim.Bird@sony.com <Tim.Bird@sony.com>
> > +    # Rebuild needs to be 'true' when the source code needs to be unpacked
> > +    # or cloned from scratch on each run. For example, git repositories that
> > +    # may be rebased. If you are always using the same tarball or a git
> > +    # repository that is not supposed to be rebased then Rebuild false
> > +    # is the fastest method. Note that if git pull is able to retrieve changes
> > +    # a (possibly partial) rebuild will happen. If you want to stick to
> > +    # a specific commit id or tag, you can specify it on your spec with gitref
> >      if [ "$Rebuild" = "false" ] && [ -e fuego_test_successfully_built ]; then
> >          echo "The test is already built"
> > +        if [ -d ".git" ]; then
> > +            echo "Trying to update the source code with git pull"
> > +            if git pull | grep "Already up-to-date"; then
> > +                echo "No source code updates"
> > +            else
> > +                echo "The source code was updated, rebuilding"
> > +                rm -f fuego_test_successfully_built
> > +            fi
> > +        fi
> I don't like this.  If the user says "Rebuild=false", then Fuego shouldn't rebuild
> the source.  It's possible (maybe even likely) that the user wants to execute the
> test again with exactly the same test program that was used in the previous
> test.
> 
> Maybe we need some other flag value, like: Rebuild=if_source_changed
> to cover this case.  Having the test source code change out-from-under Fuego is
> not something we've handled before. 

I added if_source_changed in the latest patches.

> In order to avoid confusion, we likely
> need to record the version of the test source in the run logs somehow
> (probably in run.json).

This is recorded in testsuite_version with the latest patches.

> If we introduce a new value for Rebuild, then we may want to make the new
> value the default when jobs are created, rather than "false'.

Done.

> >  function test_processing {
> > -    echo "Processing kernel build log"
> > -    if [ -z "$FUNCTIONAL_KERNEL_BUILD_REGEX_P" ]; then
> > -        log_compare "$TESTDIR" "1" "[ \t]*Kernel: arch/.* is ready" "p"
> > +    if [ -z "$BUILD_EXECUTED" ]; then
> > +        echo "test_build was not executed, so there is not log to parse"
> not->no
> 
> Hitting test_processing() with no log is what caused me to find the issue
> with test_build not getting run on subsequent runs.
> 
> Detecting this situation is good.  I had coded up a different solution, which was to
> use
> the build log from the previous run, as this run's build log.  That required adding
> back in the 'tee -a build_log' stuff we had building steps before, which I found
> annoying.
> However, having the previous run's build log solves the problem of possibly
> missing useful information.  Not for this test, which is basically a no-op, but for
> when this test is used in a bigger context.
> 
> That is...
> If Functional.kernel_build is used as part of a sequence of actions during a test, then
> it may be useful for the build_log associated with the artifacts (ie kernel) used
> for the run sequence to be present in this run's log directory.
> 
> For example, if there were a testplan (say:
> testplan_install_everything_and_run_some_test) that did:
>  Functional.kernel_build
>  Functional.kernel_install
>  Functional.distro_build
>  Functional.distro_install
>  Functional.some_test
> 
> Then, if the kernel didn't build (due to already being built), but the distro did, then
> in the
> overall results it might still be useful to save the build_log for the kernel in this
> overall test's
> log directory.
> -----
> Looking at the bigger picture,
> I think the issue here is that for this particular test we're conflating the
> software under test with the test program.
> 
> Normally, Fuego builds the test program, which is built, deployed to the board and
> executed.
> Most Fuego tests ignore the issue of how the software under test was built and
> installed.
> But the kernel is not the test program, it is the software under test (SUT).
> 
> >From Fuego's perspective, the test program for a kernel build is the compiler (and
> all other elements
> of the toolchain).  That's the thing that gives the pass/fail result in this case.
> Functional.kernel_build is sort-of the first time Fuego has built the software under
> test (that's not strictly true, but it's close enough).  We've left SUT building and
> provisioning as an exercise or the user.  But I'd like to move past that.
> 
> I would like to Fuego to be able to manage the software under test (e.g. the kernel,
> the distro,
> and individual programs and libraries).  But I don't want to turn Fuego into Yocto,
> where it's a full
> blown build system and distro creator.

OK, this sounds a bit difficult for now, so I left it as it is which works fine for the current use cases.

> > +++ b/engine/tests/Functional.kernel_build/spec.json
> > @@ -3,7 +3,9 @@
> >      "specs": {
> >          "default": {
> >              "gitrepo": "https://github.com/torvalds/linux.git",
> > -            "config": "tinyconfig"
> > +            "config": "tinyconfig",
> > +            "deploy_method": "local_dir",
> > +            "deploy_path": "/fuego-rw/"
> This doesn't look right.
> Won't this put the kernel for every board in the same directory?
> potentially with the same name?

I modified the path to /fuego-rw/boards/$NODE_NAME

> The bottom line for this commit is that I'd like to see some other value for
> the Rebuild flag, that is potentially special for this test, to avoid
> confusing behavior for users.  I think if the user specifies "Rebuild=true"
> the software should be built, and if the user specifies "Rebuild=false"
> the software should not be built.

I tried to implement that in the latest patches and did some manual testing. I will keep testing just in case I left some corner cases unsupported.

Thanks,
Daniel