Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Compiling under Windows #810

Closed
wants to merge 14 commits into from
Closed

Conversation

equalsraf
Copy link
Contributor

@equalsraf equalsraf commented Jun 4, 2014

Windows support is now tracked in #5229, check the wiki for Installation instructions and Build instructions

This PR is rebased for convinience, but it will be closed as soon as clipboard support goes into master.

@justinmk justinmk added this to the first release milestone Jun 4, 2014
@justinmk
Copy link
Member

justinmk commented Jun 4, 2014

If you did anything special to compile libuv on mingw, maybe you could send them a PR?

I think we should still aim to support MSVC 2013 also.

@equalsraf
Copy link
Contributor Author

@justinmk

  • libuv builds without incident (only some weirdness in make invocation, not sure what that was)
  • for msgpack-c there is already an issue CMake msgpack/msgpack-c#20 that is worth following
  • luarocks had some dificulties find the lua lib with Mingw - might be worth pushing that

I'm not giving up on MSVC just yet, but we might have to wait for a bit.

@tarruda
Copy link
Member

tarruda commented Jun 4, 2014

Once we manage to build on mingw, the build will be integrated with travis

@aktau
Copy link
Contributor

aktau commented Jun 4, 2014

I agree, extremely valuable. It would give us a way to check we're not using too many undue UNIXisms and get our types straight (travis), as well as deliver a working windows binaries to all those who need to work with windows. It would be very cool. Of course MSVC 2013 (or even 2014 if/when it's released and if it's easier, but it doesn't look like it) is also still a goal. Very nice work.

@aktau
Copy link
Contributor

aktau commented Jun 4, 2014

Also @equalsraf those casts to (char_u *) need to be removed as os_getenv takes const char * parameters anyway. I'm also not sure if that ifdef-path is the right one for windows, not sure if windows even has LC_ALL, et cetera...

@equalsraf
Copy link
Contributor Author

Speaking of unixsms any idea if we still need to include the sys/select.h in vim.h?

@equalsraf
Copy link
Contributor Author

@aktau You are correct that path is the wrong one. I need to get libintl/gettext for mingw, in order to avoid it

@equalsraf
Copy link
Contributor Author

On a slighty different note it seems MSYS2 has a package for libuv(along with gettext and ncurses) - that might be the easiest way to go.

@equalsraf
Copy link
Contributor Author

Looking at os_unix_defs.h there are a bunch of defines that seem to be system independant (FILETYPE_FILE, INDENT_FILE, etc), we should probably move those into a separate header.

@equalsraf
Copy link
Contributor Author

Some real issues seem to be going on at fileio.c, the buf_write code for windows was likely removed and vim_tmpname seems to be event worse.

[  2%] Building C object src/nvim/CMakeFiles/nvim.dir/fileio.c.obj
C:\Users\raf\dev\neovim\src\nvim\fileio.c: In function 'buf_write':
C:\Users\raf\dev\neovim\src\nvim\fileio.c:2582:40: error: 'file_info_old' undeclared (first use in this function)
       os_get_file_info((char *)fname, &file_info_old);
                                        ^
C:\Users\raf\dev\neovim\src\nvim\fileio.c:2582:40: note: each undeclared identifier is reported only once for each function it appears in
C:\Users\raf\dev\neovim\src\nvim\fileio.c: In function 'modname':
C:\Users\raf\dev\neovim\src\nvim\fileio.c:4403:21: error: 'BASENAMELEN' undeclared (first use in this function)
   if (STRLEN(ptr) > BASENAMELEN)
                     ^
C:\Users\raf\dev\neovim\src\nvim\fileio.c: In function 'vim_tempname':
C:\Users\raf\dev\neovim\src\nvim\fileio.c:5223:15: error: 'TEMPNAMELEN' undeclared (first use in this function)
   char_u itmp[TEMPNAMELEN];
               ^
In file included from C:\Users\raf\dev\neovim\src\nvim\fileio.c:15:0:
C:\Users\raf\dev\neovim\src\nvim\fileio.c:5332:16: error: 'TEMPNAME' undeclared (first use in this function)
   STRCPY(itmp, TEMPNAME);
                ^
C:/Users/raf/dev/neovim/src/nvim/vim.h:977:58: note: in definition of macro 'STRCPY'
 #define STRCPY(d, s)        strcpy((char *)(d), (char *)(s))
                                                          ^
C:\Users\raf\dev\neovim\src\nvim\fileio.c:5223:10: error: unused variable 'itmp' [-Werror=unused-variable]
   char_u itmp[TEMPNAMELEN];
          ^
C:\Users\raf\dev\neovim\src\nvim\fileio.c:5341:1: error: control reaches end of non-void function [-Werror=return-type]
 }
 ^

@aktau
Copy link
Contributor

aktau commented Jun 5, 2014

C:\Users\raf\dev\neovim\src\nvim\fileio.c: In function 'buf_write':
C:\Users\raf\dev\neovim\src\nvim\fileio.c:2582:40: error: 'file_info_old' undeclared (first use in this function)
       os_get_file_info((char *)fname, &file_info_old);

This is strange, the preceding code is:

#if defined(UNIX)
  perm = -1;
  FileInfo file_info_old;
  if (!os_get_file_info((char *)fname, &file_info_old)) {
    newfile = TRUE;
  } else {
   // ...
  }

So, UNIX seems to be defined (I think that's fine here since libuv handles the conversions), but I don't see why it would think that file_info_old is undeclared. It's declared the line before. Did you change something @equalsraf?

@aktau
Copy link
Contributor

aktau commented Jun 5, 2014

Also, sadly libuv doesn't seem to provide support for creating tempfiles, so we'll probably have to make os_tempfile as a cleaned up version of vim_tempfile ourselves.

@equalsraf
Copy link
Contributor Author

@aktau UNIX is undefined in my branch, that is why it is throwing the undeclared error.

The code under the UNIX check should be ok for buf_write since it uses libuv anyway (at least it builds if UNIX == 1). The remainder code should probably be removed since it lacks the proper declarations anyway.

I've defined TEMPNAMELEN/BASENAMELEN as 256 since that seems to be MAX_PATH for windows (as far as I can tell from stackoverflow and libuv pipes).

vim_tempname is the bigger problem here, should I create a new issue for it?

@aktau
Copy link
Contributor

aktau commented Jun 5, 2014

@aktau UNIX is undefined in my branch, that is why it is throwing the undeclared error.

If UNIX is undefined, then it shouldn't have tried compiling the os_get_file_info call either. So I'm not sure what's happening here. The declaration and call are in the same #ifdef block, as far as I can see.

We can probably remove a lot of code, as you said. This is one more reason why the windows build is a good thing, it will allow us to notice what's superfluous and slim down the codebase even more.

I've defined TEMPNAMELEN/BASENAMELEN as 256 since that seems to be MAX_PATH for windows (as far as I can tell from stackoverflow and libuv pipes).

Indeed, these are things that can be pretty OS-specific and I don't see a good reason for limiting all OS's to the same path length. We should probably move those defines to a platform file that CMake includes based on the OS on which it is compiling. Any input @jszakmeister?

vim_tempfile is the bigger problem here, should I create a new issue for it?

Definitely.

@equalsraf
Copy link
Contributor Author

@aktau the call to os_get_file_info does not get compiled but there is second call in the #else branch that uses the same (undeclared) var name. Line numbers may differ, declaration happens at

#if defined(UNIX)
perm = -1;
FileInfo file_info_old;
if (!os_get_file_info((char *)fname, &file_info_old)) {

undeclared use happens later in the #else clause of UNIX (:2583)

if (overwriting) {
  os_get_file_info((char *)fname, &file_info_old);
}

@aktau
Copy link
Contributor

aktau commented Jun 5, 2014

undeclared use happens later in the #else clause of UNIX (:2583)

I see. That explains it ;).

I notice that, for example, mch_nodetype is defined in os_unix.c, which seems like a poor place to place something that gets called in a non-UNIX branch. I sense a lot of refactoring coming up. Those #ifdef's need to go and those branches need to be simplified a lot. Possibly even extracted into separate functions. Perhaps we could/should make separate issues out of it and get them merged quickly so you can continue piecewise with your work while putting the codebase in order?

@equalsraf
Copy link
Contributor Author

Separate issues would be better yes. In any case there are a lot of commits in this branch that are just testing glue - I will have to rebase anyway.

See also sys/select.h and FILETYPE/INDENT comments above.

@aktau
Copy link
Contributor

aktau commented Jun 5, 2014

Pretty sure we shouldn't need sys/select.h, libuv should handle that.

@equalsraf
Copy link
Contributor Author

@aktau I ended up removing sys/select.h in #813 after I realised it was not being used

@equalsraf
Copy link
Contributor Author

Moving forward a bit, it turns out the UNIX code for vim_tempnam() might work with Mingw (with a different TEMPDIRNAMES and minus mkdtemp).

'''
[ 0%] Building C object src/nvim/CMakeFiles/nvim.dir/if_cscope.c.obj
C:\Users\raf\dev\neovim\src\nvim\if_cscope.c: In function 'cs_add_common':
C:\Users\raf\dev\neovim\src\nvim\if_cscope.c:545:3: error: implicit declaration of function '
S_ISLNK' [-Werror=implicit-function-declaration]
else if (S_ISREG(file_info.stat.st_mode) || S_ISLNK(file_info.stat.st_mode))
^
C:\Users\raf\dev\neovim\src\nvim\if_cscope.c: In function 'cs_create_connection':
C:\Users\raf\dev\neovim\src\nvim\if_cscope.c:783:3: error: 'sa' undeclared (first use in this
function)
sa.nLength = sizeof(SECURITY_ATTRIBUTES);
^
C:\Users\raf\dev\neovim\src\nvim\if_cscope.c:783:3: note: each undeclared identifier is repor
ted only once for each function it appears in
C:\Users\raf\dev\neovim\src\nvim\if_cscope.c:787:9: error: 'pipe_stdin' undeclared (first use
in this function)
if (!(pipe_stdin = CreatePipe(&stdin_rd, &stdin_wr, &sa, 0))
^
C:\Users\raf\dev\neovim\src\nvim\if_cscope.c:787:34: error: 'stdin_rd' undeclared (first use
in this function)
if (!(pipe_stdin = CreatePipe(&stdin_rd, &stdin_wr, &sa, 0))
^
C:\Users\raf\dev\neovim\src\nvim\if_cscope.c:787:45: error: 'stdin_wr' undeclared (first use
in this function)
if (!(pipe_stdin = CreatePipe(&stdin_rd, &stdin_wr, &sa, 0))
'''

@equalsraf
Copy link
Contributor Author

There is a bunch of stuff in if_cscope.c that does not seem to be ported to libuv, same goes for filesystem bits like IS_LNK, etc

@equalsraf
Copy link
Contributor Author

Using a cross compiler toolchain to build this on Linux saves me a lot of trouble of having to deal with dependencies in Windows like luajit/luarocks.

Following the same strategy as in #696 I've added dummy mch_* functions to try to get to the linking phase. Remember this is cross compiled I cannot be sure it actually runs yet - but here are some promising results:

Linking C executable ../../bin/nvim.exe
CMakeFiles/nvim.dir/objects.a(path.c.obj):path.c:(.text+0x24fb): undefined reference to `unix_expandpath'
CMakeFiles/nvim.dir/objects.a(os_mingw.c.obj):os_mingw.c:(.text+0x374): undefined reference to `getlinecol'
CMakeFiles/nvim.dir/objects.a(shell.c.obj):shell.c:(.text+0x422): undefined reference to `append_ga_line'
CMakeFiles/nvim.dir/objects.a(shell.c.obj):shell.c:(.text+0x8fc): undefined reference to `append_ga_line'
CMakeFiles/nvim.dir/objects.a(input.c.obj):input.c:(.text+0x1cc): undefined reference to `read_from_input_buf'
CMakeFiles/nvim.dir/objects.a(input.c.obj):input.c:(.text+0x214): undefined reference to `fill_input_buf'
CMakeFiles/nvim.dir/objects.a(input.c.obj):input.c:(.text+0x23a): undefined reference to `input_available'
collect2: error: ld returned 1 exit status
make[2]: *** [bin/nvim.exe] Error 1
make[1]: *** [src/nvim/CMakeFiles/nvim.dir/all] Error 2
make: *** [all] Error 2

The linking errors are almost the same as in #696. Next steps rebase against master and start merging the code with #696.

@tarruda
Copy link
Member

tarruda commented Jun 25, 2014

@equalsraf thank you for this amazing contribution 👍

@equalsraf
Copy link
Contributor Author

@timofonic no problem, you might want keep track of #5229, I will be removing this PR as soon as one or two more patches make it into master.

equalsraf and others added 14 commits October 14, 2016 09:24
The 64bit check for the libuv recipe worked for the VS generator
but not for NMake.
Look for runtime dependencies diff.exe and win32yank.exe (and
recursively for DLL dependencies) and install them with nvim.exe. If a
dependency is missing a warning will be issued.
Winpty has x86/x64 binary builds, download them when building
Neovim.
In Windows we cannot rely on absolute install paths to point to the location of
the runtime. Vim used the path of the current binary as a possible location for
the runtime folder. In Neovim the install locations places the runtime folder in
../share/nvim/runtime.
This commit implements clipboard support for windows, using win32yank.
FFI unittests require header preprocessing. Previously the headers
in tests/include/pre/ were preprocessed by CMake, and the Neovim/
system headers were preprocessed by lua in preprocess.lua. This
commit moves all header preprocessing into CMake.

The following headers are processed:

1. All headers under src/nvim/ with the exception of os/unix_defs.h and
   os/win_defs.h.
2. All headers under test/unit/fixtures/
2. All headers under test/includes/pre/

Generated headers go into <builddir>/test/include/ffi-headers/.
Handling of process exit is still broken.  It detects the moment when the
child process exits, then quickly stops polling for process output.  It
should continue polling for output until the agent has scraped all of the
process' output.  This problem is easy to noticed by running a command like
"dir && exit", but even typing "exit<ENTER>" can manifest the problem --
the "t" might not appear.

winpty's Cygwin adapter handles shutdown by waiting for the agent to close
the CONOUT pipe, which it does after it has scraped the child's last
output.  AFAIK, neovim doesn't do anything interesting when winpty closes
the CONOUT pipe.
kvec.h uses "restrict" which is not defined in MSVC, but there is a definition
in win_defs.h
@equalsraf
Copy link
Contributor Author

The master branch should now have feature parity with this, closing. Keep track of windows support in #5229. I'm updating the wiki links accordingly.

@equalsraf equalsraf closed this Jan 24, 2017
@jszakmeister jszakmeister removed the WIP label Jan 24, 2017
dwb pushed a commit to dwb/neovim that referenced this pull request Feb 21, 2017
Same as here: neomake/neomake#532
I was able to get airline support only when enabled %E in error format
And it's related to neomake/neomake#336
@equalsraf equalsraf deleted the tb-mingw branch March 17, 2017 22:44
butwerenotthereyet pushed a commit to butwerenotthereyet/neovim that referenced this pull request Dec 30, 2019
If there's an autocmd for BufReadPost that jumps to the last position,
searching for the tag location sometimes results in vim printing the
wrapscan warning and then requires hit-enter. Execute the search
silently to suppress the warning (errors are still visible!).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build building and installing Neovim using the provided scripts platform:windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet