Hi Justin, Thanks for voting (and for your +1 despite there being some questions). Responses inline:
On Mon, Mar 7, 2016 at 7:01 PM, Justin Mclean <justinmcl...@me.com> wrote: > Hi, > > +1 binding > > I checked: > - signatures and hashes fine > - name contains incubating > - DISCLAIMER exists > - LICENSE and NOTICE good > - No unexpected binaries > - Source file have apache headers (some have extra ones) > - Can compile on OS X (but it takes a while) > > I still think there’s a couple of minor issues with license. > - For instance the added text "The following dependencies or pieces of > incorporated source code have licenses such that either:...” is IMO > incorrect. For instance BSD requires license to be both in source and > binary distributions. > Right, that's true that the BSD license does require that (and that's why most of the other licenses above in the file are for BSD source). The specific cases of BSD-licensed software here is FindProtobuf.cmake, which is a build-time-only dependency and does not become part of the binary distribution. I should add a note to clarify that. > - It’s unclear why the 1/2 dozen non Apache license software listed under > this text are treated in a different way to the other bundled > software.Wouldn’t it be better to be consistent and handle all licenses > the same way? > Yes, that's what I thought, but in your previous vote, I understood that you and others preferred the LICENSE.txt file to be "minimal" and include "pointers" in cases where the license didn't _require_ the full text to be reproduced. I'm happy to revert the change we made for this release and go back to what we did in the previous one, but I thought this was what was requested. > - This file [1] isn’t BSD as noted in the license but a modified zlib, > notice the clauses about modifications > > The license file says "half BSD, half zlib". The file self-describes as a "BSD-style license": https://github.com/svn2github/valgrind/blob/master/include/valgrind.h#L4 But as you noted, it has some zlib-like clauses. So I think "half BSD half zlib" is relatively accurate, no? There also looks to be some minor issues with Apache headers in several > files > > Several files have double Apache headers. For instance in src/kudu/util: > bit-stream-utils.h, bit-stream-utils.inline.h, bit-util-test.cc, > bit-util.h, logging.cclogging.h, rle-encoding.h, rle-test.cc, > url-coding-test.cc, url-coding.h > Thanks. I think these were files that we borrowed from Apache Impala (incubating) at some point when it already had the license header, and our scripting which prepended the header got them wrong. Will fix. > It also may be that Apache headers have been added to files that shouldn’t > have them? For Instance [2] is stated as BSD in the license file but has > an Apache header. Was the original header removed? Also [3] has an Apache > header but notes it's BSD licensed. These are not the only examples. > Aha, I see what happened with random-util.{cc,h}. In fact these files were authored as part of Kudu, and _not_ BSD licensed. The mention of them in LICENSE.txt is a hold-over from an earlier revision which contained some a single function from WebRTC. That code was later refactored into random.h as Random::Normal(...). We should just update the LICENSE.txt file to say that random.h contains the BSD-licensed code and not random_util.cc. I'll try to take a swing through for any other double-licensed stuff I can find. -Todd
