Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
6746009:
Add piglit ebuilds (Closed)
|
Created:
9 years, 9 months ago by ilja Modified:
9 years, 7 months ago CC:
chromium-os-reviews_chromium.org, msb+crosoverlay_chromium.org, adlr+crosoverlay_chromium.org, anush Visibility:
Public. |
DescriptionAdd ebuilds to cross-compile piglit OpenGL library.
See also http://codereview.chromium.org/6745001
for autotest setup.
BUG=chromium-os:13932
TEST= ./build_packages --board=${BOARD} --oldchromebinary --withautotest | grep piglit
ls /build/${BOARD}/usr/local/autotest/packages/piglit.tar.gz
Takes about 12 minutes on my mario. Should say "Pass" and return count_subtests_pass etc.
Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=65bbe2c
Patch Set 1 #Patch Set 2 : Skip building and running on non-x86 boards due to lack of OpenGL #Patch Set 3 : Undelete autotest.ebuild #Patch Set 4 : add chromeous-test dependency #
Total comments: 10
Patch Set 5 : responding to sosa #Patch Set 6 : responding to zdenek #
Total comments: 5
Patch Set 7 : fix comment #Patch Set 8 : responding to zdenek #
Messages
Total messages: 26 (0 generated)
The main problem I had with piglit is that it has no install. the binaries are run against the config files that live with the sources. Hence after building the binaries I merge them back into the tarball for later use (which is fine for autotest as the library gets zipped up there again). As this is the first time setting up ebuilds + autotest I appreciate detailed feedback. Thanks!
+ericli I don't think this is the right way to go about this, especially since you're cherry-picking a CL. If you look at glbench, iotools, and libaio you'll see that they're all stubbed ebuilds with the actual work happening in autotest/files/deps/[glbench|iotools|libaio].
Question: can you elaborate "the binaries are run against the config files that live with the source"? If the test binaries are run inside a chrome netbook, where is the source you refer to? On Fri, Mar 25, 2011 at 3:44 PM, <dalecurtis@chromium.org> wrote: > +ericli > > I don't think this is the right way to go about this, especially since > you're > cherry-picking a CL. If you look at glbench, iotools, and libaio you'll see > that > they're all stubbed ebuilds with the actual work happening in > autotest/files/deps/[glbench|iotools|libaio]. > > > http://codereview.chromium.org/6746009/ > -- Eric Li 李咏竹 Google Kirkland
-me, +zbehan -- he understands more how these autotest-deps ebuilds work.
On 2011/03/25 23:02:32, ericli wrote: > Question: can you elaborate "the binaries are run against the config files > that live with the source"? > If the test binaries are run inside a chrome netbook, where is the source > you refer to? The compilation step of the piglit source tree produces about 400 independent binaries which all live in the bin directory. The binaries are independent of each other, but many are not independent of the source tree. This is primarily because the often load extra files that were placed somewhere near their sources. These extra files could be images, 3D models or shaders. Also there are several python scripts and configuration files that will run these 400 binaries to create a summary. In any case, this means the whole downloaded tarball plus the bin directory have to go together onto the netbook for testing (which is what I am creating here).
On 2011/03/25 22:44:50, dalec wrote: > I don't think this is the right way to go about this, especially since you're > cherry-picking a CL. If you look at glbench, iotools, and libaio you'll see that > they're all stubbed ebuilds with the actual work happening in > autotest/files/deps/[glbench|iotools|libaio]. I am fine to consider changing everything to how glbench|iotools|libaio are doing. I don't know the limitations of the current autotest setup. But let me give you some more background information first: 1) glbench et al. are tiny. A handful of c/c++ files and < 100k in size. Compile in seconds. Piglit is 20MB sources plus 100MB x86 binaries. Compile takes 3 minutes. Not sure if this matters. 2) There is a need for cherry picking commits as these are external tests and I don't want somebody accidentally close our tree. We plan to update the external checkins maybe once a month. In addition we consider adding our own tests to piglit (and upstream periodically after some soaking). I don't know how this setup will look like at the end, but I don't want to go a route that will not support this. Do glbench et al. allow for easy periodic source sync and patching? Thanks.
Got it. I would suggest to take a look how page cycler test was integrated into autotest. For now, I think it is OK to check in the tarball. You mentioned 100M x86 binaries, so this test is not compatible with ARM? On Fri, Mar 25, 2011 at 5:52 PM, <ihf@chromium.org> wrote: > On 2011/03/25 22:44:50, dalec wrote: > >> I don't think this is the right way to go about this, especially since >> you're >> cherry-picking a CL. If you look at glbench, iotools, and libaio you'll >> see >> > that > >> they're all stubbed ebuilds with the actual work happening in >> autotest/files/deps/[glbench|iotools|libaio]. >> > > I am fine to consider changing everything to how glbench|iotools|libaio are > doing. I don't know the limitations of the current autotest setup. But let > me > give you some more background information first: > 1) glbench et al. are tiny. A handful of c/c++ files and < 100k in size. > Compile > in seconds. Piglit is 20MB sources plus 100MB x86 binaries. Compile takes 3 > minutes. Not sure if this matters. > 2) There is a need for cherry picking commits as these are external tests > and I > don't want somebody accidentally close our tree. We plan to update the > external > checkins maybe once a month. In addition we consider adding our own tests > to > piglit (and upstream periodically after some soaking). I don't know how > this > setup will look like at the end, but I don't want to go a route that will > not > support this. Do glbench et al. allow for easy periodic source sync and > patching? Thanks. > > > http://codereview.chromium.org/6746009/ > -- Eric Li 李咏竹 Google Kirkland
Hi Ilja, A comment regarding ebuild setup: Ebuilds don't replace a build system, merely wrap around an existing one to make all packages installable in a single unified way. For autotest, ebuilds call autotest to do all the work, and all the magic they do is creating a proper autotest enviroment to build everything correctly. The point is to still be able to run autotest the "old way", using just autotest scripts, without any ebuilds, and ebuilds serve as packaging. In practice, that means that you should write all this code as part of the standard setup() of your test/dep, and rely on the existing infrastructure (autotest eclass) to build it in portage. A note on the big outside sources: I was fixing a similar problem with O3D, where the test (graphics_O3DSelenium) needs a specific revision checkout of a subset of O3D sources inside. Ultimately, I changed it to use a tarball and download that tarball off our local portage mirror. Perhaps another option that has not been used/explored so far would be git submodules to have your test sources directly point at the dependent upstream sources, or even mirroring the upstream sources in your test repository. That would be an entirely source-level solution, and would have nothing to do with autotest. Other tests needing smaller tarballs that [almost] never change simply commit the .tgz into the git repository along with the test. Zdenek On Sat, Mar 26, 2011 at 2:15 AM, Eric Li(李咏竹) <ericli@chromium.org> wrote: > Got it. I would suggest to take a look how page cycler test was integrated > into autotest. For now, I think it is OK to check in the tarball. > > You mentioned 100M x86 binaries, so this test is not compatible with ARM? > > On Fri, Mar 25, 2011 at 5:52 PM, <ihf@chromium.org> wrote: > >> On 2011/03/25 22:44:50, dalec wrote: >> >>> I don't think this is the right way to go about this, especially since >>> you're >>> cherry-picking a CL. If you look at glbench, iotools, and libaio you'll >>> see >>> >> that >> >>> they're all stubbed ebuilds with the actual work happening in >>> autotest/files/deps/[glbench|iotools|libaio]. >>> >> >> I am fine to consider changing everything to how glbench|iotools|libaio >> are >> doing. I don't know the limitations of the current autotest setup. But let >> me >> give you some more background information first: >> 1) glbench et al. are tiny. A handful of c/c++ files and < 100k in size. >> Compile >> in seconds. Piglit is 20MB sources plus 100MB x86 binaries. Compile takes >> 3 >> minutes. Not sure if this matters. >> 2) There is a need for cherry picking commits as these are external tests >> and I >> don't want somebody accidentally close our tree. We plan to update the >> external >> checkins maybe once a month. In addition we consider adding our own tests >> to >> piglit (and upstream periodically after some soaking). I don't know how >> this >> setup will look like at the end, but I don't want to go a route that will >> not >> support this. Do glbench et al. allow for easy periodic source sync and >> patching? Thanks. >> >> >> http://codereview.chromium.org/6746009/ >> > > > > -- > Eric Li > 李咏竹 > Google Kirkland > > >
On 2011/03/26 01:15:55, ericli wrote: > You mentioned 100M x86 binaries, so this test is not compatible with ARM? I tried to compile it for ARM and about 20 percent into it broke. I think ultimately there shouldn't be a reason why it wouldn't run on ARM, but for now the answer is no.
Hi Zdenek, Ok. I got that ebuild calls autotest and autotest should do all the making. Now I don't know the "old" way where just the autotest scripts are used. piglit needs glut and glut is put by ebuild into SYSROOT. I guess autotest could use a different version of glut which I could build just before piglit (say like deps/mysql/mysql.py). This approach though seems wrong to me, as we would not be running the same code as users. Please take a look at http://codereview.chromium.org/6745001/ and let me know what could be improved so it works with the old approach. Thank you, Ilja. On 2011/03/27 11:09:39, zbehan wrote: > Hi Ilja, > > A comment regarding ebuild setup: > Ebuilds don't replace a build system, merely wrap around an existing one to > make all packages installable in a single unified way. For autotest, ebuilds > call autotest to do all the work, and all the magic they do is creating a > proper autotest enviroment to build everything correctly. The point is to > still be able to run autotest the "old way", using just autotest scripts, > without any ebuilds, and ebuilds serve as packaging. > In practice, that means that you should write all this code as part of the > standard setup() of your test/dep, and rely on the existing infrastructure > (autotest eclass) to build it in portage.
Sorry for the delay. If this breaks on ARM then it'll break the ARM builders when it's checked in and have to be rolled back. You'll need to fix it to work on ARM. I don't think we have a way for a test to exist on just X86 or just ARM. It wouldn't be impossible to do, but I'm not sure we want to start that rift. On 2011/03/30 05:38:35, ihf wrote: > Hi Zdenek, > > Ok. I got that ebuild calls autotest and autotest should do all the making. Now > I don't know the "old" way where just the autotest scripts are used. piglit > needs glut and glut is put by ebuild into SYSROOT. I guess autotest could use a > different version of glut which I could build just before piglit (say like > deps/mysql/mysql.py). This approach though seems wrong to me, as we would not be > running the same code as users. Please take a look at > http://codereview.chromium.org/6745001/ > and let me know what could be improved so it works with the old approach. > > Thank you, > > Ilja. > > On 2011/03/27 11:09:39, zbehan wrote: > > Hi Ilja, > > > > A comment regarding ebuild setup: > > Ebuilds don't replace a build system, merely wrap around an existing one to > > make all packages installable in a single unified way. For autotest, ebuilds > > call autotest to do all the work, and all the magic they do is creating a > > proper autotest enviroment to build everything correctly. The point is to > > still be able to run autotest the "old way", using just autotest scripts, > > without any ebuilds, and ebuilds serve as packaging. > > In practice, that means that you should write all this code as part of the > > standard setup() of your test/dep, and rely on the existing infrastructure > > (autotest eclass) to build it in portage.
Hi Dale, thanks for letting me know about the ARM blocker. I will work on it. Ilja. On 2011/04/04 17:46:50, dalec wrote: > Sorry for the delay. > > If this breaks on ARM then it'll break the ARM builders when it's checked in and > have to be rolled back. You'll need to fix it to work on ARM. > > I don't think we have a way for a test to exist on just X86 or just ARM. It > wouldn't be impossible to do, but I'm not sure we want to start that rift. >
On Mon, Apr 4, 2011 at 10:46, <dalecurtis@chromium.org> wrote: > Sorry for the delay. > > If this breaks on ARM then it'll break the ARM builders when it's checked > in and > have to be rolled back. You'll need to fix it to work on ARM. > > I don't think we have a way for a test to exist on just X86 or just ARM. It > wouldn't be impossible to do, but I'm not sure we want to start that rift. > > Hi, Given that piglit is an OpenGL test and that ARM uses OpenGL ES for all rendering, running piglit on ARM doesn't seem make much sense anyway. If we run piglit on ARM it'll link against our software-rendered mesa library instead, and that is not what we want to test here. We do recognize the lack of GLES testing but that'll have to be addressed at some later point. Stéphane
So, we are in a situation where a feature (OpenGL) is only available on some
platforms but not others. I checked how this was handled previously and some
smaller tests were ported (like GLBench or TearTest) to OpenGL ES. I don't
think we can do this here. Which leaves two options
a) fix autotest such that it is able to schedule jobs to run only for
particular boards
b) change graphics_Piglit.py::setup to skip compilation when SYSROOT does
not contain x86
also change graphics_Piglit.py::run_once to skip running the test and
return bogus numbers (all 0) if there is no binary to run (I think SYSROOT
is not available on test targets)
Let me know if there are other options. I spoke with Chris Sosa yesterday
and he thinks Zdenek might have experience with it.
Going the second route seems faster to implement, but is more hacky and
would lead to overhead as we would essentially compile and run empty tests.
It places the burden on the test owner to maintain a list of boards to run
(once we say have a non-Tegra2 ARM that can run the test). On the other hand
essentially the same code could take be placed a level higher to solve this
problem across all tests and skip the overhead.
Ilja.
On Mon, Apr 4, 2011 at 3:06 PM, Stéphane Marchesin <marcheu@chromium.org>wrote:
>
>
> On Mon, Apr 4, 2011 at 10:46, <dalecurtis@chromium.org> wrote:
>
>> Sorry for the delay.
>>
>> If this breaks on ARM then it'll break the ARM builders when it's checked
>> in and
>> have to be rolled back. You'll need to fix it to work on ARM.
>>
>> I don't think we have a way for a test to exist on just X86 or just ARM.
>> It
>> wouldn't be impossible to do, but I'm not sure we want to start that rift.
>>
>>
> Hi,
>
> Given that piglit is an OpenGL test and that ARM uses OpenGL ES for all
> rendering, running piglit on ARM doesn't seem make much sense anyway. If we
> run piglit on ARM it'll link against our software-rendered mesa library
> instead, and that is not what we want to test here.
>
> We do recognize the lack of GLES testing but that'll have to
> be addressed at some later point.
>
> Stéphane
>
>
We already have a). It doesn't solve the build breaking on ARM though. b) or a variant there of is the only way to solve that. We could also create an autotest-tests-x86 ebuild and autotest-test-arm ebuild for tests which can only run on one or the other. On 2011/04/05 18:49:01, ihf_google.com wrote: > So, we are in a situation where a feature (OpenGL) is only available on some > platforms but not others. I checked how this was handled previously and some > smaller tests were ported (like GLBench or TearTest) to OpenGL ES. I don't > think we can do this here. Which leaves two options > > a) fix autotest such that it is able to schedule jobs to run only for > particular boards > b) change graphics_Piglit.py::setup to skip compilation when SYSROOT does > not contain x86 > also change graphics_Piglit.py::run_once to skip running the test and > return bogus numbers (all 0) if there is no binary to run (I think SYSROOT > is not available on test targets) > > Let me know if there are other options. I spoke with Chris Sosa yesterday > and he thinks Zdenek might have experience with it. > > Going the second route seems faster to implement, but is more hacky and > would lead to overhead as we would essentially compile and run empty tests. > It places the burden on the test owner to maintain a list of boards to run > (once we say have a non-Tegra2 ARM that can run the test). On the other hand > essentially the same code could take be placed a level higher to solve this > problem across all tests and skip the overhead. > > > Ilja. > > On Mon, Apr 4, 2011 at 3:06 PM, Stéphane Marchesin <marcheu@chromium.org>wrote: > > > > > > > On Mon, Apr 4, 2011 at 10:46, <mailto:dalecurtis@chromium.org> wrote: > > > >> Sorry for the delay. > >> > >> If this breaks on ARM then it'll break the ARM builders when it's checked > >> in and > >> have to be rolled back. You'll need to fix it to work on ARM. > >> > >> I don't think we have a way for a test to exist on just X86 or just ARM. > >> It > >> wouldn't be impossible to do, but I'm not sure we want to start that rift. > >> > >> > > Hi, > > > > Given that piglit is an OpenGL test and that ARM uses OpenGL ES for all > > rendering, running piglit on ARM doesn't seem make much sense anyway. If we > > run piglit on ARM it'll link against our software-rendered mesa library > > instead, and that is not what we want to test here. > > > > We do recognize the lack of GLES testing but that'll have to > > be addressed at some later point. > > > > Stéphane > > > >
Ok. I'll implement b) and skip build if SYSROOT does not contain x86. If there is any other variable I should check instead please let me know. Ilja. On Tue, Apr 5, 2011 at 12:03 PM, <dalecurtis@chromium.org> wrote: > We already have a). It doesn't solve the build breaking on ARM though. b) > or a > variant there of is the only way to solve that. We could also create an > autotest-tests-x86 ebuild and autotest-test-arm ebuild for tests which can > only > run on one or the other. > > > On 2011/04/05 18:49:01, ihf_google.com wrote: > >> So, we are in a situation where a feature (OpenGL) is only available on >> some >> platforms but not others. I checked how this was handled previously and >> some >> smaller tests were ported (like GLBench or TearTest) to OpenGL ES. I don't >> think we can do this here. Which leaves two options >> > > a) fix autotest such that it is able to schedule jobs to run only for >> particular boards >> b) change graphics_Piglit.py::setup to skip compilation when SYSROOT does >> not contain x86 >> also change graphics_Piglit.py::run_once to skip running the test and >> return bogus numbers (all 0) if there is no binary to run (I think SYSROOT >> is not available on test targets) >> > > Let me know if there are other options. I spoke with Chris Sosa yesterday >> and he thinks Zdenek might have experience with it. >> > > Going the second route seems faster to implement, but is more hacky and >> would lead to overhead as we would essentially compile and run empty >> tests. >> It places the burden on the test owner to maintain a list of boards to run >> (once we say have a non-Tegra2 ARM that can run the test). On the other >> hand >> essentially the same code could take be placed a level higher to solve >> this >> problem across all tests and skip the overhead. >> > > > Ilja. >> > > On Mon, Apr 4, 2011 at 3:06 PM, Stéphane Marchesin >> > <marcheu@chromium.org>wrote: > > > >> > >> > On Mon, Apr 4, 2011 at 10:46, <mailto:dalecurtis@chromium.org> wrote: >> > >> >> Sorry for the delay. >> >> >> >> If this breaks on ARM then it'll break the ARM builders when it's >> checked >> >> in and >> >> have to be rolled back. You'll need to fix it to work on ARM. >> >> >> >> I don't think we have a way for a test to exist on just X86 or just >> ARM. >> >> It >> >> wouldn't be impossible to do, but I'm not sure we want to start that >> rift. >> >> >> >> >> > Hi, >> > >> > Given that piglit is an OpenGL test and that ARM uses OpenGL ES for all >> > rendering, running piglit on ARM doesn't seem make much sense anyway. If >> we >> > run piglit on ARM it'll link against our software-rendered mesa library >> > instead, and that is not what we want to test here. >> > >> > We do recognize the lack of GLES testing but that'll have to >> > be addressed at some later point. >> > >> > Stéphane >> > >> > >> > > > > http://codereview.chromium.org/6746009/ >
I suggested that we use the ebuild keywords in autotest-tests to conditionalize whether to build / include the test in autotest-tests. Zdenek, does this sound reasonable? I much prefer this than doing some logic to read a hard coded board in the sysroot. On Tue, Apr 5, 2011 at 1:28 PM, Ilja Friedel <ihf@google.com> wrote: > Ok. I'll implement b) and skip build if SYSROOT does not contain x86. If > there is any other variable I should check instead please let me know. > Ilja. > > On Tue, Apr 5, 2011 at 12:03 PM, <dalecurtis@chromium.org> wrote: >> >> We already have a). It doesn't solve the build breaking on ARM though. b) >> or a >> variant there of is the only way to solve that. We could also create an >> autotest-tests-x86 ebuild and autotest-test-arm ebuild for tests which can >> only >> run on one or the other. >> >> On 2011/04/05 18:49:01, ihf_google.com wrote: >>> >>> So, we are in a situation where a feature (OpenGL) is only available on >>> some >>> platforms but not others. I checked how this was handled previously and >>> some >>> smaller tests were ported (like GLBench or TearTest) to OpenGL ES. I >>> don't >>> think we can do this here. Which leaves two options >> >>> a) fix autotest such that it is able to schedule jobs to run only for >>> particular boards >>> b) change graphics_Piglit.py::setup to skip compilation when SYSROOT does >>> not contain x86 >>> also change graphics_Piglit.py::run_once to skip running the test and >>> return bogus numbers (all 0) if there is no binary to run (I think >>> SYSROOT >>> is not available on test targets) >> >>> Let me know if there are other options. I spoke with Chris Sosa yesterday >>> and he thinks Zdenek might have experience with it. >> >>> Going the second route seems faster to implement, but is more hacky and >>> would lead to overhead as we would essentially compile and run empty >>> tests. >>> It places the burden on the test owner to maintain a list of boards to >>> run >>> (once we say have a non-Tegra2 ARM that can run the test). On the other >>> hand >>> essentially the same code could take be placed a level higher to solve >>> this >>> problem across all tests and skip the overhead. >> >> >>> Ilja. >> >>> On Mon, Apr 4, 2011 at 3:06 PM, Stéphane Marchesin >> >> <marcheu@chromium.org>wrote: >> >>> > >>> > >>> > On Mon, Apr 4, 2011 at 10:46, <mailto:dalecurtis@chromium.org> wrote: >>> > >>> >> Sorry for the delay. >>> >> >>> >> If this breaks on ARM then it'll break the ARM builders when it's >>> >> checked >>> >> in and >>> >> have to be rolled back. You'll need to fix it to work on ARM. >>> >> >>> >> I don't think we have a way for a test to exist on just X86 or just >>> >> ARM. >>> >> It >>> >> wouldn't be impossible to do, but I'm not sure we want to start that >>> >> rift. >>> >> >>> >> >>> > Hi, >>> > >>> > Given that piglit is an OpenGL test and that ARM uses OpenGL ES for all >>> > rendering, running piglit on ARM doesn't seem make much sense anyway. >>> > If we >>> > run piglit on ARM it'll link against our software-rendered mesa library >>> > instead, and that is not what we want to test here. >>> > >>> > We do recognize the lack of GLES testing but that'll have to >>> > be addressed at some later point. >>> > >>> > Stéphane >>> > >>> > >> >> >> >> http://codereview.chromium.org/6746009/ > >
All tests in autotest-tests are exposed as a USE flag by itself, so that they can be disabled/masked in a profile, but we don't quite have the infrastructure ready for any fine-grained selection of per-profile use flags. To conditionalize, the IUSE_TESTS list would have to be constructed conditionally based on keywords in the global scope, which is forbidden (but in this particular case may be possible, after silencing the QA checks). Another option would be adding autotest-tests-x86 ebuild which would have x86-specific tests, but as I understand, the test is not strictly x86, it just doesn't fully work anywhere else yet, so that's probably not the best option. Maybe create an ebuild specifically for this test, and handle the condition elsewhere? On Tue, Apr 5, 2011 at 10:31 PM, Chris Sosa <sosa@google.com> wrote: > I suggested that we use the ebuild keywords in autotest-tests to > conditionalize whether to build / include the test in autotest-tests. > Zdenek, does this sound reasonable? I much prefer this than doing > some logic to read a hard coded board in the sysroot. > > On Tue, Apr 5, 2011 at 1:28 PM, Ilja Friedel <ihf@google.com> wrote: > > Ok. I'll implement b) and skip build if SYSROOT does not contain x86. If > > there is any other variable I should check instead please let me know. > > Ilja. > > > > On Tue, Apr 5, 2011 at 12:03 PM, <dalecurtis@chromium.org> wrote: > >> > >> We already have a). It doesn't solve the build breaking on ARM though. > b) > >> or a > >> variant there of is the only way to solve that. We could also create an > >> autotest-tests-x86 ebuild and autotest-test-arm ebuild for tests which > can > >> only > >> run on one or the other. > >> > >> On 2011/04/05 18:49:01, ihf_google.com wrote: > >>> > >>> So, we are in a situation where a feature (OpenGL) is only available on > >>> some > >>> platforms but not others. I checked how this was handled previously and > >>> some > >>> smaller tests were ported (like GLBench or TearTest) to OpenGL ES. I > >>> don't > >>> think we can do this here. Which leaves two options > >> > >>> a) fix autotest such that it is able to schedule jobs to run only for > >>> particular boards > >>> b) change graphics_Piglit.py::setup to skip compilation when SYSROOT > does > >>> not contain x86 > >>> also change graphics_Piglit.py::run_once to skip running the test > and > >>> return bogus numbers (all 0) if there is no binary to run (I think > >>> SYSROOT > >>> is not available on test targets) > >> > >>> Let me know if there are other options. I spoke with Chris Sosa > yesterday > >>> and he thinks Zdenek might have experience with it. > >> > >>> Going the second route seems faster to implement, but is more hacky and > >>> would lead to overhead as we would essentially compile and run empty > >>> tests. > >>> It places the burden on the test owner to maintain a list of boards to > >>> run > >>> (once we say have a non-Tegra2 ARM that can run the test). On the other > >>> hand > >>> essentially the same code could take be placed a level higher to solve > >>> this > >>> problem across all tests and skip the overhead. > >> > >> > >>> Ilja. > >> > >>> On Mon, Apr 4, 2011 at 3:06 PM, Stéphane Marchesin > >> > >> <marcheu@chromium.org>wrote: > >> > >>> > > >>> > > >>> > On Mon, Apr 4, 2011 at 10:46, <mailto:dalecurtis@chromium.org> > wrote: > >>> > > >>> >> Sorry for the delay. > >>> >> > >>> >> If this breaks on ARM then it'll break the ARM builders when it's > >>> >> checked > >>> >> in and > >>> >> have to be rolled back. You'll need to fix it to work on ARM. > >>> >> > >>> >> I don't think we have a way for a test to exist on just X86 or just > >>> >> ARM. > >>> >> It > >>> >> wouldn't be impossible to do, but I'm not sure we want to start that > >>> >> rift. > >>> >> > >>> >> > >>> > Hi, > >>> > > >>> > Given that piglit is an OpenGL test and that ARM uses OpenGL ES for > all > >>> > rendering, running piglit on ARM doesn't seem make much sense anyway. > >>> > If we > >>> > run piglit on ARM it'll link against our software-rendered mesa > library > >>> > instead, and that is not what we want to test here. > >>> > > >>> > We do recognize the lack of GLES testing but that'll have to > >>> > be addressed at some later point. > >>> > > >>> > Stéphane > >>> > > >>> > > >> > >> > >> > >> http://codereview.chromium.org/6746009/ > > > > >
I went through and implemented b). I've moved code around such that it looks more like glbench. Otherwise I just added two if-statements in the python code. There was a problem with how to get libtiff* and libglut* to the target machine only for test builds, which Zdenek thankfully resolved by pumping the r-number in chromeos-test ebuild. I verified with the current CL 1) piglit builds inside of autotest 2) dependencies are installed only on test system images (x86) 3) compiles and runs on x86 systems (I will own this switch and adjust it as we get new boards depending on availability of OpenGL/ES) 4) does not break compile on tegra2_seaboard (nor does it run there as discussed before) It would be great to get a final review. Thanks again, Ilja. On Tue, Apr 5, 2011 at 1:28 PM, Ilja Friedel <ihf@google.com> wrote: > Ok. I'll implement b) and skip build if SYSROOT does not contain x86. If > there is any other variable I should check instead please let me know. > > Ilja. > > > On Tue, Apr 5, 2011 at 12:03 PM, <dalecurtis@chromium.org> wrote: > >> We already have a). It doesn't solve the build breaking on ARM though. b) >> or a >> variant there of is the only way to solve that. We could also create an >> autotest-tests-x86 ebuild and autotest-test-arm ebuild for tests which can >> only >> run on one or the other. >> >> >> On 2011/04/05 18:49:01, ihf_google.com wrote: >> >>> So, we are in a situation where a feature (OpenGL) is only available on >>> some >>> platforms but not others. I checked how this was handled previously and >>> some >>> smaller tests were ported (like GLBench or TearTest) to OpenGL ES. I >>> don't >>> think we can do this here. Which leaves two options >>> >> >> a) fix autotest such that it is able to schedule jobs to run only for >>> particular boards >>> b) change graphics_Piglit.py::setup to skip compilation when SYSROOT does >>> not contain x86 >>> also change graphics_Piglit.py::run_once to skip running the test and >>> return bogus numbers (all 0) if there is no binary to run (I think >>> SYSROOT >>> is not available on test targets) >>> >> >> Let me know if there are other options. I spoke with Chris Sosa yesterday >>> and he thinks Zdenek might have experience with it. >>> >> >> Going the second route seems faster to implement, but is more hacky and >>> would lead to overhead as we would essentially compile and run empty >>> tests. >>> It places the burden on the test owner to maintain a list of boards to >>> run >>> (once we say have a non-Tegra2 ARM that can run the test). On the other >>> hand >>> essentially the same code could take be placed a level higher to solve >>> this >>> problem across all tests and skip the overhead. >>> >> >> >> Ilja. >>> >> >> On Mon, Apr 4, 2011 at 3:06 PM, Stéphane Marchesin >>> >> <marcheu@chromium.org>wrote: >> >> > >>> > >>> > On Mon, Apr 4, 2011 at 10:46, <mailto:dalecurtis@chromium.org> wrote: >>> > >>> >> Sorry for the delay. >>> >> >>> >> If this breaks on ARM then it'll break the ARM builders when it's >>> checked >>> >> in and >>> >> have to be rolled back. You'll need to fix it to work on ARM. >>> >> >>> >> I don't think we have a way for a test to exist on just X86 or just >>> ARM. >>> >> It >>> >> wouldn't be impossible to do, but I'm not sure we want to start that >>> rift. >>> >> >>> >> >>> > Hi, >>> > >>> > Given that piglit is an OpenGL test and that ARM uses OpenGL ES for all >>> > rendering, running piglit on ARM doesn't seem make much sense anyway. >>> If we >>> > run piglit on ARM it'll link against our software-rendered mesa library >>> > instead, and that is not what we want to test here. >>> > >>> > We do recognize the lack of GLES testing but that'll have to >>> > be addressed at some later point. >>> > >>> > Stéphane >>> > >>> > >>> >> >> >> >> http://codereview.chromium.org/6746009/ >> > >
It seems like you have too many copies of this RDEPEND. You should only need it in autotest-tests I believe since Factory and Test already both pull this in. http://codereview.chromium.org/6746009/diff/13002/chromeos-base/autotest-deps... File chromeos-base/autotest-deps-piglit/autotest-deps-piglit-9999.ebuild (right): http://codereview.chromium.org/6746009/diff/13002/chromeos-base/autotest-deps... chromeos-base/autotest-deps-piglit/autotest-deps-piglit-9999.ebuild:14: KEYWORDS="~amd64 arm x86" Don't unmask keywords yourself. They will be unmasked themselves when you commit them in a 9999 ebuild. http://codereview.chromium.org/6746009/diff/13002/chromeos-base/autotest-test... File chromeos-base/autotest-tests/autotest-tests-9999.ebuild (right): http://codereview.chromium.org/6746009/diff/13002/chromeos-base/autotest-test... chromeos-base/autotest-tests/autotest-tests-9999.ebuild:13: KEYWORDS="x86 ~arm ~amd64" Don't change this. http://codereview.chromium.org/6746009/diff/13002/chromeos-base/autotest-test... chromeos-base/autotest-tests/autotest-tests-9999.ebuild:47: chromeos-base/autotest-deps-piglit This contradicts the chromeos-test ebuild. Just in case arm is ever switched we should prob set this to be consistent with x86? here as well. http://codereview.chromium.org/6746009/diff/13002/chromeos-base/chromeos-test... File chromeos-base/chromeos-test/chromeos-test-0.1.0.ebuild (right): http://codereview.chromium.org/6746009/diff/13002/chromeos-base/chromeos-test... chromeos-base/chromeos-test/chromeos-test-0.1.0.ebuild:18: # specific tests. Libraries are installed in /usr/local/lib. This isn't necessarily generic enough. Perhaps an extra stanza below (before the TODO) saying something along the lines of: Developers should be aware that packages installed by this ebuild are rooted in /usr/local. This means that libraries are installed in /usr/local/lib, executables in /usr/local/bin, etc. http://codereview.chromium.org/6746009/diff/13002/chromeos-base/chromeos-test... chromeos-base/chromeos-test/chromeos-test-0.1.0.ebuild:36: x86? ( chromeos-base/autotest-deps-piglit ) Don't put this here. You have 3 copies of it around. You only need the one where it really is necessary.
http://codereview.chromium.org/6746009/diff/13002/chromeos-base/autotest-deps... File chromeos-base/autotest-deps/autotest-deps-0.0.1-r551.ebuild (right): http://codereview.chromium.org/6746009/diff/13002/chromeos-base/autotest-deps... chromeos-base/autotest-deps/autotest-deps-0.0.1-r551.ebuild:1: # Copyright (c) 2010 The Chromium OS Authors. All rights reserved. Automatic rev, just change -9999 version. http://codereview.chromium.org/6746009/diff/13002/chromeos-base/autotest-fact... File chromeos-base/autotest-factory/autotest-factory-0.0.1-r233.ebuild (right): http://codereview.chromium.org/6746009/diff/13002/chromeos-base/autotest-fact... chromeos-base/autotest-factory/autotest-factory-0.0.1-r233.ebuild:1: # Copyright (c) 2010 The Chromium OS Authors. All rights reserved. Automatic rev, just change -9999 version. http://codereview.chromium.org/6746009/diff/13002/chromeos-base/autotest-test... File chromeos-base/autotest-tests/autotest-tests-0.0.1-r572.ebuild (right): http://codereview.chromium.org/6746009/diff/13002/chromeos-base/autotest-test... chromeos-base/autotest-tests/autotest-tests-0.0.1-r572.ebuild:1: # Copyright (c) 2010 The Chromium OS Authors. All rights reserved. Automatic rev, just change -9999 version.
http://codereview.chromium.org/6746009/diff/13002/chromeos-base/autotest-deps... File chromeos-base/autotest-deps/autotest-deps-0.0.1-r551.ebuild (right): http://codereview.chromium.org/6746009/diff/13002/chromeos-base/autotest-deps... chromeos-base/autotest-deps/autotest-deps-0.0.1-r551.ebuild:1: # Copyright (c) 2010 The Chromium OS Authors. All rights reserved. Automatic rev, just change -9999 version. http://codereview.chromium.org/6746009/diff/13002/chromeos-base/autotest-fact... File chromeos-base/autotest-factory/autotest-factory-0.0.1-r233.ebuild (right): http://codereview.chromium.org/6746009/diff/13002/chromeos-base/autotest-fact... chromeos-base/autotest-factory/autotest-factory-0.0.1-r233.ebuild:1: # Copyright (c) 2010 The Chromium OS Authors. All rights reserved. Automatic rev, just change -9999 version. http://codereview.chromium.org/6746009/diff/13002/chromeos-base/autotest-test... File chromeos-base/autotest-tests/autotest-tests-0.0.1-r572.ebuild (right): http://codereview.chromium.org/6746009/diff/13002/chromeos-base/autotest-test... chromeos-base/autotest-tests/autotest-tests-0.0.1-r572.ebuild:1: # Copyright (c) 2010 The Chromium OS Authors. All rights reserved. Automatic rev, just change -9999 version.
Spoke with Zdenek and Sosa and addressed their concerns. http://codereview.chromium.org/6746009/diff/13002/chromeos-base/autotest-deps... File chromeos-base/autotest-deps-piglit/autotest-deps-piglit-9999.ebuild (right): http://codereview.chromium.org/6746009/diff/13002/chromeos-base/autotest-deps... chromeos-base/autotest-deps-piglit/autotest-deps-piglit-9999.ebuild:14: KEYWORDS="~amd64 arm x86" On 2011/04/11 21:12:30, sosa wrote: > Don't unmask keywords yourself. They will be unmasked themselves when you > commit them in a 9999 ebuild. Done. http://codereview.chromium.org/6746009/diff/13002/chromeos-base/autotest-test... File chromeos-base/autotest-tests/autotest-tests-9999.ebuild (right): http://codereview.chromium.org/6746009/diff/13002/chromeos-base/autotest-test... chromeos-base/autotest-tests/autotest-tests-9999.ebuild:13: KEYWORDS="x86 ~arm ~amd64" On 2011/04/11 21:12:30, sosa wrote: > Don't change this. Done.
http://codereview.chromium.org/6746009/diff/19003/chromeos-base/autotest-deps... File chromeos-base/autotest-deps-piglit/autotest-deps-piglit-9999.ebuild (right): http://codereview.chromium.org/6746009/diff/19003/chromeos-base/autotest-deps... chromeos-base/autotest-deps-piglit/autotest-deps-piglit-9999.ebuild:7: inherit cros-workon autotest-deponly conflict conflict is unused here, i suspect that's due to a copy/paste from the other -deps ebuilds? You should get rid of it. http://codereview.chromium.org/6746009/diff/19003/chromeos-base/autotest-deps... File chromeos-base/autotest-deps/autotest-deps-9999.ebuild (right): http://codereview.chromium.org/6746009/diff/19003/chromeos-base/autotest-deps... chromeos-base/autotest-deps/autotest-deps-9999.ebuild:39: chromeos-base/autotest-deps-piglit I don't think this is needed or relevant. -libaio is here, because one of the deps still provided by this ebuild does actually need one of the deps that was split. As in, it doesn't compile without it. That is not the case of piglit though, right? http://codereview.chromium.org/6746009/diff/19003/chromeos-base/autotest-fact... File chromeos-base/autotest-factory/autotest-factory-9999.ebuild (right): http://codereview.chromium.org/6746009/diff/19003/chromeos-base/autotest-fact... chromeos-base/autotest-factory/autotest-factory-9999.ebuild:26: chromeos-base/autotest-deps-piglit I think this dependency is unnecessary as well. Unless you're going to be adding the piglit test into the factory image. In that case, it should have nsanders' approval.
http://codereview.chromium.org/6746009/diff/19003/chromeos-base/autotest-deps... File chromeos-base/autotest-deps-piglit/autotest-deps-piglit-9999.ebuild (right): http://codereview.chromium.org/6746009/diff/19003/chromeos-base/autotest-deps... chromeos-base/autotest-deps-piglit/autotest-deps-piglit-9999.ebuild:7: inherit cros-workon autotest-deponly conflict On 2011/04/11 23:37:20, zbehan wrote: > conflict is unused here, i suspect that's due to a copy/paste from the other > -deps ebuilds? You should get rid of it. Done. http://codereview.chromium.org/6746009/diff/19003/chromeos-base/autotest-deps... File chromeos-base/autotest-deps/autotest-deps-9999.ebuild (right): http://codereview.chromium.org/6746009/diff/19003/chromeos-base/autotest-deps... chromeos-base/autotest-deps/autotest-deps-9999.ebuild:39: chromeos-base/autotest-deps-piglit On 2011/04/11 23:37:20, zbehan wrote: > I don't think this is needed or relevant. -libaio is here, because one of the > deps still provided by this ebuild does actually need one of the deps that was > split. As in, it doesn't compile without it. That is not the case of piglit > though, right? Done.
Looking much better now, thanks! LGTM On 2011/04/11 23:51:44, ihf wrote: > http://codereview.chromium.org/6746009/diff/19003/chromeos-base/autotest-deps... > File chromeos-base/autotest-deps-piglit/autotest-deps-piglit-9999.ebuild > (right): > > http://codereview.chromium.org/6746009/diff/19003/chromeos-base/autotest-deps... > chromeos-base/autotest-deps-piglit/autotest-deps-piglit-9999.ebuild:7: inherit > cros-workon autotest-deponly conflict > On 2011/04/11 23:37:20, zbehan wrote: > > conflict is unused here, i suspect that's due to a copy/paste from the other > > -deps ebuilds? You should get rid of it. > > Done. > > http://codereview.chromium.org/6746009/diff/19003/chromeos-base/autotest-deps... > File chromeos-base/autotest-deps/autotest-deps-9999.ebuild (right): > > http://codereview.chromium.org/6746009/diff/19003/chromeos-base/autotest-deps... > chromeos-base/autotest-deps/autotest-deps-9999.ebuild:39: > chromeos-base/autotest-deps-piglit > On 2011/04/11 23:37:20, zbehan wrote: > > I don't think this is needed or relevant. -libaio is here, because one of the > > deps still provided by this ebuild does actually need one of the deps that was > > split. As in, it doesn't compile without it. That is not the case of piglit > > though, right? > > Done. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
