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

Fix get_database_size on Windows #9663

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

iamamyth
Copy link

Replace all calls to epee::file_io::get_file_size with boost::filesystem::file_size in order to avoid lossy conversions from paths to strings, which tend to break filename resolution. This commit fixes a bug on Windows where the get_info RPC call reported a zero database size because BlockchainLMBD::get_database_size returned zero.

Replace all calls to epee::file_io::get_file_size with
boost::filesystem::file_size in order to avoid lossy conversions from
paths to strings, which tend to break filename resolution. This commit
fixes a bug on Windows where the get_info RPC call reported a zero
database size because BlockchainLMBD::get_database_size returned zero.
@iamamyth
Copy link
Author

iamamyth commented Dec 30, 2024

Fixes #9513. Replacing boost::filesystem with std::filesystem across all platforms would require a more recent version of the Android NDK (can't recall the minimum version, but there's an issue for it in the Android repo; #9523 suffices) and MacOS (10.5+, if memory serves).

@iamamyth
Copy link
Author

Worth noting: I think there are build system PRs in queue which would upgrade the relevant components to make std::filesystem available everywhere; however, linking the stdc++fs library properly across environments poses its own challenges, so I don't think the change from boost to std will be a simple find/replace.

Copy link
Collaborator

@0xFFFC0000 0xFFFC0000 left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants