Skip to content

Adhere to XDG base directory spec #53312

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

Closed
wants to merge 3 commits into from

Conversation

SingularisArt
Copy link

I moved the default configuration directory from ~/.dart and ~/.dartServer to:

  1. Linux: ~/.config/dart and ~/.config/dartServer.
  2. Mac: ~/Library/Application Support/dart/.
  3. Windows: %APPDATA%/dart/

You can overwrite the ~/.config/dart path with the DART_CONFIG_DIR variable.
I believe this resolves #41560 for the most part.

@google-cla
Copy link

google-cla bot commented Aug 23, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@copybara-service
Copy link

Thank you for your contribution! This project uses Gerrit for code reviews. Your pull request has automatically been converted into a code review at:

https://dart-review.googlesource.com/c/sdk/+/322300

Please wait for a developer to review your code review at the above link; you can speed up the review if you sign into Gerrit and manually add a reviewer that has recently worked on the relevant code. See CONTRIBUTING.md to learn how to upload changes to Gerrit directly.

Additional commits pushed to this PR will update both the PR and the corresponding Gerrit CL. After the review is complete on the CL, your reviewer will merge the CL (automatically closing this PR).

@mraleph
Copy link
Member

mraleph commented Aug 23, 2023

/cc @srawlins @bwilkerson

@ykmnkmi
Copy link
Contributor

ykmnkmi commented Aug 23, 2023

Please add custom location support for both folders.

@srawlins
Copy link
Member

I love it. Also related, #42499 (DAS should use ~/.dart instead of .dartServer), #42813 (customize location), and #49166 (use ~/Library/Application Support, ~/Library/Caches and ~/Library/Preferences on mac.).

@SingularisArt
Copy link
Author

@ykmnkmi The ~/.config/dartServer path with the ANALYZER_STATE_LOCATION_OVERRIDE variable.

I'll update the macOS paths.

@srawlins I'm not sure if you're suggesting that we put the ~/.config/dart and the ~/.config/dartServer into the same folder.

@srawlins
Copy link
Member

I'm not suggesting anything. Just linking issues.

@SingularisArt
Copy link
Author

Is there anything I need to change to have this PR merged?

@srawlins
Copy link
Member

Lemme review with the team. Not sure of any consequences of moving this directory between releases.

@jacob314
Copy link
Member

jacob314 commented Oct 7, 2023

Ping?

@mraleph
Copy link
Member

mraleph commented Oct 19, 2023

@srawlins is this on your plate now?

@srawlins
Copy link
Member

Ah sorry, missed the ping during the conference. Will discuss this morning in a team meeting.

@srawlins
Copy link
Member

Sorry for the delay in pointing this out, but as copybara-service points out above, the code review is done at https://dart-review.googlesource.com/c/sdk/+/322300, and there are some unresolved comments there. Please look at those and upload a new commit if you make changes in response to the comments.

@srawlins
Copy link
Member

XDG actually specifies a few override environment variables, which might be interesting or important here:

There is a single base directory relative to which user-specific non-essential (cached) data should be written. This directory is defined by the environment variable $XDG_CACHE_HOME.

This change could be an opportunity to use $XDG_CACHE_HOME, which might allow the OS to clean the cache occasionally. Today, with DAS's cache in $HOME/.dartServer, we can store GBs of cache in there, and the OS does not clean it up.

I think not necessary for this PR, but it would be good to address this while it's paged into our brains.

@bas-ie
Copy link

bas-ie commented Oct 25, 2023

Yeah, agree with @srawlins. Most people use XDG to keep clutter out of their $HOME and have a predictable location to find various file types. Which means:

  • config lives in $XDG_CONFIG_HOME (usually ~/.config)
  • cache lives in $XDG_CACHE_HOME (usually ~/.cache)
  • data lives in $XDG_DATA_HOME (usually ~/.local/share)
  • executables live in $XDG_BIN_HOME (usually ~/.local/bin)
  • libraries live in $XDG_LIB_HOME (usually ~/.local/lib)

So for example, just now when I installed patrol, I'd normally expect to see it show up in ~/.local/bin, but instead it's in ~/.pub-cache/bin which is a bit odd as it's not a cache file. This prompts pub to tell me to add ~/.pub-cache/bin to my path, which it wouldn't have to do with XDG since ~/.local/bin is already on my path.

Note also that people do use XDG on MacOS (me, for one, on my work laptop) so we probably shouldn't assume it's a Linux-only phenomenon.

@bas-ie
Copy link

bas-ie commented Oct 25, 2023

If it's helpful (and mostly for selfish reasons, so I get to clean up my $HOME 😎) I could submit a PR that more completely follows the spec, after this one is merged?

@srawlins
Copy link
Member

Actually, sorry to say this, but I think in order to minimize any intermediate locations, we should switch from $XDG_CONFIG_HOME to $XDG_CACHE_HOME before landing this; we offer nightly (ish?) builds on the Flutter master channel, so any commit could be published to users.

We chatted internally, and we'd like to provide a mechanism that slowly cleans up the data in the old location, using an existing mechanism: there is code that slowly removes old cache entries, and we can use that to remove the data in ~/.dartServer after users switch to $XDG_CACHE_HOME. I like this mechanism, but wouldn't want to apply it to more and more directories as we shift where the data is stored.

CC @DanTup @scheglov would you say everything in ~/.dartServer is "cache" information, and none of it is "configuration" data? I think so, but I'm not sure. And it looks like the analytics data is moved (in this PR) from ~/.dart/ to $DART_CONFIG_DIR ?? $XDG_CONFIG_HOME/dart/ (basically), so that seems good.

I think all I'm asking for then is to change pkg/analyzer/lib/file_system/physical_file_system.dart to use $XDG_CACHE_HOME instead of $XDG_CONFIG_HOME.

@bas-ie
Copy link

bas-ie commented Oct 26, 2023

If it's analytics data, I'd argue it belongs in $XDG_DATA_HOME/dart TBH! ~/.config/dart would be reserved for things like configuration files specific to Dart itself which, I guess there aren't any? 🙂 And cache data is things that are actually cache files i.e. semi-temporary, won't be a disaster if they get deleted etc.

@srawlins
Copy link
Member

I'd argue it belongs in $XDG_DATA_HOME/dart TBH!

That sounds fair.

And cache data is things that are actually cache files i.e. semi-temporary, won't be a disaster if they get deleted etc.

True. But we want to be careful not to blow away these directories with abandon. For example, one design might be to delete the ~/.dartServer directory any time we see it on server start up. But if the user has another Dart SDK in play, an older Flutter or an in-progress migration, and an IDE launching an older Dart Analysis Server, then there could be performance or stability issues. Similarly, if we stored server cache data in one directory, and then a week later we stored it in another directory, as commits land, we'd have to track those intermediate locations for the gradual data removal.

@DanTup
Copy link
Collaborator

DanTup commented Oct 26, 2023

CC @DanTup @scheglov would you say everything in ~/.dartServer is "cache" information, and none of it is "configuration" data?

Here's what I have in this folder (C:\Users\danny\AppData\Local\.dartServer on Windows) and my understanding of them (which may or may not be completely correct!)

  • .analysis-driver - cache - I believe this is all cache that could be thrown away (and I think is periodically cleaned up by _cacheCleanUpFunction in analyzer\lib\src\dart\analysis\file_byte_store.dart)
  • .dcm-teams-170 - cache - I think this is the same cache for DCM.. I don't know if the path comes from the analyzer or DCM itself (or whether it gets the same cleanup - @incendial?)
  • .instrumentation - ??? this contains a single uuid file - unsure what this is used for
  • .plugin_manager - cache - I think these are cached versions of plugins that could also be thrown away (and will rebuild)?
  • .prompts - not cache these are user preferences (such as saying "don't show me again" to the prompt about being able to run "dart fix"
  • .pub-package-details-cache - cache - this is a list of pub packages we fetch from Pub used in code-completion in pubspec.yaml
  • one - test - generated by server integration tests
  • two - test - generated by server integration tests

There's also some Dart-related stuff in (note: Roaming, not Local):

  • C:\Users\danny\AppData\Roaming\.dart
  • C:\Users\danny\AppData\Roaming\.dart-tool

These both seem telemetry related, but I'm not sure who is writing them (I suspect the new unified analytics package is doing at least one of them). I'm not sure how these two different Windows paths (Local AppData vs RoamingAppData) map on to Mac/Linux (I suspect they're merging into the home dir?).

@incendial
Copy link
Contributor

.dcm-teams-170 - cache - I think this is the same cache for DCM.. I don't know if the path comes from the analyzer or DCM itself (or whether it gets the same cleanup - @incendial?)

Yes, it's just cache and we use the analyzer's API to get the location. And yes, the same bytestore is used so _cacheCleanUpFunction gets called as well.

@bwilkerson
Copy link
Member

@eliasyishak Could you look at the #53312 (comment) for the telemetry question?

@eliasyishak
Copy link
Contributor

eliasyishak commented Oct 26, 2023

There's also some Dart-related stuff in (note: Roaming, not Local):

C:\Users\danny\AppData\Roaming.dart
C:\Users\danny\AppData\Roaming.dart-tool

Yes, the .dart directory was used for the legacy analytics and we are now using .dart-tool for the new unified analytics approach. If we wanted to move these files around, we may have to look into updating our privacy docs right? Those locations were already approved

@mraleph
Copy link
Member

mraleph commented Nov 20, 2023

@srawlins friendly ping.

@srawlins
Copy link
Member

Thanks for the ping, @mraleph . Closing.

OP has not reponded since I asked on Oct 19 to respond to code review comments.

@SingularisArt feel free to re-open.

@srawlins srawlins closed this Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adhere to XDG base directory spec
10 participants