Conversation
ejgallego
left a comment
There was a problem hiding this comment.
Commenting to remove my review request [which seems spurious]
4b888b0 to
48f47b4
Compare
|
I also made our symlink check warn instead of error. That seems to be a bit more friendly. |
48f47b4 to
3c11afe
Compare
|
I've reverted to the old behavior regarding symlinks. It's not working in CI for some reason. |
|
@diml this is ready by the way |
3c11afe to
aa31743
Compare
|
One concrete situation where the extra laziness helped was to avoid sorting directories when they were all ignored. That seems like an edge case though. Now that we're only catching Unix errors, I've changed the result type to |
5bd289b to
6cf0253
Compare
|
@emillon or @aalekseyev, mind taking over the review? |
48fbbbd to
b446fd6
Compare
|
I'll add a test here by the way. |
|
By the way, this is the last PR that I'd like to get in for 1.9 |
|
Ok, I had another look. This looks good, but why did you make the loading of dune-project files lazy? In practice, it means that errors in dune-project files might not be reported as eagerly as before which is not great. Apart from this, it looks good to me. |
6ac5efb to
3651804
Compare
|
@diml I removed the laziness and I've also added another feature: If the directory is not readable, it will not be added as a node to |
5f17026 to
a2b62d4
Compare
|
Ok, that seems good |
Any node that fails to be read will now just emit a warning and be otherwise ignored in the build. Signed-off-by: Rudi Grinberg <rudi.grinberg@gmail.com>
|
@diml your last change makes it so that |
7687d35 to
888178d
Compare
|
Ok. Yh it doesn't matter in practice, the only reason |
CHANGES: - Warn when generated `.merlin` does not reflect the preprocessing specification. This occurs when multiple stanzas in the same directory use different preprocessing specifications. This warning can now be disabled with `allow_approx_merlin` (ocaml/dune#1947, fix ocaml/dune#1946, @rgrinberg) - Watch mode: display "Success" in green and "Had errors" in red (ocaml/dune#1956, @emillon) - Fix glob dependencies on installed directories (ocaml/dune#1965, @rgrinberg) - Add support for library variants and default implementations. (ocaml/dune#1900, @TheLortex) - Add experimental `$ dune init` command. This command is used to create or update project boilerplate. (ocaml/dune#1448, fixes ocaml/dune#159, @shonfeder) - Experimental Coq support (fix ocaml/dune#1466, @ejgallego) - Install .cmi files of private modules in a `.private` directory (ocaml/dune#1983, fix ocaml/dune#1973 @rgrinberg) - Fix `dune subst` attempting to substitute on directories. (ocaml/dune#2000, fix ocaml/dune#1997, @rgrinberg) - Do not list private modules in the generated index. (ocaml/dune#2009, fix ocaml/dune#2008, @rgrinberg) - Warn instead of failing if an opam file fails to parse. This opam file can still be used to define scope. (ocaml/dune#2023, @rgrinberg) - Do not crash if unable to read a directory when traversing to find root (ocaml/dune#2024, @rgrinberg) - Do not exit dune if some source directories are unreadable. Instead, warn the user that such directories need to be ignored (ocaml/dune#2004, fix ocaml/dune#310, @rgrinberg) - Fix nested `(binaries ..)` fields in the `env` stanza. Previously, parent `binaries` fields would be ignored, but instead they should be combined. (ocaml/dune#2029, @rgrinberg) - Allow "." in `c_names` and `cxx_names` (ocaml/dune#2036, fix ocaml/dune#2033, @rgrinberg)
CHANGES: - Warn when generated `.merlin` does not reflect the preprocessing specification. This occurs when multiple stanzas in the same directory use different preprocessing specifications. This warning can now be disabled with `allow_approx_merlin` (ocaml/dune#1947, fix ocaml/dune#1946, @rgrinberg) - Watch mode: display "Success" in green and "Had errors" in red (ocaml/dune#1956, @emillon) - Fix glob dependencies on installed directories (ocaml/dune#1965, @rgrinberg) - Add support for library variants and default implementations. (ocaml/dune#1900, @TheLortex) - Add experimental `$ dune init` command. This command is used to create or update project boilerplate. (ocaml/dune#1448, fixes ocaml/dune#159, @shonfeder) - Experimental Coq support (fix ocaml/dune#1466, @ejgallego) - Install .cmi files of private modules in a `.private` directory (ocaml/dune#1983, fix ocaml/dune#1973 @rgrinberg) - Fix `dune subst` attempting to substitute on directories. (ocaml/dune#2000, fix ocaml/dune#1997, @rgrinberg) - Do not list private modules in the generated index. (ocaml/dune#2009, fix ocaml/dune#2008, @rgrinberg) - Warn instead of failing if an opam file fails to parse. This opam file can still be used to define scope. (ocaml/dune#2023, @rgrinberg) - Do not crash if unable to read a directory when traversing to find root (ocaml/dune#2024, @rgrinberg) - Do not exit dune if some source directories are unreadable. Instead, warn the user that such directories need to be ignored (ocaml/dune#2004, fix ocaml/dune#310, @rgrinberg) - Fix nested `(binaries ..)` fields in the `env` stanza. Previously, parent `binaries` fields would be ignored, but instead they should be combined. (ocaml/dune#2029, @rgrinberg) - Allow "." in `c_names` and `cxx_names` (ocaml/dune#2036, fix ocaml/dune#2033, @rgrinberg) - Format rules: if a dune file uses OCaml syntax, do not format it. (ocaml/dune#2014, fix ocaml/dune#2012, @emillon)
This PR makes it so that unreadable directories are treated as empty. Any time an unreadable dir is encountered, we report this as a warning.
One thing that would have been nicer would be to completely exclude them from the contents, but that breaks laziness a bit. Because we have to try reading the dir before adding it to the map.