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

Revised commits for Windows/mingw32 #409

Merged
merged 6 commits into from Jul 1, 2015
Merged

Revised commits for Windows/mingw32 #409

merged 6 commits into from Jul 1, 2015

Conversation

@dra27
Copy link
Contributor

@dra27 dra27 commented Jun 23, 2015

Finally found some time for upgrading Merlin from the patched 1.6 I'd been using. Patches very similar to those for #194 (some issues had already been fixed or become irrelevant) only as I'm a bit less unfamiliar with git now, I've broken the changes into multiple commits for easier cherry picking if necessary.
An adapted version of this branch also compiles against v2.2 (the whitespace clean-up of vim/merlin/autoload/merlin.py in 8c28e23 means the patch doesn't apply cleaning against v2.2)
I've tested it with gVim 7.4.752 x64 on Windows 7 with Python 2.7.10/3.4.3 for OCaml 4.02.2 i686-w64-mingw32 and it seems to be working beautifully.
a3b240b is critical for building with native Windows ports as ln isn't available
acc4ceb is critical for Vim support on Windows - since reporting that in #194, I've discovered that it is indeed a long-standing bug in Python (and the code is commented accordingly)
Remaining commits, at least with a very current Cygwin, seem to be cosmetic.

dra27 added 6 commits Jun 23, 2015
Even with "-", the error message is disconcerting in the quite large
output of make install - it's not totally clear if it's caused anything
else to fail.
ocamlopt adds .exe by default and install usually copes too, but update
the Makefile to add .exe so that it looks correct in the output as well...
Windows ports (mingw64 or msvc) can't use Cygwin's symlinks, so cp -R
instead of ln for the appropriate OCaml version.
Primarily for building on Windows (ncurses isn't installed by default for
Cygwin)
Commit fixes two problems:
1. Sub-processes must have all three standard handles specified (bug in
Python)
2. Starting the command causes a console window to be visible (containing
ocamlmerlin) - use a STARTUPINFO structure to tell Windows to hide it.
@let-def
Copy link
Contributor

@let-def let-def commented Jul 1, 2015

Looks good (interestingly, I had no problems using the vim current mode on windows, but I can't tell what my exact configuration was).

let-def added a commit that referenced this pull request Jul 1, 2015
Revised commits for Windows/mingw32
@let-def let-def merged commit 1a5100d into ocaml:master Jul 1, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dra27
Copy link
Contributor Author

@dra27 dra27 commented Jul 1, 2015

Were you using console vim or gVim? Thining about it, the Python issue would affect gVim only.

@let-def
Copy link
Contributor

@let-def let-def commented Oct 29, 2015

I took some time to play with windows build... An installer is available here:

https://github.com/the-lambda-church/merlin/releases/download/v2.3/Merlin.installer.for.OCaml.4.02.on.Windows.exe

It targets @protz OCaml on Windows distribution, but should work as long as ocaml and findlib paths are set.
I have no idea what windows customs are, it might feel a bit alien :).

@protz
Copy link

@protz protz commented Oct 30, 2015

I'm thinking of providing opam with my windows installer (see the beta
version under "other versions"). Any chance you could make merlin build
"out-of-the-box" with opam on windows?

Here's what I'm getting in the error log for "opam install merlin":

protz@manaslu:~/Code/fstar/runtime/allocator (master) $ cat
~/.opam/system/build/merlin.2.2/merlin_.err
Bad -I option: src/ocaml/: Invalid argument
make[2]: *_* Deleting file '._ncdi/src/frontend/command.di'

Please note that all dependencies of Merlin work fine on Windows.

Thanks,

Jonathan

On 10/29/2015 7:05 PM, Frédéric Bour wrote:

I took some time to play with windows build... An installer is
available here:

https://github.com/the-lambda-church/merlin/releases/download/v2.3/Merlin.installer.for.OCaml.4.02.on.Windows.exe

It targets @protz https://github.com/protz OCaml on Windows
http://protz.github.io/ocaml-installer/ distribution, but should
work as long as ocaml and findlib paths are set.
I have no idea what windows customs are, it might feel a bit alien :).


Reply to this email directly or view it on GitHub
#409 (comment).

@let-def
Copy link
Contributor

@let-def let-def commented Oct 30, 2015

I guess it is a problem with symbolic links.
What is the appropriate way to check for windows in the configure?

@protz
Copy link

@protz protz commented Oct 30, 2015

Sys.os_type == "win32" ?

On 10/30/2015 3:25 AM, Frédéric Bour wrote:

I guess it is a problem with symbolic links.
What is the appropriate way to check for windows in the configure?


Reply to this email directly or view it on GitHub
#409 (comment).

@dra27
Copy link
Contributor Author

@dra27 dra27 commented Oct 30, 2015

Sys.os_type == "Win32" (capital 'W') or I check for the presence of $COMSPEC if trying to determine it before I know if I have OCaml (it's not perfect, but neither is uname in Cygwin!)
It would indeed be symlink-related - I compile elevated with CYGWIN=winsymlinks:nativestrict (so I get real symbolic links, not Cygwin's fakery which ocamlopt, or in this case ocamldep, can't see)

@let-def
Copy link
Contributor

@let-def let-def commented Oct 30, 2015

Then I will fix the Makefile to avoid symbolic links under windows.
Checking for $COMSPEC will do it (I would like to stay in the shell script without spawning ocaml... Although we are sure OCaml is available at that point).
May I expect it to always be set in windows?

@let-def
Copy link
Contributor

@let-def let-def commented Oct 30, 2015

Hmm the script is already checking for windows using:

SYSTEM=$(ocamlfind c -config | grep system|cut -d' ' -f2)                       
 case "$SYSTEM" in                                                               
   mingw*|win*)      

@protz is it possible that this check failed on your setup?

@dra27
Copy link
Contributor Author

@dra27 dra27 commented Oct 30, 2015

Yes - Windows really doesn't run without it set (the Command Processor doesn't work and it affects the shell in other unexpected ways). What I've never been utterly certain is whether you can reasonably depend on COMSPEC not being used on any Unix variant.
Incidentally, having done quite a lot of work on symbolic links on Windows recently, it would be good to see the test correctly checking whether ln works, rather than just simply disabling it completely on Windows - see https://github.com/dra27/dose/blob/windows/configure.ac#L265-L273 (that work is subsequent to my putting that patch for cp -R, so that's more a ToDo item on my part!)

@let-def
Copy link
Contributor

@let-def let-def commented Oct 30, 2015

I changed the way type checker version is selected in latest master.
It is passed through a Makefile variable rather than a symlink on windows system (as defined by COMSPEC).

Tell me if it improves the build with opam.

@protz
Copy link

@protz protz commented Nov 2, 2015

Yes it works now. Supercool! Feel free to point your users to
http://protz.github.io/ocaml-installer/ and the "WITH OPAM" version.

On 10/30/2015 10:18 AM, Frédéric Bour wrote:

I changed the way type checker version is selected in latest master.
It is passed through a Makefile variable rather than a symlink on
windows system (as defined by COMSPEC).

Tell me if it improves the build with opam.


Reply to this email directly or view it on GitHub
#409 (comment).

@let-def
Copy link
Contributor

@let-def let-def commented Nov 5, 2015

Nice, thanks for your work.

First feedback, on http://ocaml.org/docs/install.html#Windows :
This is potentially confusing for a large part of the audience (students).
When you/we get more confident that your distribution is stable, it should be the only one advertised (a single binary solving all problems).
More involved options should be relegated to an "Advanced section".

@dra27
Copy link
Contributor Author

@dra27 dra27 commented Nov 5, 2015

That is indeed the goal - I'm certainly hoping that it will be a case of downloading a very lean binary installer for OPAM only which will then carry out the rest.

@protz
Copy link

@protz protz commented Nov 5, 2015

Yes, I'm actually thinking of just distributing the OPAM version of my
installer and dropping the other options... that's actually less work
for me to repackage.

~ jonathan

On 11/05/2015 03:25 AM, David Allsopp wrote:

That is indeed the goal - I'm certainly hoping that it will be a case
of downloading a very lean binary installer for OPAM only which will
then carry out the rest.


Reply to this email directly or view it on GitHub
#409 (comment).

@dra27 dra27 deleted the dra27:mingw32-vim branch Jul 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants