2009/5/15 Nicolas Sylvain <nsylv...@chromium.org>: > > > 2009/5/15 Zhanyong Wan (λx.x x) <w...@google.com> >> >> 2009/5/15 Nicolas Sylvain <nsylv...@chromium.org>: >> > >> > >> > 2009/5/15 Zhanyong Wan (λx.x x) <w...@google.com> >> >> >> >> 2009/5/15 Nicolas Sylvain <nsylv...@chromium.org>: >> >> > >> >> > >> >> > 2009/5/15 Nicolas Sylvain <nsylv...@chromium.org> >> >> >> >> >> >> >> >> >> On Fri, May 15, 2009 at 12:15 PM, Albert J. Wong (王重傑) >> >> >> <ajw...@chromium.org> wrote: >> >> >>> >> >> >>> [ +zhanyong ] >> >> >>> I chatted with Zhanyong about both swapping out tr1::tuple, and >> >> >>> about >> >> >>> removing the svn:external reference in the repository. >> >> >>> The gmock actions use tr1:tuple as part of the public interface for >> >> >>> defining new actions. Thus, changing the tuple implementation >> >> >>> would >> >> >>> basically mean that any other googlemock client using tr1::tuple in >> >> >>> their >> >> >>> code would have clashing tuple implementations. This will only get >> >> >>> worse as >> >> >>> tr1 is adopted more widely. >> >> >>> As for getting our own version of gtest instead of retrieving it >> >> >>> through >> >> >>> svn:external, the implementation of gmock and gtest are pretty >> >> >>> tightly >> >> >>> coupled. Thus, versioning gtest separately might be a bad idea. >> >> >>> The >> >> >>> suggestion is to consider gmock a super-set of gtest and drop the >> >> >>> original >> >> >>> dependency. >> >> >>> Zhanyong, please correct me if I got something wrong here. >> >> >>> >> >> >>> >> >> >>> Regarding the svn:external reference to google test, currently, the >> >> >>> best >> >> >>> solution seems to be either >> >> >>> a) having 2 copies of gtest in the tree (a bit dangerous due to >> >> >>> possible shadowing issues) >> >> >>> b) passing --ignore-externals to svn in gclient (an incomplete >> >> >>> solution >> >> >>> because not every svn access goes through gclient) >> >> >>> How bad is it if we don't explicitly version gtest ourselves? >> >> >> >> >> >> Very. googlemock is at version 238 right now. But we are using >> >> >> version >> >> >> 243, which is , afaik, the first revision that is stable and has >> >> >> support for >> >> >> the new sharding feature. If we start using the one in googlemock, >> >> >> the >> >> >> buildbots are going to break. >> >> > >> >> > To be clear: "googlemock is at version 238" should have been: "trunk >> >> > of >> >> > googlemock is fetching gtest at version 238". >> >> >> >> I can make googlemock pull in gtest r243 or a later revision. >> >> >> >> > Nicolas >> >> > >> >> >> >> >> >> We need to be able to control which version we pull, and we need to >> >> >> be >> >> >> able to change it fast if we realize there is a bug. (rollback to a >> >> >> previous >> >> >> version, or go to a newer version with a fix). >> >> >> >> I understand. The issue is that gtest and gmock are meant to version >> >> in lock steps (almost), as the latest revision of gmock often needs >> >> features from a newer revision of gtest. In general, you cannot >> >> freely combine two revisions of gtest and gmock. >> >> >> >> I view gtest as part of gmock. We release gtest separately for people >> >> not interested in mocking or not using a compiler gmock requires. >> >> However if you want gmock, it's recommended to let gmock decide which >> >> version of gtest is compatible. >> >> >> >> In the end, I guess it depends on how fine-grained you want to control >> >> your dependency on gtest/gmock. Dealing with them separately gives >> >> you more flexibility (at least in theory), but also incurs more >> >> maintenance overhead. You need to decide whether it's worth it. My >> >> hunch, is that it's not. >> > >> > >> > That's not changing the fact that we don't want code to be pulled >> > auto-magically. svn:external >> > is clearly one of the biggest scm user-unfriendly feature that I know >> > of.... and it's why we have gclient. >> >> Sorry I was late to the discussion - could you explain what you mean >> by code being pulled auto-magically? svn:externals lets you specify >> the revision number you want to pull in. If you set that, you'll >> always get the same code whenever you sync, until the next time you >> update the svn:externals property. > > I mean: > http://googlemock.googlecode.com/svn/trunk/ : I'm browsing the code, and I > have no indication that something is going to be pulled, and at what > version. > same here : http://code.google.com/p/googlemock/source/browse/#svn/trunk > I'm familiar with svn, and it still took me a while to be able to query the > right property and be able to know what version it was pulling. > I don't think our code review tool supports review for svn properties > either. > Feel free to get a second opinion though. But personally I think it would be > a big set back if chrome was to depend on svn:externals to be able to build. > Nicolas
FYI, as a data point. For historical reasons (compatibility with our perforce layout), o3d pulls both gmock an gtest with DEPS. So it means that technically we pull 2 versions of gtest, but we use only one. We make sure that gmock uses the same version of gtest as we do, by adding the correct include path (and making babies cry, yeah, I know). Ideally we'd love to have only one copy of gtest, and a single way to do deps, but in the mean time, it can work. Antoine > > >> >> > I agree that this is a difficult choice, there are no obvious good >> > solutions. >> > I don't think we like to pull in third party dependencies that are not >> > self >> > contained. >> > This is far from being a great solution, but I would personally just >> > paste a >> > copy of gmock in our repo (without fetching it with deps), like we do >> > for >> > the majority of our third_party. And make this version of gmock self >> > contained with the version of gtest it needs, while the rest of chrome >> > continues to use our version of gtest. Not making it clash might be a >> > challenge though. >> >> Unless you can guarantee that code using gmock will never be linked >> with code in "the rest of chrome" (which uses a separate version of >> gtest), this won't work. I do not recommend to have two copies of >> gtest in your source tree. If you have to, you should make sure one >> copy is "dead", i.e. no one actually uses it. >> >> > Nicolas >> > >> >> >> >> >> >> >> Nicolas >> >> >>> >> >> >>> -Albert >> >> >>> >> >> >>> >> >> >>> 2009/5/15 Scott Hess <sh...@chromium.org> >> >> >>>> >> >> >>>> If this is the only reason gmock needs boost, it seems like a >> >> >>>> better >> >> >>>> idea would be to push a copy of tuple.h into gmock and submit a >> >> >>>> patch >> >> >>>> to make it more self-contained in the first place. >> >> >>>> >> >> >>>> -scott >> >> >>>> >> >> >>>> >> >> >>>> On Fri, May 15, 2009 at 11:17 AM, Albert J. Wong (王重傑) >> >> >>>> <ajw...@chromium.org> wrote: >> >> >>>> > One other idea to explore...what about "reimplementing" >> >> >>>> > tr1::tuple >> >> >>>> > using >> >> >>>> > base::Tuple? It'd be a pretty naughty hack (adding something to >> >> >>>> > the >> >> >>>> > tr1:: >> >> >>>> > namespace), but for the limited use-case of gmock, it could be >> >> >>>> > good >> >> >>>> > enough? >> >> >>>> > -Albert >> >> >>>> > On Fri, May 15, 2009 at 11:11 AM, Marc-Antoine Ruel >> >> >>>> > <mar...@chromium.org> >> >> >>>> > wrote: >> >> >>>> >> >> >> >>>> >> [-chromium-reviews, +chromium-dev] >> >> >>>> >> (take 2) >> >> >>>> >> From their website, <<To use Google Mock, you will need the TR1 >> >> >>>> >> tuple >> >> >>>> >> C++ >> >> >>>> >> library installed.>> and not directly boost. Up to now, >> >> >>>> >> chromium >> >> >>>> >> source tree >> >> >>>> >> assumed "defined(_MSC_VER) == No TR1", which is not exactly >> >> >>>> >> true. >> >> >>>> >> This is >> >> >>>> >> particularly not true on VS2008 + SP1 + Feature Pack. >> >> >>>> >> Since it's included in VS2008 as an addon and there's only >> >> >>>> >> VS2005 >> >> >>>> >> that >> >> >>>> >> truly lacks it, it could be a compelling reason to drop support >> >> >>>> >> for >> >> >>>> >> VS2005. >> >> >>>> >> We'd be at odds with WebKit but 'eh' is all I have to say. :) >> >> >>>> >> It'd be a bit awkward with a potentially eminent move to VS2010 >> >> >>>> >> within a >> >> >>>> >> year or so. >> >> >>>> >> So to summarize my mind; >> >> >>>> >> If TR1 is available natively on MSVC, I want its stl tr1 >> >> >>>> >> library >> >> >>>> >> to >> >> >>>> >> be >> >> >>>> >> used with conditional include magic. I'm fine to include boost >> >> >>>> >> only >> >> >>>> >> as a >> >> >>>> >> supplicant to continue supporting MSVC8 and MSVC9 without FP. >> >> >>>> >> Is that fine? >> >> >>>> >> M-A >> >> >>>> >> 2009/5/15 John Grabowski <j...@chromium.org> >> >> >>>> >>> >> >> >>>> >>> I did a quick test. The minimal set of files needed to get >> >> >>>> >>> only >> >> >>>> >>> boost's >> >> >>>> >>> tuple is 390 (down from ~1200 in the zip), and size drops from >> >> >>>> >>> 9M >> >> >>>> >>> to >> >> >>>> >>> 1.3M. >> >> >>>> >>> Windows may differ a tad that OSX (e.g. uses >> >> >>>> >>> platform/win32.hpp >> >> >>>> >>> instead of >> >> >>>> >>> platform/macos.hpp) but it'll be in the same ballpark. >> >> >>>> >>> Is an extra 1.3M in the source tree acceptable for the benefit >> >> >>>> >>> of >> >> >>>> >>> getting >> >> >>>> >>> gmock? I think yes. maruel brettw? >> >> >>>> >>> jrg >> >> >>>> >>> >> >> >>>> >>> >> >> >>>> >>> On Fri, May 15, 2009 at 9:48 AM, Albert J. Wong (王重傑) >> >> >>>> >>> <ajw...@chromium.org> wrote: >> >> >>>> >>>> >> >> >>>> >>>> >> >> >>>> >>>> On Fri, May 15, 2009 at 9:14 AM, Steven Knight >> >> >>>> >>>> <s...@google.com> >> >> >>>> >>>> wrote: >> >> >>>> >>>>>> >> >> >>>> >>>>>> Guys, it would be a major win to get gmock landed. I'd >> >> >>>> >>>>>> like >> >> >>>> >>>>>> to >> >> >>>> >>>>>> keep >> >> >>>> >>>>>> trying here, even if not trivially small. >> >> >>>> >>>>>> Re: boost size. If necessary we could probably checkin >> >> >>>> >>>>>> only >> >> >>>> >>>>>> the >> >> >>>> >>>>>> few >> >> >>>> >>>>>> files actually needed (e.g. tuple.hpp, boost/config.hpp, >> >> >>>> >>>>>> boost/static_assert.hpp, and perhaps 10 more). maruel, is >> >> >>>> >>>>>> that >> >> >>>> >>>>>> something >> >> >>>> >>>>>> you'd be happier with? >> >> >>>> >>>>> >> >> >>>> >>>>> That seems much more acceptable to me. Especially if it >> >> >>>> >>>>> doing >> >> >>>> >>>>> it >> >> >>>> >>>>> also >> >> >>>> >>>>> sidesteps the svn:external issue. >> >> >>>> >>>> >> >> >>>> >>>> Unfortunately, it does not sidestep svn:external. What about >> >> >>>> >>>> just >> >> >>>> >>>> adding --ignore-externals to all our svn commands in gclient? >> >> >>>> >>>> I >> >> >>>> >>>> don't think >> >> >>>> >>>> anyone else uses externals, and give people's reactions, I >> >> >>>> >>>> don't >> >> >>>> >>>> think they >> >> >>>> >>>> should be. >> >> >>>> >>>> >> >> >>>> >>>> As for reducing boost to something sane, this is supposedly >> >> >>>> >>>> the >> >> >>>> >>>> reduced >> >> >>>> >>>> subset... >> >> >>>> >>>> >> >> >>>> >>>> -Albert >> >> >>>> >>>> >> >> >>>> >>>>> >> >> >>>> >>>>> (Seriously, svn:external really only works for such a narrow >> >> >>>> >>>>> use >> >> >>>> >>>>> case, >> >> >>>> >>>>> and introduces so many other problems down the road when >> >> >>>> >>>>> things >> >> >>>> >>>>> need to >> >> >>>> >>>>> change (branching+merging, local mods, etc.) that I'd really >> >> >>>> >>>>> try >> >> >>>> >>>>> to wave off >> >> >>>> >>>>> upstream gmock from using it.) >> >> >>>> >>>>> --SK >> >> >>>> >>>>> >> >> >>>> >>>>>> >> >> >>>> >>>>>> On Fri, May 15, 2009 at 7:40 AM, <nsylv...@chromium.org> >> >> >>>> >>>>>> wrote: >> >> >>>> >>>>>>> >> >> >>>> >>>>>>> LGTM with my comment and sgk's comments. >> >> >>>> >>>>>>> >> >> >>>> >>>>>>> As for maruel's comment : It made me sad too. gmock seems >> >> >>>> >>>>>>> to >> >> >>>> >>>>>>> be >> >> >>>> >>>>>>> a lot >> >> >>>> >>>>>>> of >> >> >>>> >>>>>>> troubles (svn:external, then ugly dependencies). Have we >> >> >>>> >>>>>>> at >> >> >>>> >>>>>>> least >> >> >>>> >>>>>>> considered using something else? Or not using it at all? >> >> >>>> >>>>>>> >> >> >>>> >>>>>>> >> >> >>>> >>>>>>> http://codereview.chromium.org/115398/diff/1/2 >> >> >>>> >>>>>>> File third_party/boost/README.chromium (right): >> >> >>>> >>>>>>> >> >> >>>> >>>>>>> http://codereview.chromium.org/115398/diff/1/2#newcode3 >> >> >>>> >>>>>>> Line 3: >> >> >>>> >>>>>>> >> >> >>>> >>>>>>> >> >> >>>> >>>>>>> >> >> >>>> >>>>>>> http://googlemock.googlecode.com/files/boost_tr1_tuple_1_36_0.zip >> >> >>>> >>>>>>> Can you add a line that says what the license is. >> >> >>>> >>>>>>> >> >> >>>> >>>>>>> http://codereview.chromium.org/115398 >> >> >> >> -- >> >> Zhanyong >> > >> > >> >> >> >> -- >> Zhanyong > > > > > -- A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? --~--~---------~--~----~------------~-------~--~----~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
