That all sounds great! At this point, I suggest filing one ZooKeeper JIRA issue to track the short-term stdint.h fix. Please also feel free to file any number of additional issues for the longer-term requests, such as the request to support static linking.
--Chris Nauroth On 2/5/16, 4:40 PM, "Alex Clemmer" <[email protected]> wrote: >Ah, ok, I've subscribed to dev@ to avoid this mistake in the future. >Thanks a lot for the heads up! > >Let me answer your questions directly, and then step back and address >the broader scope of maintaining robust support for modern platforms. > >We currently have no specific reason to believe that the Mesos code >base is suffering bugs from the `stdint.h` warnings, but it is >nonetheless worrying to us, because it means that the platform is >probably not well-trafficked for our target. That said, in the >immediate term, I'm willing to start with the fairly modest goal of >figuring out the "best way" to get the C client to integrate with >realistic codebases, which I personally think includes getting the C >client to compile in the presence of the totally-reasonable-to-include >`stdint.h` header. > >For next steps, I think the best thing to do is to open a ticket to >just get this to compile in the presence of `stdint.h`, and then >incrementally build out more robust, modern support from there. From >there I'm happy to do the leg work to fix these issues. > >Stepping back: given that we expect this code to run on tens of >thousands of more-modern Windows nodes (via Mesos), I think we >probably all agree that it would be a good outcome to have a high >level of operational predictability in the client. I am guessing we >are also in agreement that this probably involves cleaning up the >warnings. In the longer term I think this also means exposing more >levers to people consuming these libraries -- for example, right now >the `.vcxproj` files only build DLLs, but for a lot of reasons we want >to allow people to choose, if not actually actually make it >static-by-default (for example, DLLs have totally separate heaps in >win32, and this presents some interesting issues at scale) . > >I'm happy to collate my suggestions into a list in the future. Let me >know what would be useful for that conversation. I'm happy to make >this an extended discussion and co-evolve the codebase as we find >issues deploying it at scale. > >On Fri, Feb 5, 2016 at 3:58 PM, Chris Nauroth <[email protected]> >wrote: >> Hi Alex, >> >> I replied to this on Monday, 2/1. I'm copy-pasting the same reply >>below, >> and this time I've put your email address on the To: line explicitly. >> >> >> >> I looked into this, and I can repro. I think this is a bug, and I don't >> think any of the recent Windows fixes addresses it. Although "make sure >> you don't include stdint.h" is a viable workaround, it's not best for us >> to put artificial constraints on our users. >> >> Modern versions of Visual Studio do ship a stdint.h header, which >>negates >> the need for the winstdint.h compatibility shim. I think a stdint.h is >> available in at least Visual Studio 2010 and later. One potential fix >> would be to remove winstdint.h, transition completely to use of >>stdint.h, >> and declare that our supported build tool chain is Visual Studio 2010 >>and >> later. I'm reluctant to do that on the 3.4 maintenance line though on >> grounds of backwards-compatibility. Another approach could be >>conditional >> compilation to include either stdint.h or winstdint.h based on choice. >>I >> see a flag named HAVE_STDINT_H mentioned in winconfig.h, so it seems >>like >> someone was thinking of this. However, the flag is unused from what I >>can >> tell, so it wouldn't actually help for a project to define it before >> compiling. We'd need to make code changes to support it. >> >> On a side note, there are numerous other compilation warnings on >>Windows, >> unrelated to stdint.h. >> >> Alex, do you think you are experiencing a bug from the stdint.h >>warnings? >> I'm trying to decide if we should file a single JIRA for a mass cleanup >>of >> warnings, or if the stdint.h warnings you reported are somehow more >> critical than the rest and need tracking in their own separate issue. >> >> >> --Chris Nauroth >> >> >> >> >> On 2/5/16, 3:53 PM, "Alex Clemmer" <[email protected]> wrote: >> >>>Hey folks. Not sure if someone responded and I just didn't see it >>>because I'm not on the dev@ list, or if there just isn't that much >>>interest in these questions. Can someone please at least confirm that >>>I've not screwed up sending this message somehow? >>> >>>On Thu, Jan 28, 2016 at 3:24 PM, Alex Clemmer >>><[email protected]> wrote: >>>> I should note also that I have looked closely at the issue tracker >>>> (e.g., https://issues.apache.org/jira/browse/ZOOKEEPER-1953) and >>>> various related changesets (e.g., >>>> http://svn.apache.org/viewvc?view=revision&revision=1148116) but being >>>> young and raised mostly on git I could not find a discussion of why >>>> this file is the way that it is. So it could well be that I am simply >>>> missing simple guidance like "make sure not to include `stdint.h` in >>>> any project consuming the ZK C client library." >>>> >>>> On Thu, Jan 28, 2016 at 3:18 PM, Alex Clemmer >>>> <[email protected]> wrote: >>>>> Hey folks. >>>>> >>>>> We (the Apache Mesos project) are building against the ZK C client on >>>>> Windows, with VS 2015 (and MSVC v.1900). When we attempt a vanilla >>>>> build, including both `zookeeper.h` and `stdint.h` causes the >>>>>compiler >>>>> to complain that we're re-typedef'ing a few types (such as >>>>> `int_fast8_t` with different underlying types) in the file, >>>>> `winstdint.h`. >>>>> >>>>> I have scoured the Internet to see if I'm missing a -D flag >>>>>somewhere, >>>>> but it does not appear that this is the case. >>>>> >>>>> The comments in this file state that it's meant to provide a >>>>> C9X-compliant version of `stdint.h` for Windows, but for later >>>>> versions of MSVC some of the definitions seem to be redundant or >>>>> different. (For example, on VS 2013 `int_fast16_t` is redefined with >>>>>a >>>>> different underlying type.) >>>>> >>>>> My question for you all is: am I missing something obvious, or should >>>>> I submit a bug and a patch to resolve this issue for you? >>>>> >>>>> >>>>> -- >>>>> Alex >>>>> >>>>> Theory is the first term in the Taylor series of practice. -- Thomas >>>>>M >>>>> Cover (1992) >>>> >>>> >>>> >>>> -- >>>> Alex >>>> >>>> Theory is the first term in the Taylor series of practice. -- Thomas M >>>> Cover (1992) >>> >>> >>> >>>-- >>>Alex >>> >>>Theory is the first term in the Taylor series of practice. -- Thomas M >>>Cover (1992) >>> >> > > > >-- >Alex > >Theory is the first term in the Taylor series of practice. -- Thomas M >Cover (1992) >
