<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Nov 18, 2015 at 10:34 AM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.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"><br><br><div class="gmail_quote"><span class=""><div dir="ltr">On Wed, Nov 18, 2015 at 10:00 AM Todd Fiala <<a href="mailto:todd.fiala@gmail.com" target="_blank">todd.fiala@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Checking in the static bindings is no different than projects checking in autoconf config baked scripts so that the vast majority of people don't need to run autoconf just to get a setup that rarely changes.  There is precedent for this going back a long way on open source projects.<div><br></div><div>I'm backing off having anyone else use them if they don't want, and we (Apple) will keep those up to date.  Nobody else will use them.  Totally fine.</div><div><br></div><div>On the swig-as-a-service front, I think the idea is interesting but there are several practical holes with that:</div><div><br></div><div>* What does one do when the server is unavailable?  Needs to be a local-only backup.  So whatever service that provides isn't a guaranteed working solution (think on an airplane, network outage, other server outage, etc.)</div></div></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><br></div><div>* Security: building code that has other code injected in it by another server. Safe? Attack vector?  I could argue so is a git/svn repo susceptible to this, so maybe this isn't a huge deal, but it's big enough and smells enough like "introduce random unvetted code that can't be reviewed as easily as a VCS tag" that I doubt we would ever use this in practice.</div><div><br></div><div>* Security 2: what is the service really running?  Not obvious on the build machine accessing the service.</div></div></blockquote></span><div>The end result of the service is a copy of LLDBWrapPython.cpp and lldb.py that you check into the repo. </div></div></div></blockquote><div><br></div><div>Ah, I see.  That definitely sounds like an improvement from my interpretation :-)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> You still have a chance to diff this source code against the repository's copy before committing, same as you would if you had swig locally. </div></div></div></blockquote><div><br></div><div>Yeah, non-issue at that point.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> Vast majority of changes to SB interfaces are going to be the addition of a couple methods, or maybe a class, and the diff should be very easy to look at and understand.</div></div></div></blockquote><div><br></div><div>Yes.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><br></div><div>> In all of these cases (except the proposed), the matrix choices are justifiable because they are there to support a hard requirement of someone's environment, and I do not think we should grow for anything that is not also a hard requirement of someone's environment.  </div><div><br></div></div><div dir="ltr"><div>I'm going to call that overreaching.  We are not in the business of dictating that one of the developers of the code "should not do something unless there is a hard requirement." </div></div></blockquote></span><div>I'm not saying you shouldn't do *anything* if there's not a hard requirement.  I agree that's overreaching.  I'm saying we should not increase the number of ways of doing *the same thing* unless there is a hard requirement.  Especially if one of the ways of doing the same thing exists solely to save someone from running one command to install the package (sorry if I'm not doing justice to how difficult it is on OSX, the last time I did something on OSX it seemed fairly easy to install macports, and I thought it's a wildly common thing for people to already have installed).  For example, wouldn't people need to already have macports in order to install CMake -- a necessary component of building LLVM?</div></div></div></blockquote><div><br></div><div>In the case of cmake, no. The cmake website provides a simple-to-add binary that doesn't depend on anything else.  (Okay, I'm long winded here so Enrico just beat me to the punch).</div><div><br></div><div>For swig, the solution is homebrew/macports.  The challenge with systems like homebrew and macports is that they will install other packages that are dependencies (sometimes build-time, sometimes runtime), typically get them added to various paths, and (during the process), unintentionally hide system libraries.  It is not uncommon for us to see crash reports on OS X where Xcode or lldb is picking up the wrong python or other component that was built by one of these package managers.  They're often good about saying "this might cause trouble" in some obvious cases, but less so in less obvious cases (like, a swig installing python bindings, which installs a new shiny python, which hides the system one, which breaks the lldb bindings).  It really is messier there.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div> Apple wants to eliminate the need for people to *require* swig.  The goal there is reducing the build requirements for the average person building lldb, not growing it.</div></div></blockquote></span><div>I agree, which is why it is so important to keep the number of different ways of building to a minimum.</div></div></div></blockquote><div><br></div><div>Glad to hear the intent here.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>  It's the same reason that the autoconf build is being removed wholesale from LLVM and people have decided that CMake is the one true way.</div></div></div></blockquote><div><br></div><div>(Coincidentally - I am in the process of trying to eliminate the old make-swig-bindings.sh, which is still used by the Makefile/autoconf build of lldb, and I saw the nice banner from llvm saying autoconf support is going out for 3.9 while testing the replacement).</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>  Because even if it isn't perfect for everyone, it works for everyone. </div></div></div></blockquote><div><br></div><div>Unless you can't get swig on your system in a way that doesn't break other items.  And then it doesn't work (as things stand now).</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> And there is inherent simplicity in having fewer ways to do things, as well as reducing maintenance cost.</div><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><br></div></div><div dir="ltr"><div>So for the more common, casual lldb build environment where the developer is not touching SB API, help me understand how reducing the need for swig (without introducing the need for hitting another server) is increasing the requirement load?  (Especially if we --- our local dev group sitting by me --- maintains those static bindings)?</div></div></blockquote><div><br></div></span><div>Well, we would need to disable static bindings on the OSX buildbots for starters, otherwise when someone not using static bindings makes a change, the buildbots break, and we cannot leave buildbots in a broken state.  So I assume that will still be possible.  So now you don't have a buildbot testing the static binding configuration.</div></div></div></blockquote><div><br></div><div>I was actually going to add a verifier stage to the public OS X buildbot.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>  </div><div><br></div><div>Secondly, LLDB already has a problem (IMHO) of having too many things that only work for a few people, instead of having things that work for everyone.  It's gotten better, and even your work right now to port the Xcode build over to these new python scripts is helping to make that better.  So regardless of the outcome here, the end result will still be an improvement over before when Xcode build was using the shells cripts and CMake build was using python scripts.</div><div><br></div><div>But I still think it's important to take a hardline against introducing new build configurations.  Static vs on-the-fly bindings are going to need different logic for getting the resulting code into the place where it can be compiled / imported, different logic for creating the symlinks, etc. </div></div></div></blockquote><div><br></div><div>Yep - it was in the vicinity of 25 lines of python.  An extra option to say "okay, only try this if there is no swig found", a few lines around "if we can't find a swig, and we say it's okay to copy the static bindings, then do it, otherwise fail", and a method to copy over the LLDBWrapPython.cpp and lldb.py files.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> And now we're duplicating effort when one person changes the logic on their side but not the other side.</div><div><br></div></div></div></blockquote><div><br></div><div>I'll offer again that we'll take care of the static bindings.  It's on us if they're broken.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div></div><div>I guess if this entire thing is hidden away in the Xcode build and everyone using the Xcode build wants things to work this way then I can't really argue with that, but I don't think we should provide the option to use static bindings in the CMake build.  (If anyone who uses the CMake build disagrees though, please speak up)</div></div></div>
</blockquote></div><div class="gmail_extra"><br></div>My intent here is to only provide the --allow-static-binding option on cmake if it is explicitly turned on.  And it will be off by default.</div><div class="gmail_extra"><br clear="all"><div><br></div><div>Tell me more about the service idea.  How would you envision it working?</div><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr">-Todd</div></div>
</div></div>