ocaml / dune

A composable build system for OCaml.
https://dune.build/
MIT License
1.63k stars 401 forks source link

Linker is invoked from unexpected directory #7146

Open gridbugs opened 1 year ago

gridbugs commented 1 year ago

Expected Behavior

If a library specifies custom linker flags in (c_library_flags ...) which reference files relative to the directory containing the dune file, those files should be discoverable by the linker. For example, if I have a project with a static library src/libfoo.a and src/dune is

(library
 ...
 (c_library_flags :standard -lfoo -L.))

...or...

(library
 ...
 (c_library_flags :standard -lfoo -L%{project_root}/src))

...the linker should be able to find the archive libfoo.a.

Actual Behavior

The linker can't find the library: cannot find -lfoo: No such file or directory. This is because the linker is not invoked from the directory containing the dune file, and %{project_root} resolves to a relative path (relative to the directory containing the dune).

Reproduction

Specifications

nojb commented 1 year ago

This is a well known issue (that several others have reported in the past); it could be solved if the compilation and linking commands were invoked from the current directory instead of the workspace root. But I have a faint recollection that the conclusion was that this was not a good idea (I don't remember why...).

Alizter commented 1 year ago

Isnt the whole point of running ocaml at the workspace root so that we have more cache sharing. I.e. Moving an ocaml project deeper into the workspace will still get a cache hit since the command should be the same.

At least I think this is the motivation, I have yet to confirm it actually works in practice.

gridbugs commented 1 year ago

Which cache are you referring to? Memo?

rgrinberg commented 1 year ago

This is a well known issue (that several others have reported in the past); it could be solved if the compilation and linking commands were invoked from the current directory instead of the workspace root. But I have a faint recollection that the conclusion was that this was not a good idea (I don't remember why...).

It's because we need error messages to be relative to the workspace root

@gridbugs I think we need a fix that's similar to your preprocessing fix

gridbugs commented 1 year ago

Why do we need error messages to be relative to the workspace root?

rgrinberg commented 1 year ago

To make it easier for editors that rely on regex to jump to the locations in the errors.

Today, it's all probably redundant as we have an LSP based mechanism for jumping to errors.

nojb commented 1 year ago

I think this issue is unfortunate, but it is not common enough to justify a big change such as changing the directory from which commands are triggered. For this specific case you can work around it by generating an absolute path for your linking command by hand (ie with a rule).

rgrinberg commented 1 year ago

Yes indeed. I'm wondering if the issue is that we're expanding the paths too early. We should be expand these paths relative to the chdir we're currently in rather than the directory of the dune file.