Modifications to support GloVe on Windows

[gaebor]

This makes the code compile natively on Windows with nmake

[ghost]

Thanks for sharing this! Great to see that you were able to get it working on windows. I'm planning to just leave this pull request open for windows users to merge unless there is a large user demand for it to be merged into the main repo. Hopefully that is okay with you.

[gaebor]

Okay!

[aKzenT]

Hey, just wanted to vote to have this included by default in the main branch.

I also want to use glove in windows and had to do several modifications to make it run as I did not see this branch.

I was able to compile the code though with fewer modifications. E.g. for the posix_memalign I used a macro:

#define posix_memalign(p, a, s) (((*(p)) = _aligned_malloc((s), (a))), *(p) ?0 :errno)

For pthreads I used the pthread for windows library at https://www.sourceware.org/pthreads-win32/.

With this I was able to compile on MinGW GCC.

The only problem I face now is that the file modes used by glove seem to be incorrect sometimes, which makes cooccur produce invalid files (output is treated as text by windows instead of binary).

[ghost]

@aKzenT @gaebor The difficulty is not in adding Windows functionality but providing a high level of support for that functionality as well. That being said, your points about discoverability of the pull request could be dealt with in other ways. If one of you wants to create a Windows fork, and submit a pull request of something to the tune of #ifdef _MSC_VER ... yell some huge warning to windows users to check out github.com/yourname/yourrepo #endif, or some similar strategy to avoid making all of you guys reinvent the wheel, I think that would be great.

[gaebor]

@aKzenT is right about the binary/text file handling thing.
https://msdn.microsoft.com/en-us/library/h9t88zwz.aspx

I'm surprised that I haven't ran into it. I should correct it..

[aKzenT]

@gaebor I got it to run locally by fixing the "b" flag in some of the fopen() calls. Unfortunately that is not enough and you also have to do

setmode(fileno(stdout), O_BINARY)

or

setmode(fileno(stdin), O_BINARY)

in the programs that read from stdin or write to stdout.

@Russell91 I understand your concerns. However there could be some things that you could do which would make compiling on windows easier out of the box, without adding maintenance overhead. For example the binary flags in the fopen calls at the moment are simply wrong in some places. This doesn't matter on linux, of course, but it is easy to fix and would keep the changes for windows to a minimum.

As to stdin and stdout, maybe instead of #ifdeffing "setmode", the program could support reading or writing to files in addition to using stdin and stdout. This would provide a way to run the commands on windows without changes while also benefitting linux as well and would not require adding windows specific functions to the source code.

With the changes above the only thing missing would be the posix_memalign funcion, which in fact would require an #ifdef.

All in all the changes are quite minimal if you run in a MinGW environment and are willing to download the posix thread library. There is no need for separate Makefiles or using the windows API. For those users who want to use Visual C++ compilers and threading without extra libraries a separate fork would still be helpful of course.

[gaebor]

@aKzenT I see now, the problem is the reading/writing of the binary via the console.
I think an -input and -output command line argument would help the shuffle tool.

The purpose of my pull request was the native Windows support.
The pthread and mingw style modifications are clearly a different flavor. Go for it and let the user decide which one to use on his/her windows machine!

[mwdavids]

Thanks all (@gaebor @aKzenT @Russell91 ) for the work on this tool and to make it windows compatible.

After making the changes described in this pull, everything works smoothly on my windows 10 machine except for shuffle. Has anyone submitted a modified shuffle.c file to overcome this problem? When ever I try to execute it I get the 'shuffle.exe has stopped working' dialog.

[hfxunlp]

Hi, Have you test your changes? I have done the similiar job, but it fails to work, seems the shuffle file can not be read correctly by glove and here is my repository. @gaebor

[gaebor]

Yes, just as mentioned above. I'll try to make the modifications for the shuffle but it hasn't been a priority for me.

[AngledLuffa]

Looking at a pull request from a long time ago. How necessary is the posix_memalign anyway? My impression is at most it's going to occasionally save on a page swap. Changing it to malloc might be easier for portability.

[gaebor]

@AngledLuffa you're right