This is an archive of the discontinued LLVM Phabricator instance.

[zorg] Fix LLDBCmakeBuildFactory
ClosedPublic

Authored by labath on Oct 29 2019, 3:11 AM.

Details

Summary

The attempt in D69341 was not complete, as in the monorepo world, we
also need to pass LLVM_ENABLE_PROJECTS in order to build lldb (and
clang and lld).

This also updates the code to use the CmakeCommand class instead of the
generic ShellCommand.

Diff Detail

Event Timeline

labath created this revision.Oct 29 2019, 3:11 AM
jankratochvil requested changes to this revision.Oct 29 2019, 3:39 AM
2019-10-29 11:38:04+0100 [-] error while parsing config file
2019-10-29 11:38:04+0100 [-] Unhandled Error
	Traceback (most recent call last):
	  File "/home/buildbot/.local/lib/python2.7/site-packages/buildbot-latest-py2.7.egg/buildbot/master.py", line 197, in loadTheConfigFile
	    d = self.loadConfig(f)
	  File "/home/buildbot/.local/lib/python2.7/site-packages/buildbot-latest-py2.7.egg/buildbot/master.py", line 579, in loadConfig
	    d.addCallback(do_load)
	  File "/usr/lib64/python2.7/site-packages/twisted/internet/defer.py", line 322, in addCallback
	    callbackKeywords=kw)
	  File "/usr/lib64/python2.7/site-packages/twisted/internet/defer.py", line 311, in addCallbacks
	    self._runCallbacks()
	--- <exception caught here> ---
	  File "/usr/lib64/python2.7/site-packages/twisted/internet/defer.py", line 654, in _runCallbacks
	    current.result = callback(current.result, *args, **kw)
	  File "/home/buildbot/.local/lib/python2.7/site-packages/buildbot-latest-py2.7.egg/buildbot/master.py", line 226, in do_load
	    exec f in localDict
	  File "/quad/home/buildbot/zorg-git/buildbot/osuosl/master/master.cfg", line 83, in <module>
	    c['builders'] = builders = list(config.builders.get_builders())
	  File "/quad/home/buildbot/zorg-git/buildbot/osuosl/master/config/builders.py", line 1456, in get_builders
	    for b in _get_lldb_builders():
	  File "/quad/home/buildbot/zorg-git/buildbot/osuosl/master/config/builders.py", line 829, in _get_lldb_builders
	    '-DLLVM_LIT_ARGS="-v"'])},
	  File "/quad/home/buildbot/zorg-git/buildbot/osuosl/master/zorg/buildbot/builders/LLDBBuilder.py", line 129, in getLLDBCMakeBuildFactory
	    doStepIf=FileDoesNotExist(
	exceptions.NameError: global name 'FileDoesNotExist' is not defined
	
	  File "/quad/home/buildbot/zorg-git/buildbot/osuosl/master/master.cfg", line 83, in <module>
	    c['builders'] = builders = list(config.builders.get_builders())
	  File "/quad/home/buildbot/zorg-git/buildbot/osuosl/master/config/builders.py", line 1456, in get_builders
	    for b in _get_lldb_builders():
	  File "/quad/home/buildbot/zorg-git/buildbot/osuosl/master/config/builders.py", line 829, in _get_lldb_builders
	    '-DLLVM_LIT_ARGS="-v"'])},
	  File "/quad/home/buildbot/zorg-git/buildbot/osuosl/master/zorg/buildbot/builders/LLDBBuilder.py", line 129, in getLLDBCMakeBuildFactory
	    doStepIf=FileDoesNotExist(
	exceptions.NameError: global name 'FileDoesNotExist' is not defined
This revision now requires changes to proceed.Oct 29 2019, 3:39 AM
labath updated this revision to Diff 226872.Oct 29 2019, 4:38 AM

import FileDoesNotExist

jankratochvil accepted this revision.Oct 29 2019, 6:29 AM

It does run now, I did not test it with a slave.

This revision is now accepted and ready to land.Oct 29 2019, 6:29 AM
This revision was automatically updated to reflect the committed changes.

Sadly, I think this broke the windows bot now (assuming it got applied to staging, otherwise I have to figure out what did):

http://lab.llvm.org:8014/builders/lldb-x64-windows-ninja/builds/46/steps/cmake-configure/logs/stdio

The cmake command redefinition effectively is not supported any more, but the 'cmake' arg is left behind.
+ inline comments.

These have been addressed by https://reviews.llvm.org/rZORG425eeb1bf21b.

zorg/buildbot/builders/LLDBBuilder.py
110

This would build a path per notations on master. Which could be different than what a builder expects. Think of master running on Windows, for example.

Better use a "/" path separator as these days it seems supported by all platforms we care about.

130

It is better to always run the cmake configure step. It doesn't take long but handles some dependencies.

labath marked an inline comment as done.Oct 31 2019, 1:12 AM

Sadly, I think this broke the windows bot now (assuming it got applied to staging, otherwise I have to figure out what did):

http://lab.llvm.org:8014/builders/lldb-x64-windows-ninja/builds/46/steps/cmake-configure/logs/stdio

I am afraid that was due to 425eeb1b (or its combination with this patch, if you will). I am afraid we have too many quotes now :(

In an separate email, @gkistanova wrote:

In D69555 the problem of quotations around -DLLVM_ENABLE_PROJECTS remains.

"-DLLVM_ENABLE_PROJECTS=" + ";".join(f.depends_on_projects),

Actually, that's not true. :( The way that this patch resolved the quoting problem is by passing the command as an array. You'll notice that originally the command was being passed as a string, while now it's an array of strings. One does not need to add extra quotes when passing commands as arrays, as the array already provides the information about argument boundaries. Adding the extra quotes is not necessary and it seems to break stuff in some situations. The linux bot does not seem to be affected by this (which actually surprises me a lot), but the windows bot gets utterly confused by that.

I think that just removing the extra quotes should make everything happy.

zorg/buildbot/builders/LLDBBuilder.py
110

Thanks. I didn't know about the LLVMBuildFactory.pathRelativeToBuild stuff.

(which actually surprises me a lot)

Actually, nevermind. The linux bot fails now in the same way as the windows one (http://lab.llvm.org:8014/builders/lldb-x86_64-debian/builds/6262). Galina, will you remove the extra quotes, or should I?