fortheusers / hb-appstore

Homebrew App Store - GUI for downloading/managing homebrew apps
https://hb-app.store
GNU General Public License v3.0
1.09k stars 64 forks source link

freezing issue when SD card is full on wii u #118

Open dwskcw opened 6 months ago

dwskcw commented 6 months ago

platform: Wii U

it seems when attempting to update an app (specifically inkay) with a full sd card, store gets stuck on the downloading popup, beeps at full volume and is stuck in this state until force shutting down the console. does the app store check in any capacity for free space before downloading updates? sorry for the largely non technical description, i haven't filed a bug report before

vgmoose commented 6 months ago

Thanks for the bug report! It doesn't check indeed, and it probably should. This likely requires system specific APIs to check remaining free space prior to downloading/extracting.

This check should happen in libget around the Package:: downloadZip method, where it should return false if the Package's download_size + extracted_size (in KB) is greater than the remaining space, plus some padding.

I'm definitely interested in adding this kind of check and eventually reporting a good human-readable error message as well.

chrisrq commented 1 month ago

Looking to contribute for the first time, is this still an issue and am I free to implement this if I'm able to? Thank you!

vgmoose commented 1 month ago

Absolutely!

chrisrq commented 1 month ago

I should preface this by saying that I'm new to Nintendo homebrew development, so I apologize for any trouble I might cause while discussing proposed changes or if I ask any dumb questions. I wrote a draft for the relevant changes to be made to Package.cpp after checking if zipUrl is empty and before returning:

// check if there is enough free space to download this package
int space_needed = (this.getDownloadSize() + this.getExtractedSize()) / 1000;
int space_available = get_remaining_disk_space(tmp_path.data());
if (space_needed > space_available) {
    printf("--> ERROR: Space needed to  %s is %d KB, only %d KB of disk space remaining\n", this->pkg_name.c_str(), space_needed, space_available);
    return false;
}

get_remaining_disk_space() is a function I made a signature for in Util.hpp and am trying to work on a definition for in Util.cpp. I wasn't sure how to make this cleaner, but it would look something like this:

// returns remaining disk space
int get_remaining_disk_space(path) {
    #if defined(WIN32)
        // handle Win32 case
    #endif
    #if defined(SWITCH)
        // handle Nintendo Switch case
    #endif
    #if defined(_3DS)
        // handle Nintendo 3DS case
    #endif
    #if defined(_WII)
        // handle Wii case
    #endif
    // handle default case
}

I just needed to make sure my approach for how to handle the different cases is in the right direction (any feedback on the aforementioned functions is also appreciated). For the case of the 3DS, I read the libctru documentation and came up with this rough scheme of getting the remaining disk space:

- run fsInit()
- run fsMakePath() to get FS_Path from path specified in downloadZip (?)
- run FSUSER_OpenArchive() using the path from fsMakePath()
- run FSUSER_GetFreeBytes() using the path from OpenArchive()
- run fsExit() and FSUSER_CloseArchive()
- return result from GetFreeBytes() / 1000

Would this more or less be what we're trying to do? Am I missing some implications that this implementation might have or am I using the wrong functions? Any feedback is appreciated.

vgmoose commented 1 month ago

Yep, that looks like the right approach. I thought about mentioning those platform-specific variables in my last comment too. That's the current paradigm used throughout the codebase for platform-specific code. They are: SWITCH, __WIIU__, WII, _3DS, _WIN32, and __APPLE__ (reason for the underscore inconsistencies has to do with existing/common expectations)

In the default (aka unimplemented) case, a really large number (or a negative number, checked for positivity?) could be returned. And then piecewise the methods for each platform would be added to this method for calculating space remaining.

For the context of hb-appstore and its concerns, we only really need to check the total size remaining on the SD card, and not worry too much about the path of the downloaded/extracted content. Libget at this time only operates from wherever the SD card path is, which is defined in ROOT_PATH in Package.hpp. If there's other homebrew that is calculating SD card space remaining already, that would be good place to look to know how that can be implemented.

chrisrq commented 1 month ago

Got it on the SD card part, I think I did see some libctru functions that would let me get the SD card path too which I can then use to open the relevant archive and get the remaining space using the other filesystem functions in fs.h, so I might be able to use that if I don't find something that does it all in one go.

And then piecewise the methods for each platform would be added to this method for calculating space remaining.

Just to clarify, do you mean I should format the get_remaining_disk_space() function the way I'm already doing it or a different way? Thank you for the help so far!

vgmoose commented 1 month ago

Yep, the way you have it with the ifdefs is good. What I meant by piecewise is just that we don't necessarily need to have every single platform solved at the same time.

chrisrq commented 1 month ago

Sounds good! I just finished writing code to check SD card space for Switch, Wii U, 3DS, Wii, and Windows 32-bit (didn't write any for Mac since I don't have a Mac to test), a good amount of which I got from other homebrew apps (cited in my comments). I've verified that all the standalone code works for each platform and made the necessary changes to libget, but I have not properly tested that the changes I made to libget work. How could I go about testing it if I need to? Should I just try to make a test for it under tests/, build hb-appstore with my modified libget and test it out that way, or something else? In case this is relevant to the question, I'm going to throw this out there: I decided to run the test suite that comes with libget to see if that much would still be good to go. Though it was working for me before, now only two tests run before the main test program crashes.

---------------------------
 Test #1 - Initialization Check
---------------------------
Checking repo count...
Get library and repos initialized successfully
---------------------------
✅ Test [Initialization Check] passed!
---------------------------

---------------------------
 Test #2 - Check packages loaded
---------------------------
---------------------------
❌ Test [Check packages loaded] failed: There should be 3 packages enabled, found 0

---------------------------

---------------------------
 Test #3 - Install packages
---------------------------
terminate called after throwing an instance of 'std::bad_optional_access'
  what():  bad optional access

I'm not sure this has to do with my modifications, though, since I commented them out and the same thing happens. This might be more relevant:

Traceback (most recent call last):
  File "/usr/lib/python3.8/runpy.py", line 194, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/lib/python3.8/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/usr/lib/python3.8/http/server.py", line 1294, in <module>
    test(
  File "/usr/lib/python3.8/http/server.py", line 1249, in test
    with ServerClass(addr, HandlerClass) as httpd:
  File "/usr/lib/python3.8/socketserver.py", line 452, in __init__
    self.server_bind()
  File "/usr/lib/python3.8/http/server.py", line 1292, in server_bind
    return super().server_bind()
  File "/usr/lib/python3.8/http/server.py", line 138, in server_bind
    socketserver.TCPServer.server_bind(self)
  File "/usr/lib/python3.8/socketserver.py", line 466, in server_bind
    self.socket.bind(self.server_address)
OSError: [Errno 98] Address already in use

Thank you!