-
Notifications
You must be signed in to change notification settings - Fork 414
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix dune clean
on esy
#5507
fix dune clean
on esy
#5507
Conversation
- this patch can be removed after ocaml/dune#5507 is merged
- this patch can be removed after ocaml/dune#5507 is merged
- this patch can be removed after ocaml/dune#5507 is merged
- this patch can be removed after ocaml/dune#5507 is merged
Linked issue #5377 |
@@ -116,9 +116,7 @@ and rm_rf_dir path = | |||
deleted the directory. *) | |||
()) | |||
|
|||
let rm_rf ?(allow_external = false) fn = | |||
if (not allow_external) && not (Filename.is_relative fn) then | |||
Code_error.raise "Path.rm_rf called on external dir" [ ("fn", String fn) ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a good check to keep when performing an operation this destructive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But yes in general I would prefer removing this function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you update CHANGES?
DCO Action required after 1s — DCO
And sign the DCO
b9d9eb1
to
b1ae470
Compare
Signed-off-by: EduardoRFS <[email protected]>
b1ae470
to
81d58e4
Compare
Done boss |
…ne-site, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info and dune-action-plugin (3.1.0) CHANGES: - Add `sourcehut` as an option for defining project sources in dune-project files. For example, `(source (sourcehut user/repo))`. (ocaml/dune#5564, @rgrinberg) - Add `dune coq top` command for running a Coq toplevel (ocaml/dune#5457, @rlepigre) - Fix dune exec dumping database in wrong directory (ocaml/dune#5544, @bobot) - Always output absolute paths for locations in RPC reported diagnostics (ocaml/dune#5539, @rgrinberg) - Add `(deps <deps>)` in ctype field (ocaml/dune#5346, @bobot) - Add `(include <file>)` constructor to dependency specifications. This can be used to introduce dynamic dependencies (ocaml/dune#5442, @anmonteiro) - Ensure that `dune describe` computes a transitively closed set of libraries (ocaml/dune#5395, @esope) - Add direct dependencies to $ dune describe output (ocaml/dune#5412, @esope) - Show auto-detected concurrency on Windows too (ocaml/dune#5502, @MisterDA) - Fix operations that remove folders with absolute path. This happens when using esy (ocaml/dune#5507, @EduardoRFS) - Dune will not fail if some directories are non-empty when uninstalling. (ocaml/dune#5543, fixes ocaml/dune#5542, @nojb) - `coqdep` now depends only on the filesystem layout of the .v files, and not on their contents (ocaml/dune#5547, helps with ocaml/dune#5100, @ejgallego) - The mdx stanza 0.2 can now be used with `(implicit_transitive_deps false)` (ocaml/dune#5558, fixes ocaml/dune#5499, @emillon) - Fix missing parenthesis in printing of corresponding terminal command for `(with-outputs-to )` (ocaml/dune#5551, fixes ocaml/dune#5546, @Alizter)
Problem
esy uses
DUNE_BUILD_DIR
pointing to an absolute directory, but since 0f31294 dune rejects removing any absolute directory, as a new checking was added.Leading to
dune clean
to fail, and also basic functionality such asdune build -w
.Solution
This PR removes the checking, as the patch was likely a mistaken and not all absolute directories are external.