system: use %LOCALAPPDATA% as default datadir on windows #27064

pinheadmz · 2023-02-08T20:44:10Z

This PR changes the default datadir location on Windows from C:\Users\Username\AppData\Roaming\Bitcoin to C:\Users\Username\AppData\Local\Bitcoin. This change only applies to fresh installs. To preserve backwards compatibility, on startup we check for the existence of C:\Users\Username\AppData\Roaming\Bitcoin\chainstate and if it is there, we continue using the "Roaming" directory as the default datadir location.

Note that in Windows 11 this change may be moot:

Roaming data and settings is no longer supported as of Windows 11. The recommended replacement is Azure App Service. Azure App Service is widely supported, well documented, reliable, and supports cross-platform/cross-ecosystem scenarios such as iOS, Android and web. Settings stored here no longer roam (as of Windows 11), but the settings store is still available.

DrahtBot · 2023-02-08T20:44:13Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	hebasto, achow101, BenWestgate

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

hebasto · 2023-02-08T21:17:34Z

cc @sipsorcery

ghost

utACK a2bfd52

Note that in Windows 11 this change may be moot

Win 11 isn't used a lot though.

Usage: https://gs.statcounter.com/os-version-market-share/windows/desktop/worldwide

Failure discussion: https://techcommunity.microsoft.com/t5/windows-11/why-users-are-not-upgrading-to-windows-11-is-windows-11-a/m-p/3611663

pinheadmz · 2023-02-16T16:08:15Z

Would this change need a release-notes-27064.md file? It is kinda changing a configuration model

pinheadmz · 2023-04-26T13:07:50Z

Push to 60cd07b781bfe46262a5d66c97e257e0dd378f5c:

rebase on master
add PR release note
change how we detect "old" directory use from looking for blocks/ to looking for chainstate/

hebasto · 2023-05-22T15:56:53Z

Concept ACK.

hebasto · 2023-05-22T16:00:14Z

src/common/args.cpp

Update the PR description:

we check for the existence of C:\Users\Username\AppData\Roaming\Bitcoin\blocks

?

Good catch thanks. (fixed) First draft of this PR did check for blocks directory but of course that is unreliable because it could be customized by the user.

hebasto · 2023-05-23T10:48:24Z

This change ignores non-main network data. I'm OK with that. But it seems worth mentioning in the docs.

pinheadmz · 2023-05-24T19:43:51Z

This change ignores non-main network data.

Ah I see, because I'm checking for Bitcoin\chainstate specifically but if a user only ever used signet, there would be a nested signet directory in there too. Is there anything else I can check for that always gets written to that root? Or maybe just check if Bitcoin\ is empty at all?

I'm OK with that. But it seems worth mentioning in the docs.

Sure but I feel like that's not super clean either.

luke-jr · 2023-06-22T20:08:45Z

Why would we prefer local over roaming explicitly?

pinheadmz · 2023-06-22T20:16:19Z

Why would we prefer local over roaming explicitly?

From https://www.makeuseof.com/tag/appdata-roaming-vs-local/

In the end, Roaming and Local are functionally identical for a home PC running Windows. In a domain environment, data in the Roaming folder will stay with a user's profile if they move to a different computer.

In other words, 500 GB of blockchain data would be copied to a user's remote workstation as they move around. Roaming is just supposed to be for small files like profile preferences, etc. Not big data stores.

More discussion in #2391

achow101 · 2023-09-26T15:56:10Z

on startup we check for the existence of C:\Users\Username\AppData\Roaming\Bitcoin\chainstate

I think we should check for existence of Roaming\Bitcoin rather than any subdirectory. This would not only catch non-mainnet datadirs, but also avoid compatibility issues with users who are looking at older instructions. For example, it's conceivable that a user following a guide that tells them to use Roaming would create Roaming\Bitcoin\bitcoin.conf before their first startup, and thus expect any settings in that conf file to be picked up. Currently, that would not be the case.

pinheadmz · 2023-10-03T12:16:44Z

I think we should check for existence of Roaming\Bitcoin rather than any subdirectory.

revised, rebased on master and pushed thanks.

I tested this "in production" on my windows laptop but found writing a functional test to be impossible. I thought I could model a test after @ryanofsky commit here: 8d777c0

...but for default datadir we use windows' SHGetSpecialFolderPathW with CSIDL_APPDATA which override environment variables: https://learn.microsoft.com/en-us/windows/win32/shell/csidl#remarks

DrahtBot · 2024-04-05T07:51:10Z

Could rebase for fresh CI, if still relevant?

achow101 · 2024-04-15T21:37:23Z

crACK c941712c269d34e6496db35424e567c7f6ba34e8

Have not yet tested on a Windows machine.

pinheadmz · 2024-04-16T18:26:38Z

Could rebase for fresh CI, if still relevant?

rebased on master

hebasto

ACK c59b1e45db7b62ba0e0c833e6517045d3dfeacd0, tested on Windows 11 Pro 23H2.

hebasto · 2024-04-29T14:42:20Z

doc/bitcoin-conf.md

Suggested change

Windows | `%APPDATA%\Bitcoin\` | `C:\Users\username\AppData\Local\Bitcoin\bitcoin.conf`

Windows | `%LOCALAPPDATA%\Bitcoin\` | `C:\Users\username\AppData\Local\Bitcoin\bitcoin.conf`

hebasto · 2024-04-29T14:42:46Z

doc/release-notes/release-notes-27064.md

typo:

Suggested change

compatability if it is present.

compatibility if it is present.

hebasto · 2024-04-29T14:43:26Z

src/common/args.cpp

nit: Could add curly braces or put in a single line?

pinheadmz · 2024-04-30T15:04:43Z

@hebasto thanks for the review, I addressed your comments

hebasto

re-ACK 84900ac, only addressed feedback since my recent review.

achow101 · 2024-05-01T20:09:09Z

ACK 84900ac

Tested on Windows. %APPDATA%/Bitcoin continues to be used if it exists, %LOCALAPPDATA% if not.

BenWestgate · 2024-05-22T22:20:57Z

crACK 84900ac

Have not tested on a Windows machine.

glozow · 2024-07-09T14:43:23Z

doc/release-notes/release-notes-27064.md

Btw, this file should go in the doc/ dir instead of doc/release-notes/ (which is the archive of old release notes by version)

oh whoops, thanks. Should I move it now?

Not a big deal, just don't want it to get missed when releasing 28.0

Ok thanks again, I'll make sure it gets corrected at that time

hebasto added the Windows label Feb 8, 2023

ghost approved these changes Feb 9, 2023

View reviewed changes

DrahtBot mentioned this pull request Apr 4, 2023

move-only: Extract common/args from util/system #27419

Merged

DrahtBot added the Needs rebase label Apr 21, 2023

achow101 requested review from hebasto and jarolrod April 25, 2023 16:19

pinheadmz force-pushed the windows-local-datadir branch from a2bfd52 to d0b715a Compare April 26, 2023 13:01

DrahtBot removed the Needs rebase label Apr 26, 2023

hebasto reviewed May 22, 2023

View reviewed changes

achow101 requested review from achow101 and ryanofsky September 20, 2023 16:51

pinheadmz force-pushed the windows-local-datadir branch from 60cd07b to c941712 Compare October 3, 2023 12:13

DrahtBot added CI failed and removed CI failed labels Nov 13, 2023

DrahtBot added the CI failed label Jan 13, 2024

scarletquasar approved these changes Mar 22, 2024

View reviewed changes

DrahtBot requested a review from hebasto April 15, 2024 21:37

achow101 removed their request for review April 15, 2024 21:37

pinheadmz force-pushed the windows-local-datadir branch from c941712 to c59b1e4 Compare April 16, 2024 18:26

DrahtBot removed the CI failed label Apr 16, 2024

hebasto approved these changes Apr 29, 2024

View reviewed changes

DrahtBot requested a review from achow101 April 29, 2024 14:43

Matthew Zipkin added 2 commits April 30, 2024 11:03

system: use %LOCALAPPDATA% as default datadir on windows

855dd8d

doc: add release-notes-27064.md

84900ac

pinheadmz force-pushed the windows-local-datadir branch from c59b1e4 to 84900ac Compare April 30, 2024 15:04

hebasto approved these changes Apr 30, 2024

View reviewed changes

achow101 merged commit 915d727 into bitcoin:master May 23, 2024

glozow reviewed Jul 9, 2024

View reviewed changes

xyephy mentioned this pull request Feb 26, 2025

Upgrade to bitcoin v28 whiveio/whive#12

Open

	Windows \| `%APPDATA%\Bitcoin\` \| `C:\Users\username\AppData\Local\Bitcoin\bitcoin.conf`
	Windows \| `%LOCALAPPDATA%\Bitcoin\` \| `C:\Users\username\AppData\Local\Bitcoin\bitcoin.conf`

	compatability if it is present.
	compatibility if it is present.

system: use %LOCALAPPDATA% as default datadir on windows #27064

system: use %LOCALAPPDATA% as default datadir on windows #27064

Uh oh!

Conversation

pinheadmz commented Feb 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Feb 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage

Reviews

Conflicts

Uh oh!

hebasto commented Feb 8, 2023

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

pinheadmz commented Feb 16, 2023

Uh oh!

pinheadmz commented Apr 26, 2023

Uh oh!

hebasto commented May 22, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pinheadmz May 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hebasto commented May 23, 2023

Uh oh!

pinheadmz commented May 24, 2023

Uh oh!

luke-jr commented Jun 22, 2023

Uh oh!

pinheadmz commented Jun 22, 2023

Uh oh!

achow101 commented Sep 26, 2023

Uh oh!

pinheadmz commented Oct 3, 2023

Uh oh!

DrahtBot commented Apr 5, 2024

Uh oh!

achow101 commented Apr 15, 2024

Uh oh!

pinheadmz commented Apr 16, 2024

Uh oh!

hebasto left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pinheadmz commented Apr 30, 2024

Uh oh!

hebasto left a comment

Choose a reason for hiding this comment

Uh oh!

achow101 commented May 1, 2024

Uh oh!

BenWestgate commented May 22, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

pinheadmz commented Feb 8, 2023 •

edited

Loading

DrahtBot commented Feb 8, 2023 •

edited

Loading

pinheadmz May 24, 2023 •

edited

Loading