labath marked an inline comment as done.
labath added a comment.

In D69555#1727882 <https://reviews.llvm.org/D69555#1727882>, @stella.stamenova 
wrote:

> 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 
<https://reviews.llvm.org/rZORG425eeb1bf21b7f507905f17970f667263d630a81> (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 <https://reviews.llvm.org/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.



================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:110
 
-    cmake_cmd = [
-        "cmake", "-G", "Ninja", os.path.join(os.pardir, f.monorepo_dir, 
"llvm"),
+    path = os.path.join(os.pardir, f.monorepo_dir, "llvm")
+    cmake_options = [
----------------
gkistanova wrote:
> 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.
Thanks. I didn't know about the `LLVMBuildFactory.pathRelativeToBuild` stuff.


Repository:
  rZORG LLVM Github Zorg

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69555/new/

https://reviews.llvm.org/D69555



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to