Skip to content
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

opam clean: Ignore errors trying to remove directories #3732

Merged
merged 1 commit into from Feb 5, 2019

Conversation

Projects
None yet
2 participants
@kit-ty-kate
Copy link
Contributor

commented Jan 28, 2019

The opam clean command seems slightly too strict when it comes to errors in subcommands.
My use-case here is that I have mounted ~/.opam/4.07.1/.opam-switch/build into tmpfs to avoid unnecessary write and disk space taken on my SSD but when I try to execute opam clean to remove the archive cache for example, it fails trying to remove this directory.

My proposal here is to ignore all failures to when trying to remove directories during opam clean and instead display a note to the user telling this command failed and has been ignored.

@@ -2930,14 +2930,16 @@ let clean =
OpamConsole.msg "rm -rf \"%s\"/*\n"
(OpamFilename.Dir.to_string d)
else
OpamFilename.cleandir d
try OpamFilename.cleandir d
with OpamSystem.Internal_error msg -> OpamConsole.note "Error ignored: %s" msg

This comment has been minimized.

Copy link
@rjbou

rjbou Jan 28, 2019

Collaborator

I'd prefer warning instead of simple note

This comment has been minimized.

Copy link
@kit-ty-kate

kit-ty-kate Jan 28, 2019

Author Contributor

Fair enough, fixed

in
let rmdir d =
if dry_run then
OpamConsole.msg "rm -rf \"%s\"\n"
(OpamFilename.Dir.to_string d)
else
OpamFilename.rmdir d
try OpamFilename.rmdir d
with OpamSystem.Internal_error msg -> OpamConsole.note "Error ignored: %s" msg

This comment has been minimized.

Copy link
@rjbou

rjbou Jan 28, 2019

Collaborator

same here

@kit-ty-kate kit-ty-kate force-pushed the kit-ty-kate:opam-clean-ignore-errors branch from c33bc6d to a13d544 Jan 28, 2019

@rjbou

This comment has been minimized.

Copy link
Collaborator

commented Jan 28, 2019

lgtm, thanks!

@rjbou rjbou merged commit 13a48b5 into ocaml:master Feb 5, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

rjbou added a commit to rjbou/opam that referenced this pull request Feb 20, 2019

@rjbou rjbou added this to the 2.1.0 milestone Feb 28, 2019

rjbou added a commit to rjbou/opam that referenced this pull request Mar 27, 2019

rjbou added a commit to rjbou/opam that referenced this pull request Mar 27, 2019

rjbou added a commit to rjbou/opam that referenced this pull request Mar 27, 2019

@rjbou rjbou referenced this pull request Mar 27, 2019

Merged

2.0.4 backported commits #3805

@rjbou rjbou modified the milestones: 2.1.0, 2.0.4 Mar 28, 2019

rjbou added a commit to rjbou/opam that referenced this pull request Mar 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.