[Fuego] issue report: “PER_JOB_BUILD” related

[Fuego] issue report: “PER_JOB_BUILD” related

[Liu, Wenlong]

[-- Attachment #1: Type: text/plain, Size: 709 bytes --]

Hi,

Just report some issue with “PER_JOB_BUILD”.

Issue 1：
Jobs “boardx.bc-add.Functional.bc’ and “boardx.bc-by2.Functional.bc’cannot all PASS in the current Fuego because “Functional.bc” has 2 different tarball.
So, variable “PER_JOB_BUILD” might be added to spec file.

Is there any doc for the spec file variables?
If not，maybe those variables should be well documented.

Issue 2：
For “Functional.glib2”, the current path will be changed when running “test_build” and then file “fuego_test_succesfully_built” will be created in a wrong path.
I fixed this and sent patch to the maillist, please review.

Please correct me if you find any problems.

Best regards
Liu



[-- Attachment #2: Type: text/html, Size: 9393 bytes --]

Re: [Fuego] issue report: “PER_JOB_BUILD” related

[Bird, Timothy]

> -----Original Message-----
> From: Liu, Wenlong on Monday, November 06, 2017 7:54 AM
> Just report some issue with “PER_JOB_BUILD”.
> 
> Issue 1：
> 
> Jobs “boardx.bc-add.Functional.bc’ and “boardx.bc-by2.Functional.bc’cannot
> all PASS in the current Fuego because “Functional.bc” has 2 different tarball.
> 
> So, variable “PER_JOB_BUILD” might be added to spec file.

That would solve this problem.  But looking at the Functional.bc test, having
two different tar files, each of which contains a ~150-byte script, seems really
silly.  But this test is used for demo/tutorial purposes.

Daniel - was this just done as a demonstration of a putting the tarball build
variable in a spec file?

If so, I'll a per_job_build var to this, to get the desired behavior.  Another
option would be to use a different script name (rather than have both
tarballs provide bc-device.sh).

> 
> 
> Is there any doc for the spec file variables?
Not really.

There are a few generic ones documented here:
http://fuegotest.org/wiki/Test_Specs_and_Plans
and here:
http://fuegotest.org/wiki/Variables

But we have only a few test variables for specific
tests documented.  For an example, see
http://fuegotest.org/wiki/Functional.LTP#Variables

> 
> If not，maybe those variables should be well documented.

The intention is to document any test-specific test
variables on a page dedicated to that test.  You can see
the list of test pages I want to create here:
http://fuegotest.org/wiki/Tests

Unfortunately, the tbwiki engine doesn't show missing pages
in red links, but you can see that a lot of desired pages are missing
at: http://fuegotest.org/wiki/MissingPages

Thanks for the bug report!

> Issue 2：
> 
> For “Functional.glib2”, the current path will be changed when running
> “test_build” and then file “fuego_test_succesfully_built” will be created in a
> wrong path.
> 
> I fixed this and sent patch to the maillist, please review.
I did.  Thanks.

 -- Tim

Re: [Fuego] issue report: “PER_JOB_BUILD” related

[Daniel Sangorrin]

> -----Original Message-----
> From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-bounces@lists.linuxfoundation.org] On Behalf Of Bird, Timothy
> Sent: Wednesday, November 08, 2017 9:36 AM
> To: Liu, Wenlong; fuego@lists.linuxfoundation.org
> Subject: Re: [Fuego] issue report: “PER_JOB_BUILD” related
> 
> 
> 
> > -----Original Message-----
> > From: Liu, Wenlong on Monday, November 06, 2017 7:54 AM
> > Just report some issue with “PER_JOB_BUILD”.
> >
> > Issue 1：
> >
> > Jobs “boardx.bc-add.Functional.bc’ and “boardx.bc-by2.Functional.bc’cannot
> > all PASS in the current Fuego because “Functional.bc” has 2 different tarball.
> >
> > So, variable “PER_JOB_BUILD” might be added to spec file.

I didn't know about this functionality.

> 
> That would solve this problem.  But looking at the Functional.bc test, having
> two different tar files, each of which contains a ~150-byte script, seems really
> silly.  But this test is used for demo/tutorial purposes.
> 
> Daniel - was this just done as a demonstration of a putting the tarball build
> variable in a spec file?

Yes, in this case it was just a demonstration.
But in the kernel build test for example each kernel is built from a different git
repository.

> If so, I'll a per_job_build var to this, to get the desired behavior.  Another
> option would be to use a different script name (rather than have both
> tarballs provide bc-device.sh).

I think we should use a more unique name for the TEST_BUILD_DIR and remove
PER_JOB_BUILD. Even with a normal test, the current TEST_BUILD_DIR has issues.

Current:
  TEST_BUILD_DIR="${TESTDIR}-${PLATFORM}"  (Ex: Benchmark.cyclictest-x86_64)

This has several issues:
  - When running the same test on multiple boards that use the same toolchain we
     have a race condition.
  - As Liu pointed, if the spec points to a different tarball or git repository there will
     be conflicts

Proposal: we should use JOB_BUILD_DIR instead:
   <board>.<spec>.<testdir>-<platform>

Thanks,
Daniel


> 
> >
> >
> > Is there any doc for the spec file variables?
> Not really.
> 
> There are a few generic ones documented here:
> http://fuegotest.org/wiki/Test_Specs_and_Plans
> and here:
> http://fuegotest.org/wiki/Variables
> 
> But we have only a few test variables for specific
> tests documented.  For an example, see
> http://fuegotest.org/wiki/Functional.LTP#Variables
> 
> >
> > If not，maybe those variables should be well documented.
> 
> The intention is to document any test-specific test
> variables on a page dedicated to that test.  You can see
> the list of test pages I want to create here:
> http://fuegotest.org/wiki/Tests
> 
> Unfortunately, the tbwiki engine doesn't show missing pages
> in red links, but you can see that a lot of desired pages are missing
> at: http://fuegotest.org/wiki/MissingPages
> 
> Thanks for the bug report!
> 
> > Issue 2：
> >
> > For “Functional.glib2”, the current path will be changed when running
> > “test_build” and then file “fuego_test_succesfully_built” will be created in a
> > wrong path.
> >
> > I fixed this and sent patch to the maillist, please review.
> I did.  Thanks.
> 
>  -- Tim
> 
> _______________________________________________
> Fuego mailing list
> Fuego@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/fuego

Re: [Fuego] issue report: “PER_JOB_BUILD” related

[Bird, Timothy]

> -----Original Message-----
> From: Daniel Sangorrin [mailto:daniel.sangorrin@toshiba.co.jp]
> > -----Original Message-----
> > From: Bird, Timothy on Wednesday, November 08, 2017 9:36 AM
> > > -----Original Message-----
> > > From: Liu, Wenlong on Monday, November 06, 2017 7:54 AM
> > > Just report some issue with “PER_JOB_BUILD”.
> > >
> > > Issue 1：
> > >
> > > Jobs “boardx.bc-add.Functional.bc’ and “boardx.bc-
> by2.Functional.bc’cannot
> > > all PASS in the current Fuego because “Functional.bc” has 2 different
> tarball.
> > >
> > > So, variable “PER_JOB_BUILD” might be added to spec file.
> 
> I didn't know about this functionality.

Yeah.  Sorry about that.  This was added after LTP builds started driving
me nuts.  It is on my list of things to document for the 1.2(.1) release.

> 
> >
> > That would solve this problem.  But looking at the Functional.bc test, having
> > two different tar files, each of which contains a ~150-byte script, seems
> really
> > silly.  But this test is used for demo/tutorial purposes.
> >
> > Daniel - was this just done as a demonstration of a putting the tarball build
> > variable in a spec file?
> 
> Yes, in this case it was just a demonstration.
> But in the kernel build test for example each kernel is built from a different
> git
> repository.

Yes, I see that.

> 
> > If so, I'll a per_job_build var to this, to get the desired behavior.  Another
> > option would be to use a different script name (rather than have both
> > tarballs provide bc-device.sh).
> 
> I think we should use a more unique name for the TEST_BUILD_DIR and
> remove
> PER_JOB_BUILD. Even with a normal test, the current TEST_BUILD_DIR has
> issues.
> 
> Current:
>   TEST_BUILD_DIR="${TESTDIR}-${PLATFORM}"  (Ex: Benchmark.cyclictest-
> x86_64)
> 
> This has several issues:
>   - When running the same test on multiple boards that use the same
> toolchain we
>      have a race condition.
I changed the locking to address this.  The locking is per $JOB_BUILD_DIR.
When the build directories are shared, this works because then the lock file
is actually in the correct $TEST_BUILD_DIR, and it locks all jobs out of 
the shared directory.

>   - As Liu pointed, if the spec points to a different tarball or git repository
> there will
>      be conflicts
 
That's exactly what PER_JOB_BUILD addresses.  I should have added it to
Functional.kernel_build.

> Proposal: we should use JOB_BUILD_DIR instead:
>    <board>.<spec>.<testdir>-<platform>

In terms of visibility to the test, this is actually what we do.
We always build in $JOB_BUILD_DIR.  However, most of the time
(ie, when specs use the same source and binaries as other specs,
and do not have 'per_job_build' defined), this is a symlink
to $TEST_BUILD_DIR, which is shared among all specs (for
builds for the same $PLATFORM)

Sharing the source is fairly important for extremely large tests
like LTP.  I run LTP with multiple specs quite frequently, and I was
ending up 10s of gigabytes of duplicated source code in my
build directory.  We are likely to have many specs for Functional.kselftest,
which will have the same problem, and for which sharing the source
should not be a problem.  Note that sharing the build directory
also saves us a lot of build time (besides file system space). In
the case of LTP, the build takes about 10 minutes, and it was really
rankling me to have to build for myboard.smoketest.Functional.LTP
and myboard.quickhit.Functional.LTP and myboard.ptsonly.Functional.LTP,
etc., when they all had identical source.

I think it might be useful, in the case of Functional.kernel_build, to 
put the per_job_build variable into fuego_test.sh, instead of into
the spec.json file.
We should be able to define FUNCTIONAL_KERNEL_BUILD_PER_JOB_BUILD="true"
in fuego_test.sh, and not have to put 'per_job_build' into every single
spec entry for that test.  I could also modify the test in functions.sh:pre_build,
to check for just PER_JOB_BUILD, to make use of this variable simpler
from fuego_test.sh.

Let me know what you think.
 -- Tim

Re: [Fuego] issue report: “PER_JOB_BUILD” related

[Daniel Sangorrin]

> -----Original Message-----
> From: Bird, Timothy [mailto:Tim.Bird@sony.com]
> Sent: Wednesday, November 08, 2017 10:20 AM
> To: Daniel Sangorrin; 'Liu, Wenlong'; fuego@lists.linuxfoundation.org
> Subject: RE: [Fuego] issue report: “PER_JOB_BUILD” related
> 
> 
> 
> > -----Original Message-----
> > From: Daniel Sangorrin [mailto:daniel.sangorrin@toshiba.co.jp]
> > > -----Original Message-----
> > > From: Bird, Timothy on Wednesday, November 08, 2017 9:36 AM
> > > > -----Original Message-----
> > > > From: Liu, Wenlong on Monday, November 06, 2017 7:54 AM
> > > > Just report some issue with “PER_JOB_BUILD”.
> > > >
> > > > Issue 1：
> > > >
> > > > Jobs “boardx.bc-add.Functional.bc’ and “boardx.bc-
> > by2.Functional.bc’cannot
> > > > all PASS in the current Fuego because “Functional.bc” has 2 different
> > tarball.
> > > >
> > > > So, variable “PER_JOB_BUILD” might be added to spec file.
> >
> > I didn't know about this functionality.
> 
> Yeah.  Sorry about that.  This was added after LTP builds started driving
> me nuts.  It is on my list of things to document for the 1.2(.1) release.
> 
> >
> > >
> > > That would solve this problem.  But looking at the Functional.bc test, having
> > > two different tar files, each of which contains a ~150-byte script, seems
> > really
> > > silly.  But this test is used for demo/tutorial purposes.
> > >
> > > Daniel - was this just done as a demonstration of a putting the tarball build
> > > variable in a spec file?
> >
> > Yes, in this case it was just a demonstration.
> > But in the kernel build test for example each kernel is built from a different
> > git
> > repository.
> 
> Yes, I see that.
> 
> >
> > > If so, I'll a per_job_build var to this, to get the desired behavior.  Another
> > > option would be to use a different script name (rather than have both
> > > tarballs provide bc-device.sh).
> >
> > I think we should use a more unique name for the TEST_BUILD_DIR and
> > remove
> > PER_JOB_BUILD. Even with a normal test, the current TEST_BUILD_DIR has
> > issues.
> >
> > Current:
> >   TEST_BUILD_DIR="${TESTDIR}-${PLATFORM}"  (Ex: Benchmark.cyclictest-
> > x86_64)
> >
> > This has several issues:
> >   - When running the same test on multiple boards that use the same
> > toolchain we
> >      have a race condition.
> I changed the locking to address this.  The locking is per $JOB_BUILD_DIR.
> When the build directories are shared, this works because then the lock file
> is actually in the correct $TEST_BUILD_DIR, and it locks all jobs out of
> the shared directory.
> 
> >   - As Liu pointed, if the spec points to a different tarball or git repository
> > there will
> >      be conflicts
> 
> That's exactly what PER_JOB_BUILD addresses.  I should have added it to
> Functional.kernel_build.
> 
> > Proposal: we should use JOB_BUILD_DIR instead:
> >    <board>.<spec>.<testdir>-<platform>
> 
> In terms of visibility to the test, this is actually what we do.
> We always build in $JOB_BUILD_DIR.  However, most of the time
> (ie, when specs use the same source and binaries as other specs,
> and do not have 'per_job_build' defined), this is a symlink
> to $TEST_BUILD_DIR, which is shared among all specs (for
> builds for the same $PLATFORM)
> 
> Sharing the source is fairly important for extremely large tests
> like LTP.  I run LTP with multiple specs quite frequently, and I was
> ending up 10s of gigabytes of duplicated source code in my
> build directory.  We are likely to have many specs for Functional.kselftest,
> which will have the same problem, and for which sharing the source
> should not be a problem.  Note that sharing the build directory
> also saves us a lot of build time (besides file system space). In
> the case of LTP, the build takes about 10 minutes, and it was really
> rankling me to have to build for myboard.smoketest.Functional.LTP
> and myboard.quickhit.Functional.LTP and myboard.ptsonly.Functional.LTP,
> etc., when they all had identical source.
> 
> I think it might be useful, in the case of Functional.kernel_build, to
> put the per_job_build variable into fuego_test.sh, instead of into
> the spec.json file.
> We should be able to define FUNCTIONAL_KERNEL_BUILD_PER_JOB_BUILD="true"
> in fuego_test.sh, and not have to put 'per_job_build' into every single
> spec entry for that test.  I could also modify the test in functions.sh:pre_build,
> to check for just PER_JOB_BUILD, to make use of this variable simpler
> from fuego_test.sh.
> 
> Let me know what you think.

OK, I see your point about sharing the build time by default. I will work
on that for the kernel build fuego_test.sh.

At some point in the future, we will need to configure the Jenkins per-node
"# of executors" to something bigger than 1 so that other jobs on the same
board don't have to wait for the build lock to be released. Not sure how far
is Fuego from being able to dispatch several jobs on the same board. I suppose
we are not that far. We will probably need a per-board LOCK. If you want 
I can look into that as well. 

Thanks,
Daniel