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

Respect XDG directory settings on macOS #25205

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

aaronsky
Copy link

@aaronsky aaronsky commented Feb 5, 2025

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.

@aaronsky aaronsky marked this pull request as ready for review February 5, 2025 14:49
@github-actions github-actions bot added team-Rules-CPP Issues for C++ rules team-Documentation Documentation improvements that cannot be directly linked to other team labels awaiting-review PR is awaiting review from an assigned reviewer labels Feb 5, 2025
tetromino

This comment was marked as outdated.

@tetromino
Copy link
Contributor

(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 tetromino added team-CLI Console UI and removed team-Rules-CPP Issues for C++ rules labels Feb 5, 2025
@tetromino
Copy link
Contributor

tetromino commented Feb 5, 2025

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, find /private/var/tmp/_bazel* -type s finds nothing; it looks like we really are not using Unix domain sockets any more.

My conclusion is that this PR is mostly ok, but we should follow up by also moving to the expected cache dir on mac.

Copy link
Contributor

@tetromino tetromino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
  ...

@aaronsky
Copy link
Author

aaronsky commented Feb 5, 2025

@tetromino Thank you for the feedback – I believe I've incorporated what you've asked for.

@aaronsky aaronsky requested a review from tetromino February 5, 2025 22:55
@aaronsky
Copy link
Author

aaronsky commented Feb 7, 2025

I don't think these Windows failures are due to my changes.

@aaronsky aaronsky requested a review from tetromino February 7, 2025 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review PR is awaiting review from an assigned reviewer team-CLI Console UI team-Documentation Documentation improvements that cannot be directly linked to other team labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Respect XDG directory settings on macOS
2 participants