jbuilder shouldn't fail on unreadable directories

[mars0i]

jbuilder fails if there is any unreadable directory under the current directory:

~$ jbuilder external-lib-deps --missing @install
Error: exception Sys_error("cloj/popco-x: Permission denied")
... [long, ugly stacktrace]

It's arguable that this isn't a serious problem for building one's own projects, but it's common to be told to run the above command e.g. when there's a problem with an installation of some package via opam. If you run that command from e.g. your $HOME directory (why wouldn't you?), then any unreadable directory under that location that can be found e.g. via links stop the process. (In my case, I have a couple of directories of my own that I've set to be unreadable for my own reasons. One could imagine cases in which someone has a link into a tree belonging to another user or root or something like that, where only some of the directories are readable.)

It seems like it would make more sense to simply ignore unreadable directories, or possibly issue a warning and continue.

[dra27]

It's not clear to me why you'd want to run that from $HOME, but crashing with Sys_error is certainly a bug - we should display a warning and continue scanning

[mars0i]

Thanks.

As to why, I’m just following instructions from opam packages that fail to build. I don’t actually know what those jbuilder args do, although I can kind of guess. Would be worse for novice users, though. (The instructions never say “Go to a directory without unreadable subdirs and run this.” :-)

[dra27]

The opam package is (I hope) not telling you to run arbitrary commands from your $HOME directory! Would it be possible to see the complete failing output of the opam command?

[mars0i]

Jane Street Core can generate output that suggests running the command in response to opam install core:

#=== ERROR while compiling core_kernel.v0.9.1 =================================#
# context      2.0.0~beta4 | darwin/x86_64 | ocaml-variants.4.05.0+flambda | https://opam.ocaml.org/2.0#2017-11-05 03:04
# path         ~/.opam/4.05.0+flambda/.opam-switch/build/core_kernel.v0.9.1
# command      jbuilder build -p core_kernel -j 1
# exit-code    1
# env-file     ~/.opam/log/core_kernel-6933-703493.env
# output-file  ~/.opam/log/core_kernel-6933-703493.out
### output ###
# Error: External library "base" not found.
# -> required by "src/config/jbuild (context default)"
# Hint: try: jbuilder external-lib-deps --missing -p core_kernel @install

I think this is what originally caused me to run jbuilder from $HOME (which was where I was running opam from) and got a screenful of stackdump from an unreadable directory.

I originally assumed that the last line in the message was generated by something in the Core config, but I searched for parts of it on the Core repo on github and didn't find it. I think I got a similar "Hint" from some other opam install, but I don't remember what it was. Just tried to reproduce it but failed. Anyway, this all raises the possibility that opam is generating the hint, but I don't know.

(That's one case. I got a similar message when a different package that core needed was missing--don't remember which one.)

[dra27]

Hmm - the second line of opam's output says what path it started in. I must confess I'm not convinced that one would ever arbitrarily run a command in $HOME because "well, why not". However, jbuilder build commands are displayed as cd _build/foo && command... I can't decide if doing that for the hints too would be helpful or clutter.

@rgrinberg Thoughts?

[dra27]

(Unrelated to jbuilder, but this looks like a packaging problem of some kind)

[mars0i]

(Unrelated to jbuilder, but this looks like a packaging problem of some kind)

Right. Only the failing on unreadable directories issue is jbuilder's. The packaging problem just illustrates a case in which that matters more.

(This probably goes without saying, but I think it's important that one of the positive benefits of the ease of opam (for which I'm grateful) is that it allows new users to start using OCaml without being knowledgeable about OCaml compilation or dependencies or any particular build tool. Then you can gradually learn different pieces of what is, overall, a very rich set of tools and libraries. If a less-knowledgeable user gets a message while running opam that says "that didn't work, but try this", it should be entirely appropriate for the user simply try that in the way specified, without first, say, figuring out what jbuilder does. The user didn't know what opam was doing before that point, either, after all, and simply trusted it. (In my case, I no longer consider myself a rank novice, but there's a lot that I don't know or don't understand, and I don't have time to become an OCaml expert before proceeding. If I get a message from opam that says "try this because I couldn't build your package", then yeah, I'm going to try it, just like I tried running opam to install the package in the first place. I only started using jbuilder a couple of months ago--before that I only (kind of) knew oasis--and I know enough to do what I do with jbuilder, and keep learning more, but I don't have time to understand everything about it. The fact that the jbuilder docs aren't very good for the initial learning process doesn't help, but that's another issue.)

[dra27]

Sorry if the way I put the aside made it unclear - there's no question that your original issue about scanning unreadable directories is a bug to be fixed!

What I'm musing on is the value of changing the error message, and it's not totally clear-cut. For example, it's interesting that you dived in running a command rather than reading the line above which said that a package is missing (by 'interesting', I don't necessarily mean you wrong!)

[lpw25]

The issue here is that opam is trying to install core without all its required dependencies for some reason. When the build of core fails opam -- not unreasonably -- shows you the error message from the build to help you figure out what has gone wrong. Since core is using jbuilder for its build, this ends up showing you the message from jbuilder.

When jbuilder tries to build something and kind find a dependency it prints the hint "try: jbuilder external-lib-deps ..." to help you figure out what dependencies are missing. If you had been running jbuilder yourself in the core source directory this would be a very helpful message, but in the context of a package being built by opam it is actively harmful/confusing.

The solution is probably to try to distinguish these two different use cases -- building a package in a package manager and running jbuilder by hand -- and then tailor the error messages and hints accordingly.

[mars0i]

@lpw25 Ah, I see. It's jbuilder that inserts the hint message. I couldn't figure out where that was coming from.

@dra27, I guess what happens is that I expect opam to manage dependencies. If core needs base, I would expect base to get installed when I try to install core. If that doesn't happen, it seems weird, and I'm not sure what to do. As it turns out, running opam install base (and one or two other installs) before opam install core solves the problem, but that wasn't obvious to me. (I probably did that before, too, but that was a while ago, and I didn't remember how it was I got core installed.) If there's a problem I don't understand, and there's a hint at the end of the error message, it's natural for me to follow the wise advice of whoever who wrote the hint. Presumably they know more than I do. Understanding that the message is from jbuilder, now, it makes sense to possibly ignore it.

[mars0i]

@lpw25 Maybe it would be simpler for the hint to include full build directory path when the problem occurs. That should work both for the normal jbuild process and for opam use.

[lpw25]

The directory isn't what's wrong with the hint. Running that command is not going to help a user work out what is wrong in this situation any more than the error message Error: External library "base" not found. which appears before the hint. Indeed thinking that they are supposed to run that command is likely to confuse people, making the hint harmful even if it directed people to a directory where the command would be able to successfully run.

[mars0i]

Indeed thinking that they are supposed to run that command is likely to confuse people

Ah, I get it.

I am exhibit A. :-)

I think I should let someone else submit an issue about this, if that makes sense, since I don't understand the situation as well.

[rgrinberg]

#2004 fixes this. Interested parties should test that PR.