[Lldb-commits] [PATCH] D69341: [zorg] Port LLDB cmake build factory to git

[Pavel Labath via Phabricator via lldb-commits]

labath marked 2 inline comments as done.
labath added inline comments.


================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:71
+            is_legacy_mode=False,
+            depends_on_projects=["llvm", "clang", "lldb", "lld"])
 
----------------
sivachandra wrote:
> sivachandra wrote:
> > I wonder why it is getting git.lab.llvm.org! Can you try with an additional arg to the LLVMBuildFactory constructor:
> > 
> > ```
> >   repourl_prefix="http://github.com/llvm/"
> > ```
> May be https instead of http.
Are you sure that's needed? I don't see anyone else setting that. @jankratochvil, could this be something specific to your setup?


================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:110
     cmake_cmd = [
-        "cmake", "-G", "Ninja", "../llvm",
+        "cmake", "-G", "Ninja", "../" + self.monorepo_dir,
         "-DCMAKE_BUILD_TYPE=" + config,
----------------
jankratochvil wrote:
> sivachandra wrote:
> > Should this be os.path.join(os.pardir, f.monorepo_dir, "llvm") ?
> I was curious there is no longer trailing `"llvm"` as I am using `cmake ../llvm-monorepo/llvm/`.  Yes, it does compile with the new expression there:
> ```
>         "cmake", "-G", "Ninja", os.path.join(os.pardir, f.monorepo_dir, "llvm"),
> ```
Technically, using `os.path.join`, is not correct here, because this is a path on the buildbot, not the path on the host which runs the master. In practice that doesn't matter because the master runs on a posix system. The existing code is pretty inconsistent about the usage, but the `../` seems to be a bit more common.


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

https://reviews.llvm.org/D69341