hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Colin Patrick McCabe (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-7188) support build libhdfs3 on windows
Date Mon, 29 Dec 2014 21:00:13 GMT

    [ 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)

Mime
View raw message