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

[Siva Chandra via Phabricator via lldb-commits]

sivachandra added inline comments.


================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:71
+            is_legacy_mode=False,
+            depends_on_projects=["llvm", "clang", "lldb", "lld"])
 
----------------
labath wrote:
> 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?
If I am reading the code right, it should not be needed. I suggested that to see if @jankratochvil can make progress, which validates the rest of this patch.


================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:110
     cmake_cmd = [
-        "cmake", "-G", "Ninja", "../llvm",
+        "cmake", "-G", "Ninja", "../" + self.monorepo_dir,
         "-DCMAKE_BUILD_TYPE=" + config,
----------------
labath wrote:
> 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.
Right. The main point I was trying to convey was to add the path to the llvm directory.

I am only interested in seeing what you eventually land as I want to use the same for the LLVM libc builders.


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

https://reviews.llvm.org/D69341