Respect XDG directory settings on macOS #25205
Conversation
github-actions
bot
added
team-Rules-CPP
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
(For general background on XDG_* env variables on mac, see e.g. https://stackoverflow.com/questions/3373948/equivalents-of-xdg-config-home-and-xdg-data-home-on-mac-os-x) |
tetromino
added
team-CLI
|
Ok, I've done some git archaeology. Already in 2016 (commit f107deb) @lberki removed the Unix domain socket file on both linux and mac, and after a rollback and a fix, the change stuck. In the internal review for f107deb a reviewer noted that this change meant Bazel can switch back from /var/tmp to ${HOME}/Library/Caches (the "normal" cache directory on mac), however this was not followed up upon and not done. Currently on a mac, My conclusion is that this PR is mostly ok, but we should follow up by also moving to the expected cache dir on mac. |
tetromino
left a comment
tetromino
left a comment
There was a problem hiding this comment.
Please add a comment explaining.
Something like:
string GetCacheDir() {
// See https://stackoverflow.com/questions/3373948/equivalents-of-xdg-config-home-and-xdg-data-home-on-mac-os-x
// for general background. On macOS, the normal cache directory is
// ${HOME}/Library/Caches; Bazel historically instead used /var/tmp due to
// path length limits on Unix domain sockets. (Those limits are no longer
// relevant since we no longer create Unix domain sockets under output_base.)
// However, it is still useful to respect ${XDG_CACHE_HOME} because it allows
// users to easily override the cache directory and is respected by many
// Linux-derived tools.
...
|
@tetromino Thank you for the feedback – I believe I've incorporated what you've asked for. |
|
I don't think these Windows failures are due to my changes. |
tetromino
added
awaiting-PR-merge
github-actions
bot
removed
the
awaiting-PR-merge
|
@philwo benchmarked that this change noticeably improves build performance on Macs with many cores (e.g. Mac Studio M2 Ultra). Maybe we ought to cherry-pick it into older branches... |
|
@bazel-io fork 8.4.0 |
Fixes bazelbuild#25167 Copies much of the logic over from `blaze_util_linux.cc` and its tests into the Darwin space to implement the feature request. Closes bazelbuild#25205. PiperOrigin-RevId: 725622080 Change-Id: I46a65076bdea4c0b1989b4816bc41efb123b14be
Fixes #25167 Copies much of the logic over from `blaze_util_linux.cc` and its tests into the Darwin space to implement the feature request. Closes #25205. PiperOrigin-RevId: 725622080 Change-Id: I46a65076bdea4c0b1989b4816bc41efb123b14be Commit f6d71e5 Co-authored-by: Aaron Sky <[email protected]>
Fixes #25260 Implements the request made [here](#25205 (comment)) and moves the default output root on macOS from `/private/var/tmp` to `$HOME/Library/Caches`. May constitute an incompatible change, though on a cursory glance I didn't see any precedent for incompatible startup options in source. Closes #25262. PiperOrigin-RevId: 840604399 Change-Id: I391b805b2c8e99d3ffe44a76e916d1c9839225e5
Fixes bazelbuild#25260 Implements the request made [here](bazelbuild#25205 (comment)) and moves the default output root on macOS from `/private/var/tmp` to `$HOME/Library/Caches`. May constitute an incompatible change, though on a cursory glance I didn't see any precedent for incompatible startup options in source. Closes bazelbuild#25262. PiperOrigin-RevId: 840604399 Change-Id: I391b805b2c8e99d3ffe44a76e916d1c9839225e5
…27875) Fixes #25260 Implements the request made [here](#25205 (comment)) and moves the default output root on macOS from `/private/var/tmp` to `$HOME/Library/Caches`. May constitute an incompatible change, though on a cursory glance I didn't see any precedent for incompatible startup options in source. Closes #25262. PiperOrigin-RevId: 840604399 Change-Id: I391b805b2c8e99d3ffe44a76e916d1c9839225e5 Commit 13da9a1 Co-authored-by: Aaron Sky <[email protected]>
3 participants
Fixes #25167
Copies much of the logic over from
blaze_util_linux.ccand its tests into the Darwin space to implement the feature request.