Tracking paths to error locations

[mmottl]

My directory structure is as follows:

jbuild
jbuild-workspace
src/
  jbuild
  ...
examples/
  foo.ml
  jbuild
  ...

src contains a library and examples example applications. If I go to the examples directory and issue a build command from there (e.g. from an editor), an error in example foo.ml would be reported using location examples/foo.ml. This is a problem for inferring correct error locations. The editor does not know where the root of this path is, because we are in a (potentially even deeper) subdirectory.

Could you please print the absolute path of the root for error messages for a particular build when jbuilder is invoked so editors can infer the correct path? E.g. Vim allows you to pattern match on the build output to keep track of directory stacks for correct errors locations. A somewhat unique string should be used to correctly identify this location message, preferably an existing make-like Entering directory message that Vim or Emacs already understand.

[ghost]

I admit I'm a bit fuzzy on what is recommended way of invoking the build system from the editor. Myself I use some emacs lisp code that guesses the project root by looking for a few key files (Makefile, _oasis, jbuild-workspace...) and then set the build command to something like cd ... && make.

So the idea would be to have an option --make-style-errors and/or possibly an environment variable to make jbuilder print:

Entering "<absolute path>"

before reporting an error?

[mmottl]

Yes, something like that would work. I don't think it would be a good idea to have everybody write their own editor scripts for how jbuilder determines build roots or maybe even changes directories during build operations.

[bobot]

Instead of an option that should be added manually, can't jbuilder always print Entering "<absolute path>" when it is not the current directory?

PS: I personally use projectile, where projectile-compile run the command where the .git is.

[ghost]

Entering ... doesn't make much sense, jbuilder builds everything at once. I'm ok adding a compatibility option for editors but making it the default will make things confusing

[mmottl]

It's not a big deal. Various messages can coexist. You can choose your own, maybe something like jbuilder root is ....

I'm not sure how jbuilder works, but it would certainly be necessary to never change directories if different directories are built in parallel. All paths would have to start at the root to avoid confusing editors.

[mmottl]

Btw., if anybody needs a quickfix for now when building from within a subdirectory of the root, just create a Makefile containing e.g.:

.PHONY: all
all:
        @echo jbuilder: Entering directory \`$(abspath $(dir $(lastword $(PWD))))\'
        @jbuilder build --dev @all

Then a simple call to make should do what you want.

[ghost]

It's not a big deal. Various messages can coexist. You can choose your own, maybe something like jbuilder root is ....

That seems fine to me

All paths would have to start at the root to avoid confusing editors.

It's almost the case. Basically most commands run by jbuilder are run from the root, except for building C files as the .o is always generated in the cwd. User actions are also run from the directory they are defined. In any cases, jbuilder always print the directory in which the action is run, using this syntax:

(cd <dir> && ...)

[mmottl]

The handling of C-files makes things more complicated then. Since you presumably build them in parallel, I guess the best way would be to only print the directory change right before emitting an error message, making sure that there is no race condition that could insert another message between the directory change message and the error message.

I'm not sure (cd <dir> && ...) is unique enough. There may also be a chance that the S-expression pretty printer, if it's being used here, will break lines, which makes pattern matching in editors more difficult.

[Chris00]

The standard way of launching a build system in Emacs is M-x compile, usually bound to C-c C-c.

Unless there is a strong motivation, it would be better to keep the sentence Entering directory '<dir>' — you can put anything you want before it but it must end the line —, otherwise editors will need to be told about it.

Another possibility (which I'm not particularly fond of) is to invoke OCaml compilers with absolute file names, the errors will then contain the full path and links to errors will work.

[ghost]

I implemented @bobot's suggestion. Initially I was thinking about printing Entering directory ... before every command that output something given that commands are not all run from the same directory. However this clutters the output and given that all ocamlXXX commands are run from the root, printing it only once at the beginning is enough for most cases.

Unfortunately, it still doesn't help for C stubs. The problem with C stubs is that when you call ocamlc -c src/file.c, you get file.o rather than src/file.o. As a result the command has to be executed in src. Maybe we could workaround this problem by doing a rename just after the ocamlc -c ... and sandbox the action to avoid race conditions.

[mmottl]

jbuilder now prints directory changes, but the format does not match the patterns typically defined in Vim configurations for make. E.g. it prints:

Entering directory '...'

But most Vim errorformat configurations would expect something like the following:

jbuilder: Entering directory `...'

This would require the following changes:

1. Print jbuilder: before Entering directory to indicate the build tool.
2. Use a backtick to open the quoted directory (keep the regular tick at the end).

I have updated my personal (public) OCaml Vim configuration to support both kinds of ticks, which is what other configurations do now, too. GNU make is apparently switching to saner quoting. But it would help if jbuilder stayed compatible with legacy make error formats.

[bobot]

But it would help if jbuilder stayed compatible with legacy make error formats.

The format that jbuilder is currently using is the output used by gnu make since 4.0 (2013). Editors have been updated (for example four years ago for emacs).

Even if gnu make 4.0 had been adopted slowly it is now. So I don't want jbuilder to support an old format that is deprecated.

[Chris00]

I agree about not supporting deprecated formats but @mmottl has a point when he says that the line is still prefixed by make: (hence, here, jbuilder:). Emacs is not sensitive to that but Vim may be.

[bobot]

Indeed I forget to say that I don't see any problem for the first point.

[mmottl]

Some platforms (e.g. Mac OS X) still ship with old make programs by default. I agree that in case of the backtick it's probably better to push editor maintainers to update accordingly. I still think the prefix should be there, not just for make, but because it also helps determine what program produced a log message. This may be helpful for projects with mixed build tools.

[bobot]

I agree, I don't see any problem for the implementation and inclusion of the first point. We can add the prefix "jbuilder:". Sorry if I was not clear.

[cpitclaudel]

Any chance that the corresponding "Leaving directory …" messages could be added? Otherwise, when dune is run in a sequence including other programs, the messages produced by those programs are misinterpreted by editors.

For example, the following case is broken: project root is in /project/, and I have the following makefile in /project/codegen/:

../build/codegen/cppgen.exe:
	dune build cppgen.exe

example.cpp: build example.in
	../build/codegen/cppgen.exe example.in

a.out: example.cpp
	g++ example.cpp

run: a.out
	./a.out

When I execute make run, dune says that it's entering /project/, and then when g++ runs and prints errors or warnings Emacs is confused and understands these relative to /project/. This wouldn't happen if dune printed Leaving directory `/project' after completing the build.

[ghost]

It just feels a tad heavy :/ In your particular case, why not put the rules in the dune file directly? Then you just need one single call to dune.

[cpitclaudel]

It just feels a tad heavy :/

That's a fair point. Maybe under a flag?

In your particular case, why not put the rules in the dune file directly? Then you just need one single call to dune.

I didn't realize this was possible. In practice, the rules tend to be more complicated. Can I do things like enumerating all files with a certain extension and run a command on each in dune? (Also, I (and to a larger extent people that I work with) have more experience with make, so I'm wary of using dune for more than the OCaml and Coq parts of my projects — but I'm open to being convinced ^^).

[ghost]

Well, I won't try to convince you as the truth is, as soon as you go off the beaten track it's indeed pretty verbose compared to make. That said, if you do write the custom dune rules, then you get all the benefits that come with dune: polling mode, soon the shared cache, etc...

Adding a flag would be an option yeah. On the other hand having too many specific options tends to clutter the documentation. We generally try to just do the right thing without offering configuration unless it's really needed.

Thinking about this again, it's only one line at the end of the build and the Entering ... line was added precisely for Emacs. So if adding the Leaving ... line helps integration in Emacs, let's just do it.

Do you want to write the patch? The Entering ... line is printed in the prepare function in src/dune/scheduler.ml. You can just setup a at_exit hook at the same time as printing this line.

[cpitclaudel]

Neat. Yes, I can (try to) prepare a patch.

One more question, though, to make sure that I have this right. Why was the fix for the original issue to print the directory when compilation starts, rather than printing error messages relative to the current directory? Is it because the messages are printed by OCaml itself (and hence can't be adjusted to be relative to the directory in which dune was invoked? (I imagine that even if OCaml could be adjusted, any other binary would need the same treatment, which is unmanageable)

[ghost]

That's exactly the idea. We would need to parse the output of commands and rewrite filenames. There is no universal format for printing locations, so we'd need to hardcode a bunch of formats.

[cpitclaudel]

Done (roughly 4 months later 🎉) in #3072