Fix macOS dependency cache. #14795
Fix macOS dependency cache. #14795
Conversation
aarlt
force-pushed
the
osx-caching
branch
3 times, most recently
from
717648f
to
aff81f8
Compare
|
I just wondered about the message while saving the cache
So the change seem to be ok - even for intel macs ;) |
aarlt
force-pushed
the
osx-caching
branch
2 times, most recently
from
78a3545
to
d6bb160
Compare
aarlt
force-pushed
the
osx-caching
branch
2 times, most recently
from
24a8c68
to
fd699df
Compare
.circleci/config.yml
Outdated
| sudo chmod 777 /usr/local | ||
| sudo chmod 777 /usr/local/bin |
There was a problem hiding this comment.
This yields
Unable to set permissions on "/usr/local/bin" - chmod /usr/local/bin: operation not permitted
Unable to set permissions on "/usr/local" - chmod /usr/local: operation not permitted
in the b_osx restore cache step, although it doesn't seem to cause any issues.
There was a problem hiding this comment.
the thing is that it still works and its also needed. if not set, the cache restore is not possible, because it's not allowed to write into these directories. I don't know why circleci is generating these messages.
.circleci/config.yml
Outdated
| sudo chmod 777 /usr/local | ||
| sudo chmod 777 /usr/local/bin |
There was a problem hiding this comment.
Why it needs to be set to 777 and not 755 (which I believe is the default)?
There was a problem hiding this comment.
I was just seeing strange errors when I was restoring the cache.. but you might be right, that 755 is sufficient
There was a problem hiding this comment.
It might be needed to have those set to 777 before restoring the cache.. lets see
There was a problem hiding this comment.
ok its needed before the cache gets restored:
Skipping writing "usr/local/Cellar/boost/1.83.0/include/boost/align/detail/alignment_of_gcc.hpp" - mkdir /usr/local/Cellar: permission denied
Skipping writing "usr/local/Cellar/boost/1.83.0/include/boost/align/detail/alignment_of_msvc.hpp" - mkdir /usr/local/Cellar: permission denied
Skipping writing "usr/local/Cellar/boost/1.83.0/include/boost/align/detail/assume_aligned.hpp" - mkdir /usr/local/Cellar: permission denied
Skipping writing "usr/local/Cellar/boost/1.83.0/include/boost/align/detail/assume_aligned_clang.hpp" - mkdir /usr/local/Cellar: permission denied
Skipping writing "usr/local/Cellar/boost/1.83.0/include/boost/align/detail/assume_aligned_gcc.hpp" - mkdir /usr/local/Cellar: permission denied
Skipping writing "usr/local/Cellar/boost/1.83.0/include/boost/align/detail/assume_aligned_intel.hpp" - mkdir /usr/local/Cellar: permission denied
Skipping writing "usr/local/Cellar/boost/1.83.0/include/boost/align/detail/assume_aligned_msvc.hpp" - mkdir /usr/local/Cellar: permission denied
Skipping writing "usr/local/Cellar/boost/1.83.0/include/boost/align/detail/element_type.hpp" - mkdir /usr/local/Cellar: permission denied
Skipping writing "usr/local/Cellar/boost/1.83.0/include/boost/align/detail/integral_constant.hpp" - mkdir /usr/local/Cellar: permission denied
Skipping writing "usr/local/Cellar/boost/1.83.0/include/boost/align/detail/is_aligned.hpp" - mkdir /usr/local/Cellar: permission denied
There was a problem hiding this comment.
but youre right that its better to have those not set to 777 after the cache restore
There was a problem hiding this comment.
I think we can just ignore them. The fact that it fails when you set 755 means that they probably are set to 777 in the image to begin with. I'd rather not restore them and keep the CI script less complicated.
There was a problem hiding this comment.
the cache restore is not executed as root, an thus mean that with 755 the directory is not writable for others (owner is root here). however, I agree to let the CI script as simple as possible. I will remove it.
There was a problem hiding this comment.
the cache restore is not executed as root, an thus mean that with
755the directory is not writable for others (owner is root here
Which means that, like I said, it's probably not 755 but rather 777 by default, right?
I mean, I'm not sure what you meant. Your comment does not contradict what I said but, the "however" makes it sound as it was meant to say it's wrong? :)
aarlt
force-pushed
the
osx-caching
branch
2 times, most recently
from
880095d
to
36b14b6
Compare
There was a problem hiding this comment.
I have some suggestions but I'm going to just push them as fixups. The only remaining things are the rm /usr/local/* vs rm /opt/homebrew thing and the permissions. Both trivial to change.
If you're fine with my fixups when once the comments are resolved, we can merge.
.circleci/config.yml
Outdated
| sudo chmod 777 /usr/local | ||
| sudo chmod 777 /usr/local/bin |
There was a problem hiding this comment.
I think we can just ignore them. The fact that it fails when you set 755 means that they probably are set to 777 in the image to begin with. I'd rather not restore them and keep the CI script less complicated.
aarlt
force-pushed
the
osx-caching
branch
2 times, most recently
from
2288f72
to
98f2b83
Compare
There was a problem hiding this comment.
Did one last look through this and noticed a few more things. The important one is whether we're not forgetting to cache /usr/local/Cellar/ (or its equivalent on ARM). The rest is about adding some more comments that explain why things are done the way they are.
.circleci/config.yml
Outdated
| - run: | ||
| name: chmod 755 for /opt/homebrew & /usr/local | ||
| command: | | ||
| [[ -d /opt/homebrew ]] && sudo chmod 755 /opt/{homebrew,homebrew/bin} | ||
| sudo chmod 755 /usr/{local,local/bin} |
There was a problem hiding this comment.
Not sure if I missed it when I looked last time or if you added it back. I thought we agree to remove it (#14795 (comment)).
There was a problem hiding this comment.
ups, yeah looks like it just came back.. will remove that now
Fixes #12925.