This is an archive of the discontinued LLVM Phabricator instance.

[buildbot] Added CUDATestsuiteBuilder.py
ClosedPublic

Authored by tra on Dec 14 2016, 10:51 AM.

Details

Summary

getCUDATestsuiteBuildFactory() returns a factory that:

  • checks out clang, libcxx(+deps), and test-suite.
  • compiles & tests clang.
  • compiles and runs CUDA tests.

Added checkout_libcxx and checkout_test_suite knobs to
getClangCmakeBuildFactory() so it can be reused for other
clang-based buildbots.

Tested on a local buildmaster+buildslave.

Diff Detail

Repository
rL LLVM

Event Timeline

tra updated this revision to Diff 81421.Dec 14 2016, 10:51 AM
tra retitled this revision from to [buildbot] Added CUDATestsuiteBuilder.py.
tra updated this object.
tra added a reviewer: gkistanova.
tra added a subscriber: llvm-commits.
jlebar added inline comments.Dec 14 2016, 11:55 AM
zorg/buildbot/builders/CUDATestsuiteBuilder.py
86 ↗(On Diff #81421)

Nit, this would be clearer to me if it was indented like cuda_test_env above.

135 ↗(On Diff #81421)

Nit, no parens in if.

tra updated this revision to Diff 81434.Dec 14 2016, 12:03 PM

fixed formatting nits.

tra marked 2 inline comments as done.Dec 14 2016, 12:04 PM
gkistanova requested changes to this revision.Dec 17 2016, 11:09 PM
gkistanova edited edge metadata.

Thanks for working on this, Artem.

zorg/buildbot/builders/CUDATestsuiteBuilder.py
16 ↗(On Diff #81434)

Please do not use mutable default arguments.

52 ↗(On Diff #81434)

Maybe adding an extra indent here? This is cosmetic, though.

101 ↗(On Diff #81434)

Could you use CmakeCommand instead of ShellCommand here, please?

113 ↗(On Diff #81434)

Could you use NinjaCommand instead of WarningCountingShellCommand here and in all the other similar places, please?

This revision now requires changes to proceed.Dec 17 2016, 11:09 PM
tra updated this revision to Diff 82004.Dec 19 2016, 1:58 PM
tra edited edge metadata.
tra marked 4 inline comments as done.

Addressed Galina's comments.

It is almost there.

Few small things to address and the patch would be ready to land.

zorg/buildbot/builders/CUDATestsuiteBuilder.py
2 ↗(On Diff #82004)

For the version of the buildbot we use, it should be
from buildbot.steps.slave import RemoveDirectory

and it used just as RemoveDirectory below.

18 ↗(On Diff #82004)

Please do not use mutable default arguments.

42 ↗(On Diff #82004)

You do not need this any more. Instead feel free to pass jobs straight to the NinjaCommand as is, and it will take care of the correct propagation of the jobs from a factory argument, or from a buildslave, a builder, or even a build parameters.

84 ↗(On Diff #82004)

Just RemoveDirectory here. Please see my comment about the buildbot version above.

132 ↗(On Diff #82004)

You do not need this any more, do you?

156 ↗(On Diff #82004)

If it takes a long to build and there is no TTY output during that time, you may want to set a longer timeout for this step.

tra updated this revision to Diff 82245.Dec 21 2016, 10:16 AM
tra edited edge metadata.
tra marked 5 inline comments as done.

Addressed Galina's comments.

tra added inline comments.Dec 21 2016, 10:17 AM
zorg/buildbot/builders/CUDATestsuiteBuilder.py
2 ↗(On Diff #82004)

Buildbot seems to be moving RemoveDirectory all over the place in different versions.
I've changed the code to try loading it in a way that should work with both 0.8.5 and 0.8.12.

156 ↗(On Diff #82004)

Individual compilations should be well within default timeout of 20 minutes.

gkistanova accepted this revision.Dec 21 2016, 1:22 PM
gkistanova edited edge metadata.

LGTM.
Thanks for the changes, Artem.

This revision is now accepted and ready to land.Dec 21 2016, 1:22 PM
This revision was automatically updated to reflect the committed changes.