ocaml icon indicating copy to clipboard operation
ocaml copied to clipboard

ocamldep: add a robust mode

Open ghost opened this issue 5 years ago • 10 comments

ocamldep has a tendency to silently do nothing in case of errors. For instance:

$ ls toto.ml
ls: cannot access toto.ml: No such file or directory
$ ocamldep toto.ml
$ echo $?
0

I'm guessing that this behaviour was designed for use in makefiles. However, it tends to delay errors and make debugging build errors harder. I propose to add a -robust option to ocamldep so that it fails with an exit code of 1 when the input file doesn't exist or cannot be read or parsed.

/cc @sliquister

ghost avatar Apr 18 '19 09:04 ghost

I'm guessing that this behaviour was designed for use in makefiles.

There is the idiom ocamldep *.ml *.mli where, if there are no .mli files for example, the shell passes *.mli to ocamldep and it is somewhat helpful that this argument is ignored.

I agree that in general it's bad to ignore nonexistent files listed on the command-line.

If you want to add an option, go for it. (But I'd rather call it -strict than -robust; ignoring *.mli is an example of robustness :-)).

But maybe we could do the right thing: detect and ignore failed globbings, but exit with an error in other cases of files not found.

xavierleroy avatar Apr 18 '19 15:04 xavierleroy

I see, thanks for the info :) Calling the option -strict seems good to me.

But maybe we could do the right thing: detect and ignore failed globbings, but exit with an error in other cases of files not found.

That makes sense. Though for my main use case (dune and jenga) the strict mode will still be useful as we are calling ocamldep directly via execve rather than go through a shell. I guess ml files with * in the name are not very common, but it's still easier to reason about all this when the various components don't try to be too clever.

ghost avatar Apr 18 '19 15:04 ghost

OK, I guess we can have both: the -strict option and a more clever handling of nonexistent file names. Go for the option, and I'll try to think of something sensible to do with the nonexistent file names.

xavierleroy avatar Apr 18 '19 15:04 xavierleroy

Seems good. I had a look this morning. If I implement the option in a naive way, I end up with this output which doesn't look good:

$ ocamldep -strict '*.ml'
File "*.ml", line 1:
Error: I/O error: *.ml: No such file or directory
*.cmo :
*.cmx :

Ideally, I would prefer to not display the last two lines. It's easy enough to do, but in the end the code feels more complicated than it should.

It seems that in case of error, ocamldep systematically displays all the errors and the partial results. Do you know why this is so? It would seem much simpler to me to immediately fail as soon as we get an error. In practice, the user cannot really trust the output if the call fails.

ghost avatar Apr 23 '19 11:04 ghost

@xavierleroy Ping on @diml 's question.

mshinwell avatar Apr 16 '20 16:04 mshinwell

I tend to agree with @jeremiedimino : ocamldep should not output any dependencies if there's an error. But there's not telling what strange things build systems will do, so a change like that should be tested on a large number of opam packages.

damiendoligez avatar Sep 09 '20 07:09 damiendoligez

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

github-actions[bot] avatar Sep 13 '21 04:09 github-actions[bot]

We were talking about this problem just yesterday as it has been biting us again. @snowleopard is going to submit a PR to implement the -strict option.

ghost avatar Sep 14 '21 09:09 ghost

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

github-actions[bot] avatar Sep 16 '22 04:09 github-actions[bot]

I believe @emillon is working on this.

snowleopard avatar Sep 16 '22 12:09 snowleopard