[
https://issues.apache.org/jira/browse/HDFS-7188?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14260439#comment-14260439
]
Colin Patrick McCabe commented on HDFS-7188:
--------------------------------------------
Thanks for sticking with this. The new patch looks better... good work.
I don't understand why you changed this to depend on krb5_32 instead of krb5,
can you explain? It seems like we might not want that change for UNIX, if it
makes us use a 32-bit version of the library (is that what it does?)
Also, I would prefer not to edit ENABLE_BOOST in this JIRA if possible.
{{hadoop-hdfs-project/hadoop-hdfs/src/contrib/libhdfs3/README.txt}}: It seems a
little odd to provide instructions only for Windows. Perhaps let's do this in
a follow-on JIRA when we can write a full README. I also don't think we need
to explain how to use "git diff"... that's explained in our HowToContribute
entry in the wiki.
{code}
#define THREAD_LOCAL __thread
{code}
This doesn't work on MacOS X. It's fine for us to deal with that in a
follow-on JIRA, but we should be aware of it. I mention this because I see you
have some text about MacOS elsewhere in this file.
I would prefer that we rename {{platform.h.in}} to {{build.h.in}}, and have
platform-specific stuff in normal header files like {{windows/platform.h}} and
{{posix/platform.h}}. It seems like there is no need to have cmake generate
the platform-specific headers... the only purpose of having the .h.in files is
to pass along CMake settings, but those should operate the same on each
platform, right?
{code}
// Code is copied from here
// http://code.google.com/p/mman-win32/
// MIT License
{code}
I can see that this person's Google Code landing page says "BSD license," but
it doesn't explain which BSD license it is (2-clause, 3-clause, etc.) Can we
either reimplement this (I would prefer this), or get this guy to add a
LICENSE.txt with the license to his repo? Or did I miss somewhere where he put
the full license text?
{{InputStreamImpl.cc}}: there's still a huge #ifdefed lump of Windows-specific
code calling {{GetAdaptersAddresses}} here. Let's move this to a
platform-specific file. It seems like we should just move the whole function
{{unordered_set<std::string> BuildLocalAddrSet()}} into the platform directory.
{code}
131 #ifdef _WIN32
132 // In windows, vsnprintf(NULL, 0, fmp, ap) returns -1.
133 // Hence use _vscprintf to get the required size instead.
134 int size = _vscprintf(fmt, ap);
135 #else
136 int size = vsnprintf(NULL, 0, fmt, ap);
137 #endif
{code}
Rather than scattering this kind of ifdef around, I would prefer that we
implemented a working version of vsnprintf for windows in the platform
directory. It shouldn't be too hard, right? Then everyone could call
{{platform_vsnprintf}} to get the same result on each platform.
One thing that might help you make progress here is to split off some of this
into smaller patches. For example, I can see a few of your changes just add
additional standard header files. These changes should be easy to get in very
quickly. Perhaps create a jira like "add additional header includes needed by
windows" and just add that part. Then I can get that in right away.
> support build libhdfs3 on windows
> ---------------------------------
>
> Key: HDFS-7188
> URL: https://issues.apache.org/jira/browse/HDFS-7188
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
> Environment: Windows System, Visual Studio 2010
> Reporter: Zhanwei Wang
> Assignee: Thanh Do
> Attachments: HDFS-7188-branch-HDFS-6994-0.patch,
> HDFS-7188-branch-HDFS-6994-1.patch
>
>
> libhdfs3 should work on windows
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
