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 Directory::get_space_left() result on macOS and Linux. #49222

Merged
merged 1 commit into from
May 31, 2021

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented May 31, 2021

  • Use fragme_size instead of block_size as base unit.
  • Use "free space for unprivileged users" instead of "total free space" (which corresponds to the Windows implementation and value shown by df and mc and some other Linux file managers).

Tested on macOS 11.4, Ubuntu 21.04 (x86-64), Mageia 8 (x86-64 and i586) with different file systems.

Fixes #47262

@bruvzg bruvzg added bug platform:linuxbsd platform:macos topic:porting cherrypick:3.x Considered for cherry-picking into a future 3.x release labels May 31, 2021
@bruvzg bruvzg added this to the 4.0 milestone May 31, 2021
@akien-mga akien-mga merged commit afe776c into godotengine:master May 31, 2021
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 3.4.

@bruvzg bruvzg deleted the fix_unix_free_space branch May 31, 2021 10:42
@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label May 31, 2021
@akien-mga
Copy link
Member

Cherry-picked for 3.3.3.

@bruvzg
Copy link
Member Author

bruvzg commented May 31, 2021

By the way, is this "Truncate to closest MiB" part of the GDScript binding necessary for something, or it's just an artifact from the broken conversion to MiB? Previously it has comment "return value in megabytes, given binding is int", but code was the same (missing bracket?).

return d->get_space_left() / 1024 * 1024; // Truncate to closest MiB.

@akien-mga
Copy link
Member

I changed the comment because the previous one didn't make sense given the code and was too orthogonal to the changes in #47254.

But yeah the intent was maybe actually to return MiB instead of bytes since those are typically big values which could easily be more than INT32_MAX, which I guess was a limit in Godot in the past?

I think we could either remove the truncation and keep bytes (and document it, current classref doesn't say the unit), or actually convert to MiB. The latter is maybe the most user-friendly?

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

Successfully merging this pull request may close these issues.

Directory::get_space_left() returns invalid result on Unix
2 participants