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

Do not fail if $HOME/.ccache is missing #3957

Merged
merged 3 commits into from Sep 2, 2019
Merged

Do not fail if $HOME/.ccache is missing #3957

merged 3 commits into from Sep 2, 2019

Conversation

@mseri
Copy link
Member

mseri commented Aug 15, 2019

mseri added 2 commits Aug 15, 2019
    See https://discuss.ocaml.org/t/dune-1-11-1-compilation-failed/4248

Signed-off-by: Marcello Seri <marcello.seri@gmail.com>
@rjbou

This comment has been minimized.

Copy link
Collaborator

rjbou commented Aug 29, 2019

Thanks for the PR!
It remains a tricky case that is not handled by current proposition : ccache configuration file only system wide, cache_dir changed in this conf file, and $HOME/.ccache directory missing.
Moving existence directory check as a condition for add_mount call would handle all cases.

This was already present in bwrap, so only sandbox needs to be updated

Signed-off-by: Marcello Seri <marcello.seri@gmail.com>
@mseri

This comment has been minimized.

Copy link
Member Author

mseri commented Aug 29, 2019

Indeed! This was actually checked in bwrap.sh in the add_mounts function, I have reverted the change and updated sandbox_exec.sh moving the check as you suggested.

@rjbou

This comment has been minimized.

Copy link
Collaborator

rjbou commented Aug 29, 2019

Thanks! If no objection, i'll change PR description when merging.

@mseri

This comment has been minimized.

Copy link
Member Author

mseri commented Aug 29, 2019

Sure, do as you see fit!

@AltGr AltGr added the PR: LGTM label Aug 30, 2019
@rjbou rjbou merged commit b963ef2 into ocaml:master Sep 2, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mseri mseri deleted the mseri:patch-1 branch Sep 2, 2019
rjbou added a commit to OCamlPro/opam that referenced this pull request Sep 4, 2019
bwrap/osx: do not fail if $HOME/.ccache is missing: check for directory existence in `add_mounts`

See https://discuss.ocaml.org/t/dune-1-11-1-compilation-failed/4248

This was already present in bwrap, so only sandbox needs to be updated

Signed-off-by: Marcello Seri <marcello.seri@gmail.com>
@rjbou rjbou mentioned this pull request Sep 5, 2019
rjbou added a commit to OCamlPro/opam that referenced this pull request Oct 16, 2019
bwrap/osx: do not fail if $HOME/.ccache is missing: check for directory existence in `add_mounts`

See https://discuss.ocaml.org/t/dune-1-11-1-compilation-failed/4248

This was already present in bwrap, so only sandbox needs to be updated

Signed-off-by: Marcello Seri <marcello.seri@gmail.com>
@rjbou rjbou removed the PR: LGTM label Oct 29, 2019
@rjbou rjbou added this to the 2.0.6 milestone Oct 29, 2019
rjbou added a commit to rjbou/opam that referenced this pull request Dec 10, 2019
bwrap/osx: do not fail if $HOME/.ccache is missing: check for directory existence in `add_mounts`

See https://discuss.ocaml.org/t/dune-1-11-1-compilation-failed/4248

This was already present in bwrap, so only sandbox needs to be updated

Signed-off-by: Marcello Seri <marcello.seri@gmail.com>
rjbou added a commit to rjbou/opam that referenced this pull request Dec 11, 2019
bwrap/osx: do not fail if $HOME/.ccache is missing: check for directory existence in `add_mounts`

See https://discuss.ocaml.org/t/dune-1-11-1-compilation-failed/4248

This was already present in bwrap, so only sandbox needs to be updated

Signed-off-by: Marcello Seri <marcello.seri@gmail.com>
rjbou added a commit to OCamlPro/opam that referenced this pull request Dec 13, 2019
bwrap/osx: do not fail if $HOME/.ccache is missing: check for directory existence in `add_mounts`

See https://discuss.ocaml.org/t/dune-1-11-1-compilation-failed/4248

This was already present in bwrap, so only sandbox needs to be updated

Signed-off-by: Marcello Seri <marcello.seri@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.