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

Include OCaml lib directory in search path in ocamlmklib #7373

Closed
vicuna opened this issue Sep 26, 2016 · 7 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link

commented Sep 26, 2016

Original bug ID: 7373
Reporter: @dra27
Assigned to: @dra27
Status: resolved (set by @dra27 on 2017-06-10T12:02:42Z)
Resolution: fixed
Priority: normal
Severity: major
Version: 4.03.0
Target version: 4.04.1
Fixed in version: 4.04.1
Category: tools (ocaml{lex,yacc,dep,debug,...})
Monitored by: @gasche

Bug description

ocamlmklib doesn't include OCaml's lib directory in the search path for invocations of the linker.

For a FlexDLL 0.35+ installation where the FlexDLL object files are allowed to be installed in a different directory from flexlink.exe itself, this causes an error when using ocamlmklib to compile a DLL.

Given that the two compilers always include OCaml's lib dir for everything (using -I or -L as appropriate), is there any reason not to do this for ocamlmklib? It's just a matter of including the standard library directory as the base option for clibs?

Steps to reproduce

$ cd ocaml-4.03.0/testsuite/tests/lib-dynlink-bytecode

$ ocamlc -c -verbose stub1.c

  • i686-w64-mingw32-gcc -O -mms-bitfields -Wall -Wno-unused -c -I"C:/ocamlmgw/lib" "stub1.c"
    $ ocamlmklib -verbose -o plug1 stub1.o
  • flexlink -chain mingw -stack 16777216 -link -static-libgcc -o .\dllplug1.dll stub1.o
    ** Fatal error: Cannot find file "flexdll_initer_mingw.o"
    ** Fatal error: Cannot find file "flexdll_initer_mingw.o"
    $ ocamlmklib -verbose -o plug1 stub1.o -Locamlc -where
  • flexlink -chain mingw -stack 16777216 -link -static-libgcc -o .\dllplug1.dll stub1.o -LC:/ocamlmgw/lib
  • rm -f .\libplug1.a && i686-w64-mingw32-ar rcs .\libplug1.a stub1.o

Additional information

This isn't caught when running the testsuite, because at that time the object file is in the same directory as flexlink itself (it's all compiled inside the submodule). Not sure that it's worth the effort of torturing the test suite to catch this case.

@vicuna

This comment has been minimized.

Copy link
Author

commented Sep 26, 2016

Comment author: @dra27

I'm quite happy to implement this - just figured it might need discussion first!

@vicuna

This comment has been minimized.

Copy link
Author

commented Sep 27, 2016

Comment author: @damiendoligez

I'm confused: why is it OCaml's lib dir that you want to include? Is there a good reason to believe the FlexDLL library files will be found there?

Maybe I'm too naive, but ocamlmklib does not link an executable, so it seems to me it shouldn't need to find the library files. FlexDLL seems to be a special case here.

@vicuna

This comment has been minimized.

Copy link
Author

commented Sep 27, 2016

Comment author: @dra27

flexlink when called without -exe builds a DLL - which is what ocamlmklib is doing in that example (bytecode stubs library).

flexlink 0.35 relaxed the requirement that the object files be located in the same directory as the flexlink.exe binary and uses the library search path if it can't find them there (though for maximal backwards compatibility, it looks in the same directory as flexlink.exe first).

The reason for that change are twofold - principally, installing object files to a bin directory is inherently unhygienic; they are not executables or scripts! More importantly, it's loosely related to the Visual Studio 2015 thing - there are setups where you want multiple copies of flexdll_msvc.obj but only one flexlink.exe.

FlexDLL is indeed a special case - though a special case imposed by the compiler.

Possibly (and easily) ocamlmklib could add OCaml's lib dir only when it detects it's needed? That detection could be belt-and-braces: so a search of PATH for flexlink.exe and a test for the object files in that directory or a simpler "only do it on Windows/Cygwin"?

@vicuna

This comment has been minimized.

Copy link
Author

commented Sep 28, 2016

Comment author: @damiendoligez

I'd go for the simpler "only do it on Windows/Cygwin", but I'm a bit wary of breaking things for people who don't want to use OCaml's lib dir (for example because they have special implementations of libraries).

What about adding a subdirectory of $LIBDIR instead? For example $LIBDIR/flexdll ? Who decides where the FlexDLL files are installed?

@vicuna

This comment has been minimized.

Copy link
Author

commented Sep 28, 2016

Comment author: @dra27

OCaml decides where the FlexDLL files are installed - if OCaml compiled the FlexDLL objects, then Makefile.nt installs them, so the location can be changed.

Obviously, there'd be other places where -L$OCAMLLIB/flexdll and equivalents would be needed, but I certainly agree that would be much neater.

That would move the configuration to a new variable in config/Makefile.* (and so ultimately to configure).

Note that the Cygwin ports do not yet support this bootstrapped mode, so having a OCaml/FlexDLL set-up this way is unlikely on Cygwin (at least at the moment).

@vicuna

This comment has been minimized.

Copy link
Author

commented Oct 4, 2016

Comment author: @damiendoligez

@dra Could you please do a github PR? Preferably with the $OCAMLLIB/flexdll subdirectory, but if it looks too hard to catch all the things that need to be changed, let's go for $OCAMLLIB directly.

@vicuna

This comment has been minimized.

Copy link
Author

commented Jun 10, 2017

Comment author: @dra27

@doligez This is long since done, but I forgot to update Mantis - sorry.

Fixed in 4.04.1 by #1023 (40c4bb); 4.05 by #1101 (c8d9d5) and trunk by #1145 (80e752)

@vicuna vicuna closed this Jun 10, 2017

@vicuna vicuna added the tools label Mar 14, 2019

@vicuna vicuna added this to the 4.04.1 milestone Mar 14, 2019

@vicuna vicuna added the bug label Mar 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.