LGTM.<br><br><div class="gmail_quote">On Sat, Jul 13, 2013 at 4:30 PM, Virgile Bello <span dir="ltr"><<a href="mailto:virgile.bello@gmail.com" target="_blank">virgile.bello@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">Sorry, quick patch update to fix DataBufferMemoryMap::MemoryMapFromFileDescriptor.</div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><br><div class="gmail_quote">On Sat, Jul 13, 2013 at 12:24 PM, Virgile Bello <span dir="ltr"><<a href="mailto:virgile.bello@gmail.com" target="_blank">virgile.bello@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Ok, attached the updated patch.<div>It improves quite a few things:</div><div>- readded some libraries such as PluginObjectContainerBSDArchive and SymbolVendorELF</div>
<div>- added missing functions in Windows.cpp</div>
<div>- full support for cmake under MingW.</div><div><br></div><div>Note that for automake build, you need to add std=c++11 and -DLLDB_DISABLE_PYTHON (automatic for cmake build).</div></div><div><div>
<div class="gmail_extra"><br><br>
<div class="gmail_quote">On Thu, Jul 11, 2013 at 10:02 AM, Virgile Bello <span dir="ltr"><<a href="mailto:virgile.bello@gmail.com" target="_blank">virgile.bello@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div>On Thu, Jul 11, 2013 at 4:18 AM, Joćo Matos <span dir="ltr"><<a href="mailto:ripzonetriton@gmail.com" target="_blank">ripzonetriton@gmail.com</a>></span> wrote:<br></div><div class="gmail_extra">
<div class="gmail_quote"><div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div>On Sun, Jul 7, 2013 at 10:44 AM, Virgile Bello <span dir="ltr"><<a href="mailto:virgile.bello@gmail.com" target="_blank">virgile.bello@gmail.com</a>></span> wrote:<br>
</div></div><div class="gmail_quote"><div><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr">Here is a patch allowing compilation of LLDB with MinGW.<br><div>(it still compiles on Linux after the patch)</div><div>Can someone please review (and merge in trunk if everything seems good)?</div><div><br>
</div><div>Soon some other patches should follow:</div><div>- 1 for MSVC compilation</div><div>- 1 to add a lldbProcessWindows using Win32 debugging API, so that gdb/clang compiled programs can be debugged in Windows.</div>
<div><br></div><div>Hopefully I can get all this into trunk quickly so that I don't need to rebase/merge too much.</div><div>Thanks!</div></div></blockquote><div><br></div></div></div><div>Just looked at the patch and it mostly LGTM. I think long-term it would be nice to have the POSIX-specific stuff a little better abstracted, instead of trying to hack around it in the Windows port, though for now it seems the way forward and it can be fixed incrementally in the future.</div>
</div></blockquote></div><div>I agree. I tried to keep the changes as small as possible (my priority was that it would be easy to integrate into trunk) but long-term it might require some more abstraction.</div><div>
<div>
</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="gmail_quote">
<div><br></div><div>Some questions:</div><div><br></div><div>1. Why do we need to define these in "lldb-socket.h"?</div><div><br></div><div><div>+#define NTDDI_VERSION NTDDI_VISTA</div><div>+#define _WIN32_WINNT _WIN32_WINNT_VISTA</div>
</div><div><br></div><div>Can't you just include "lldb-windows.h" which already includes Windows.h?</div></div></blockquote></div><div>You're totally right.</div><div>Actually I thought that was in the MinGW patch but I did right after in the MSVC branch.</div>
<div>I will backport it and improve this patch a little bit then, in order to avoid further changes later.</div><div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class="gmail_quote"><div><br></div><div>2. Why are you excluding lldbPluginObjectContainerBSDArchive and lldbPluginSymbolVendorELF from the MinGW build?</div></div></blockquote></div><div>No ar.h for lldbPluginObjectContainerBSDArchive (same as before, I patched it in MSVC branch so I should probably backport that as well).</div>
<div>For lldbPluginSymbolVendorELF I will double-check.</div><div><br></div><div>I will soon get back to you with an updated patch covering 1 and 2.</div><div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class="gmail_quote">
<div><br></div><div>3. Why add support for two build systems (CMake and automake)?</div><div><br></div><div>IIRC LLVM and Clang are looking to kill all non-CMake build scripts to ease maintenance and reduce duplication, so IMHO LLDB being an umbrella project should do the same.</div>
</div></blockquote></div><div>I see, so I don't need to support Automake makefile anymore? Good to know. Actually I was still using it for the MingW build (and CMake for MSVC build), but never tested CMake + MingW. I have to give it a try and check everything works fine, esp. if it is the future-proof way.</div>
<div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="gmail_quote">
<div><br></div><div>Apart from that, looking forward to see this getting in and for further Windows debugging patches.</div><span><font color="#888888"><div><br></div></font></span></div><span><font color="#888888">-- <br>
Joćo Matos
</font></span></blockquote></div></div><br></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br>Joćo Matos