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] Make checkDiskSpace look at the correct drive again #3654

Merged
merged 2 commits into from
Mar 31, 2024

Conversation

CommandMC
Copy link
Collaborator

In my checkDiskSpace refactor (#3440), I used the root property of the object returned by Node's path.parse, thinking that it'd, logically, give me the root folder (or in other words the mount point) of the disk the path is on. This is not the case on Linux/macOS, it's just simply always / there.
This issue would manifest itself in the wrong disk space info being displayed when installing a game onto a drive other than the primary one.

Resolving this was somewhat tricky. Since this function runs on both macOS and Linux, we can't use commandline tools like findmnt to help us here, so I went back to what we did before the refactor: Going up the directory tree and using the first path that exists. This works just fine on Unix, since there will always be a / path (as opposed to Windows, where entire drive letters can be missing)

I've also added a test for these functions now, so this issue can't happen again.

To test this:

  1. Install a game onto another hard drive on the current main branch, see that the total/free disk space is reported incorrectly
  2. Try the same thing with this PR, info will now be correct

Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@CommandMC CommandMC added the pr:ready-for-review Feature-complete, ready for the grind! :P label Mar 29, 2024
@CommandMC CommandMC requested a review from a team March 29, 2024 20:31
@CommandMC CommandMC self-assigned this Mar 29, 2024
@CommandMC CommandMC requested review from arielj, flavioislima, Etaash-mathamsetty, Nocccer and imLinguin and removed request for a team March 29, 2024 20:31
@CommandMC CommandMC force-pushed the fix/getDiskInfo-root-path branch from bad37f2 to eefdab1 Compare March 29, 2024 20:35
@arielj arielj added this to the 2.13.1 milestone Mar 29, 2024
Copy link
Member

@Etaash-mathamsetty Etaash-mathamsetty left a comment

Choose a reason for hiding this comment

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

looks like it's working again

@CommandMC CommandMC merged commit 394ecee into main Mar 31, 2024
9 checks passed
@CommandMC CommandMC deleted the fix/getDiskInfo-root-path branch March 31, 2024 18:30
@Heroic-Games-Launcher Heroic-Games-Launcher locked and limited conversation to collaborators Mar 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr:ready-for-review Feature-complete, ready for the grind! :P
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants