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

fix openbsd build #659

Merged
merged 1 commit into from
Jun 27, 2017
Merged

fix openbsd build #659

merged 1 commit into from
Jun 27, 2017

Conversation

xguerin
Copy link
Contributor

@xguerin xguerin commented Jun 2, 2017

No description provided.

@let-def
Copy link
Contributor

let-def commented Jun 5, 2017

Hi. Thanks for the patch.

In the light of #658, maybe it is better to find a more robust way to generate the socket path.
So I prefer to wait a bit and if we come up with a nicer solution, check that it doesn't break on OpenBSD before merging (Merlin 3 hasn't been released yet anyway).

@xguerin
Copy link
Contributor Author

xguerin commented Jun 5, 2017

Ok, thanks. Since I am working on OpenBSD I'll be glad to take care of the testing if needed.

@let-def
Copy link
Contributor

let-def commented Jun 19, 2017

xguerin: can you try to rebase on current master? If it works, I will happily merge your PR. Thanks.

@xguerin
Copy link
Contributor Author

xguerin commented Jun 20, 2017

Done. However, recent versions of merlin 3.0 are behaving strangely on OpenBSD. I am getting the following error when running ocamlmerlin:

execvp(ocamlmerlin-server): No such file or directory
merlin path: /home/xrg/merlin/ocamlmerlin-server
socket path: /tmp/<not computed yet>

I have not taken an in-depth look at what may be the source of the problem (ocamlmerlin-server is in my PATH). If I find something I'll let you know.

@let-def let-def merged commit cdb6b71 into ocaml:master Jun 27, 2017
@let-def
Copy link
Contributor

let-def commented Jun 27, 2017

I was able to run your patched version on OpenBSD (6.1) without problem, so the problem must come from a difference in your setup. I tried with vim.
Are you using emacs?

@xguerin
Copy link
Contributor Author

xguerin commented Jun 27, 2017

How do you call ocamlmerlin ? If I call it with its full path, it indeed works. If I call it with just ocamlmerlin, it fails.

It seems that realpath in ocamlmerlin.c:378 expands ocamlmerlin (argv0) into $PWD/ocamlmerlin, therefore skipping the search_in_path call. Removing line 378 fixes it for me.

It looks like the culprit may be w0rp/ale, which calls ocamlmerlin without its full path.

However, I don't really see the point of calling realpath above. I'm also quite flabbergasted that it would work out of the box on your OpenBSD installation. AFAICT, that call is not biased by any outside environment.

@let-def
Copy link
Contributor

let-def commented Jun 28, 2017

I see. My .vimrc for merlin development might workaround the issue.

The purpose of realpath is to follow symbolic links, but this approach is fragile, I have to think about something more robust for finding ocamlmerlin-server from ocamlmerlin.

@xguerin
Copy link
Contributor Author

xguerin commented Jun 28, 2017

What about checking realpath only when argv0 is actually a path, i.e. skipping it when argv0 = "ocamlmerlin" ? That should cover pretty much all corner cases.

EDIT I apologize for being so dense, but although I get the reason why you want to use realpath, I am still not quite sure about that particular check on line 377:

if (realpath(argv0, merlin_path) == NULL)

From the OpenBSD MAN:

All but the last component of pathname must exist when realpath() is called.

Which differs apparently from the Linux version where all components must exist, period. However, since realpath is called on argv0, which for all intent and purpose is always going to be valid (otherwise the shell could not have run it), the return result of realpath is unlikely to be NULL.

My understanding is that you want to check whether or not a the binary was called using a path so you can either use that path (if it exists) to run ocamlmerlin-server or search PATH to find the proper location.

If my interpretation is correct, I gave it a shot here: #667.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants