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

Switch to using LOCALAPPDATA as default location on Windows #946

Closed
mrjoel opened this issue Oct 15, 2021 · 8 comments · Fixed by #1124
Closed

Switch to using LOCALAPPDATA as default location on Windows #946

mrjoel opened this issue Oct 15, 2021 · 8 comments · Fixed by #1124
Labels
improvement Improvement that is not a bug fix or new feature os: windows Related to building or running on Windows
Milestone

Comments

@mrjoel
Copy link

mrjoel commented Oct 15, 2021

Description

The current default on Windows of using getenv("APPDATA") results in the default location being in "${USERHOME}/AppData/Roaming". In many enterprise environments that is part of a Windows roaming profile, and is synchronized across a potentially slow network connection on login (i.e. during the login screen when no interaction is possible). The usage of this location by ccache has the unfortunate side-effect of synchronizing a large cache of binary data that is [mostly] inherently tied to a local machine (compiler versions, paths, etc.).

Instead, ccache should default to using LOCALAPPDATA instead which is intended for use by application to store local-only data (email cache, browser cache, etc.).

I'll try to draft a PR in the next week or so, with the following considerations in mind:

  • The new default precedence for the config file should be to use ccache.conf in LOCALAPPDATA, then APPDATA. This provides a way to share config while allowing override on a per-machine basis. It also maps well to the existing configuration locations so no migration is required.
  • The new default cache directory should be at LOCALAPPDATA/ccache
  • Issue a warning if a cache directory is detected at APPDATA/ccache but is not explicitly configured for that location. The warning should include a recommendation to migrate/delete the cache location.
  • Continue using the APPDATA/ccache as a default iff it exists but the LOCALAPPDATA/ccache version does not
@mrjoel mrjoel added the improvement Improvement that is not a bug fix or new feature label Oct 15, 2021
@jrosdahl jrosdahl added the os: windows Related to building or running on Windows label Oct 30, 2021
@jrosdahl
Copy link
Member

Instead, ccache should default to using LOCALAPPDATA instead which is intended for use by application to store local-only data (email cache, browser cache, etc.).

Sounds good to me. For reference, the use of APPDATA comes from 92cc99e.

  • The new default precedence for the config file should be to use ccache.conf in LOCALAPPDATA, then APPDATA. This provides a way to share config while allowing override on a per-machine basis. It also maps well to the existing configuration locations so no migration is required.

OK.

  • The new default cache directory should be at LOCALAPPDATA/ccache

OK.

  • Issue a warning if a cache directory is detected at APPDATA/ccache but is not explicitly configured for that location. The warning should include a recommendation to migrate/delete the cache location.

This is unfortunately not possible since ccache should not emit anything other than what the compiler would do. The options are either to silently use the new location or to print an error message and exit with an error code.

  • Continue using the APPDATA/ccache as a default iff it exists but the LOCALAPPDATA/ccache version does not

OK.

@mrjoel
Copy link
Author

mrjoel commented Oct 30, 2021

  • Issue a warning if a cache directory is detected at APPDATA/ccache but is not explicitly configured for that location. The warning should include a recommendation to migrate/delete the cache location.

This is unfortunately not possible since ccache should not emit anything other than what the compiler would do. The options are either to silently use the new location or to print an error message and exit with an error code.

Ah, yes indeed. Offhand I lean toward the side of exiting with an error, but will consider it further. While the error would be a breaking change on upgrade, combined with point four (continue using previous location of two don't exist) I think it'd be a) a very uncommon case, and b) alerting to a legitimately ambiguous situation, and c) alert to a situation of possibly large cache using disk space without serving any purpose. On the other hand, depending on build system etc. actually seeing the error message may be hit or miss, leading to undue confusion.

@jrosdahl
Copy link
Member

@mrjoel: Are you still interested in working on this?

@mrjoel
Copy link
Author

mrjoel commented Mar 26, 2022

Yes, although it's been back burnered for the last bit. If you're asking because someone else would like to do it before I get to it then that's completely fine. If you're asking whether it's worth keeping open then please do.

@jrosdahl
Copy link
Member

jrosdahl commented May 1, 2022

@mrjoel: In #1023, there is a suggestion to use either %HOMEPATH% or %USERPROFILE% (with higher priority than %HOME% so that the location is consistent in Windows native environments as well as Unix-like environments like Git Bash). What is your opinion on this?

@rkitover
Copy link
Contributor

rkitover commented Jul 1, 2022

I'll work on implementing this, I think this design is reasonable, and I also think a fatal error when a cache is detected in APPDATA is the best option.

In addition, I will change the secondary/global config from C:/Program Files (x86)/ccache/etc/ccache.conf to C:\ProgramData\ccache\ccache.conf if there are no objections.

We should also support a system-wide cache there in multi-user scenarios, @jrosdahl @Predelnik @cristianadam any suggestions on the best way to do that? Setting cache_dir in that config would work for example.

@jrosdahl
Copy link
Member

jrosdahl commented Jul 5, 2022

I'll work on implementing this, I think this design is reasonable

Thanks. Do I understand correctly that you're aiming for %LOCALAPPDATA%/ccache as the cache location? As mentioned in #1023 (comment), I think I would prefer the same location regardless of execution environment on Windows. What is your opinion on this?

and I also think a fatal error when a cache is detected in APPDATA is the best option.

That's OK with me.

In addition, I will change the secondary/global config from C:/Program Files (x86)/ccache/etc/ccache.conf to C:\ProgramData\ccache\ccache.conf if there are no objections.

Sounds good.

We should also support a system-wide cache there in multi-user scenarios, @jrosdahl @Predelnik @cristianadam any suggestions on the best way to do that? Setting cache_dir in that config would work for example.

Yes, setting cache_dir in the system-wide configuration is one way. Setting a global CCACHE_DIR variable is another.

@jrosdahl jrosdahl added this to the 4.7 milestone Jul 5, 2022
@rkitover
Copy link
Contributor

rkitover commented Jul 5, 2022

I think I would prefer the same location regardless of execution environment on Windows. What is your opinion on this?

That's what I was going to do, and I think this makes the most sense. If someone wants UNIX semantics for config/cache they can use a cygwin/msys2/wsl ccache which is compiled that way. Most native UNIX ports do not make this kind of distinction in any case, it's confusing and error-prone.

rkitover added a commit to rkitover/ccache that referenced this issue Jul 21, 2022
Throw a fatal error if a cache is detected in APPDATA.

Set systemwide config to C:\ProgramData\ccache\ccache.conf .

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 21, 2022
Throw a fatal error if a cache is detected in APPDATA.

Set system-wide config to C:\ProgramData\ccache\ccache.conf .

Update manual.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 21, 2022
Throw a fatal error if a cache is detected in APPDATA.

Set system-wide config to C:\ProgramData\ccache\ccache.conf .

Update manual.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 21, 2022
Throw a fatal error if a cache is detected in APPDATA.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf .

Update manual.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 21, 2022
Throw a fatal error if a cache is detected in APPDATA.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf .

Update manual.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 21, 2022
Throw a fatal error if a cache is detected in APPDATA.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf .

Update manual.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 21, 2022
Throw a fatal error if a cache is detected in APPDATA.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf .

Update manual.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 21, 2022
Throw a fatal error if a cache is detected in APPDATA.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf .

Update manual.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 21, 2022
Throw a fatal error if a cache is detected in APPDATA.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf .

Update manual.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 21, 2022
Add a variadic template helper function PATH(std::string...) to
construct native paths using std::filesystem.

Throw a fatal error if a cache is detected in APPDATA.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf .

Update manual.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 22, 2022
Add a variadic template helper function PATH(std::string...) to
construct native paths using std::filesystem.

Throw a fatal error if a cache is detected in APPDATA.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf .

Update manual.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 22, 2022
Add a variadic template helper function PATH(std::string...) to
construct native paths using std::filesystem.

Throw a fatal error if a cache is detected in APPDATA.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf .

Update manual.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 22, 2022
Add a variadic template helper function PATH(std::string...) to
construct native paths using std::filesystem.

Throw a fatal error if a cache is detected in APPDATA.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf .

Update manual.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 22, 2022
Add a variadic template helper function PATH(std::string...) to
construct native paths using std::filesystem.

Throw a fatal error if a cache is detected in APPDATA.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf .

Update manual.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 22, 2022
Add a variadic template helper function PATH(std::string...) to
construct native paths using std::filesystem.

Throw a fatal error if a cache is detected in APPDATA.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf .

Update manual.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 22, 2022
Add a variadic template helper function PATH(std::string...) to
construct native paths using std::filesystem.

Throw a fatal error if a cache is detected in APPDATA.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf .

Update manual.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 22, 2022
Add a variadic template helper function PATH(std::string...) to
construct native paths using std::filesystem.

Throw a fatal error if a cache is detected in APPDATA.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf .

Update manual.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 23, 2022
Add a variadic template helper function PATH(std::string...) to
construct native paths using std::filesystem.

Throw a fatal error if a cache is detected in APPDATA.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf .

Update manual.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 23, 2022
Add a variadic template helper function PATH(std::string...) to
construct native paths using std::filesystem.

Throw a fatal error if a cache is detected in APPDATA.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf .

Update manual.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 23, 2022
Add a variadic template helper function PATH(std::string...) to
construct native paths using std::filesystem.

Throw a fatal error if a cache is detected in APPDATA.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf .

Update manual.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 25, 2022
Add a variadic template helper function PATH(...) to construct native
paths using std::filesystem.

Throw a fatal error if a cache is detected in APPDATA.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf .

Update manual.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 25, 2022
Add a variadic template helper function PATH(...) to construct native
paths using std::filesystem.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf .

Update manual.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 25, 2022
Add a variadic template helper function PATH(...) to construct native
paths using std::filesystem.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf on Windows.

Update manual to describe behavior specific to Windows and cache
directory finding heuristics on all systems.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 25, 2022
Add a variadic template helper function PATH(...) to construct native
paths using std::filesystem.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf on Windows.

Update manual to describe behavior specific to Windows and cache
directory finding heuristics on all systems.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 25, 2022
Add a variadic template helper function PATH(...) to construct native
paths using std::filesystem.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf on Windows.

Update manual to describe behavior specific to Windows and cache
directory finding heuristics on all systems.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 27, 2022
Add a variadic template helper function PATH(...) to construct native
paths using std::filesystem.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf on Windows.

Update manual to describe behavior specific to Windows and cache
directory finding heuristics on all systems.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 28, 2022
Add a variadic template helper function PATH(...) to construct native
paths using std::filesystem.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf on Windows.

Update manual to describe behavior specific to Windows and cache
directory finding heuristics on all systems.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 28, 2022
Add a variadic template helper function Util::make_path(...) to
construct normalized native paths using std::filesystem.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf on Windows.

Update manual to describe behavior specific to Windows and cache
directory finding heuristics on all systems.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 29, 2022
Add a variadic template helper function Util::make_path(...) to
construct normalized native paths using std::filesystem.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf on Windows.

Update manual to describe and clarify configuration file finding
behavior specific to Windows and cache directory finding behavior on all
systems.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
rkitover added a commit to rkitover/ccache that referenced this issue Jul 29, 2022
Add a variadic template helper function Util::make_path(...) to
construct normalized native paths using std::filesystem.

Fix-up default cmake install directories and set system-wide config to
C:\ProgramData\ccache\ccache.conf on Windows.

Update manual to describe and clarify configuration file finding
behavior specific to Windows and cache directory finding behavior on all
systems.

Fix ccache#1023
Fix ccache#946

Signed-off-by: Rafael Kitover <rkitover@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement that is not a bug fix or new feature os: windows Related to building or running on Windows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants